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 default 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, tests for negated options were added/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 23, 2019
1 parent 3598303 commit a830068
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 19 deletions.
4 changes: 2 additions & 2 deletions Readme.md
Expand Up @@ -355,13 +355,13 @@ Usage: pizza [options]
An application for pizzas ordering
Options:
-h, --help output usage information
-V, --version output the version number
-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 (default: "marble")
-C, --no-cheese You do not want any cheese
-h, --help output usage information
```
## Custom help
Expand Down
6 changes: 2 additions & 4 deletions examples/pizza
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 All @@ -21,9 +21,7 @@ if (program.peppers) console.log(' - peppers');
if (program.pineapple) console.log(' - pineapple');
if (program.bbq) console.log(' - bbq');

var cheese = true === program.cheese
? 'marble'
: program.cheese || 'no';
var cheese = !program.cheese ? 'no' : program.cheese;

console.log(' - %s cheese', cheese);
console.log(program.args);
24 changes: 14 additions & 10 deletions index.js
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 @@ -335,14 +333,17 @@ Command.prototype.action = function(fn) {
*
* Examples:
*
* // simple boolean defaulting to false
* // simple boolean defaulting to undefined
* program.option('-p, --pepper', 'add pepper');
*
* program.pepper
* // => undefined
*
* --pepper
* program.pepper
* // => Boolean
* // => true
*
* // simple boolean defaulting to true
* // simple boolean defaulting to true (unless non-negated option is also defined)
* program.option('-C, --no-cheese', 'remove cheese');
*
* program.cheese
Expand Down Expand Up @@ -392,8 +393,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 Expand Up @@ -424,7 +428,7 @@ Command.prototype.option = function(flags, description, fn, defaultValue) {
}
} else if (val !== null) {
// reassign
self[name] = val;
self[name] = option.bool ? val : false;
}
});

Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 18 additions & 2 deletions test/test.options.bool.no.js
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();
23 changes: 23 additions & 0 deletions test/test.options.defaults.no.js
@@ -0,0 +1,23 @@
/**
* Module dependencies.
*/

var program = require('../')
, should = require('should');

program
.version('0.0.1')
.option('-p, --pepper [type]', 'add pepper', 'red')
.option('-P, --no-pepper', 'remove pepper');

program.parse(['node', 'test']);
program.pepper.should.equal('red');

program.parse(['node', 'test', '--pepper']);
program.pepper.should.equal('red');

program.parse(['node', 'test', '--pepper', 'jalapeño']);
program.pepper.should.equal('jalapeño');

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

0 comments on commit a830068

Please sign in to comment.