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

No defaultValue if @ArgGroup #844

Closed
AdrienDS72 opened this issue Oct 28, 2019 · 10 comments
Closed

No defaultValue if @ArgGroup #844

AdrienDS72 opened this issue Oct 28, 2019 · 10 comments
Milestone

Comments

@AdrienDS72
Copy link

When using @ArgGroup, we can't use defaultValue

import picocli.CommandLine;
import picocli.CommandLine.ArgGroup;
import picocli.CommandLine.Command;
import picocli.CommandLine.Option;

@Command()
public class Cmd {

    @Option(names = { "-name" }, required = true, defaultValue = "${env:USERNAME}")
    String name;

    @ArgGroup(multiplicity = "1", exclusive = true)
    Groups groups;

    static class Groups {
        @Option(names = { "-nameGrp" }, required = true, defaultValue = "${env:USERNAME}")
        String name;
    }

    public static void main(String[] args) {
        Cmd cmd = new Cmd();
        new CommandLine(cmd).parseArgs("-nameGrp=foo");
        System.out.println(cmd.name);
        System.out.println(cmd.groups.name);

        cmd = new Cmd();
        new CommandLine(cmd).parseArgs();
        System.out.println(cmd.name);
        // Why groups is null ?
        System.out.println(cmd.groups.name);
    }
}

Output is :

Adrien
foo
Exception in thread "main" picocli.CommandLine$MissingParameterException: Error: Missing required argument (specify one of these): ([-nameGrp=])
at picocli.CommandLine$Model$ArgGroupSpec.validate(CommandLine.java:8687)
at picocli.CommandLine$Model$ArgGroupSpec.access$15700(CommandLine.java:8307)
at picocli.CommandLine$ParseResult$GroupMatchContainer.validate(CommandLine.java:10567)
at picocli.CommandLine$ParseResult.validateGroups(CommandLine.java:10247)
at picocli.CommandLine$Interpreter.validateConstraints(CommandLine.java:11055)
at picocli.CommandLine$Interpreter.parse(CommandLine.java:11031)
at picocli.CommandLine$Interpreter.parse(CommandLine.java:10908)
at picocli.CommandLine.parseArgs(CommandLine.java:1271)
at Cmd.main(Cmd.java:27)

@remkop
Copy link
Owner

remkop commented Oct 28, 2019

Thank you for raising this. I could reproduce your result, thanks for providing the example program.

Picocli does not instantiate an @ArgGroup-annotated field until it matches one of the options in that group on the command line. So the groups field is null unless the end user specifies the -nameGrp option on the command line.

The multiplicity = "1" of the group is not satisfied by any default values of the options in that group; multiplicity = "1" means that the user must specify this group on the command line. If that is not the desired behaviour, then the group should be defined with multiplicity = "0..1" to make the group itself optional.

The defaultValue of options in a group are applied only when picocli instantiates the user object of the group (the Groups class in this example). The exact logic of when this happens is complex, but it is basically when an option in the group is matched for the first time - I'd be happy to explain more details if you're interested. The point is that the defaultValues of options in a group don't come into play if no option in that group has been matched.

The user manual does not mention anything about this, and you are not the first to ask about this, see this StackOverflow question.

So I need to at least update the user manual to clarify the current behaviour.

@remkop remkop added this to the 4.1 milestone Oct 28, 2019
@remkop
Copy link
Owner

remkop commented Oct 31, 2019

I updated the user manual to clarify when @ArgGroup-annotated fields are initialized.

Can you take a look and see if this is sufficient?

@remkop
Copy link
Owner

remkop commented Nov 1, 2019

