Skip to content

Commit

Permalink
Add support for variadic options (#1250)
Browse files Browse the repository at this point in the history
* First cut at variadic option implementation

* Rework variadic regexp
- remove ellipsis for symmetry with .arguments
- require word character before dots

* Add tests for variadic options

* Add test on non-variadic pattern

* Rethink interation of variadic and coercion function

* Fix typos

* Raise ecmaVersion to eliminate some false positive errors in example files

* Improve variadic option with optional value, zero or more values

* Reorder comparison to match previous test

* Use consistent test names

* Add example and README for variadic option

* Fix typo
  • Loading branch information
shadowspawn authored Jun 1, 2020
1 parent e9a6109 commit 913389f
Show file tree
Hide file tree
Showing 10 changed files with 243 additions and 12 deletions.
3 changes: 3 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ const typescriptSettings = {

module.exports = {
plugins: ['jest'],
parserOptions: {
ecmaVersion: 8
},
overrides: [
javascriptSettings,
typescriptSettings
Expand Down
33 changes: 33 additions & 0 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Read this in other languages: English | [简体中文](./Readme_zh-CN.md)
- [Other option types, negatable boolean and flag|value](#other-option-types-negatable-boolean-and-flagvalue)
- [Custom option processing](#custom-option-processing)
- [Required option](#required-option)
- [Variadic option](#variadic-option)
- [Version option](#version-option)
- [Commands](#commands)
- [Specify the argument syntax](#specify-the-argument-syntax)
Expand Down Expand Up @@ -278,6 +279,38 @@ $ pizza
error: required option '-c, --cheese <type>' not specified
```
### Variadic option
You may make an option variadic by appending `...` to the value placeholder when declaring the option. On the command line you
can then specify multiple option arguments, and the parsed option value will be an array. The extra arguments
are read until the first argument starting with a dash. The special argument `--` stops option processing entirely. If a value
is specified in the same argument as the option then no further values are read.
Example file: [options-variadic.js](./examples/options-variadic.js)
```js
program
.option('-n, --number <numbers...>', 'specify numbers')
.option('-l, --letter [letters...]', 'specify letters');
program.parse();
console.log('Options: ', program.opts());
console.log('Remaining arguments: ', program.args);
```
```bash
$ collect -n 1 2 3 --letter a b c
Options: { number: [ '1', '2', '3' ], letter: [ 'a', 'b', 'c' ] }
Remaining arguments: []
$ collect --letter=A -n80 operand
Options: { number: [ '80' ], letter: [ 'A' ] }
Remaining arguments: [ 'operand' ]
$ collect --letter -n 1 -n 2 3 -- operand
Options: { number: [ '1', '2', '3' ], letter: true }
Remaining arguments: [ 'operand' ]
```
### Version option
The optional `version` method adds handling for displaying the command version. The default option flags are `-V` and `--version`, and when present the command prints the version number and exits.
Expand Down
21 changes: 21 additions & 0 deletions examples/options-variadic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#!/usr/bin/env node

// This is used as an example in the README for variadic options.

// const commander = require('commander'); // (normal include)
const commander = require('../'); // include commander in git clone of commander repo
const program = new commander.Command();

program
.option('-n, --number <value...>', 'specify numbers')
.option('-l, --letter [value...]', 'specify letters');

program.parse();

console.log('Options: ', program.opts());
console.log('Remaining arguments: ', program.args);

// Try the following:
// node options-variadic.js -n 1 2 3 --letter a b c
// node options-variadic.js --letter=A -n80 operand
// node options-variadic.js --letter -n 1 -n 2 3 -- operand
32 changes: 25 additions & 7 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ class Option {
this.flags = flags;
this.required = flags.includes('<'); // A value must be supplied when the option is specified.
this.optional = flags.includes('['); // A value is optional when the option is specified.
// variadic test ignores <value,...> et al which might be used to describe custom splitting of single argument
this.variadic = /\w\.\.\.[>\]]$/.test(flags); // The option can take multiple values.
this.mandatory = false; // The option must have a value after parsing, which usually means it must be specified on command line.
const optionFlags = _parseOptionFlags(flags);
this.short = optionFlags.shortFlag;
Expand Down Expand Up @@ -266,7 +268,7 @@ class Command extends EventEmitter {
*
* addHelpCommand() // force on
* addHelpCommand(false); // force off
* addHelpCommand('help [cmd]', 'display help for [cmd]'); // force on with custom detais
* addHelpCommand('help [cmd]', 'display help for [cmd]'); // force on with custom details
*
* @return {Command} `this` command for chaining
* @api public
Expand Down Expand Up @@ -434,7 +436,7 @@ class Command extends EventEmitter {
* @param {Object} config
* @param {string} flags
* @param {string} description
* @param {Function|*} [fn] - custom option processing function or default vaue
* @param {Function|*} [fn] - custom option processing function or default value
* @param {*} [defaultValue]
* @return {Command} `this` command for chaining
* @api private
Expand Down Expand Up @@ -482,13 +484,21 @@ class Command extends EventEmitter {
// when it's passed assign the value
// and conditionally invoke the callback
this.on('option:' + oname, (val) => {
// coercion
const oldValue = this._getOptionValue(name);

// custom processing
if (val !== null && fn) {
val = fn(val, this._getOptionValue(name) === undefined ? defaultValue : this._getOptionValue(name));
val = fn(val, oldValue === undefined ? defaultValue : oldValue);
} else if (val !== null && option.variadic) {
if (oldValue === defaultValue || !Array.isArray(oldValue)) {
val = [val];
} else {
val = oldValue.concat(val);
}
}

// unassigned or boolean value
if (typeof this._getOptionValue(name) === 'boolean' || typeof this._getOptionValue(name) === 'undefined') {
if (typeof oldValue === 'boolean' || typeof oldValue === 'undefined') {
// if no value, negate false, and we have a default, then use it!
if (val == null) {
this._setOptionValue(name, option.negate
Expand Down Expand Up @@ -552,7 +562,7 @@ class Command extends EventEmitter {
*
* @param {string} flags
* @param {string} description
* @param {Function|*} [fn] - custom option processing function or default vaue
* @param {Function|*} [fn] - custom option processing function or default value
* @param {*} [defaultValue]
* @return {Command} `this` command for chaining
* @api public
Expand All @@ -570,7 +580,7 @@ class Command extends EventEmitter {
*
* @param {string} flags
* @param {string} description
* @param {Function|*} [fn] - custom option processing function or default vaue
* @param {Function|*} [fn] - custom option processing function or default value
* @param {*} [defaultValue]
* @return {Command} `this` command for chaining
* @api public
Expand Down Expand Up @@ -1004,6 +1014,7 @@ class Command extends EventEmitter {
}

// parse options
let activeVariadicOption = null;
while (args.length) {
const arg = args.shift();

Expand All @@ -1014,6 +1025,12 @@ class Command extends EventEmitter {
break;
}

if (activeVariadicOption && !maybeOption(arg)) {
this.emit(`option:${activeVariadicOption.name()}`, arg);
continue;
}
activeVariadicOption = null;

if (maybeOption(arg)) {
const option = this._findOption(arg);
// recognised option, call listener to assign value with possible custom processing
Expand All @@ -1032,6 +1049,7 @@ class Command extends EventEmitter {
} else { // boolean flag
this.emit(`option:${option.name()}`);
}
activeVariadicOption = option.variadic ? option : null;
continue;
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/command.addCommand.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ test('when commands added using .addCommand and .command then internals similar'
case 'boolean':
case 'number':
case 'undefined':
// Compare vaues in a way that will be readable in test failure message.
// Compare values in a way that will be readable in test failure message.
expect(`${key}:${cmd1[key]}`).toEqual(`${key}:${cmd2[key]}`);
break;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/command.executableSubcommand.signals.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const path = require('path');
// https://nodejs.org/api/process.html#process_signal_events
const describeOrSkipOnWindows = (process.platform === 'win32') ? describe.skip : describe;

// Note: the previous (sinon) test had custom code for SIGUSR1, revist if required:
// Note: the previous (sinon) test had custom code for SIGUSR1, revisit if required:
// As described at https://nodejs.org/api/process.html#process_signal_events
// this signal will start a debugger and thus the process might output an
// additional error message:
Expand Down
2 changes: 1 addition & 1 deletion tests/command.exitOverride.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const path = require('path');

// Test details of the exitOverride errors.
// The important checks are the exitCode and code which are intended to be stable for
// semver minor versions. For now, also testing the error.message and that output occured
// semver minor versions. For now, also testing the error.message and that output occurred
// to detect accidental changes in behaviour.

/* eslint jest/expect-expect: ["error", { "assertFunctionNames": ["expect", "expectCommanderError"] }] */
Expand Down
2 changes: 1 addition & 1 deletion tests/command.help.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Commands:
expect(helpInformation).toBe(expectedHelpInformation);
});

test('when use .description for command then help incudes description', () => {
test('when use .description for command then help includes description', () => {
const program = new commander.Command();
program
.command('simple-command')
Expand Down
2 changes: 1 addition & 1 deletion tests/options.mandatory.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ describe('required command option with mandatory value not specified', () => {
consoleErrorSpy.mockRestore();
});

test('when command has required value not specified then eror', () => {
test('when command has required value not specified then error', () => {
const program = new commander.Command();
program
.exitOverride()
Expand Down
156 changes: 156 additions & 0 deletions tests/options.variadic.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
const commander = require('../');

describe('variadic option with required value', () => {
test('when variadic with value missing then error', () => {
// Optional. Use internal knowledge to suppress output to keep test output clean.
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => { });

const program = new commander.Command();
program
.exitOverride()
.option('-r,--required <value...>');

expect(() => {
program.parse(['--required'], { from: 'user' });
}).toThrow();

consoleErrorSpy.mockRestore();
});

test('when variadic with one value then set in array', () => {
const program = new commander.Command();
program
.option('-r,--required <value...>');

program.parse(['--required', 'one'], { from: 'user' });
expect(program.opts().required).toEqual(['one']);
});

test('when variadic with two values then set in array', () => {
const program = new commander.Command();
program
.option('-r,--required <value...>');

program.parse(['--required', 'one', 'two'], { from: 'user' });
expect(program.opts().required).toEqual(['one', 'two']);
});

test('when variadic with repeated values then set in array', () => {
const program = new commander.Command();
program
.option('-r,--required <value...>');

program.parse(['--required', 'one', '--required', 'two'], { from: 'user' });
expect(program.opts().required).toEqual(['one', 'two']);
});

test('when variadic with short combined argument then not variadic', () => {
const program = new commander.Command();
program
.option('-r,--required <value...>')
.arguments('[arg]');

program.parse(['-rone', 'operand'], { from: 'user' });
expect(program.opts().required).toEqual(['one']);
});

test('when variadic with long combined argument then not variadic', () => {
const program = new commander.Command();
program
.option('-r,--required <value...>')
.arguments('[arg]');

program.parse(['--required=one', 'operand'], { from: 'user' });
expect(program.opts().required).toEqual(['one']);
});

test('when variadic with value followed by option then option not eaten', () => {
const program = new commander.Command();
program
.option('-r,--required <value...>')
.option('-f, --flag')
.arguments('[arg]');

program.parse(['-r', 'one', '-f'], { from: 'user' });
const opts = program.opts();
expect(opts.required).toEqual(['one']);
expect(opts.flag).toBe(true);
});

test('when variadic with no value and default then set to default', () => {
const program = new commander.Command();
program
.option('-r,--required <value...>', 'variadic description', 'default');

program.parse([], { from: 'user' });
expect(program.opts().required).toEqual('default');
});

test('when variadic with coercion then coercion sets value', () => {
const program = new commander.Command();
program
.option('-r,--required <value...>', 'variadic description', parseFloat);

// variadic processing reads the multiple values, but up to custom coercion what it does.
program.parse(['--required', '1e2', '1e3'], { from: 'user' });
expect(program.opts().required).toEqual(1000);
});
});

// Not retesting everything, but do some tests on variadic with optional
describe('variadic option with optional value', () => {
test('when variadic not specified then value undefined', () => {
const program = new commander.Command();
program
.option('-o,--optional [value...]');

program.parse([], { from: 'user' });
expect(program.opts().optional).toBeUndefined();
});

test('when variadic used as boolean flag then value true', () => {
const program = new commander.Command();
program
.option('-o,--optional [value...]');

program.parse(['--optional'], { from: 'user' });
expect(program.opts().optional).toBe(true);
});

test('when variadic with one value then set in array', () => {
const program = new commander.Command();
program
.option('-o,--optional [value...]');

program.parse(['--optional', 'one'], { from: 'user' });
expect(program.opts().optional).toEqual(['one']);
});

test('when variadic with two values then set in array', () => {
const program = new commander.Command();
program
.option('-o,--optional [value...]');

program.parse(['--optional', 'one', 'two'], { from: 'user' });
expect(program.opts().optional).toEqual(['one', 'two']);
});
});

describe('variadic special cases', () => {
test('when option flags has word character before dots then is variadic', () => {
const program = new commander.Command();
program
.option('-c,--comma [value...]');

expect(program.options[0].variadic).toBeTruthy();
});

test('when option flags has special characters before dots then not variadic', () => {
// This might be used to describe coercion for comma separated values, and is not variadic.
const program = new commander.Command();
program
.option('-c,--comma [value,...]');

expect(program.options[0].variadic).toBeFalsy();
});
});

0 comments on commit 913389f

Please sign in to comment.