-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
doc: "if and only if" should be "if" in util.parseArgs default value #58958
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
base: main
Are you sure you want to change the base?
Conversation
The default value is legal by means other than defaulting.
@@ -1956,7 +1956,7 @@ changes: | |||
`false`, values for the option are last-wins. **Default:** `false`. | |||
* `short` {string} A single character alias for the option. | |||
* `default` {string | boolean | string\[] | boolean\[]} The default value to | |||
be used if (and only if) the option does not appear in the arguments to be | |||
be used if the option does not appear in the arguments to be |
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.
upstream comment:
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.
Re: #54431, the critical part is "the option does not appear in the arguments to be parsed".
However, the way it's currently worded "if and only if", it literally means that the value passed as default
behaves as a sentinel value: when it matches, the option was missing; otherwise, the option was in the arguments provided.
But that's not how it works. It's a one-side implication.
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.
Well it says "to be used", so I don't think it says the value could not be the default value if the option appears in the arguments.
Maybe we can try to rephrase it further to remove the ambiguity:
* `default` {string | boolean | string\[] | boolean\[]} It must be of the
same type as the `type` property. When `multiple` is `true`, it must be an
array. It is ignored when when the option appears in the arguments to be
parsed.
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.
@aduh95 That leaves out what default
is actually used for. Adding your suggestion to the previous text:
`default` {string | boolean | string\[] | boolean\[]} The default value to
be used if the option does not appear in the arguments to be parsed,
and ignored if the option does appear. It must be of the same type as the
`type` property. When `multiple` is `true`, it must be an array.
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.
`default` {string | boolean | string\[] | boolean\[]} The default value to be used if the option does not appear in the arguments to be parsed, and ignored if the option does appear. It must be of the same type as the `type` property. When `multiple` is `true`, it must be an array.
This wording is also an "if and only if", just more verbose. This wording implies that if string option --foo
has a default value of "bar"
, then --foo=bar
is ignored.
I'd go with this:
* `default` {string | boolean | string\[] | boolean\[]} The value to be used if the option does not appear in the arguments to be parsed. It must be of the same type as the `type` property. When `multiple` is `true`, it must be an array. It is ignored when the option appears in the arguments to be parsed.
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.
(There is a "when when" in the last line.)
The default value is legal by means other than defaulting.