From 5a27ec4e58c38c7ef9479bad246e3d771129e404 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 30 Jun 2019 12:52:51 +1200 Subject: [PATCH 1/5] Add support for default value for boolean flags --- index.js | 6 +++--- test/test.options.defaults.given.js | 8 ++++++-- test/test.options.defaults.js | 6 +++++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/index.js b/index.js index 73570914c..751b53648 100644 --- a/index.js +++ b/index.js @@ -402,8 +402,8 @@ Command.prototype.option = function(flags, description, fn, defaultValue) { } } - // preassign default value only for --no-*, [optional], or - if (option.negate || option.optional || option.required) { + // preassign default value for --no-*, [optional], , or plain flag if boolean value + if (option.negate || option.optional || option.required || typeof defaultValue === 'boolean') { // when --no-foo we make sure default is true, unless a --foo option is already defined if (option.negate) { var opts = self.opts(); @@ -427,7 +427,7 @@ Command.prototype.option = function(flags, description, fn, defaultValue) { val = fn(val, self[name] === undefined ? defaultValue : self[name]); } - // unassigned or bool + // unassigned or boolean value if (typeof self[name] === 'boolean' || typeof self[name] === 'undefined') { // if no value, negate false, and we have a default, then use it! if (val == null) { diff --git a/test/test.options.defaults.given.js b/test/test.options.defaults.given.js index d0846d7e0..ac572190e 100644 --- a/test/test.options.defaults.given.js +++ b/test/test.options.defaults.given.js @@ -8,15 +8,19 @@ var program = require('../') program .version('0.0.1') .option('-a, --anchovies', 'Add anchovies?') - .option('-o, --onions', 'Add onions?', true) + .option('-o, --onions', 'Add onions?') + .option('-O, --no-onions', 'No onions') + .option('-t, --tomatoes', 'Add tomatoes?', false) + .option('-T, --no-tomatoes', 'No tomatoes') .option('-v, --olives', 'Add olives? Sorry we only have black.', 'black') .option('-s, --no-sauce', 'Uh… okay') .option('-r, --crust ', 'What kind of crust would you like?', 'hand-tossed') .option('-c, --cheese [type]', 'optionally specify the type of cheese', 'mozzarella'); -program.parse(['node', 'test', '--anchovies', '--onions', '--olives', '--no-sauce', '--crust', 'thin', '--cheese', 'wensleydale']); +program.parse(['node', 'test', '--anchovies', '--onions', '--tomatoes', '--olives', '--no-sauce', '--crust', 'thin', '--cheese', 'wensleydale']); program.should.have.property('anchovies', true); program.should.have.property('onions', true); +program.should.have.property('tomatoes', true); program.should.have.property('olives', 'black'); program.should.have.property('sauce', false); program.should.have.property('crust', 'thin'); diff --git a/test/test.options.defaults.js b/test/test.options.defaults.js index 71ea24e8c..3881c163f 100644 --- a/test/test.options.defaults.js +++ b/test/test.options.defaults.js @@ -8,7 +8,10 @@ var program = require('../') program .version('0.0.1') .option('-a, --anchovies', 'Add anchovies?') - .option('-o, --onions', 'Add onions?', true) + .option('-o, --onions', 'Add onions?') + .option('-O, --no-onions', 'No onions') + .option('-t, --tomatoes', 'Add tomatoes?', false) + .option('-T, --no-tomatoes', 'No tomatoes') .option('-v, --olives', 'Add olives? Sorry we only have black.', 'black') .option('-s, --no-sauce', 'Uh… okay') .option('-r, --crust ', 'What kind of crust would you like?', 'hand-tossed') @@ -20,6 +23,7 @@ program.parse(['node', 'test']); program.should.have.property('_name', 'test'); program.should.not.have.property('anchovies'); program.should.not.have.property('onions'); +program.should.have.property('tomatoes', false); program.should.not.have.property('olives'); program.should.have.property('sauce', true); program.should.have.property('crust', 'hand-tossed'); From 3ff940b144378c570c0b2bc633cc976719e02bbd Mon Sep 17 00:00:00 2001 From: John Gee Date: Wed, 24 Jul 2019 21:43:16 +1200 Subject: [PATCH 2/5] Expand testing for boolean flags to cover new features --- test/test.options.bool.js | 37 +++++++---- test/test.options.bool.no.js | 95 +++++++++++++++++++---------- test/test.options.bool.small.js | 15 ----- test/test.options.defaults.given.js | 16 ++--- test/test.options.defaults.js | 18 ++---- 5 files changed, 101 insertions(+), 80 deletions(-) delete mode 100644 test/test.options.bool.small.js diff --git a/test/test.options.bool.js b/test/test.options.bool.js index 508b84b09..352495609 100644 --- a/test/test.options.bool.js +++ b/test/test.options.bool.js @@ -1,15 +1,28 @@ -/** - * Module dependencies. - */ +const commander = require('../'); +// eslint-disable-next-line no-unused-vars +const should = require('should'); -var program = require('../') - , should = require('should'); +// Test simple flag and negatable flag -program - .version('0.0.1') - .option('-p, --pepper', 'add pepper') - .option('-c, --no-cheese', 'remove cheese'); +function simpleFlagProgram() { + const program = new commander.Command(); + program + .option('-p, --pepper', 'add pepper') + .option('-C, --no-cheese', 'remove cheese'); + return program; +} -program.parse(['node', 'test', '--pepper']); -program.pepper.should.be.true(); -program.cheese.should.be.true(); +const simpleFlagNoOptions = simpleFlagProgram(); +simpleFlagNoOptions.parse(['node', 'test']); +simpleFlagNoOptions.should.not.have.property('pepper'); +simpleFlagNoOptions.cheese.should.be.true(); + +const simpleFlagLong = simpleFlagProgram(); +simpleFlagLong.parse(['node', 'test', '--pepper', '--no-cheese']); +simpleFlagLong.pepper.should.be.true(); +simpleFlagLong.cheese.should.be.false(); + +const simpleFlagShort = simpleFlagProgram(); +simpleFlagShort.parse(['node', 'test', '-p', '-C']); +simpleFlagShort.pepper.should.be.true(); +simpleFlagShort.cheese.should.be.false(); diff --git a/test/test.options.bool.no.js b/test/test.options.bool.no.js index 0a3eaaefc..09b15350f 100644 --- a/test/test.options.bool.no.js +++ b/test/test.options.bool.no.js @@ -1,31 +1,64 @@ -/** - * Module dependencies. - */ - -var program = require('../') - , should = require('should'); - -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']); -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(); +const commander = require('../'); +// eslint-disable-next-line no-unused-vars +const should = require('should'); + +// Test combination of flag and --no-flag +// (negatable flag on its own is tested in test.options.bool.js) + +function flagProgram(defaultValue) { + const program = new commander.Command(); + program + .option('-p, --pepper', 'add pepper', defaultValue) + .option('-P, --no-pepper', 'remove pepper'); + return program; +} + +// Flag with no default, normal usage. + +const programNoDefaultNoOptions = flagProgram(); +programNoDefaultNoOptions.parse(['node', 'test']); +programNoDefaultNoOptions.should.not.have.property('pepper'); + +const programNoDefaultWithFlag = flagProgram(); +programNoDefaultWithFlag.parse(['node', 'test', '--pepper']); +programNoDefaultWithFlag.pepper.should.be.true(); + +const programNoDefaultWithNegFlag = flagProgram(); +programNoDefaultWithNegFlag.parse(['node', 'test', '--no-pepper']); +programNoDefaultWithNegFlag.pepper.should.be.false(); + +// Flag with default, say from an environment variable. + +const programTrueDefaultNoOptions = flagProgram(true); +programTrueDefaultNoOptions.parse(['node', 'test']); +programTrueDefaultNoOptions.pepper.should.be.true(); + +const programTrueDefaultWithFlag = flagProgram(true); +programTrueDefaultWithFlag.parse(['node', 'test', '-p']); +programTrueDefaultWithFlag.pepper.should.be.true(); + +const programTrueDefaultWithNegFlag = flagProgram(true); +programTrueDefaultWithNegFlag.parse(['node', 'test', '-P']); +programTrueDefaultWithNegFlag.pepper.should.be.false(); + +const programFalseDefaultNoOptions = flagProgram(false); +programFalseDefaultNoOptions.parse(['node', 'test']); +programFalseDefaultNoOptions.pepper.should.be.false(); + +const programFalseDefaultWithFlag = flagProgram(false); +programFalseDefaultWithFlag.parse(['node', 'test', '-p']); +programFalseDefaultWithFlag.pepper.should.be.true(); + +const programFalseDefaultWithNegFlag = flagProgram(false); +programFalseDefaultWithNegFlag.parse(['node', 'test', '-P']); +programFalseDefaultWithNegFlag.pepper.should.be.false(); + +// Flag specified both ways, last one wins. + +const programNoYes = flagProgram(); +programNoYes.parse(['node', 'test', '--no-pepper', '--pepper']); +programNoYes.pepper.should.be.true(); + +const programYesNo = flagProgram(); +programYesNo.parse(['node', 'test', '--pepper', '--no-pepper']); +programYesNo.pepper.should.be.false(); diff --git a/test/test.options.bool.small.js b/test/test.options.bool.small.js deleted file mode 100644 index 45c6a76b8..000000000 --- a/test/test.options.bool.small.js +++ /dev/null @@ -1,15 +0,0 @@ -/** - * Module dependencies. - */ - -var program = require('../') - , should = require('should'); - -program - .version('0.0.1') - .option('-p, --pepper', 'add pepper') - .option('-c, --no-cheese', 'remove cheese'); - -program.parse(['node', 'test', '-p', '-c']); -program.pepper.should.be.true(); -program.cheese.should.be.false(); diff --git a/test/test.options.defaults.given.js b/test/test.options.defaults.given.js index ac572190e..f78fb08c3 100644 --- a/test/test.options.defaults.given.js +++ b/test/test.options.defaults.given.js @@ -1,14 +1,10 @@ -/** - * Module dependencies. - */ - -var program = require('../') - , should = require('should'); +const program = require('../'); +// eslint-disable-next-line no-unused-vars +const should = require('should'); program - .version('0.0.1') .option('-a, --anchovies', 'Add anchovies?') - .option('-o, --onions', 'Add onions?') + .option('-o, --onions', 'Add onions?', true) .option('-O, --no-onions', 'No onions') .option('-t, --tomatoes', 'Add tomatoes?', false) .option('-T, --no-tomatoes', 'No tomatoes') @@ -17,9 +13,9 @@ program .option('-r, --crust ', 'What kind of crust would you like?', 'hand-tossed') .option('-c, --cheese [type]', 'optionally specify the type of cheese', 'mozzarella'); -program.parse(['node', 'test', '--anchovies', '--onions', '--tomatoes', '--olives', '--no-sauce', '--crust', 'thin', '--cheese', 'wensleydale']); +program.parse(['node', 'test', '--anchovies', '--no-onions', '--tomatoes', '--olives', '--no-sauce', '--crust', 'thin', '--cheese', 'wensleydale']); program.should.have.property('anchovies', true); -program.should.have.property('onions', true); +program.should.have.property('onions', false); program.should.have.property('tomatoes', true); program.should.have.property('olives', 'black'); program.should.have.property('sauce', false); diff --git a/test/test.options.defaults.js b/test/test.options.defaults.js index 3881c163f..5dee92d99 100644 --- a/test/test.options.defaults.js +++ b/test/test.options.defaults.js @@ -1,14 +1,10 @@ -/** - * Module dependencies. - */ - -var program = require('../') - , should = require('should'); +const program = require('../'); +// eslint-disable-next-line no-unused-vars +const should = require('should'); program - .version('0.0.1') .option('-a, --anchovies', 'Add anchovies?') - .option('-o, --onions', 'Add onions?') + .option('-o, --onions', 'Add onions?', true) .option('-O, --no-onions', 'No onions') .option('-t, --tomatoes', 'Add tomatoes?', false) .option('-T, --no-tomatoes', 'No tomatoes') @@ -17,12 +13,10 @@ program .option('-r, --crust ', 'What kind of crust would you like?', 'hand-tossed') .option('-c, --cheese [type]', 'optionally specify the type of cheese', 'mozzarella'); -program.should.have.property('_name', ''); - program.parse(['node', 'test']); -program.should.have.property('_name', 'test'); + program.should.not.have.property('anchovies'); -program.should.not.have.property('onions'); +program.should.have.property('onions', true); program.should.have.property('tomatoes', false); program.should.not.have.property('olives'); program.should.have.property('sauce', true); From b3ccac24763b0af0795c39b39fd8c63e18cc526a Mon Sep 17 00:00:00 2001 From: John Gee Date: Wed, 24 Jul 2019 22:20:52 +1200 Subject: [PATCH 3/5] Add written description of boolean default value for --foo/--no-foo --- Readme.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Readme.md b/Readme.md index 9ec05af3f..6d1091be5 100644 --- a/Readme.md +++ b/Readme.md @@ -101,8 +101,9 @@ cheese: stilton You can specify a boolean option long name with a leading `no-` to set the option value to false when used. Defined alone this also makes the option true by default. -If you define `foo` first, adding `--no-foo` does not change the default value. +If you define `--foo` first, adding `--no-foo` does not change the default value from what it would +otherwise be. You can specify a default boolean value for a boolean flag and it can be overridden on command line. ```js const program = require('commander'); From e346d6767499de7b68bd960460d3a3a256d6f2ae Mon Sep 17 00:00:00 2001 From: John Gee Date: Thu, 25 Jul 2019 19:45:21 +1200 Subject: [PATCH 4/5] Avoid eslint warnings for requiring shouldjs in simple way --- test/test.options.bool.js | 3 +-- test/test.options.bool.no.js | 3 +-- test/test.options.defaults.given.js | 3 +-- test/test.options.defaults.js | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/test/test.options.bool.js b/test/test.options.bool.js index 352495609..3fb22018f 100644 --- a/test/test.options.bool.js +++ b/test/test.options.bool.js @@ -1,6 +1,5 @@ const commander = require('../'); -// eslint-disable-next-line no-unused-vars -const should = require('should'); +require('should'); // Test simple flag and negatable flag diff --git a/test/test.options.bool.no.js b/test/test.options.bool.no.js index 09b15350f..caab58351 100644 --- a/test/test.options.bool.no.js +++ b/test/test.options.bool.no.js @@ -1,6 +1,5 @@ const commander = require('../'); -// eslint-disable-next-line no-unused-vars -const should = require('should'); +require('should'); // Test combination of flag and --no-flag // (negatable flag on its own is tested in test.options.bool.js) diff --git a/test/test.options.defaults.given.js b/test/test.options.defaults.given.js index f78fb08c3..b9bf46c10 100644 --- a/test/test.options.defaults.given.js +++ b/test/test.options.defaults.given.js @@ -1,6 +1,5 @@ const program = require('../'); -// eslint-disable-next-line no-unused-vars -const should = require('should'); +require('should'); program .option('-a, --anchovies', 'Add anchovies?') diff --git a/test/test.options.defaults.js b/test/test.options.defaults.js index 5dee92d99..54b72fc40 100644 --- a/test/test.options.defaults.js +++ b/test/test.options.defaults.js @@ -1,6 +1,5 @@ const program = require('../'); -// eslint-disable-next-line no-unused-vars -const should = require('should'); +require('should'); program .option('-a, --anchovies', 'Add anchovies?') From bb4af520453c91f977a50c87a87381e45aca6477 Mon Sep 17 00:00:00 2001 From: John Gee Date: Thu, 25 Jul 2019 19:52:24 +1200 Subject: [PATCH 5/5] Restore test.options.bool.small.js, KISS --- test/test.options.bool.js | 5 ----- test/test.options.bool.small.js | 11 +++++++++++ 2 files changed, 11 insertions(+), 5 deletions(-) create mode 100644 test/test.options.bool.small.js diff --git a/test/test.options.bool.js b/test/test.options.bool.js index 3fb22018f..e992b9ea6 100644 --- a/test/test.options.bool.js +++ b/test/test.options.bool.js @@ -20,8 +20,3 @@ const simpleFlagLong = simpleFlagProgram(); simpleFlagLong.parse(['node', 'test', '--pepper', '--no-cheese']); simpleFlagLong.pepper.should.be.true(); simpleFlagLong.cheese.should.be.false(); - -const simpleFlagShort = simpleFlagProgram(); -simpleFlagShort.parse(['node', 'test', '-p', '-C']); -simpleFlagShort.pepper.should.be.true(); -simpleFlagShort.cheese.should.be.false(); diff --git a/test/test.options.bool.small.js b/test/test.options.bool.small.js new file mode 100644 index 000000000..0199450fd --- /dev/null +++ b/test/test.options.bool.small.js @@ -0,0 +1,11 @@ +var program = require('../'); +require('should'); + +program + .version('0.0.1') + .option('-p, --pepper', 'add pepper') + .option('-c, --no-cheese', 'remove cheese'); + +program.parse(['node', 'test', '-p', '-c']); +program.pepper.should.be.true(); +program.cheese.should.be.false();