From a83006813979ff0f9c683b5221cd031d4962b87b 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 | 4 ++-- examples/pizza | 6 ++---- index.js | 24 ++++++++++++++---------- package-lock.json | 2 +- test/test.options.bool.no.js | 20 ++++++++++++++++++-- test/test.options.defaults.no.js | 23 +++++++++++++++++++++++ 6 files changed, 60 insertions(+), 19 deletions(-) create mode 100644 test/test.options.defaults.no.js diff --git a/Readme.md b/Readme.md index b82afd892..075b47752 100644 --- a/Readme.md +++ b/Readme.md @@ -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 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 diff --git a/examples/pizza b/examples/pizza index f67f9cba2..7e939d7fe 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') .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 289814d12..cf1932727 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/package-lock.json b/package-lock.json index 9f11925a2..099ff46c4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2023,7 +2023,7 @@ }, "text-encoding": { "version": "0.6.4", - "resolved": "http://registry.npmjs.org/text-encoding/-/text-encoding-0.6.4.tgz", + "resolved": "https://registry.npmjs.org/text-encoding/-/text-encoding-0.6.4.tgz", "integrity": "sha1-45mpgiV6J22uQou5KEXLcb3CbRk=", "dev": true }, 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();