@AdrienDS72 I also added a new section Default Values in Argument Groups to the user manual and furthermore made the defaultValue annotation the preferred way to define defaults (you are already doing this in your example, this is related to another issue #839).

Could you take a look at the docs and see if the updated section on Argument Groups explains things better? Any feedback welcome!

@remkop
Copy link
Owner

remkop commented Nov 4, 2019

@AdrienDS72, can you give feedback on whether this meets your needs?

@AdrienDS72
Copy link
Author

AdrienDS72 commented Nov 5, 2019

Yes it's clearer to me
In order to properly handle the environment variables I had to instantiate the option object.
Here is an example :

import picocli.CommandLine;
import picocli.CommandLine.ArgGroup;
import picocli.CommandLine.Command;
import picocli.CommandLine.Option;

@Command()
public class Cmd {

    @ArgGroup()
    Groups groups = new Groups();

    static class Groups {
        @Option(names = { "-nameGrp" }, defaultValue = "${env:USERNAME}")
        String name;

        String getName() {
            if (name == null) {
                name = System.getenv("USERNAME");
            }

            return name;
        }
    }

    public static void main(String[] args) {
        Cmd cmd = new Cmd();
        new CommandLine(cmd).parseArgs("-nameGrp=foo");
        System.out.println(cmd.groups.getName());

        cmd = new Cmd();
        new CommandLine(cmd).parseArgs();
        System.out.println(cmd.groups.getName());
    }
}

@remkop
Copy link
Owner

remkop commented Nov 5, 2019

Hmm, yes the duplication there does not look ideal.
I've been thinking a bit on how to let picocli instantiate the Groups object and give its options default values like when the group is matched on the command line.

Thinking out loud:

One idea is to make this possible programmatically, maybe by making the ArgGroupSpec.initUserObject method public. Your code would then look like this:

@Command public class Cmd {

    @ArgGroup
    Groups groups; // we instantiate with initUserObject (see main method below)

    static class Groups {
        @Option(names = { "-nameGrp" }, defaultValue = "${env:USERNAME}")
        String name;
    }

    public static void main(String[] args) {
        Cmd cmd = new Cmd();
        CommandLine commandLine = new CommandLine(cmd);

        // this would create a new Groups instance,
        // set all its options to their default value
        // and assign it to the `groups` field.
        commandLine.getCommandSpec().argGroups().get(0).initUserObject(commandLine);

        commandLine.parseArgs("-nameGrp=foo");
        System.out.println(cmd.groups.name);

        cmd = new Cmd();
        commandLine = new CommandLine(cmd);
        commandLine.getCommandSpec().argGroups().get(0).initUserObject(commandLine);
        commandLine.parseArgs();
        System.out.println(cmd.groups.name);
    }
}

This is verbose but that may not be a bad thing if this is a rare use case.

Another idea is to do this via the annotations API. That would make it much easier to get a non-null group even if there was no match. I can't think of a good name yet, and also I am not yet convinced that this should be encouraged by making this API such a first-class citizen...

// one idea is to do something like this in the annotations
@ArgGroup(initializeBeforeMatch = true)
Groups groups; // initialize this group even if none of its options are matched

CAUTION multi-value groups should not do this, or you would not be able to distinguish zero matches from one match:

@ArgGroup(multiplicity = "0..*", initializeBeforeMatch = true) // this would be bad...
List<Groups> groupList; // this list would now always have a Groups object even if nothing matched

Thoughts?

@clausnagel
Copy link

I wonder if there has been any update on this in the meantime? I am facing the same issue here. I have defined a class DatabaseOptions with CLI options for database connection details (host, port, username, password, etc). I want to use environment variables as default values for these options. My software has several commands that need to be able to connect to a database, so I reuse the DatabaseOptions class in these commands using the @ArgGroup annotation. Very much like in the example above.

Now, my DatabaseOptions only gets instantiated in my command classes if at least one CLI options was passed on the command line but not if a corresponding environment variable was set. It would be nice if this behaviour could be influenced.

@remkop
Copy link
Owner

remkop commented Aug 28, 2021

@clausnagel
There has been no progress on this issue.

My understanding is that there is a workaround that will get the above use case to work, but the drawback is that there is some duplication.

If you can provide a pull request that addresses this I will certainly look at it.

@siilike
Copy link

siilike commented Jun 22, 2022

@remkop, could we please reopen this issue as the workaround is not working. The solution seems to be rather simple.

class TestCmd
{
	@CommandLine.ArgGroup(exclusive = true, multiplicity = "1")
	GroupOpts group = new GroupOpts()

	static class GroupOpts
	{
		@CommandLine.Option(names = ["--test1"], defaultValue = '${env:TEST}')
		String test1

		@CommandLine.Option(names = ["--test2"], defaultValue = '${env:TEST2}')
		String test2
	}
}

def cmd = new CommandLine(...).addSubcommand(TestCmd.class)

cmd.parseArgs(args)

logs:

[picocli DEBUG] Applying default values for group '([--test1=<test1>] | [--test2=<test2>])'
[picocli DEBUG] Applying defaultValue (test) to field String Test$GroupOpts.test1 on GroupOpts@13579834
[picocli DEBUG] 'test' doesn't resemble an option: 0 matching prefix chars out of 18 option names
[picocli INFO] Setting field String Test$GroupOpts.test1 to 'test' (was 'null') for field String Test$GroupOpts.test1 on GroupOpts@13579834
[picocli DEBUG] Applying defaultValue (test2) to field String ee.keel.sync.cli.commands.Setup$GroupOpts.test2 on GroupOpts@13579834
[picocli DEBUG] 'test2' doesn't resemble an option: 0 matching prefix chars out of 18 option names
[picocli INFO] Setting field String Test$GroupOpts.test2 to 'test2' (was 'null') for field String Test$GroupOpts.test2 on GroupOpts@13579834

so when manually initializing the group field the values are set.

The validation still fails, though:

picocli.CommandLine$MissingParameterException: Error: Missing required argument (specify one of these): ([--test1=<test1>] | [--test2=<test2>])
	at picocli.CommandLine$Model$ArgGroupSpec.validate(CommandLine.java:10552)
	at picocli.CommandLine$Model$ArgGroupSpec.access$19300(CommandLine.java:10126)
	at picocli.CommandLine$ParseResult$GroupMatchContainer.validate(CommandLine.java:12659)
	at picocli.CommandLine$ParseResult.validateGroups(CommandLine.java:12303)
	at picocli.CommandLine$Interpreter.validateConstraints(CommandLine.java:13252)
	at picocli.CommandLine$Interpreter.parse(CommandLine.java:13200)
	at picocli.CommandLine$Interpreter.processSubcommand(CommandLine.java:13468)
	at picocli.CommandLine$Interpreter.processArguments(CommandLine.java:13374)
	at picocli.CommandLine$Interpreter.parse(CommandLine.java:13177)
	at picocli.CommandLine$Interpreter.parse(CommandLine.java:13146)
	at picocli.CommandLine$Interpreter.parse(CommandLine.java:13047)
	at picocli.CommandLine.parseArgs(CommandLine.java:1478)
	at Test.createCommandLine(Test.groovy:281)
	at Test.executeResult(Test.groovy:324)
	at Test.main(Test.groovy:370)

In GroupMatchContainer.validate() the group is treated as an unmatched subgroup:

                for (ArgGroupSpec missing : unmatchedSubgroups) {
                    if (missing.validate() && missing.multiplicity().min > 0) {
                        int presentCount = 0;
                        boolean haveMissing = true;
                        boolean someButNotAllSpecified = false;
                        String exclusiveElements = missing.synopsisUnit();
                        String missingElements = missing.synopsisUnit(); //ArgSpec.describe(missing.requiredArgs());
                        validationResult = missing.validate(commandLine, presentCount, haveMissing, someButNotAllSpecified, exclusiveElements, missingElements, missingElements);
                    }
                }

and in that case haveMissing is always set to true which causes validate() to fail. Instead, it should check whether any of the arguments has a default value and set haveMissing = false in that case.

@remkop
Copy link
Owner

remkop commented Jun 22, 2022

@siilike thank you for raising this!

Could you open a separate ticket for this? That makes it easier for me to track which changes went into which release.

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

No branches or pull requests

4 participants