From 0a2e7605b1f09ce65b84cbac02a6894bf530d269 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 115ff995f..cc10e4b88 100644 --- a/Readme.md +++ b/Readme.md @@ -343,13 +343,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 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..269c0cd4a 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 ', '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 da594e9cd..e1096d9a7 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 @@ -394,8 +395,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; @@ -426,7 +430,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 2d030a3bc..119bada6a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2275,7 +2275,7 @@ "dependencies": { "minimist": { "version": "1.2.0", - "resolved": "http://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=", "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();