-
Notifications
You must be signed in to change notification settings - Fork 425
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
Documentation: Update boolean arity #1400
Conversation
There might be some doc missing. @remkop I am assuming the html is somehow autogenerated? Or do I need to update it as well? |
Some background on how this is implemented: If a positional parameter is defined without explicitly setting the arity, picocli automatically assigns arity="1". During parsing, picocli creates a "derived arity" for each option it recognizes: named options with a value attached to the option name will have derived arity="1". For example, Picocli uses this "derived arity" if the option was defined without an explicit arity (so when "unspecified=true"). Effectively, this means that from a validation point of view, booleans have arity="0..1". Anyway, that is why the docs say that arity=0 for booleans, when arity=0..1 is more accurate. (It does not tell the whole story, but I guess that is okay.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are almost there.
I have a few suggested changes. Can you take a look?
@@ -3746,22 +3746,12 @@ private static String format(String formatString, Object... params) { | |||
* </p> | |||
* <b>A note on boolean options</b> | |||
* <p> | |||
* By default picocli does not expect boolean options (also called "flags" or "switches") to have a parameter. | |||
* You can make a boolean option take a required parameter by annotating your field with {@code arity="1"}. | |||
* By default picocli allows boolean options (also called "flags" or "switches") to have an optional parameter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small addition:
(...) an optional parameter, which must be either
true
orfalse
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one is very redundant, it's a boolean option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you are saying, but I think it makes sense to be very specific about what parameters are allowed. For example, the following parameters would be rejected: TRUE, FALSE, True, False, 1, 0, YES, NO, Yes, No.
Contrast this to some languages like Groovy and Python where Strings with this wider range of values are translated to true/false boolean values.
The default arity for boolean options is 0..1 and no longer 0.
Merged. |
The default arity for boolean options is 0..1 and no longer 0.
This reverts commit 4ac3aba.
This reverts commit 9ef376a.
This reverts commit c0039ae.
This reverts commit 4ac3aba.
This reverts commit 9ef376a.
This reverts commit c0039ae.
The default arity for boolean options is 0..1 and no longer 0.
Closes #1398.