Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework option defaults and add preset #1652

Merged
merged 24 commits into from Dec 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
1cbf7e4
Add defaultOption, but not implemented
shadowspawn Dec 4, 2021
604f531
Start refactoring addOption to clarify logic
shadowspawn Dec 7, 2021
7c483ec
Update comments
shadowspawn Dec 7, 2021
ed7b74d
Implement separate default and preset for options
shadowspawn Dec 12, 2021
693e629
Changed behaviour of boolean default, fix and extend test
shadowspawn Dec 14, 2021
154adba
Update unit tests with new default/preset behaviour
shadowspawn Dec 15, 2021
ffcbced
Add tests for default with all option types
shadowspawn Dec 17, 2021
d15519d
Add tests that default does get overwritten
shadowspawn Dec 17, 2021
c2186c0
Fix JSDoc for new routines
shadowspawn Dec 17, 2021
b72b73e
Add tests for preset
shadowspawn Dec 17, 2021
3f96d6a
Add typings
shadowspawn Dec 18, 2021
b3a6c1c
Merge branch 'release/9.x' into feature/optional-value
shadowspawn Dec 18, 2021
da7635d
Merge branch 'release/9.x' into feature/optional-value
shadowspawn Dec 18, 2021
86739d0
Merge branch 'release/9.x' into feature/optional-value
shadowspawn Dec 18, 2021
4bddfe5
Update README
shadowspawn Dec 18, 2021
d606493
Add test of preset with negated
shadowspawn Dec 18, 2021
8ea7136
Update comments
shadowspawn Dec 18, 2021
86aa16d
Add tests of options being used twice
shadowspawn Dec 18, 2021
fe4c3ca
Modify documentation for preset
shadowspawn Dec 18, 2021
d5afd61
Add prefix example usage in README
shadowspawn Dec 18, 2021
9c0f036
Add preset to help
shadowspawn Dec 18, 2021
1426bb4
Update code example
shadowspawn Dec 18, 2021
fce2be8
Use same format for preset as for default, add test
shadowspawn Dec 18, 2021
9eec174
Be selective about default and preset shown in help
shadowspawn Dec 19, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 7 additions & 5 deletions Readme.md
Expand Up @@ -146,7 +146,7 @@ pizza details:

### Default option value

You can specify a default value for an option which takes a value.
You can specify a default value for an option.

Example file: [options-defaults.js](./examples/options-defaults.js)

Expand All @@ -172,7 +172,7 @@ You can define a boolean option long name with a leading `no-` to set the option
Defined alone this also makes the option true by default.

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 option and it can be overridden on command line.
otherwise be.

Example file: [options-negatable.js](./examples/options-negatable.js)

Expand Down Expand Up @@ -312,7 +312,8 @@ program
.addOption(new Option('-s, --secret').hideHelp())
.addOption(new Option('-t, --timeout <delay>', 'timeout in seconds').default(60, 'one minute'))
.addOption(new Option('-d, --drink <size>', 'drink size').choices(['small', 'medium', 'large']))
.addOption(new Option('-p, --port <number>', 'port number').env('PORT'));
.addOption(new Option('-p, --port <number>', 'port number').env('PORT'))
.addOption(new Option('--donate [amount]', 'optional donation in dollars').preset('20').argParser(parseFloat));
```

```bash
Expand All @@ -323,13 +324,14 @@ Options:
-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] optional donation in dollars (preset: 20)
-h, --help display help for command

$ extra --drink huge
error: option '-d, --drink <size>' argument 'huge' is invalid. Allowed choices are small, medium, large.

$ PORT=80 extra
Options: { timeout: 60, port: '80' }
$ PORT=80 extra --donate
Options: { timeout: 60, donate: 20, port: '80' }
```

### Custom option processing
Expand Down
5 changes: 4 additions & 1 deletion examples/options-extra.js
Expand Up @@ -11,7 +11,8 @@ program
.addOption(new Option('-s, --secret').hideHelp())
.addOption(new Option('-t, --timeout <delay>', 'timeout in seconds').default(60, 'one minute'))
.addOption(new Option('-d, --drink <size>', 'drink cup size').choices(['small', 'medium', 'large']))
.addOption(new Option('-p, --port <number>', 'port number').env('PORT'));
.addOption(new Option('-p, --port <number>', 'port number').env('PORT'))
.addOption(new Option('--donate [amount]', 'optional donation in dollars').preset('20').argParser(parseFloat));

program.parse();

Expand All @@ -21,3 +22,5 @@ console.log('Options: ', program.opts());
// node options-extra.js --help
// node options-extra.js --drink huge
// PORT=80 node options-extra.js
// node options-extra.js --donate
// node options-extra.js --donate 30.50
49 changes: 24 additions & 25 deletions lib/command.js
Expand Up @@ -508,33 +508,33 @@ Expecting one of '${allowedValues.join("', '")}'`);
const oname = option.name();
const name = option.attributeName();

