fix: assignment of negate property #1302
Conversation
| @@ -23,7 +23,7 @@ class Option { | |||
| this.required = flags.indexOf('<') >= 0; // A value must be supplied when the option is specified. | |||
| this.optional = flags.indexOf('[') >= 0; // A value is optional when the option is specified. | |||
| this.mandatory = false; // The option must have a value after parsing, which usually means it must be specified on command line. | |||
| this.negate = flags.indexOf('-no-') !== -1; | |||
| this.negate = flags.indexOf('--no-') !== -1; | |||
There was a problem hiding this comment.
This should test for it being at the start of the string. The other places in the code use a regular expression, but checking the index would be fine here I think.
There was a problem hiding this comment.
How about something like this?
| this.negate = flags.indexOf('--no-') !== -1; | |
| this.negate = flags.startsWith('--no-'); |
There was a problem hiding this comment.
Even better! (We moved away from using index to using routines in #1264 which is on the develop branch.)
| program | ||
| .option('--module-no-parse <value>', 'test description'); | ||
| program.parse(['node', 'test']); | ||
| expect(program.opts()).toEqual({}); |
There was a problem hiding this comment.
Thanks for adding a test.
Testing .opts() being the empty object is a little fragile as .opts() has some subtle differences between storeOptionsAsProperties true/false, but this approach does avoid needing to know what property name the flag turned into. Good enough for now!
|
I was just looking at the merge conflicts, and discovered I already fixed I'll do some extra checking, but think this is already done. |
|
Adding a regression test with @snitin315 as co-author |
|
Thanks @shadowspawn, how soon can we expect the release? |
|
We are getting ready to release v6 now, anytime from tonight to end of week. |
|
Fixed in Commander v6.0.0 which has been released : https://github.com/tj/commander.js/releases/tag/v6.0.0 |
Pull Request
Resolve #1301
Problem
We were checking for indexOf
-no-fornegate: trueinstead of--no-Solution
check index of
--no-ChangeLog