-
Notifications
You must be signed in to change notification settings - Fork 123
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
fix: do not set boolean flags if not defined in argv
#119
Conversation
f926841
to
4ecdb72
Compare
undefined
if not set in argv
argv
4ecdb72
to
cf299cb
Compare
argv
argv
index.js
Outdated
@@ -105,8 +105,10 @@ function parse (args, opts) { | |||
var argv = { _: [] } | |||
|
|||
Object.keys(flags.bools).forEach(function (key) { | |||
setArg(key, !(key in defaults) ? false : defaults[key]) | |||
setDefaulted(key) | |||
if (key in defaults) { |
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.
This should use Object.prototype.hasOwnProperty.call
, in case the "key" is "toString" or "hasOwnProperty"
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 kept key in defaults
because if was there before, but I agree we should use Object.prototype.hasOwnProperty.call
. I made the change.
BREAKING CHANGE: `boolean` flags defined without a `default` value will now behave like other option type and won't be set in the parsed results when the user doesn't set the corresponding CLI arg. Previous behavior: ```js var parse = require('yargs-parser'); parse('--flag', {boolean: ['flag']}); // => { _: [], flag: true } parse('--no-flag', {boolean: ['flag']}); // => { _: [], flag: false } parse('', {boolean: ['flag']}); // => { _: [], flag: false } ``` New behavior: ```js var parse = require('yargs-parser'); parse('--flag', {boolean: ['flag']}); // => { _: [], flag: true } parse('--no-flag', {boolean: ['flag']}); // => { _: [], flag: false } parse('', {boolean: ['flag']}); // => { _: [] } => flag not set similarly to other option type ```
cf299cb
to
9f185f2
Compare
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.
@pvdlg thanks for digging into this; I'm currently triaging issues with @evocateur and we both agree that this is a good improvement to the API.
Awesome! Thanks! |
@bcoe could you by any chance also check out yargs/yargs#1071 which is related? Thanks! |
Fix #116
boolean
flags defined without adefault
value will now behave like other option type and won't be set in the parsed results when the user doesn't set the corresponding CLI arg.Previous behavior:
New behavior: