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

PropertiesDefaultProvider fails with static ArgGroup class #876

Closed
deining opened this issue Nov 23, 2019 · 7 comments
Closed

PropertiesDefaultProvider fails with static ArgGroup class #876

deining opened this issue Nov 23, 2019 · 7 comments
Labels
theme: arg-group An issue or change related to argument groups

Comments

@deining
Copy link
Contributor

deining commented Nov 23, 2019

Minimum example:

Java source file:

1  package test;
2
3  import java.io.File;
4
5  import picocli.CommandLine;
6  import picocli.CommandLine.ArgGroup;
7  import picocli.CommandLine.Command;
8  import picocli.CommandLine.Option;
9  import picocli.CommandLine.PropertiesDefaultProvider;
10
11 public class ArgGroupPropertiesDefaultProvider {
12
13     public static void main(String... args) {
14
15     CommandLine cmd = new CommandLine(new MyCommand());
16     File defaultsFile = new File("MyCommand.properties");
17     cmd.setDefaultValueProvider(new PropertiesDefaultProvider(defaultsFile));
18     cmd.execute(args);
19     }
20 }
21
22 @Command( name="myCommand")
23 class MyCommand implements Runnable {
24
25     @Option(names = "-x")
26     int x;
27
28     @ArgGroup()
29     MyArgGroup anArgGroup;
30     //MyArgGroup anArgGroup = new MyArgGroup(); // this does work!
31    
32    static class MyArgGroup {
33         @Option(names = { "-y" }, descriptionKey= "myOption")
34         static int y;
35    }
36
37    
38     public void run() {
39         System.out.println("Option x is picked up: " + x);
40         System.out.println("Option y inside arg group is not picked up (static class!): " + MyArgGroup.y);
41     }
42 }

Associated properties file (MyCommand.properties):

myCommand.x=6
myCommand.y=9
myCommand.myOption=9

If you comment out line 29 and uncomment line 30, the value from the properties file is picked up.

Is this the intended behaviour?

@remkop
Copy link
Owner

remkop commented Nov 24, 2019

Thank you for raising this! I’ll investigate when I get to my PC.

@remkop
Copy link
Owner

remkop commented Nov 24, 2019

I have only had a chance to look at this for a few minutes, but I’m guessing that this is related to the fact that default values in general are not applied to ArgGroups until at least one option in the group is matched.

https://picocli.info/#_default_values_in_argument_groups

@remkop remkop added the theme: arg-group An issue or change related to argument groups label Nov 25, 2019
@remkop
Copy link
Owner

remkop commented Nov 26, 2019

If the class associated with the arg group is not a static class, the value from the properties file is picked up.

How can I reproduce this? If I make inner class MyArgGroup non-static, the static int y field gives a compilation error: "modifier 'static' is only allowed in constant variable declarations".
And even if I make MyArgGroup a top-level class (and keep static int y; field as static), there is no default value assigned to MyArgGroup.y... So I did not understand your comment above.

However, I believe the observed behaviour is the expected behaviour: picocli will not instantiate a MyArgGroup and assign default values to the group's options unless at least one of the options of the group is matched on the command line. If a group has multiplicity = "1" it is mandatory and in that case the business logic in the run/call method can safely assume that anArgGroup is non-null, but it should not assume that if the group has the default multiplicity = "0..1".

I understand this may not be intuitive for many users but I cannot imagine how it could work otherwise.

remkop added a commit that referenced this issue Nov 26, 2019
@deining
Copy link
Contributor Author

deining commented Nov 26, 2019

How can I reproduce this?

Sorry for not being clear here. I edited my inital posting to make it clear and unambigous. Hopefully I succeeded in doing so.

@remkop
Copy link
Owner

remkop commented Nov 26, 2019

Thanks for the clarification.
The short answer is that yes, this is working as intended. (see the if (arg.scope().get() != null) check on CommandLine.java line 11222)

Basically, if the application has instantiated the @ArgGroup-annotated field, then picocli should apply defaults before parsing the command line arguments.

If the application has not instantiated the the @ArgGroup-annotated field, then picocli should only instantiate (and apply defaults) when one of the options in that group is matched on the command line. In this case, applications can check that the group was matched on the command line by checking if the @ArgGroup-annotated field is non-null.

MadFoal added a commit to MadFoal/picocli that referenced this issue Oct 27, 2021
Classname ArgGroupDefaultValueTest

Description of issue:
I understand that by default defaultValue in ArgGroup are not applied.

The behavior is explained at : remkop#876 (comment)
(Note: The default behavior seems surprising to me at first sight but maybe it's justified)

I faced a situation where it seems that this behavior is not respected (but maybe I missed something)
Sorry for the not so short example, I was not able to make it smaller.

The goal is to have this kind of options : X AND ((1A AND 1B) OU (2A OU 2B))
I set default value for X, 2A and 2B.
@MadFoal
Copy link
Contributor

MadFoal commented Nov 18, 2021

@remkop This issue is also closed by PR 1446, correct?

@remkop
Copy link
Owner

remkop commented Nov 18, 2021

@remkop This issue is also closed by PR 1446, correct?

It is the same underlying mechanism, yes. (
The source of default values does not matter.) 😅

Oh, I see what you mean now: can this issue be closed now that we improved the documentation?
Yes, I agree, that is fine.

@remkop remkop closed this as completed Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: arg-group An issue or change related to argument groups
Projects
None yet
Development

No branches or pull requests

3 participants