From 8e35d5a751b3d7dd00cb8e642bee306e91ff340c Mon Sep 17 00:00:00 2001 From: Marcelo Boveto Shima Date: Sat, 12 Sep 2020 09:28:56 -0300 Subject: [PATCH 1/5] Implement implicitNegateOptions(). --- index.js | 29 +++++++++++++++++++++++++++-- tests/options.bool.test.js | 22 ++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index c1b67b7b2..62324938d 100644 --- a/index.js +++ b/index.js @@ -34,6 +34,13 @@ class Option { } this.description = description || ''; this.defaultValue = undefined; + this.negateOption = false; + } + + allowNegateOption(allowNegateOption) { + allowNegateOption = (allowNegateOption === undefined) || allowNegateOption; + this.negateOption = !this.negate && this.long && !this.required && allowNegateOption; + return this; } /** @@ -112,6 +119,7 @@ class Command extends EventEmitter { this.options = []; this.parent = null; this._allowUnknownOption = false; + this._implicitNegateOptions = false; this._args = []; this.rawArgs = null; this._scriptPath = null; @@ -491,7 +499,7 @@ Read more on https://git.io/JJc0W`); */ _optionEx(config, flags, description, fn, defaultValue) { - const option = new Option(flags, description); + const option = new Option(flags, description).allowNegateOption(this._implicitNegateOptions); const oname = option.name(); const name = option.attributeName(); option.mandatory = !!config.mandatory; @@ -669,6 +677,17 @@ Read more on https://git.io/JJc0W`); return this; }; + /** + * Allow implicit negate value to options on the command line. + * + * @param {Boolean} [arg] - if `true` or omitted, a negate option will be accepted for non negative options with no value or non required values. + * @api public + */ + implicitNegateOptions(arg) { + this._implicitNegateOptions = (arg === undefined) || arg; + return this; + }; + /** * Whether to store option values as properties on command object, * or store separately (specify false). In both cases the option values can be accessed using .opts(). @@ -1104,7 +1123,7 @@ Read more on https://git.io/JJc0W`); activeVariadicOption = null; if (maybeOption(arg)) { - const option = this._findOption(arg); + let option = this._findOption(arg); // recognised option, call listener to assign value with possible custom processing if (option) { if (option.required) { @@ -1123,6 +1142,12 @@ Read more on https://git.io/JJc0W`); } activeVariadicOption = option.variadic ? option : null; continue; + } else if (arg.startsWith('--no-')) { + option = this._findOption(arg.replace(/^--no-/, '--')); + if (option && option.negateOption) { + this.emit(`option:${option.name()}`, false); + continue; + } } } diff --git a/tests/options.bool.test.js b/tests/options.bool.test.js index e05825183..a8d346d29 100644 --- a/tests/options.bool.test.js +++ b/tests/options.bool.test.js @@ -35,6 +35,16 @@ describe('boolean flag on program', () => { program.parse(['node', 'test', '--no-cheese']); expect(program.cheese).toBe(false); }); + + test('when implicit negatable boolean flag specified then value is false', () => { + const program = new commander.Command(); + program + .option('--cheese', 'add cheese') + ._findOption('--cheese') + .allowNegateOption(true); + program.parse(['node', 'test', '--no-cheese']); + expect(program.cheese).toBe(false); + }); }); // boolean flag on command @@ -82,6 +92,18 @@ describe('boolean flag on command', () => { program.parse(['node', 'test', 'sub', '--no-cheese']); expect(subCommand.cheese).toBe(false); }); + + test('when implicit negatable boolean flag specified then value is false', () => { + let subCommand; + const program = new commander.Command(); + program + .command('sub') + .implicitNegateOptions() + .option('--cheese', 'add cheese') + .action((cmd) => { subCommand = cmd; }); + program.parse(['node', 'test', 'sub', '--no-cheese']); + expect(subCommand.cheese).toBe(false); + }); }); // This is a somewhat undocumented special behaviour which appears in some examples. From d01215f2a6e07a7daa7762ae08a5bd38490b92d3 Mon Sep 17 00:00:00 2001 From: Marcelo Boveto Shima Date: Sat, 12 Sep 2020 20:06:30 -0300 Subject: [PATCH 2/5] Drop implicitNegateOptions. --- index.js | 14 +------------- tests/options.bool.test.js | 12 ------------ 2 files changed, 1 insertion(+), 25 deletions(-) diff --git a/index.js b/index.js index 62324938d..b9e1e20c1 100644 --- a/index.js +++ b/index.js @@ -119,7 +119,6 @@ class Command extends EventEmitter { this.options = []; this.parent = null; this._allowUnknownOption = false; - this._implicitNegateOptions = false; this._args = []; this.rawArgs = null; this._scriptPath = null; @@ -499,7 +498,7 @@ Read more on https://git.io/JJc0W`); */ _optionEx(config, flags, description, fn, defaultValue) { - const option = new Option(flags, description).allowNegateOption(this._implicitNegateOptions); + const option = new Option(flags, description); const oname = option.name(); const name = option.attributeName(); option.mandatory = !!config.mandatory; @@ -677,17 +676,6 @@ Read more on https://git.io/JJc0W`); return this; }; - /** - * Allow implicit negate value to options on the command line. - * - * @param {Boolean} [arg] - if `true` or omitted, a negate option will be accepted for non negative options with no value or non required values. - * @api public - */ - implicitNegateOptions(arg) { - this._implicitNegateOptions = (arg === undefined) || arg; - return this; - }; - /** * Whether to store option values as properties on command object, * or store separately (specify false). In both cases the option values can be accessed using .opts(). diff --git a/tests/options.bool.test.js b/tests/options.bool.test.js index a8d346d29..eadc04798 100644 --- a/tests/options.bool.test.js +++ b/tests/options.bool.test.js @@ -92,18 +92,6 @@ describe('boolean flag on command', () => { program.parse(['node', 'test', 'sub', '--no-cheese']); expect(subCommand.cheese).toBe(false); }); - - test('when implicit negatable boolean flag specified then value is false', () => { - let subCommand; - const program = new commander.Command(); - program - .command('sub') - .implicitNegateOptions() - .option('--cheese', 'add cheese') - .action((cmd) => { subCommand = cmd; }); - program.parse(['node', 'test', 'sub', '--no-cheese']); - expect(subCommand.cheese).toBe(false); - }); }); // This is a somewhat undocumented special behaviour which appears in some examples. From aff477922f5605768d9a2535b975d628738cd2b4 Mon Sep 17 00:00:00 2001 From: Marcelo Boveto Shima Date: Sat, 12 Sep 2020 20:07:21 -0300 Subject: [PATCH 3/5] Remove required condition for allowNegateOption. --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index b9e1e20c1..929b121e6 100644 --- a/index.js +++ b/index.js @@ -39,7 +39,7 @@ class Option { allowNegateOption(allowNegateOption) { allowNegateOption = (allowNegateOption === undefined) || allowNegateOption; - this.negateOption = !this.negate && this.long && !this.required && allowNegateOption; + this.negateOption = !this.negate && this.long && allowNegateOption; return this; } From 39dec814603e9cda73adc99cec8ef6e8cd00e2b3 Mon Sep 17 00:00:00 2001 From: Marcelo Boveto Shima Date: Sat, 12 Sep 2020 20:07:42 -0300 Subject: [PATCH 4/5] Add jsDoc to Option.allowNegateOption() --- index.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/index.js b/index.js index 929b121e6..4c9ba78df 100644 --- a/index.js +++ b/index.js @@ -37,6 +37,14 @@ class Option { this.negateOption = false; } + /** + * Whether the option allows negate value with prefix `--no-`. + * + * @param {boolean} allowNegateOption + * @return {Option} + * @api public + */ + allowNegateOption(allowNegateOption) { allowNegateOption = (allowNegateOption === undefined) || allowNegateOption; this.negateOption = !this.negate && this.long && allowNegateOption; From 6a3accde9ad565bf28db2501d21a77b7cd76719c Mon Sep 17 00:00:00 2001 From: Marcelo Boveto Shima Date: Sat, 12 Sep 2020 20:08:58 -0300 Subject: [PATCH 5/5] Add addOption implementation. --- index.js | 105 +++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 74 insertions(+), 31 deletions(-) diff --git a/index.js b/index.js index 4c9ba78df..6145bbb21 100644 --- a/index.js +++ b/index.js @@ -51,6 +51,47 @@ class Option { return this; } + /** + * Set the default value, and optionally supply the description to be displayed in the help. + * + * @param {any} value + * @param {string} [description] + * @return {Option} + * @api public + */ + + default(value, description) { + this.defaultValue = value; + this.defaultValueDescription = description; + return this; + }; + + /** + * Set the custom handler for processing CLI option arguments into option values. + * + * @param {Function} [fn] + * @return {Option} + * @api public + */ + + argParser(fn) { + this.parseArg = fn; + return this; + }; + + /** + * Whether the option is mandatory and must have a value after parsing. + * + * @param {boolean} [value] + * @return {Option} + * @api public + */ + + makeOptionMandatory(value) { + this.mandatory = (value === undefined) || value; + return this; + }; + /** * Return option name. * @@ -494,40 +535,18 @@ Read more on https://git.io/JJc0W`); }; /** - * Internal implementation shared by .option() and .requiredOption() + * Add an option. * - * @param {Object} config - * @param {string} flags - * @param {string} description - * @param {Function|*} [fn] - custom option processing function or default value - * @param {*} [defaultValue] + * @param {Option} option * @return {Command} `this` command for chaining - * @api private */ - - _optionEx(config, flags, description, fn, defaultValue) { - const option = new Option(flags, description); + addOption(option) { const oname = option.name(); const name = option.attributeName(); - option.mandatory = !!config.mandatory; this._checkForOptionNameClash(option); - // default as 3rd arg - if (typeof fn !== 'function') { - if (fn instanceof RegExp) { - // This is a bit simplistic (especially no error messages), and probably better handled by caller using custom option processing. - // No longer documented in README, but still present for backwards compatibility. - const regex = fn; - fn = (val, def) => { - const m = regex.exec(val); - return m ? m[0] : def; - }; - } else { - defaultValue = fn; - fn = null; - } - } + let defaultValue = option.defaultValue; // preassign default value for --no-*, [optional], , or plain flag if boolean value if (option.negate || option.optional || option.required || typeof defaultValue === 'boolean') { @@ -539,7 +558,6 @@ Read more on https://git.io/JJc0W`); // preassign only if we have a default if (defaultValue !== undefined) { this._setOptionValue(name, defaultValue); - option.defaultValue = defaultValue; } } @@ -552,8 +570,16 @@ Read more on https://git.io/JJc0W`); const oldValue = this._getOptionValue(name); // custom processing - if (val !== null && fn) { - val = fn(val, oldValue === undefined ? defaultValue : oldValue); + if (val !== null && option.parseArg) { + try { + val = option.parseArg(val, oldValue === undefined ? defaultValue : oldValue); + } catch (err) { + if (err.code === 'commander.optionArgumentRejected') { + console.error(err.message); + this._exit(err.exitCode, err.code, err.message); + } + throw err; + } } else if (val !== null && option.variadic) { if (oldValue === defaultValue || !Array.isArray(oldValue)) { val = [val]; @@ -634,7 +660,22 @@ Read more on https://git.io/JJc0W`); */ option(flags, description, fn, defaultValue) { - return this._optionEx({}, flags, description, fn, defaultValue); + const option = new Option(flags, description); + if (typeof fn === 'function') { + option.default(defaultValue).argParser(fn); + } else if (fn instanceof RegExp) { + // legacy + const regex = fn; + fn = (val, def) => { + const m = regex.exec(val); + return m ? m[0] : def; + }; + option.default(defaultValue).argParser(fn); + } else { + option.default(fn); + } + + return this.addOption(option); }; /** @@ -652,7 +693,9 @@ Read more on https://git.io/JJc0W`); */ requiredOption(flags, description, fn, defaultValue) { - return this._optionEx({ mandatory: true }, flags, description, fn, defaultValue); + this.option(flags, description, fn, defaultValue); + this.options[this.options.length - 1].makeOptionMandatory(); + return this; }; /**