From 3f336d2863964e8319448cb1012524ec9bced1c5 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 devault 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, the tests for negated options were 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 | 2 +- index.js | 13 +++++++------ test/test.options.bool.no.js | 20 ++++++++++++++++++-- 4 files changed, 29 insertions(+), 12 deletions(-) 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..c8a6d215c 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); diff --git a/index.js b/index.js index 0f54b5ef7..b094d60a8 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-/, '')); }; /** @@ -392,8 +390,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; 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();