Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuring multi-value defaults in list option in tz example #51

Closed
remkop opened this issue Feb 12, 2020 · 7 comments
Closed

Configuring multi-value defaults in list option in tz example #51

remkop opened this issue Feb 12, 2020 · 7 comments

Comments

@remkop
Copy link

remkop commented Feb 12, 2020

Hi Max, congrats again on jbang, nice work!

The tz example mentions some difficulty with configuring defaults for multi-value options.

It is possible in picocli to configure default values for a multi-value option, if you are willing to allow comma-separated values. You would do that by setting the split = "," attribute in the @Option or @Parameters annotation. Then you can set multiple default values in a single string:

@Parameters(split = ",", defaultValue = "1,2,3") List<Integer> myNumbers;

Similarly for tz:

class ListDefaults {
    @Parameters(split = ",",
            defaultValue = "America/Los_Angeles,America/Detroit,Europe/London,Europe/Zurich,Asia/Kolkata,Australia/Brisbane",
            description = "Time zones. Defaults: ${DEFAULT-VALUE}.",
            hideParamSyntax = true, paramLabel = "[<zones>[,<zones>...]...]")
    List<ZoneId> zones;

    public static void main(String[] args) {
        ListDefaults instance = CommandLine.populateCommand(new ListDefaults()); // without args
        System.out.println("ZoneIds are: " + instance.zones);

        System.out.println();
        System.out.println("It makes the parameter label in the usage help a bit verbose though...");
        System.out.println("So we set a custom label.");
        System.out.println();
        new CommandLine(new ListDefaults())
                .setUsageHelpLongOptionsMaxWidth(30) // requires picocli 4.2
                .usage(System.out);
    }
}
@maxandersen
Copy link
Collaborator

ah that’s nice - what happens if you mark such parameter arity="0..*" ...would it be possible to mix-match comma separated vs spaces or does that end up being a List<List<ZoneId>> ?

@remkop
Copy link
Author

remkop commented Feb 12, 2020

You don’t need to specify arity.
The parser will accept both comma-separated and individual values.
It should just work. You won’t end up with nested lists.

@maxandersen
Copy link
Collaborator

not sure what I do wrong but I get an empty list on start, even tried upgrading to 4.2.

the parameter parsing works though as it does work with tz.java 15:00 cet,bdt ist where it detects all three.

but if just done as tz.java 15:00 nothing is print out as if just empty list.

@remkop
Copy link
Author

remkop commented Feb 14, 2020

Can you push your code on a branch where I can take a look at it?

maxandersen added a commit that referenced this issue Feb 15, 2020
@maxandersen
Copy link
Collaborator

#56 has the change - copied verbatim from your example + converter attribute (which does not seem to make a difference for this behavior).

With that pr tz.java 15:00 prints out nothing as if timezones are not specified.

@remkop
Copy link
Author

remkop commented Feb 16, 2020

OK, I found the problem. The timezones option needs to be defined like this:

@Parameters(index = "1..*", split = ",",
        converter = ZoneIdConverter.class, // <.>
        defaultValue = "America/Los_Angeles,America/Detroit,Europe/London,Europe/Zurich,Asia/Kolkata,Australia/Brisbane",
        description = "Time zones. Defaults: ${DEFAULT-VALUE}.",
        hideParamSyntax = true, paramLabel = "[<zones>[,<zones>...]...]")
List<ZoneId> zones;

Note the index = "1..*" part. This was missing, so both parameters were looking at index zero (the first argument). Picocli does not automatically increase the index when multiple @Parameters are defined. This is a tricky area (see for example remkop/picocli#564).

Anyway, adding the index bit will fix it.

One thing: I found this by switching on picocli's built-in debug tracing. I ended up changing the source code of the tz main method to do this, because I could not figure out how to pass -Dpicocli.trace=DEBUG on the command line. I tried these:

remko@LAPTOP-7EO0MTL0:/mnt/c/Users/remko/IdeaProjects/jbang$ jbang -Dpicocli.trace=DEBUG examples/tz.java 15:00
Unknown option: '-Dpicocli.trace=DEBUG'
Usage: jbang [-eh] [--verbose] [--version] [--clear-cache] [--completion]
...

and

remko@LAPTOP-7EO0MTL0:/mnt/c/Users/remko/IdeaProjects/jbang$ jbang examples/tz.java -Dpicocli.trace=DEBUG 15:00
Unknown option: '-Dpicocli.trace=DEBUG'

In both cases, picocli is parsing the option and treats it as an application option: first of the jbang application, and next of the tz application. It may be worth treating -Dx=y options to jbang as special and pass them as VM parameters.

maxandersen added a commit that referenced this issue Feb 28, 2020
@maxandersen
Copy link
Collaborator

thanks for the info, just pushed fixed to master and you gave me the idea to implement support for implementing -D passthrough :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants