petermr / ami3

Integration of cephis and normami code into a single base. Tests will be slimmed down
Apache License 2.0
17 stars 5 forks source link

Default values not working in `ami` commands #42

Closed petermr closed 4 years ago

remkop commented 4 years ago

Can you provide some detail on how to reproduce the issue?

petermr commented 4 years ago

This is probably a misreading of the picocli spec.

I want to have monochrome as an Option with a value, which if omitted can default to a default value

    public enum MonochromeDest implements AbstractDest {
        _delete,
        monochrome,
        ;
    }
    @Option(names = {"--monochrome"},
            arity="0..1",
            defaultValue = "monochrome",
            description = "FILTER: move monochrome images to <monochrome>; default ${DEFAULT-VALUE}; "+_DELETE+" means delete"
            )
    private MonochromeDest monochromeDirname;

and called by

    @Test
    /** 
     * 
     */
    public void testImageForestPlotsSmall() throws Exception {
        String cmd = ""
                + " -p "+FOREST_PLOT_SMALL
                + " image"
                + " --monochrome"     // note value missing
                + " --minwidth 100"
                + " --minheight 100"
                + " --smalldir small"   // explicit value
                ;
        AMIImageTool imageTool = AMI.execute(AMIImageTool.class, cmd);
    } 

gives:

Invalid value for option '--monochrome': expected one of [_delete, monochrome] (case-sensitive) but was '--minwidth'
Usage: ami image [OPTIONS]
Try 'ami image --help' for more information.
petermr commented 4 years ago

Gosh! you were too quick!

remkop commented 4 years ago

Perhaps instead of defaultValue you wanted to use fallbackValue.

However, in general I recommend against the overuse of arity="0..1". It introduces ambiguity: is the next value a parameter or a new option? The picocli parser has limitations and this may give results that are surprising to end users. Best to keep it simple.

Without the arity="0..1" bit, users can simply omit the --monochrome option and the command will use the default.

Generally, you only need the arity="0..1" bit if you need different behaviour for these 3 possibilities:

In this case, do we need these 3 cases to behave differently?

petermr commented 4 years ago

Thanks, I know you advised against arity="0..1" and I have removed this almost everywhere. I will now take your advice completely!

The idea was:

user omitted the --monochrome option altogether
user specified --monochrome with a parameter
user specified --monochrome without parameter
  1. do not take monochrome action at all
  2. use monochrome with the given parameter
  3. use monochrome with the default value

In this case, do we need these 3 cases to behave differently?

Yes. The main reason is that it's easy to misspell or misremember parameters.

But In this and other cases I favour convention-over-configuration and so will make the choices simpler by not giving them.

remkop commented 4 years ago

Closing: this was fixed by using fallbackValue = "monochrome" instead of defaultValue.