From 6c06528814315c82ee36772dd65e52563e662187 Mon Sep 17 00:00:00 2001 From: John Gee Date: Tue, 18 May 2021 18:02:31 +1200 Subject: [PATCH] Add getOptionValue and setOptionValue (#1521) * Simplify typings by removing default export of program * Switch any to unknown per lint suggestion * Add typings change to deprecated.md * Make getOptionValue and setOptionValue public * Fix comment change lost in merge * Tweak wording --- Readme.md | 5 ++++- index.js | 40 ++++++++++++++++++------------------ tests/command.chain.test.js | 6 ++++++ tests/options.getset.test.js | 24 ++++++++++++++++++++++ typings/index.d.ts | 10 +++++++++ typings/index.test-d.ts | 7 +++++++ 6 files changed, 71 insertions(+), 21 deletions(-) create mode 100644 tests/options.getset.test.js diff --git a/Readme.md b/Readme.md index 39c532f10..f4e991325 100644 --- a/Readme.md +++ b/Readme.md @@ -95,7 +95,10 @@ const program = new Command(); Options are defined with the `.option()` method, also serving as documentation for the options. Each option can have a short flag (single character) and a long name, separated by a comma or space or vertical bar ('|'). -The parsed options can be accessed by calling `.opts()` on a `Command` object, and are passed to the action handler. Multi-word options such as "--template-engine" are camel-cased, becoming `program.opts().templateEngine` etc. +The parsed options can be accessed by calling `.opts()` on a `Command` object, and are passed to the action handler. +You can also use `.getOptionValue()` and `.setOptionValue()` to work with a single option value. + +Multi-word options such as "--template-engine" are camel-cased, becoming `program.opts().templateEngine` etc. Multiple short flags may optionally be combined in a single argument following the dash: boolean flags, followed by a single option taking a value (possibly followed by the value). For example `-a -b -p 80` may be written as `-ab -p80` or even `-abp80`. diff --git a/index.js b/index.js index 0f7860c4a..862358cda 100644 --- a/index.js +++ b/index.js @@ -1094,11 +1094,11 @@ class Command extends EventEmitter { // 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; + defaultValue = this._findOption(positiveLongFlag) ? this.getOptionValue(name) : true; } // preassign only if we have a default if (defaultValue !== undefined) { - this._setOptionValue(name, defaultValue); + this.setOptionValue(name, defaultValue); } } @@ -1108,7 +1108,7 @@ class Command extends EventEmitter { // when it's passed assign the value // and conditionally invoke the callback this.on('option:' + oname, (val) => { - const oldValue = this._getOptionValue(name); + const oldValue = this.getOptionValue(name); // custom processing if (val !== null && option.parseArg) { @@ -1129,15 +1129,15 @@ class Command extends EventEmitter { 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 + this.setOptionValue(name, option.negate ? false : defaultValue || true); } else { - this._setOptionValue(name, val); + this.setOptionValue(name, val); } } else if (val !== null) { // reassign - this._setOptionValue(name, option.negate ? false : val); + this.setOptionValue(name, option.negate ? false : val); } }); @@ -1325,34 +1325,34 @@ class Command extends EventEmitter { }; /** - * Store option value + * Retrieve option value. * * @param {string} key - * @param {Object} value - * @api private + * @return {Object} value */ - _setOptionValue(key, value) { + getOptionValue(key) { if (this._storeOptionsAsProperties) { - this[key] = value; - } else { - this._optionValues[key] = value; + return this[key]; } + return this._optionValues[key]; }; /** - * Retrieve option value + * Store option value. * * @param {string} key - * @return {Object} value - * @api private + * @param {Object} value + * @return {Command} `this` command for chaining */ - _getOptionValue(key) { + setOptionValue(key, value) { if (this._storeOptionsAsProperties) { - return this[key]; + this[key] = value; + } else { + this._optionValues[key] = value; } - return this._optionValues[key]; + return this; }; /** @@ -1764,7 +1764,7 @@ class Command extends EventEmitter { // Walk up hierarchy so can call in subcommand after checking for displaying help. for (let cmd = this; cmd; cmd = cmd.parent) { cmd.options.forEach((anOption) => { - if (anOption.mandatory && (cmd._getOptionValue(anOption.attributeName()) === undefined)) { + if (anOption.mandatory && (cmd.getOptionValue(anOption.attributeName()) === undefined)) { cmd.missingMandatoryOptionValue(anOption); } }); diff --git a/tests/command.chain.test.js b/tests/command.chain.test.js index 79c08a815..9cfe15158 100644 --- a/tests/command.chain.test.js +++ b/tests/command.chain.test.js @@ -165,4 +165,10 @@ describe('Command methods that should return this for chaining', () => { const result = program.enablePositionalOptions(); expect(result).toBe(program); }); + + test('when call .setOptionValue() then returns this', () => { + const program = new Command(); + const result = program.setOptionValue(); + expect(result).toBe(program); + }); }); diff --git a/tests/options.getset.test.js b/tests/options.getset.test.js new file mode 100644 index 000000000..adb15e639 --- /dev/null +++ b/tests/options.getset.test.js @@ -0,0 +1,24 @@ +const commander = require('../'); + +describe.each([true, false])('storeOptionsAsProperties is %s', (storeOptionsAsProperties) => { + test('when option specified on CLI then value returned by getOptionValue', () => { + const program = new commander.Command(); + program + .storeOptionsAsProperties(storeOptionsAsProperties) + .option('--cheese [type]', 'cheese type'); + const cheeseType = 'blue'; + program.parse(['node', 'test', '--cheese', cheeseType]); + expect(program.getOptionValue('cheese')).toBe(cheeseType); + }); + + test('when setOptionValue then value returned by opts', () => { + const program = new commander.Command(); + const cheeseType = 'blue'; + // Note: opts() only returns declared options when options stored as properties + program + .storeOptionsAsProperties(storeOptionsAsProperties) + .option('--cheese [type]', 'cheese type') + .setOptionValue('cheese', cheeseType); + expect(program.opts().cheese).toBe(cheeseType); + }); +}); diff --git a/typings/index.d.ts b/typings/index.d.ts index 8d834449c..d0f2c7d50 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -465,6 +465,16 @@ export class Command { storeOptionsAsProperties(storeAsProperties: true): this & OptionValues; storeOptionsAsProperties(storeAsProperties?: boolean): this; + /** + * Retrieve option value. + */ + getOptionValue(key: string): any; + + /** + * Store option value. + */ + setOptionValue(key: string, value: unknown): this; + /** * Alter parsing of short flags with optional values. * diff --git a/typings/index.test-d.ts b/typings/index.test-d.ts index 82c2ad44a..0e2a389a1 100644 --- a/typings/index.test-d.ts +++ b/typings/index.test-d.ts @@ -138,6 +138,13 @@ expectType(program.storeOptionsAsPro expectType(program.storeOptionsAsProperties(true)); expectType(program.storeOptionsAsProperties(false)); +// getOptionValue +void program.getOptionValue('example'); + +// setOptionValue +expectType(program.setOptionValue('example', 'value')); +expectType(program.setOptionValue('example', true)); + // combineFlagAndOptionalValue expectType(program.combineFlagAndOptionalValue()); expectType(program.combineFlagAndOptionalValue(false));