From 11755182cd1d7fdd3eda431a1a6c736a6d468f8f Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 19 Apr 2020 18:13:49 +1200 Subject: [PATCH 01/12] First cut at variadic option implementation --- index.js | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 2ae841538..4b9c9659d 100644 --- a/index.js +++ b/index.js @@ -22,6 +22,7 @@ class Option { this.flags = flags; this.required = flags.indexOf('<') >= 0; // A value must be supplied when the option is specified. this.optional = flags.indexOf('[') >= 0; // A value is optional when the option is specified. + this.variadic = /((\.\.\.)|…)[>\]]/.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. this.negate = flags.indexOf('-no-') !== -1; const flagParts = flags.split(/[ ,|]+/); @@ -476,13 +477,24 @@ 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); + } + + // variadic processing + if (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 @@ -998,6 +1010,7 @@ class Command extends EventEmitter { } // parse options + let activeVariadicOption = null; while (args.length) { const arg = args.shift(); @@ -1008,6 +1021,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 @@ -1026,6 +1045,7 @@ class Command extends EventEmitter { } else { // boolean flag this.emit(`option:${option.name()}`); } + activeVariadicOption = option.variadic ? option : null; continue; } } From c72842044352298a24cd016a8c1377fdd97cc20b Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 19 Apr 2020 20:05:48 +1200 Subject: [PATCH 02/12] Rework variadic regexp - remove ellipsis for symmetry with .arguments - require word character before dots --- index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 4b9c9659d..80ff85c37 100644 --- a/index.js +++ b/index.js @@ -22,7 +22,8 @@ class Option { this.flags = flags; this.required = flags.indexOf('<') >= 0; // A value must be supplied when the option is specified. this.optional = flags.indexOf('[') >= 0; // A value is optional when the option is specified. - this.variadic = /((\.\.\.)|…)[>\]]/.test(flags); // The option can take multiple values. + // variadic test ignores 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. this.negate = flags.indexOf('-no-') !== -1; const flagParts = flags.split(/[ ,|]+/); From bde0e6194ff01e84b8370aadc77aecb00b50a3cc Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 19 Apr 2020 22:03:02 +1200 Subject: [PATCH 03/12] Add tests for variadic options --- tests/options.variadic.test.js | 127 +++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 tests/options.variadic.test.js diff --git a/tests/options.variadic.test.js b/tests/options.variadic.test.js new file mode 100644 index 000000000..a260f446c --- /dev/null +++ b/tests/options.variadic.test.js @@ -0,0 +1,127 @@ +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 '); + + 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 '); + + 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 '); + + 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 '); + + 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 ') + .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 ') + .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 ') + .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 ', 'variadic description', 'default'); + + program.parse([], { from: 'user' }); + expect(program.opts().required).toEqual('default'); + }); + + test('when variadic with coercion then coerced value set in array', () => { + const program = new commander.Command(); + program + .option('-r,--required ', 'variadic description', parseFloat); + + program.parse(['--required', '1e2'], { from: 'user' }); + expect(program.opts().required).toEqual([100]); + }); +}); + +// Not retesting everything, but do some tests on variadic with optional +describe('variadic option with optional value', () => { + test('when variadic with zero values 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 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']); + }); +}); From 6eb6a516b5e24bc2903a98abdbff2b7a1e25a657 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 19 Apr 2020 22:13:53 +1200 Subject: [PATCH 04/12] Add test on non-variadic pattern --- tests/options.variadic.test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/options.variadic.test.js b/tests/options.variadic.test.js index a260f446c..f5fb49781 100644 --- a/tests/options.variadic.test.js +++ b/tests/options.variadic.test.js @@ -125,3 +125,22 @@ describe('variadic option with optional value', () => { 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(); + }); +}); From 8c88da7da77af7bc32aca0f4e9d761a72efb9db1 Mon Sep 17 00:00:00 2001 From: John Gee Date: Mon, 4 May 2020 21:47:58 +1200 Subject: [PATCH 05/12] Rethink interation of variadic and coercion function --- index.js | 5 +---- tests/options.variadic.test.js | 7 ++++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/index.js b/index.js index 80ff85c37..68f4bbea4 100644 --- a/index.js +++ b/index.js @@ -483,10 +483,7 @@ class Command extends EventEmitter { // custom processing if (val !== null && fn) { val = fn(val, oldValue === undefined ? defaultValue : oldValue); - } - - // variadic processing - if (option.variadic) { + } else if (option.variadic) { if (oldValue === defaultValue || !Array.isArray(oldValue)) { val = [val]; } else { diff --git a/tests/options.variadic.test.js b/tests/options.variadic.test.js index f5fb49781..b04c35854 100644 --- a/tests/options.variadic.test.js +++ b/tests/options.variadic.test.js @@ -86,13 +86,14 @@ describe('variadic option with required value', () => { expect(program.opts().required).toEqual('default'); }); - test('when variadic with coercion then coerced value set in array', () => { + test('when variadic with coercion then coercion sets value', () => { const program = new commander.Command(); program .option('-r,--required ', 'variadic description', parseFloat); - program.parse(['--required', '1e2'], { from: 'user' }); - expect(program.opts().required).toEqual([100]); + // 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); }); }); From a6e51cc476cc8b22eaa7e9969c133c29c61b53d2 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 30 May 2020 19:34:57 +1200 Subject: [PATCH 06/12] Fix typos --- index.js | 8 ++++---- tests/command.addCommand.test.js | 2 +- tests/command.executableSubcommand.signals.test.js | 2 +- tests/command.exitOverride.test.js | 2 +- tests/command.help.test.js | 2 +- tests/options.mandatory.test.js | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/index.js b/index.js index 9b789ca9c..5acc63bbe 100644 --- a/index.js +++ b/index.js @@ -268,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 @@ -436,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 @@ -562,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 @@ -580,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 diff --git a/tests/command.addCommand.test.js b/tests/command.addCommand.test.js index d9773b17e..714128d6d 100644 --- a/tests/command.addCommand.test.js +++ b/tests/command.addCommand.test.js @@ -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; } diff --git a/tests/command.executableSubcommand.signals.test.js b/tests/command.executableSubcommand.signals.test.js index 7cdabf01d..5789d86f3 100644 --- a/tests/command.executableSubcommand.signals.test.js +++ b/tests/command.executableSubcommand.signals.test.js @@ -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: diff --git a/tests/command.exitOverride.test.js b/tests/command.exitOverride.test.js index 52917ca38..ecd1f1e46 100644 --- a/tests/command.exitOverride.test.js +++ b/tests/command.exitOverride.test.js @@ -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"] }] */ diff --git a/tests/command.help.test.js b/tests/command.help.test.js index 58b146f07..d6e0e899a 100644 --- a/tests/command.help.test.js +++ b/tests/command.help.test.js @@ -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') diff --git a/tests/options.mandatory.test.js b/tests/options.mandatory.test.js index fb80d8d03..eaed78c71 100644 --- a/tests/options.mandatory.test.js +++ b/tests/options.mandatory.test.js @@ -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() From 6bfb43f3ed5027bad116082229dbf1855212ae48 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 30 May 2020 19:45:15 +1200 Subject: [PATCH 07/12] Raise ecmaVersion to eliminate some false positive errors in example files --- .eslintrc.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.eslintrc.js b/.eslintrc.js index 1d8e6739d..96ad43527 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -44,6 +44,9 @@ const typescriptSettings = { module.exports = { plugins: ['jest'], + parserOptions: { + ecmaVersion: 8 + }, overrides: [ javascriptSettings, typescriptSettings From e5ffe3b15f7e6e0b7970cfe973f765400e9017d1 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 30 May 2020 20:23:04 +1200 Subject: [PATCH 08/12] Improve variadic option with optional value, zero or more values --- index.js | 2 +- tests/options.variadic.test.js | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 5acc63bbe..99fe389ca 100644 --- a/index.js +++ b/index.js @@ -489,7 +489,7 @@ class Command extends EventEmitter { // custom processing if (val !== null && fn) { val = fn(val, oldValue === undefined ? defaultValue : oldValue); - } else if (option.variadic) { + } else if (option.variadic && (val !== null)) { if (oldValue === defaultValue || !Array.isArray(oldValue)) { val = [val]; } else { diff --git a/tests/options.variadic.test.js b/tests/options.variadic.test.js index b04c35854..04ff3dcb2 100644 --- a/tests/options.variadic.test.js +++ b/tests/options.variadic.test.js @@ -99,7 +99,7 @@ describe('variadic option with required value', () => { // Not retesting everything, but do some tests on variadic with optional describe('variadic option with optional value', () => { - test('when variadic with zero values then value undefined', () => { + test('when option not specified then value undefined', () => { const program = new commander.Command(); program .option('-o,--optional [value...]'); @@ -108,6 +108,15 @@ describe('variadic option with optional value', () => { expect(program.opts().optional).toBeUndefined(); }); + test('when option 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 From 395f574484daf0489cec2d188867fae6ad840294 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 30 May 2020 20:29:20 +1200 Subject: [PATCH 09/12] Reorder comparison to match previous test --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 99fe389ca..5846d42e7 100644 --- a/index.js +++ b/index.js @@ -489,7 +489,7 @@ class Command extends EventEmitter { // custom processing if (val !== null && fn) { val = fn(val, oldValue === undefined ? defaultValue : oldValue); - } else if (option.variadic && (val !== null)) { + } else if (val !== null && option.variadic) { if (oldValue === defaultValue || !Array.isArray(oldValue)) { val = [val]; } else { From 8bcbdabe22980428002288a1fa32a9bace2375e9 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 31 May 2020 18:08:19 +1200 Subject: [PATCH 10/12] Use consistent test names --- tests/options.variadic.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/options.variadic.test.js b/tests/options.variadic.test.js index 04ff3dcb2..f06002ec1 100644 --- a/tests/options.variadic.test.js +++ b/tests/options.variadic.test.js @@ -99,7 +99,7 @@ describe('variadic option with required value', () => { // Not retesting everything, but do some tests on variadic with optional describe('variadic option with optional value', () => { - test('when option not specified then value undefined', () => { + test('when variadic not specified then value undefined', () => { const program = new commander.Command(); program .option('-o,--optional [value...]'); @@ -108,7 +108,7 @@ describe('variadic option with optional value', () => { expect(program.opts().optional).toBeUndefined(); }); - test('when option used as boolean flag then value true', () => { + test('when variadic used as boolean flag then value true', () => { const program = new commander.Command(); program .option('-o,--optional [value...]'); From 6c7e8c160ab723936329507dbb5a364c73cae1a3 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 31 May 2020 18:08:53 +1200 Subject: [PATCH 11/12] Add example and README for variadic option --- Readme.md | 33 +++++++++++++++++++++++++++++++++ examples/options-variadic.js | 21 +++++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 examples/options-variadic.js diff --git a/Readme.md b/Readme.md index bcc466582..7bd9e15fe 100644 --- a/Readme.md +++ b/Readme.md @@ -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) @@ -278,6 +279,38 @@ $ pizza error: required option '-c, --cheese ' 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 +in 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 ', '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. diff --git a/examples/options-variadic.js b/examples/options-variadic.js new file mode 100644 index 000000000..26de84f01 --- /dev/null +++ b/examples/options-variadic.js @@ -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 ', '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 From 970420010b4043f7b56728be8c18475bfda77602 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 31 May 2020 18:42:04 +1200 Subject: [PATCH 12/12] Fix typo --- Readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Readme.md b/Readme.md index 7bd9e15fe..404653997 100644 --- a/Readme.md +++ b/Readme.md @@ -284,7 +284,7 @@ error: required option '-c, --cheese ' not specified 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 -in specified in the same argument as the option then no further values are read. +is specified in the same argument as the option then no further values are read. Example file: [options-variadic.js](./examples/options-variadic.js)