Rework option defaults and add preset

[shadowspawn]

Pull Request

Problem

1. Using "default" value with boolean options and optional options is a bit fragile and buggy.

• non-boolean default used with boolean option is intended to replace "true" when option used, but different behaviour than normal default, never documented in README, and bugs when option repeated
• default used with optional does set the default, but breaks the use of optional without an option argument which is just ignored
• behaviour varies depending on previous value

2. No way to set value for optional used without argument .

3. Optional used without argument does not trigger custom processing, complicating some uses.

4. This area of code is a problem for adding features to the code with some difficult to understand behaviours and intentions.

Matching issue: #1648

Related: #116 #440 #928 #984 #1135 #1201 #1328 #1394

Solution

1. Make default consistently set just the default (initial) value for boolean, required, and optional options. Fix use of optional with a default value.

2. Add Option.preset() for explicitly setting the preset value to use instead of true with a boolean option, or optional without an option argument.

3. Using preset does trigger custom processing.

ChangeLog

• fixed: option with optional argument not supplied on command line now works when option already has a value, whether from default value or from previous arguments
• changed: default value specified for boolean option now always used as default value (see .preset() to match some previous behaviours)
• changed: default value for boolean option only shown in help if true/false
• added: Option.preset() allows specifying value/arg for option when used without option-argument (especially optional, but also boolean option)

[shadowspawn]

The core of the processing dates all the way back to at least v1.1.0. For interest:

v1.1.0 code setting default value (note, not including boolean):

// preassign default value only for --no-*, [optional], or <required>
if (false == option.bool || option.optional || option.required) {
    // when --no-* we make sure default is true
    if (false == option.bool) defaultValue = true;
    // preassign only if we have a default
    if (undefined !== defaultValue) self[name] = defaultValue;
 }

v1.1.0 code setting new option value. Note: null and undefined val. Default value being used in multiple places. Choosing behaviour based on previous value. Tricky!

    if (null !== val && fn) val = fn(val, undefined === self[name]
      ? defaultValue
      : self[name]);

    // unassigned or bool
    if ('boolean' == typeof self[name] || 'undefined' == typeof self[name]) {
      // if no value, bool true, and we have a default, then use it!
      if (null == val) {
        self[name] = option.bool
          ? defaultValue || true
          : false;
      } else {
        self[name] = val;
      }
    } else if (null !== val) {
      // reassign
      self[name] = val;
    }
    /// else ignoring optional so value unchanged
  });

[shadowspawn]

This PR changes the boolean behaviour with defaults, which is a breaking change. The use of defaults with boolean options has not been much documented so hopefully not in widespread use.

Long version of behaviours.

program
  .option('--plain', 'option without a default')
  .option('--default-false', 'option with a boolean default', false)
  .option('--default-string', 'option with a non-boolean default', 'stringy');

program.parse(process.argv);
console.log(program.opts());

commander@2

• "default" is not used as default
• double option with non-boolean default returns to undefined

% node bug1
{ plain: undefined, defaultFalse: undefined, defaultString: undefined }
% node bug1 --plain --default-false --default-string  
{ plain: true, defaultFalse: true, defaultString: 'stringy' }
% node bug1 --plain --plain --default-false --default-false --default-string --default-string
{ plain: true, defaultFalse: true, defaultString: undefined }

commander@8

• "default" is not used as default, except for boolean values
• double option with non-boolean default returns to undefined

% node bug1                                                                                 
{ defaultFalse: false }
% node bug1 --plain --default-false --default-string                                        
{ defaultFalse: true, plain: true, defaultString: 'stringy' }
% node bug1 --plain --plain --default-false --default-false --default-string --default-string
{ defaultFalse: true, plain: true, defaultString: undefined }

---

This PR.

• "default" is used as default

% node bug1                                                                                 
{ defaultFalse: false, defaultString: 'stringy' }
% node bug1 --plain --default-false --default-string                                        
{ defaultFalse: true, defaultString: true, plain: true }
% node bug1 --plain --plain --default-false --default-false --default-string --default-string
{ defaultFalse: true, defaultString: true, plain: true }

And new behaviour available with preset.

program.addOption(new Option('--preset-string').preset('stringy'));

% node  bug1
{}
% node  bug1 --preset-string
{ presetString: 'stringy' }
% node  bug1 --preset-string --preset-string
{ presetString: 'stringy' }

[shadowspawn]

I should probably add the preset to the usage in the help?

  -t, --timeout <delay>  timeout in seconds (default: one minute)
  -d, --drink <size>     drink cup size (choices: "small", "medium", "large")
  -p, --port <number>    port number (env: PORT)
  --donate [amount]      (preset: 20)

Edit: Done.

[shadowspawn]

This PR fixes the behaviour of optional used without an option-argument, so it works when option already has a value whether from default or from previous arguments on command line.

program
  .option('-o, --optional [value]')
  .option('-p [value]', 'description', 'default');

// program.P = 'oops';

program.parse(process.argv);
console.log(program.opts());

Commander 8

% node bug2.js           
{ p: 'default' }
% node bug2.js -o -p 
{ p: 'default', optional: true } # BUG
% node bug2.js -o cli -p cli
{ p: 'cli', optional: 'cli' }
% node bug2.js -o cli -p cli -o -p
{ p: 'cli', optional: 'cli' } #BUG

Behaviour with this PR

% node bug2.js                    
{ p: 'default' }
% node bug2.js -o -p              
{ p: true, optional: true }
% node bug2.js -o cli -p cli      
{ p: 'cli', optional: 'cli' }
% node bug2.js -o cli -p cli -o -p
{ p: true, optional: true }

[abetomo]

Great work!

[shadowspawn]

Commander v9 has been released.