let defaultValue = option.defaultValue;

// 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) {
const positiveLongFlag = option.long.replace(/^--no-/, '--');
defaultValue = this._findOption(positiveLongFlag) ? this.getOptionValue(name) : true;
}
// preassign only if we have a default
if (defaultValue !== undefined) {
this.setOptionValueWithSource(name, defaultValue, 'default');
// store default value
if (option.negate) {
// --no-foo is special and defaults foo to true, unless a --foo option is already defined
const positiveLongFlag = option.long.replace(/^--no-/, '--');
if (!this._findOption(positiveLongFlag)) {
this.setOptionValueWithSource(name, option.defaultValue === undefined ? true : option.defaultValue, 'default');
}
} else if (option.defaultValue !== undefined) {
this.setOptionValueWithSource(name, option.defaultValue, 'default');
}

// register the option
this.options.push(option);

// handler for cli and env supplied values
const handleOptionValue = (val, invalidValueMessage, valueSource) => {
// Note: using closure to access lots of lexical scoped variables.
const oldValue = this.getOptionValue(name);
// val is null for optional option used without an optional-argument.
// val is undefined for boolean and negated option.
if (val == null && option.presetArg !== undefined) {
val = option.presetArg;
}

// custom processing
const oldValue = this.getOptionValue(name);
if (val !== null && option.parseArg) {
try {
val = option.parseArg(val, oldValue === undefined ? defaultValue : oldValue);
val = option.parseArg(val, oldValue);
} catch (err) {
if (err.code === 'commander.invalidArgument') {
const message = `${invalidValueMessage} ${err.message}`;
Expand All @@ -546,18 +546,17 @@ Expecting one of '${allowedValues.join("', '")}'`);
val = option._concatValue(val, oldValue);
}

// unassigned or boolean value
if (typeof oldValue === 'boolean' || typeof oldValue === 'undefined') {
// if no value, negate false, and we have a default, then use it!
if (val == null) {
this.setOptionValueWithSource(name, option.negate ? false : defaultValue || true, valueSource);
// Fill-in appropriate missing values. Long winded but easy to follow.
if (val == null) {
if (option.negate) {
val = false;
} else if (option.isBoolean() || option.optional) {
val = true;
} else {
this.setOptionValueWithSource(name, val, valueSource);
val = ''; // not normal, parseArg might have failed or be a mock function for testing
}
} else if (val !== null) {
// reassign
this.setOptionValueWithSource(name, option.negate ? false : val, valueSource);
}
this.setOptionValueWithSource(name, val, valueSource);
};

this.on('option:' + oname, (val) => {
Expand Down Expand Up @@ -813,7 +812,7 @@ Expecting one of '${allowedValues.join("', '")}'`);

/**
* Get user arguments from implied or explicit arguments.
* Side-effects: set _scriptPath if args included application, and use that to set implicit command name.
* Side-effects: set _scriptPath if args included script. Used for default program name, and subcommand searches.
*
* @api private
*/
Expand Down
17 changes: 13 additions & 4 deletions lib/help.js
Expand Up @@ -235,15 +235,24 @@ class Help {

optionDescription(option) {
const extraInfo = [];
// Some of these do not make sense for negated boolean and suppress for backwards compatibility.

if (option.argChoices && !option.negate) {
if (option.argChoices) {
extraInfo.push(
// use stringify to match the display of the default value
`choices: ${option.argChoices.map((choice) => JSON.stringify(choice)).join(', ')}`);
}
if (option.defaultValue !== undefined && !option.negate) {
extraInfo.push(`default: ${option.defaultValueDescription || JSON.stringify(option.defaultValue)}`);
if (option.defaultValue !== undefined) {
// default for boolean and negated more for programmer than end user,
// but show true/false for boolean option as may be for hand-rolled env or config processing.
const showDefault = option.required || option.optional ||
(option.isBoolean() && typeof option.defaultValue === 'boolean');
if (showDefault) {
extraInfo.push(`default: ${option.defaultValueDescription || JSON.stringify(option.defaultValue)}`);
}
}
// preset for boolean and negated are more for programmer than end user
if (option.presetArg !== undefined && option.optional) {
extraInfo.push(`preset: ${JSON.stringify(option.presetArg)}`);
}
if (option.envVar !== undefined) {
extraInfo.push(`env: ${option.envVar}`);
Expand Down
31 changes: 31 additions & 0 deletions lib/option.js
Expand Up @@ -28,6 +28,7 @@ class Option {
}
this.defaultValue = undefined;
this.defaultValueDescription = undefined;
this.presetArg = undefined;
this.envVar = undefined;
this.parseArg = undefined;
this.hidden = false;
Expand All @@ -48,6 +49,23 @@ class Option {
return this;
};

/**
* Preset to use when option used without option-argument, especially optional but also boolean and negated.
* The custom processing (parseArg) is called.
*
* @example
* new Option('--color').default('GREYSCALE').preset('RGB');
* new Option('--donate [amount]').preset('20').argParser(parseFloat);
*
* @param {any} arg
* @return {Option}
*/

preset(arg) {
this.presetArg = arg;
return this;
};

/**
* Set environment variable to check for option value.
* Priority order of option values is default < env < cli
Expand Down Expand Up @@ -166,6 +184,19 @@ class Option {
is(arg) {
return this.short === arg || this.long === arg;
};

/**
* Return whether a boolean option.
*
* Options are one of boolean, negated, required argument, or optional argument.
*
* @return {boolean}
* @api private
*/

isBoolean() {
return !this.required && !this.optional && !this.negate;
};
}

/**
Expand Down
30 changes: 25 additions & 5 deletions tests/help.optionDescription.test.js
Expand Up @@ -17,13 +17,33 @@ describe('optionDescription', () => {
expect(helper.optionDescription(option)).toEqual(description);
});

test('when option has default value then return description and default value', () => {
test('when boolean option has default value true then return description and default value', () => {
const description = 'description';
const option = new commander.Option('-a', description).default('default');
const option = new commander.Option('-a', description).default(true);
const helper = new commander.Help();
expect(helper.optionDescription(option)).toEqual('description (default: "default")');
expect(helper.optionDescription(option)).toEqual('description (default: true)');
});

test('when boolean option has default value string then return description without default', () => {
const description = 'description';
const option = new commander.Option('-a', description).default('foo');
const helper = new commander.Help();
expect(helper.optionDescription(option)).toEqual('description');
});

test('when optional option has preset value then return description and default value', () => {
const description = 'description';
const option = new commander.Option('--aa [value]', description).preset('abc');
const helper = new commander.Help();
expect(helper.optionDescription(option)).toEqual('description (preset: "abc")');
});

test('when boolean option has preset value then return description without default', () => {
const description = 'description';
const option = new commander.Option('--bb', description).preset('abc');
const helper = new commander.Help();
expect(helper.optionDescription(option)).toEqual('description');
});
test('when option has env then return description and env name', () => {
const description = 'description';
const option = new commander.Option('-a', description).env('ENV');
Expand All @@ -34,15 +54,15 @@ describe('optionDescription', () => {
test('when option has default value description then return description and custom default description', () => {
const description = 'description';
const defaultValueDescription = 'custom';
const option = new commander.Option('-a', description).default('default value', defaultValueDescription);
const option = new commander.Option('-a <value>', description).default('default value', defaultValueDescription);
const helper = new commander.Help();
expect(helper.optionDescription(option)).toEqual(`description (default: ${defaultValueDescription})`);
});

test('when option has choices then return description and choices', () => {
const description = 'description';
const choices = ['one', 'two'];
const option = new commander.Option('-a', description).choices(choices);
const option = new commander.Option('-a <value>', description).choices(choices);
const helper = new commander.Help();
expect(helper.optionDescription(option)).toEqual('description (choices: "one", "two")');
});
Expand Down
51 changes: 37 additions & 14 deletions tests/options.bool.combo.test.js
Expand Up @@ -96,28 +96,51 @@ describe('boolean option combo, default false, short flags', () => {
});
});

// This is a somewhat undocumented special behaviour which appears in some examples.
// When a flag has a non-boolean default, it is used as the value (only) when the flag is specified.
//
// boolean option combo with non-boolean default
// boolean option combo with non-boolean default.
// Changed behaviour to normal default in Commander 9.
describe('boolean option combo with non-boolean default', () => {
test('when boolean combo not specified then value is undefined', () => {
const flagValue = 'red';
const program = createPepperProgramWithDefault(flagValue);
test('when boolean combo not specified then value is default', () => {
const program = createPepperProgramWithDefault('default');
program.parse(['node', 'test']);
expect(program.opts().pepper).toBeUndefined();
expect(program.opts().pepper).toBe('default');
});

test('when boolean combo positive then value is true', () => {
const program = createPepperProgramWithDefault('default');
program.parse(['node', 'test', '--pepper']);
expect(program.opts().pepper).toBe(true);
});

test('when boolean combo negative then value is false', () => {
const program = createPepperProgramWithDefault('default');
program.parse(['node', 'test', '--no-pepper']);
expect(program.opts().pepper).toBe(false);
});
});

describe('boolean option combo with non-boolean default and preset', () => {
function createPepperProgramWithDefaultAndPreset() {
const program = new commander.Command();
program
.addOption(new commander.Option('-p, --pepper').default('default').preset('preset'))
.option('-P, --no-pepper', 'remove pepper');
return program;
}

test('when boolean combo not specified then value is default', () => {
const program = createPepperProgramWithDefaultAndPreset();
program.parse(['node', 'test']);
expect(program.opts().pepper).toBe('default');
});

test('when boolean combo positive then value is "default" value', () => {
const flagValue = 'red';
const program = createPepperProgramWithDefault(flagValue);
test('when boolean combo positive then value is preset', () => {
const program = createPepperProgramWithDefaultAndPreset();
program.parse(['node', 'test', '--pepper']);
expect(program.opts().pepper).toBe(flagValue);
expect(program.opts().pepper).toBe('preset');
});

test('when boolean combo negative then value is false', () => {
const flagValue = 'red';
const program = createPepperProgramWithDefault(flagValue);
const program = createPepperProgramWithDefaultAndPreset();
program.parse(['node', 'test', '--no-pepper']);
expect(program.opts().pepper).toBe(false);
});
Expand Down
21 changes: 10 additions & 11 deletions tests/options.bool.test.js
Expand Up @@ -84,34 +84,33 @@ describe('boolean flag on command', () => {
});
});

// This is a somewhat undocumented special behaviour which appears in some examples.
// When a flag has a non-boolean default, it is used as the value (only) when the flag is specified.
//
// boolean flag with non-boolean default
// NB: behaviour changed in Commander v9 to have default be default.
// These tests no longer match likely uses, but retained and updated to match current behaviour.
describe('boolean flag with non-boolean default', () => {
test('when flag not specified then value is undefined', () => {
test('when flag not specified then value is "default"', () => {
const flagValue = 'black';
const program = new commander.Command();
program
.option('--olives', 'Add olives? Sorry we only have black.', flagValue);
.option('--olives', 'Add green olives?', flagValue);
program.parse(['node', 'test']);
expect(program.opts().olives).toBeUndefined();
expect(program.opts().olives).toBe(flagValue);
});

test('when flag specified then value is "default" value', () => {
test('when flag specified then value is true', () => {
const flagValue = 'black';
const program = new commander.Command();
program
.option('-v, --olives', 'Add olives? Sorry we only have black.', flagValue);
.option('-v, --olives', 'Add green olives?', flagValue);
program.parse(['node', 'test', '--olives']);
expect(program.opts().olives).toBe(flagValue);
expect(program.opts().olives).toBe(true);
});

test('when flag implied and negated then value is false', () => {
test('when combo flag and negated then value is false', () => {
const flagValue = 'black';
const program = new commander.Command();
program
.option('-v, --olives', 'Add olives? Sorry we only have black.', flagValue)
.option('-v, --olives', 'Add green olives?', flagValue)
.option('--no-olives');
program.parse(['node', 'test', '--olives', '--no-olives']);
expect(program.opts().olives).toBe(false);
Expand Down