Support defaults for boolean options (including --no-*), for env var set defaults

[michael-crawford]

It doesn't appear we can set boolean option default values, so that we can set a default value for such options via environment variables, then override these values from the command line. I can do this for options which take a value, and would like this sort of override behavior to be consistent for both option types.

We have two use cases. Default true vs default false. For the description below, assume:

Default true use-case: We want to have a method to monitor progress of the program, the options.monitor boolean should default to false. If the user has a MONITOR environment variable set to true (or 1), this boolean should instead default to true. The user can then, on a case-by-case basis, override the false default with --monitor, or override the true (set via env var) default with --no-monitor. I'd like an ability to just turn on monitoring system-wide via an env var, not have to specify --monitor each time I run a CLI command. But, having changed the default system-wide, I may want to do the opposite on a single command.

Default false use-case: We want to have a method to perform additional validation checks, the options.check boolean should default to true. If the user has a NOCHECK (or NO_CHECK) environment variable set to true (or 1), this should instead default to false. The user can then, on a case-by-case basis, override the true default with --no-check, or override the false (set by env var) default with --check. As above, I'd like to turn off these extra checks system-wide via an env var, not have to specify --no-check each time I run a CLI command.

I was asked to open this issue as a new ticket, based on a comment to a related issue (#736). I've provided a cleaner description of the two use-cases above, but below is the original description of my use-cases.

• options.monitor should default to false or undefined, but have a way to be set to true
• options.check should default to true, but have a way to be set to false.

Current design as I understand it seems to support what I want as explicit command line options:

• Add "--monitor" to CLI to get options.monitor = true, otherwise it's undefined
• Add "--no-check" to CLI to get options.check = false, otherwise it's true

But I can't see how to make this work with default values, allowing me to set the default behavior via environment variables, but still allow for a command-line override.

• If MONITOR env var exists and is 1 or true, options.monitor should default to true, and "--monitor" should have no effect. Otherwise, options.monitor should default to undefined or false, and "--monitor" would then set it to true.
• If NOCHECK env var exists and is 1 or true, options.check should default to false, and "--check" should override this default to set it to true. Otherwise, options.check should default to true, and "--no-check" should override this and set it to false.

I thought this should work, but it doesn't. If there's not a way to do this, there should be.

let MONITOR = (process.env.MONITOR && (process.env.MONITOR == 'true' || process.env.MONITOR == '1')) ? true : false;
let CHECK = (process.env.NOCHECK && (process.env.NOCHECK == 'true' || process.env.NOCHECK == '1')) ? false : true;

program.command('test')
  .option('-m, --monitor', 'Monitor build process', MONITOR)
  .option('-C, --no-check', 'Do not check prerequisites', CHECK)

[shadowspawn]

Thanks for the detailed description of a good use case and expected behaviours.

On investigation, the problem lies in the underlying code. The default value for a boolean option is ignored (i.e. an option that does not take a value), presumably because author was thinking of testing for presence of the option rather than its value. This earlier comment tracked it down too: #574 (comment) (and also related #108 and #691).

  // preassign default value only for --no-*, [optional], or <required>
  if (false == option.bool || option.optional || option.required) {

There are also discussions around improving support for mixing --foo and --no-foo, I didn't chase that aspect yet.

A possible work-around for current behaviour is to give the option a value, like:

  .option('-m, --monitor <true/false>', 'Monitor build process', MONITOR)

$ MONITOR=1 node . test 
MONITOR is true
options.monitor is true
$ MONITOR=0 node . test 
MONITOR is false
options.monitor is false
$ MONITOR=0 node . test --monitor true
MONITOR is false
options.monitor is true
$ MONITOR=1 node . test --monitor false
MONITOR is true
options.monitor is false

[michael-crawford]

I found that code too, but coudn't quite follow how it worked or was supposed to work.

I'm doing exactly the workaround you note, with a regex to validate true/false. This makes it work the same as other options which take a value like '--environment Production', but it's less than ideal.

Hopefully this doesn't come across as arrogant for a newbie to this discussion, but I really do think what I've proposed is how these boolean flags should work from a user's perspective:

• It should be possible to use '--flag' to turn on a flag, and '--no-flag' to turn off a flag. That seems to be a convention I've seen used in a lot of other open source software. This appears to mostly work, but it seems there some flaky behavior around the -no-* variant which is being improved.
• It should be possible to set the default true/false value of these flags through the default mechanism, in a way that works the same as other options that take a value. This would allow us to set these defaults through environment variables, or configuration files, on a global basis applying to all invocations of a command, but still allow a user to override true -> false or false -> true when running a specific invocation. This is also behavior I've seen in many other places in open source.

What I'm building has overrides at multiple levels. So, I have a default baked into the code, then a system configuration tree, with a default path, which can be overridden by the EXAMPLE_SYSCONFIG env var, a per-user default like ~/.example (containing config.json), which can be overridden by the EXAMPLE_USRCONFIG env var, then for some key values, per option env vars like EXAMPLE_MONITOR, EXAMPLE_CHECK, EXAMPLE_ACCOUNT, EXAMPLE_ENVIRONMENT, as I may want to turn on or off this behavior system-wide in some environments where this is used.

But last, I still want to have a way to override the baked in default, system override default, per-user override default, specific env var override default, on the command line by --monitor or --no-monitor. This type of multi-level defaults with override behavior is something I've seen often in sophisticated software, but first time I'm trying to do this in Node. I'm building a framework to manage building AWS CloudFormation based architectures for a large tech giant that can be used for multiple clients, so it needs a lot of flexibility.

I'm using the workaround, but how long does it usually take for an idea like this, which would seem to be useful to many and straightforward to implement, to make it into a release?

[michael-crawford]

I now have some code which gets me pretty close to what I was looking for, which may be helpful to those looking to do something similar to what I am, while waiting for this issue to perhaps get implemented. It's still not ideal, in that it doesn't exactly match the CLI "UI" I see elsewhere.

I can set the default value of a boolean to true or false via an env var, turn this on (set to true) via '-c' or '--check' (see example above), regardless of if the env var set default is true or false. So, this works exactly as I want.

However, I can't use the '--no-check' syntax to turn off this boolean. Instead, I use '-c false', '-c 0', --check false' or '--check 0' to turn this off (set to false), again regardless of if the env set default is true or false. So, VERY close to what I wanted. I'd still like to see the '--no-flag' syntax work, but the pressure is off from me to have this working, so I thought others might benefit from this technique.

Here's the code, a modification of the "pizza" example. "monitor" should default to false. "check" should default to true.

#!/usr/bin/env node

const program = require('commander');

const MONITOR = (process.env.MONITOR && (process.env.MONITOR == 'true' || process.env.MONITOR == '1')) ? true : false;
const CHECK = (process.env.NOCHECK && (process.env.NOCHECK == 'true' || process.env.NOCHECK == '1')) ? false : true;

const parseBool = (val) => {
  re=/^(true|1|on)$/i;
  return re.test(val);
}

program
  .version('0.0.1')
  .usage('test')
  .option('-s --size <size>', 'Pizza size', /^(large|medium|small)$/i, 'medium')
  .option('-d --drink [drink]', 'Drink', /^(coke|pepsi|izze)$/i)
  .option('-m --monitor [false]', 'Monitor', parseBool, MONITOR)
  .option('-c --check [false]', 'Check', parseBool, CHECK)
  .parse(process.argv);
  
console.log(' size: %j', program.size);
console.log(' drink: %j', program.drink);
console.log(' monitor: %j', program.monitor);
console.log(' check: %j', program.check);

[shadowspawn]

Thinking of allowing default value for boolean flags in conjunction with #795 to support this pattern of defaults coming from env .

[usmonster]

Please see my comment on #795 for an alternative approach, which I think vastly reduces the need for this kind of feature to be built-in.

In summary, you can use custom event listeners to get the exact behavior you want without hacking around the option type.

Alternatively, you can skip using custom event listeners and simply add the conditional switch flipping logic after you parse the arguments, e.g. something like:

#!/usr/bin/env node

const program = require('commander');

program
  .option('-m, --monitor', 'Monitor build process')
  .option('-C, --no-check', 'Do not check prerequisites')
  .parse(process.argv);

const trueRe = /(?:true|1)/;
if ('MONITOR' in process.env) program.monitor = trueRe.test(process.env.MONITOR);
if ('NOCHECK' in process.env) program.check = !trueRe.test(process.env.NOCHECK);

console.log(' monitor: %j', program.monitor);
console.log(' check: %j', program.check);

Output is:

$ ./test.js
 monitor: undefined
 check: true
$ ./test.js --monitor --no-check
 monitor: true
 check: false
$ MONITOR=1 NOCHECK=true ./test.js --monitor --no-check
 monitor: true
 check: false
$ MONITOR=1 NOCHECK=true ./test.js
 monitor: true
 check: false
$ MONITOR=false NOCHECK=nope ./test.js
 monitor: false
 check: true
$ MONITOR=false NOCHECK=nope ./test.js --monitor --no-check
 monitor: false
 check: true
$ MONITOR= NOCHECK= ./test.js --monitor --no-check
 monitor: false
 check: true

Does this solution satisfy your requirements, @michael-crawford?

[shadowspawn]

@usmonster

I think the desired behaviour is that the environment variable is optional and supplies the default value, and the command line flags override this. So for example (just for monitor):

$ MONITOR=false ./test.js --monitor
 monitor: true

[usmonster]

Oops! Sorry, I indeed misread. In this case, the two lines would instead change to:

// ...
if ('MONITOR' in process.env && !program.monitor) program.monitor = trueRe.test(process.env.MONITOR);
if (trueRe.test(process.env.NOCHECK)) program.check = !program.check;
// ...

...and so the output is now:

$ ./test.js
 monitor: undefined
 check: true
$ ./test.js --monitor --no-check
 monitor: true
 check: false
$ MONITOR=1 NOCHECK=true ./test.js --monitor --no-check
 monitor: true
 check: true
$ MONITOR=1 NOCHECK=true ./test.js
 monitor: true
 check: false
$ MONITOR=false NOCHECK=nope ./test.js
 monitor: false
 check: true
$ MONITOR=false NOCHECK=nope ./test.js --monitor --no-check
 monitor: true
 check: false
$ MONITOR= NOCHECK= ./test.js --monitor --no-check
 monitor: true
 check: false

Thanks for the catch, @shadowspawn!

[shadowspawn]

I have opened a Pull Request related to this issue. See #987.

[shadowspawn]

This issue will be resolved when v3.0.0 is released. Available now as a prerelease. See #1001

[shadowspawn]

Support for combined --foo and --no-foo (thanks @usmonster), and for supplying a default boolean value has shipped in v3: https://github.com/tj/commander.js/releases/tag/v3.0.0