From 7ee84bc4c979e15c191544f8581ce7ff9ecf5666 Mon Sep 17 00:00:00 2001 From: usmonster Date: Thu, 3 May 2018 00:14:13 +0200 Subject: [PATCH] fixes behavior of --no-* options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- Readme.md | 6 +++--- examples/pizza | 6 ++---- index.js | 24 ++++++++++++++---------- test/test.options.bool.no.js | 20 ++++++++++++++++++-- test/test.options.defaults.no.js | 23 +++++++++++++++++++++++ 5 files changed, 60 insertions(+), 19 deletions(-) create mode 100644 test/test.options.defaults.no.js diff --git a/Readme.md b/Readme.md index a29da4013..e4367dcda 100644 --- a/Readme.md +++ b/Readme.md @@ -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 @@ -152,7 +152,7 @@ program .option('-s --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); ``` @@ -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 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 ``` diff --git a/examples/pizza b/examples/pizza index f67f9cba2..525f85536 100755 --- a/examples/pizza +++ b/examples/pizza @@ -12,7 +12,7 @@ program .option('-p, --peppers', 'Add peppers') .option('-P, --pineapple', 'Add pineapple') .option('-b, --bbq', 'Add bbq sauce') - .option('-c, --cheese ', 'Add the specified type of cheese [marble]') + .option('-c, --cheese [type]', 'Add the specified type of cheese [marble]', 'marble') .option('-C, --no-cheese', 'You do not want any cheese') .parse(process.argv); @@ -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); diff --git a/index.js b/index.js index 0f54b5ef7..482dd86b4 100644 --- a/index.js +++ b/index.js @@ -60,9 +60,7 @@ function Option(flags, description) { */ Option.prototype.name = function() { - return this.long - .replace('--', '') - .replace('no-', ''); + return this.long.replace(/^--/, ''); }; /** @@ -74,7 +72,7 @@ Option.prototype.name = function() { */ Option.prototype.attributeName = function() { - return camelcase(this.name()); + return camelcase(this.name().replace(/^no-/, '')); }; /** @@ -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 @@ -392,8 +393,11 @@ Command.prototype.option = function(flags, description, fn, defaultValue) { // preassign default value only for --no-*, [optional], or 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; @@ -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; } }); diff --git a/test/test.options.bool.no.js b/test/test.options.bool.no.js index c8c954439..0a3eaaefc 100644 --- a/test/test.options.bool.no.js +++ b/test/test.options.bool.no.js @@ -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(); diff --git a/test/test.options.defaults.no.js b/test/test.options.defaults.no.js new file mode 100644 index 000000000..3eb38d4d8 --- /dev/null +++ b/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();