From 339eb9702a19efd57dab9629711c52f10b464f00 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 | 9 +++++++++ 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, 67 insertions(+), 17 deletions(-) create mode 100644 test/test.options.defaults.no.js diff --git a/Readme.md b/Readme.md index 7e09c21ec..6fa8f697e 100644 --- a/Readme.md +++ b/Readme.md @@ -264,6 +264,15 @@ Usage: pizza [options] An application for pizzas ordering +Options: + -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 (default: "marble") + -C, --no-cheese You do not want any cheese + -h, --help output usage information + Options: -h, --help output usage information -V, --version output the version number 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 84436d8ea..7b8b3721c 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 54f6aa992..aacf953a2 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();