-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
doc: expand description of parseArg
's default
#54431
base: main
Are you sure you want to change the base?
Conversation
From past experience, it is hard being clear about the config setup, the passed arguments, and the parsed result! I often say command-line but that does not cover custom arguments. The suggested text does not accurately cover I'll have a try, instead try changing the initial description: - The default option value when this argument is not passed.
+ The default option value when the option is not included in the arguments to parse. |
Co-authored-by: John Gee <john@ruru.gen.nz>
I always forget about |
doc/api/util.md
Outdated
`type` property. When `multiple` is `true`, it must be an array. For | ||
`string` arguments, if the option is passed it must still be followed by | ||
a value. |
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.
`type` property. When `multiple` is `true`, it must be an array. For | |
`string` arguments, if the option is passed it must still be followed by | |
a value. | |
`type` property. When `multiple` is `true`, it must be an array. |
In my personal opinion, saying "The default option value when this argument is not passed."
implies that if it passed, it must have a value.
Secondly, "it must still be followed by a value."
makes it sound like the "default"
option doesn't work on string arguments...
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.
makes it sound like the
"default"
option doesn't work on string arguments.
The default
option doesn't do anything when the option being specified is passed (except in strict: false
), so that's a correct inference (as long as you don't skip over the "if the option is passed" part). Unless I'm misunderstanding you?
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 "it must still be followed by a value" is redudant because of "The default option value when this argument is not passed.", but that might just be me.
Another possible wording (which I used in a recent comment where I was trying to clarify behaviour):
|
Tried a new wording based on @shadowspawn's suggestion, WDYT?
Hopefully that's sufficient to prevent people from coming to (incorrectly) believe that setting this would allow passing string-based options without arguments in |
Nice! Big +1 from me. |
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 but solid incremental improvement, reducing chance for confusion.
Per #53427 (comment).