Allow boolean default for flag option (#987)

* Add support for default value for boolean flags

* Expand testing for boolean flags to cover new features

* Add written description of boolean default value for --foo/--no-foo

* Avoid eslint warnings for requiring shouldjs in simple way

Author: shadowspawn

Readme.md

@@ -100,8 +100,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');

index.js

@@ -371,8 +371,8 @@ Command.prototype.option = function(flags, description, fn, defaultValue) {
     }
   }
 
-  // preassign default value only for --no-*, [optional], or <required>
-  if (option.negate || option.optional || option.required) {
+  // preassign default value for --no-*, [optional], <required>, 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();
@@ -396,7 +396,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) {

test/test.options.bool.js

@@ -1,15 +1,22 @@
-/**
- * Module dependencies.
- */
+const commander = require('../');
+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();

test/test.options.bool.no.js

@@ -1,31 +1,63 @@
-/**
- * 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('../');
+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();

test/test.options.bool.small.js

@@ -1,9 +1,5 @@
-/**
- * Module dependencies.
- */
-
-var program = require('../')
-  , should = require('should');
+var program = require('../');
+require('should');
 
 program
   .version('0.0.1')

test/test.options.defaults.given.js

@@ -1,22 +1,21 @@
-/**
- * Module dependencies.
- */
-
-var program = require('../')
-  , should = require('should');
+const program = require('../');
+require('should');
 
 program
-  .version('0.0.1')
   .option('-a, --anchovies', 'Add anchovies?')
   .option('-o, --onions', 'Add onions?', true)
+  .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 <type>', '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', '--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);
 program.should.have.property('crust', 'thin');

test/test.options.defaults.js

@@ -1,25 +1,22 @@
-/**
- * Module dependencies.
- */
-
-var program = require('../')
-  , should = require('should');
+const program = require('../');
+require('should');
 
 program
-  .version('0.0.1')
   .option('-a, --anchovies', 'Add anchovies?')
   .option('-o, --onions', 'Add onions?', true)
+  .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 <type>', '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);
 program.should.have.property('crust', 'hand-tossed');