Skip to content

Commit

Permalink
fixes behavior of --no-* options
Browse files Browse the repository at this point in the history
A bug existed whenever a "--no"-prefixed option was defined along with
its counterpart (e.g. "--status" and "--no-status"), which caused the
corresponding boolean attribute to always be `false` when either option
was specified on the command line.

This was because the definition of a negating option would overwrite the
default value of the attribute, combined with the fact that specifying either
option would emit two events with conflicting logic.

This has been corrected in two ways:
1. During the setup of a negating option, we now check to see if the
   non-negated counterpart was already defined, in which case we don't
   set/overwrite the devault value (which would be "true");
2. An option's name no longer omits its negation prefix; that is,
   "--no-cheese" is internally "no-cheese", which will distinguish it
   from "--cheese"—this will allow unique events to be registered and
   emitted, depending on which option is passed to the command, thus
   avoiding any attribute assignment collision.

Additionally, the tests for negated options were updated to more
explicitly demonstrate the expected behavior, and a couple of relevant
examples were fixed to match their intended behavior.
  • Loading branch information
usmonster committed May 2, 2018
1 parent dcddf69 commit 3f336d2
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 12 deletions.
6 changes: 3 additions & 3 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ console.log(' - %s cheese', program.cheese);

Short flags may be passed as a single arg, for example `-abc` is equivalent to `-a -b -c`. Multi-word options such as "--template-engine" are camel-cased, becoming `program.templateEngine` etc.

Note that multi-word options starting with `--no` prefix negate the boolean value of the following word. For example, `--no-sauce` sets the value of `program.sauce` to false.
Note that multi-word options starting with `--no` prefix negate the boolean value of the following word. For example, `--no-sauce` sets the value of `program.sauce` to false.

```js
#!/usr/bin/env node
Expand Down Expand Up @@ -152,7 +152,7 @@ program
.option('-s --size <size>', 'Pizza size', /^(large|medium|small)$/i, 'medium')
.option('-d --drink [drink]', 'Drink', /^(coke|pepsi|izze)$/i)
.parse(process.argv);

console.log(' size: %j', program.size);
console.log(' drink: %j', program.drink);
```
Expand Down Expand Up @@ -260,7 +260,7 @@ You can enable `--harmony` option in two ways:
-p, --peppers Add peppers
-P, --pineapple Add pineapple
-b, --bbq Add bbq sauce
-c, --cheese <type> Add the specified type of cheese [marble]
-c, --cheese [type] Add the specified type of cheese [marble]
-C, --no-cheese You do not want any cheese
```
Expand Down
2 changes: 1 addition & 1 deletion examples/pizza
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ program
.option('-p, --peppers', 'Add peppers')
.option('-P, --pineapple', 'Add pineapple')
.option('-b, --bbq', 'Add bbq sauce')
.option('-c, --cheese <type>', 'Add the specified type of cheese [marble]')
.option('-c, --cheese [type]', 'Add the specified type of cheese [marble]')
.option('-C, --no-cheese', 'You do not want any cheese')
.parse(process.argv);

Expand Down
13 changes: 7 additions & 6 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ function Option(flags, description) {
*/

Option.prototype.name = function() {
return this.long
.replace('--', '')
.replace('no-', '');
return this.long.replace(/^--/, '');
};

/**
Expand All @@ -74,7 +72,7 @@ Option.prototype.name = function() {
*/

Option.prototype.attributeName = function() {
return camelcase(this.name());
return camelcase(this.name().replace(/^no-/, ''));
};

/**
Expand Down Expand Up @@ -392,8 +390,11 @@ Command.prototype.option = function(flags, description, fn, defaultValue) {

// preassign default value only for --no-*, [optional], or <required>
if (!option.bool || option.optional || option.required) {
// when --no-* we make sure default is true
if (!option.bool) defaultValue = true;
// when --no-foo we make sure default is true, unless a --foo option is already defined
if (!option.bool) {
var opts = self.opts();
defaultValue = name in opts ? opts[name] : true;
}
// preassign only if we have a default
if (defaultValue !== undefined) {
self[name] = defaultValue;
Expand Down
20 changes: 18 additions & 2 deletions test/test.options.bool.no.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,25 @@ var program = require('../')

program
.version('0.0.1')
.option('-e, --everything', 'add all of the toppings')
.option('-p, --pepper', 'add pepper')
.option('-P, --no-pepper', 'remove pepper')
.option('-c|--no-cheese', 'remove cheese');

program.parse(['node', 'test', '--no-cheese']);
should.equal(undefined, program.pepper);
program.parse(['node', 'test']);
program.should.not.have.property('everything');
program.should.not.have.property('pepper');
program.cheese.should.be.true();

program.parse(['node', 'test', '--everything']);
program.everything.should.be.true();
program.should.not.have.property('pepper');
program.cheese.should.be.true();

program.parse(['node', 'test', '--pepper']);
program.pepper.should.be.true();
program.cheese.should.be.true();

program.parse(['node', 'test', '--everything', '--no-pepper', '--no-cheese']);
program.pepper.should.be.false();
program.cheese.should.be.false();

0 comments on commit 3f336d2

Please sign in to comment.