From 266397524f64cec1f11ff92978b850bc21cb2f1b Mon Sep 17 00:00:00 2001 From: John Gee Date: Tue, 17 Aug 2021 19:10:25 +1200 Subject: [PATCH 01/19] First cut at env support for options, including custom processing --- lib/command.js | 51 +++++++++++++++++++++++++++++++++++----------- lib/help.js | 3 +++ lib/option.js | 13 ++++++++++++ typings/index.d.ts | 4 ++++ 4 files changed, 59 insertions(+), 12 deletions(-) diff --git a/lib/command.js b/lib/command.js index 4d080b172..06d4ef6e8 100644 --- a/lib/command.js +++ b/lib/command.js @@ -516,6 +516,20 @@ Expecting one of '${allowedValues.join("', '")}'`); } } + // environment variables + if (option.envvar && option.envvar in process.env) { + let envValue; + if (option.required || option.optional) { // option takes a value + envValue = process.env[option.envvar]; + const errorMessage = `error: option '${option.flags}' value '${envValue}' from ${option.envvar} is invalid.`; + envValue = this._doParseOptionArg(option, envValue, undefined, errorMessage); + } else { // boolean + // keep very simple, only care that envvar defined and not the value + envValue = !option.negate; + } + this.setOptionValue(name, envValue); + } + // register the option this.options.push(option); @@ -525,18 +539,9 @@ Expecting one of '${allowedValues.join("', '")}'`); const oldValue = this.getOptionValue(name); // custom processing - if (val !== null && option.parseArg) { - try { - val = option.parseArg(val, oldValue === undefined ? defaultValue : oldValue); - } catch (err) { - if (err.code === 'commander.invalidArgument') { - const message = `error: option '${option.flags}' argument '${val}' is invalid. ${err.message}`; - this._displayError(err.exitCode, err.code, message); - } - throw err; - } - } else if (val !== null && option.variadic) { - val = option._concatValue(val, oldValue); + if (val !== null) { + const errorMessage = `error: option '${option.flags}' argument '${val}' is invalid.`; + val = this._doParseOptionArg(option, val, oldValue, errorMessage); } // unassigned or boolean value @@ -1411,6 +1416,28 @@ Expecting one of '${allowedValues.join("', '")}'`); this._exit(exitCode, code, message); } + /** + * Call parseArg with handling for caught error, and variadic. + * + * @api private + */ + _doParseOptionArg(option, val, oldValue, errorMessage) { + if (option.parseArg) { + try { + val = option.parseArg(val, oldValue === undefined ? option.defaultValue : oldValue); + } catch (err) { + if (err.code === 'commander.invalidArgument') { + const message = `${errorMessage} ${err.message}`; + this._displayError(err.exitCode, err.code, message); + } + throw err; + } + } else if (option.variadic) { + val = option._concatValue(val, oldValue); + } + return val; + }; + /** * Argument `name` is missing. * diff --git a/lib/help.js b/lib/help.js index 025bc0743..d71acb433 100644 --- a/lib/help.js +++ b/lib/help.js @@ -246,6 +246,9 @@ class Help { if (option.defaultValue !== undefined) { extraInfo.push(`default: ${option.defaultValueDescription || JSON.stringify(option.defaultValue)}`); } + if (option.envvar !== undefined) { + extraInfo.push(`env: ${option.envvar}`); + } if (extraInfo.length > 0) { return `${option.description} (${extraInfo.join(', ')})`; } diff --git a/lib/option.js b/lib/option.js index d8c3aa9c3..74666984d 100644 --- a/lib/option.js +++ b/lib/option.js @@ -28,6 +28,7 @@ class Option { } this.defaultValue = undefined; this.defaultValueDescription = undefined; + this.envvar = undefined; this.parseArg = undefined; this.hidden = false; this.argChoices = undefined; @@ -47,6 +48,18 @@ class Option { return this; }; + /** + * environment variable to check for option value + * + * @param {string} [name] + * @return {Option} + */ + + env(name) { + this.envvar = name; + return this; + }; + /** * Set the custom handler for processing CLI option arguments into option values. * diff --git a/typings/index.d.ts b/typings/index.d.ts index 2889e30e0..c25dc98bb 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -99,6 +99,10 @@ export class Option { */ default(value: unknown, description?: string): this; + /** + */ + env(name: string): this; + /** * Calculate the full description, including defaultValue etc. */ From ae67d5fd9610debcec3c23150d32cf9ff24682bf Mon Sep 17 00:00:00 2001 From: John Gee Date: Tue, 17 Aug 2021 21:59:11 +1200 Subject: [PATCH 02/19] Basic tests --- tests/options.env.test.js | 174 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 tests/options.env.test.js diff --git a/tests/options.env.test.js b/tests/options.env.test.js new file mode 100644 index 000000000..637dcdbf6 --- /dev/null +++ b/tests/options.env.test.js @@ -0,0 +1,174 @@ +const commander = require('../'); + +describe.each(['-f, --foo ', '-f, --foo [optional-arg]'])('option declared as: %s', (fooFlags) => { + test('when env undefined and no cli then option undefined', () => { + const program = new commander.Command(); + program.addOption(new commander.Option(fooFlags).env('BAR')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toBeUndefined(); + }); + + test('when env defined and no cli then option from env', () => { + const program = new commander.Command(); + process.env.BAR = 'env'; + program.addOption(new commander.Option(fooFlags).env('BAR')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toEqual('env'); + delete process.env.BAR; + }); + + test('when env defined and cli then option from cli', () => { + const program = new commander.Command(); + process.env.BAR = 'env'; + program.addOption(new commander.Option(fooFlags).env('BAR')); + program.parse(['--foo', 'cli'], { from: 'user' }); + expect(program.opts().foo).toEqual('cli'); + delete process.env.BAR; + }); + + test('when default and env undefined and no cli then option from default', () => { + const program = new commander.Command(); + program.addOption(new commander.Option(fooFlags).env('BAR').default('default')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toEqual('default'); + }); + + test('when default and env defined and no cli then option from env', () => { + const program = new commander.Command(); + process.env.BAR = 'env'; + program.addOption(new commander.Option(fooFlags).env('BAR').default('default')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toEqual('env'); + delete process.env.BAR; + }); + + test('when default and env defined and cli then option from cli', () => { + const program = new commander.Command(); + process.env.BAR = 'env'; + program.addOption(new commander.Option(fooFlags).env('BAR').default('default')); + program.parse(['--foo', 'cli'], { from: 'user' }); + expect(program.opts().foo).toEqual('cli'); + delete process.env.BAR; + }); +}); + +describe('boolean flag', () => { + test('when env undefined and no cli then option undefined', () => { + const program = new commander.Command(); + program.addOption(new commander.Option('-f, --foo').env('BAR')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toBeUndefined(); + }); + + test('when env defined with value and no cli then option true', () => { + const program = new commander.Command(); + process.env.BAR = 'env'; + program.addOption(new commander.Option('-f, --foo').env('BAR')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toBe(true); + delete process.env.BAR; + }); + + test('when env is "" and no cli then option true', () => { + // any string, including "" + const program = new commander.Command(); + process.env.BAR = ''; + program.addOption(new commander.Option('-f, --foo').env('BAR')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toBe(true); + delete process.env.BAR; + }); + + test('when env is "0" and no cli then option true', () => { + // any string, including "0" + const program = new commander.Command(); + process.env.BAR = '0'; + program.addOption(new commander.Option('-f, --foo').env('BAR')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toBe(true); + delete process.env.BAR; + }); + + test('when env is "false" and no cli then option true', () => { + // any string, including "false" + const program = new commander.Command(); + process.env.BAR = 'false'; + program.addOption(new commander.Option('-f, --foo').env('BAR')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toBe(true); + delete process.env.BAR; + }); +}); + +describe('boolean no-flag', () => { + test('when env undefined and no cli then option true', () => { + const program = new commander.Command(); + program.addOption(new commander.Option('-F, --no-foo').env('NO_BAR')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toBe(true); + }); + + test('when env defined and no cli then option false', () => { + const program = new commander.Command(); + process.env.NO_BAR = 'env'; + program.addOption(new commander.Option('-F, --no-foo').env('NO_BAR')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toBe(false); + delete process.env.NO_BAR; + }); +}); + +describe('boolean flag and negatable', () => { + test('when env undefined and no cli then option undefined', () => { + const program = new commander.Command(); + program + .addOption(new commander.Option('-f, --foo').env('BAR')) + .addOption(new commander.Option('-F, --no-foo').env('NO_BAR')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toBeUndefined(); + }); + + test('when env defined and no cli then option true', () => { + const program = new commander.Command(); + process.env.BAR = 'env'; + program + .addOption(new commander.Option('-f, --foo').env('BAR')) + .addOption(new commander.Option('-F, --no-foo').env('NO_BAR')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toBe(true); + delete process.env.BAR; + }); + + test('when env defined and cli --no-foo then option false', () => { + const program = new commander.Command(); + process.env.BAR = 'env'; + program + .addOption(new commander.Option('-f, --foo').env('BAR')) + .addOption(new commander.Option('-F, --no-foo').env('NO_BAR')); + program.parse(['--no-foo'], { from: 'user' }); + expect(program.opts().foo).toBe(false); + delete process.env.BAR; + }); + + test('when no_env defined and no cli then option false', () => { + const program = new commander.Command(); + process.env.NO_BAR = 'env'; + program + .addOption(new commander.Option('-f, --foo').env('BAR')) + .addOption(new commander.Option('-F, --no-foo').env('NO_BAR')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toBe(false); + delete process.env.NO_BAR; + }); + + test('when no_env defined and cli --foo then option true', () => { + const program = new commander.Command(); + process.env.NO_BAR = 'env'; + program + .addOption(new commander.Option('-f, --foo').env('BAR')) + .addOption(new commander.Option('-F, --no-foo').env('NO_BAR')); + program.parse(['--foo'], { from: 'user' }); + expect(program.opts().foo).toBe(true); + delete process.env.NO_BAR; + }); +}); From 02970587849526e0989b6fcb0e2a104e9ca560ee Mon Sep 17 00:00:00 2001 From: John Gee Date: Tue, 17 Aug 2021 22:26:20 +1200 Subject: [PATCH 03/19] Fill out special cases --- tests/option.chain.test.js | 6 ++++++ tests/options.env.test.js | 41 +++++++++++++++++++++++++++++++++----- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/tests/option.chain.test.js b/tests/option.chain.test.js index 700984d15..fd9d826ee 100644 --- a/tests/option.chain.test.js +++ b/tests/option.chain.test.js @@ -30,4 +30,10 @@ describe('Option methods that should return this for chaining', () => { const result = option.choices(['a']); expect(result).toBe(option); }); + + test('when call .env() then returns this', () => { + const option = new Option('-e,--example '); + const result = option.env('e'); + expect(result).toBe(option); + }); }); diff --git a/tests/options.env.test.js b/tests/options.env.test.js index 637dcdbf6..40f68a5ee 100644 --- a/tests/options.env.test.js +++ b/tests/options.env.test.js @@ -13,7 +13,7 @@ describe.each(['-f, --foo ', '-f, --foo [optional-arg]'])('option process.env.BAR = 'env'; program.addOption(new commander.Option(fooFlags).env('BAR')); program.parse([], { from: 'user' }); - expect(program.opts().foo).toEqual('env'); + expect(program.opts().foo).toBe('env'); delete process.env.BAR; }); @@ -22,7 +22,7 @@ describe.each(['-f, --foo ', '-f, --foo [optional-arg]'])('option process.env.BAR = 'env'; program.addOption(new commander.Option(fooFlags).env('BAR')); program.parse(['--foo', 'cli'], { from: 'user' }); - expect(program.opts().foo).toEqual('cli'); + expect(program.opts().foo).toBe('cli'); delete process.env.BAR; }); @@ -30,7 +30,7 @@ describe.each(['-f, --foo ', '-f, --foo [optional-arg]'])('option const program = new commander.Command(); program.addOption(new commander.Option(fooFlags).env('BAR').default('default')); program.parse([], { from: 'user' }); - expect(program.opts().foo).toEqual('default'); + expect(program.opts().foo).toBe('default'); }); test('when default and env defined and no cli then option from env', () => { @@ -38,7 +38,7 @@ describe.each(['-f, --foo ', '-f, --foo [optional-arg]'])('option process.env.BAR = 'env'; program.addOption(new commander.Option(fooFlags).env('BAR').default('default')); program.parse([], { from: 'user' }); - expect(program.opts().foo).toEqual('env'); + expect(program.opts().foo).toBe('env'); delete process.env.BAR; }); @@ -47,7 +47,7 @@ describe.each(['-f, --foo ', '-f, --foo [optional-arg]'])('option process.env.BAR = 'env'; program.addOption(new commander.Option(fooFlags).env('BAR').default('default')); program.parse(['--foo', 'cli'], { from: 'user' }); - expect(program.opts().foo).toEqual('cli'); + expect(program.opts().foo).toBe('cli'); delete process.env.BAR; }); }); @@ -172,3 +172,34 @@ describe('boolean flag and negatable', () => { delete process.env.NO_BAR; }); }); + +describe('custom argParser', () => { + test('when env defined and no cli then custom parse from env', () => { + const program = new commander.Command(); + process.env.BAR = 'env'; + program.addOption(new commander.Option('-f, --foo ').env('BAR').argParser(str => str.toUpperCase())); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toBe('ENV'); + delete process.env.BAR; + }); +}); + +describe('variadic', () => { + test('when env defined and no cli then array from env', () => { + const program = new commander.Command(); + process.env.BAR = 'env'; + program.addOption(new commander.Option('-f, --foo ').env('BAR')); + program.parse([], { from: 'user' }); + expect(program.opts().foo).toEqual(['env']); + delete process.env.BAR; + }); + + test('when env defined and cli then array from cli', () => { + const program = new commander.Command(); + process.env.BAR = 'env'; + program.addOption(new commander.Option('-f, --foo ').env('BAR')); + program.parse(['--foo', 'cli'], { from: 'user' }); + expect(program.opts().foo).toEqual(['cli']); + delete process.env.BAR; + }); +}); From b1d386d78692bfad610fdd773459e488e9508df4 Mon Sep 17 00:00:00 2001 From: John Gee Date: Tue, 17 Aug 2021 22:27:43 +1200 Subject: [PATCH 04/19] Skip test that picked up a problem! --- tests/options.env.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/options.env.test.js b/tests/options.env.test.js index 40f68a5ee..51e731b2f 100644 --- a/tests/options.env.test.js +++ b/tests/options.env.test.js @@ -194,7 +194,7 @@ describe('variadic', () => { delete process.env.BAR; }); - test('when env defined and cli then array from cli', () => { + test.skip('when env defined and cli then array from cli', () => { const program = new commander.Command(); process.env.BAR = 'env'; program.addOption(new commander.Option('-f, --foo ').env('BAR')); From 59ae71ff4fc0bb11ed9f74c64893b6be6a4942e0 Mon Sep 17 00:00:00 2001 From: John Gee Date: Wed, 18 Aug 2021 22:40:48 +1200 Subject: [PATCH 05/19] Add option source so can make decisions about priorities when processing env --- lib/command.js | 56 ++++++++++++++++++++++++--------------- tests/options.env.test.js | 2 +- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/lib/command.js b/lib/command.js index 06d4ef6e8..05b511041 100644 --- a/lib/command.js +++ b/lib/command.js @@ -35,6 +35,7 @@ class Command extends EventEmitter { this._scriptPath = null; this._name = name || ''; this._optionValues = {}; + this._optionValueSources = {}; this._storeOptionsAsProperties = false; this._actionHandler = null; this._executableHandler = false; @@ -512,24 +513,10 @@ Expecting one of '${allowedValues.join("', '")}'`); } // preassign only if we have a default if (defaultValue !== undefined) { - this.setOptionValue(name, defaultValue); + this.setOptionValue(name, defaultValue, 'default'); } } - // environment variables - if (option.envvar && option.envvar in process.env) { - let envValue; - if (option.required || option.optional) { // option takes a value - envValue = process.env[option.envvar]; - const errorMessage = `error: option '${option.flags}' value '${envValue}' from ${option.envvar} is invalid.`; - envValue = this._doParseOptionArg(option, envValue, undefined, errorMessage); - } else { // boolean - // keep very simple, only care that envvar defined and not the value - envValue = !option.negate; - } - this.setOptionValue(name, envValue); - } - // register the option this.options.push(option); @@ -548,15 +535,13 @@ Expecting one of '${allowedValues.join("', '")}'`); 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 - ? false - : defaultValue || true); + this.setOptionValue(name, option.negate ? false : defaultValue || true, 'cli'); } else { - this.setOptionValue(name, val); + this.setOptionValue(name, val, 'cli'); } } else if (val !== null) { // reassign - this.setOptionValue(name, option.negate ? false : val); + this.setOptionValue(name, option.negate ? false : val, 'cli'); } }); @@ -760,15 +745,17 @@ Expecting one of '${allowedValues.join("', '")}'`); * * @param {string} key * @param {Object} value + * @param {string} [source] * @return {Command} `this` command for chaining */ - setOptionValue(key, value) { + setOptionValue(key, value, source) { if (this._storeOptionsAsProperties) { this[key] = value; } else { this._optionValues[key] = value; } + this._optionValueSources[key] = source; return this; }; @@ -1136,6 +1123,7 @@ Expecting one of '${allowedValues.join("', '")}'`); _parseCommand(operands, unknown) { const parsed = this.parseOptions(unknown); + this._parseOptionsEnv(); // after cli, so parseArg not called on both cli and env operands = operands.concat(parsed.operands); unknown = parsed.unknown; this.args = operands.concat(unknown); @@ -1438,6 +1426,32 @@ Expecting one of '${allowedValues.join("', '")}'`); return val; }; + /** + * Apply any option related environment variables, if option does + * not have a value from cli or client code. + * + * @api private + */ + _parseOptionsEnv() { + this.options.forEach((option) => { + if (option.envvar && option.envvar in process.env) { + const optionKey = option.attributeName(); + if (this.getOptionValue(optionKey) === undefined || this._optionValueSources[optionKey] === 'default') { + let envValue; + if (option.required || option.optional) { // option takes a value + envValue = process.env[option.envvar]; + const errorMessage = `error: option '${option.flags}' value '${envValue}' from ${option.envvar} is invalid.`; + envValue = this._doParseOptionArg(option, envValue, undefined, errorMessage); + } else { // boolean + // keep very simple, only care that envvar defined and not the value + envValue = !option.negate; + } + this.setOptionValue(optionKey, envValue, 'env'); + } + } + }); + } + /** * Argument `name` is missing. * diff --git a/tests/options.env.test.js b/tests/options.env.test.js index 51e731b2f..40f68a5ee 100644 --- a/tests/options.env.test.js +++ b/tests/options.env.test.js @@ -194,7 +194,7 @@ describe('variadic', () => { delete process.env.BAR; }); - test.skip('when env defined and cli then array from cli', () => { + test('when env defined and cli then array from cli', () => { const program = new commander.Command(); process.env.BAR = 'env'; program.addOption(new commander.Option('-f, --foo ').env('BAR')); From ce508f31125b3d8b838a8e2a8f7e929aaf16e5b7 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 21 Aug 2021 13:44:06 +1200 Subject: [PATCH 06/19] Rework processing to preserve existing behaviour and handle cli and env same way --- lib/command.js | 68 ++++++++++++++++++++++++++++++++++---------------- lib/help.js | 4 +-- lib/option.js | 4 +-- 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/lib/command.js b/lib/command.js index 05b511041..bb9527d45 100644 --- a/lib/command.js +++ b/lib/command.js @@ -35,7 +35,7 @@ class Command extends EventEmitter { this._scriptPath = null; this._name = name || ''; this._optionValues = {}; - this._optionValueSources = {}; + this._optionValueSources = {}; // default < env < cli this._storeOptionsAsProperties = false; this._actionHandler = null; this._executableHandler = false; @@ -513,38 +513,59 @@ Expecting one of '${allowedValues.join("', '")}'`); } // preassign only if we have a default if (defaultValue !== undefined) { - this.setOptionValue(name, defaultValue, 'default'); + this._setOptionValueWithSource(name, defaultValue, 'default'); } } // register the option this.options.push(option); - // when it's passed assign the value - // and conditionally invoke the callback - this.on('option:' + oname, (val) => { + // 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); // custom processing - if (val !== null) { - const errorMessage = `error: option '${option.flags}' argument '${val}' is invalid.`; - val = this._doParseOptionArg(option, val, oldValue, errorMessage); + if (val !== null && option.parseArg) { + try { + val = option.parseArg(val, oldValue === undefined ? defaultValue : oldValue); + } catch (err) { + if (err.code === 'commander.invalidArgument') { + const message = `${invalidValueMessage} ${err.message}`; + this._displayError(err.exitCode, err.code, message); + } + throw err; + } + } else if (val !== null && option.variadic) { + 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.setOptionValue(name, option.negate ? false : defaultValue || true, 'cli'); + this._setOptionValueWithSource(name, option.negate ? false : defaultValue || true, valueSource); } else { - this.setOptionValue(name, val, 'cli'); + this._setOptionValueWithSource(name, val, valueSource); } } else if (val !== null) { // reassign - this.setOptionValue(name, option.negate ? false : val, 'cli'); + this._setOptionValueWithSource(name, option.negate ? false : val, valueSource); } + }; + + this.on('option:' + oname, (val) => { + const invalidValueMessage = `error: option '${option.flags}' argument '${val}' is invalid.`; + handleOptionValue(val, invalidValueMessage, 'cli'); }); + if (option.envVar) { + this.on('optionEnv:' + oname, (val) => { + const invalidValueMessage = `error: option '${option.flags}' value '${val}' from ${option.envVar} is invalid.`; + handleOptionValue(val, invalidValueMessage, 'env'); + }); + } + return this; } @@ -755,10 +776,17 @@ Expecting one of '${allowedValues.join("', '")}'`); } else { this._optionValues[key] = value; } - this._optionValueSources[key] = source; return this; }; + /** + * @api private + */ + _setOptionValueWithSource(key, value, source) { + this.setOptionValue(key, value); + this._optionValueSources[key] = source; + } + /** * Get user arguments implied or explicit arguments. * Side-effects: set _scriptPath if args included application, and use that to set implicit command name. @@ -1434,19 +1462,17 @@ Expecting one of '${allowedValues.join("', '")}'`); */ _parseOptionsEnv() { this.options.forEach((option) => { - if (option.envvar && option.envvar in process.env) { + if (option.envVar && option.envVar in process.env) { const optionKey = option.attributeName(); + // env is second lowest priority source, above default if (this.getOptionValue(optionKey) === undefined || this._optionValueSources[optionKey] === 'default') { - let envValue; - if (option.required || option.optional) { // option takes a value - envValue = process.env[option.envvar]; - const errorMessage = `error: option '${option.flags}' value '${envValue}' from ${option.envvar} is invalid.`; - envValue = this._doParseOptionArg(option, envValue, undefined, errorMessage); + if (option.required || option.optional) { // option can take a value + // keep very simple, optional always takes value + this.emit(`optionEnv:${option.name()}`, process.env[option.envVar]); } else { // boolean - // keep very simple, only care that envvar defined and not the value - envValue = !option.negate; + // keep very simple, only care that envVar defined and not the value + this.emit(`optionEnv:${option.name()}`); } - this.setOptionValue(optionKey, envValue, 'env'); } } }); diff --git a/lib/help.js b/lib/help.js index d71acb433..c19c3238f 100644 --- a/lib/help.js +++ b/lib/help.js @@ -246,8 +246,8 @@ class Help { if (option.defaultValue !== undefined) { extraInfo.push(`default: ${option.defaultValueDescription || JSON.stringify(option.defaultValue)}`); } - if (option.envvar !== undefined) { - extraInfo.push(`env: ${option.envvar}`); + if (option.envVar !== undefined) { + extraInfo.push(`env: ${option.envVar}`); } if (extraInfo.length > 0) { return `${option.description} (${extraInfo.join(', ')})`; diff --git a/lib/option.js b/lib/option.js index 74666984d..5a37d272e 100644 --- a/lib/option.js +++ b/lib/option.js @@ -28,7 +28,7 @@ class Option { } this.defaultValue = undefined; this.defaultValueDescription = undefined; - this.envvar = undefined; + this.envVar = undefined; this.parseArg = undefined; this.hidden = false; this.argChoices = undefined; @@ -56,7 +56,7 @@ class Option { */ env(name) { - this.envvar = name; + this.envVar = name; return this; }; From d40e3d73fd170a5ea482e2c63167f4a29d756be6 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 21 Aug 2021 14:11:35 +1200 Subject: [PATCH 07/19] Remove _doParseOptionArg as no longer used, refactored in a more direct way --- lib/command.js | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/lib/command.js b/lib/command.js index bb9527d45..97ea97193 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1432,28 +1432,6 @@ Expecting one of '${allowedValues.join("', '")}'`); this._exit(exitCode, code, message); } - /** - * Call parseArg with handling for caught error, and variadic. - * - * @api private - */ - _doParseOptionArg(option, val, oldValue, errorMessage) { - if (option.parseArg) { - try { - val = option.parseArg(val, oldValue === undefined ? option.defaultValue : oldValue); - } catch (err) { - if (err.code === 'commander.invalidArgument') { - const message = `${errorMessage} ${err.message}`; - this._displayError(err.exitCode, err.code, message); - } - throw err; - } - } else if (option.variadic) { - val = option._concatValue(val, oldValue); - } - return val; - }; - /** * Apply any option related environment variables, if option does * not have a value from cli or client code. From cda8654a741a5de841a17a05e99dd5408b001ac5 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 21 Aug 2021 16:56:32 +1200 Subject: [PATCH 08/19] Update JSDoc/TSDoc/tsd --- lib/option.js | 3 ++- typings/index.d.ts | 14 ++++++++------ typings/index.test-d.ts | 3 +++ 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/option.js b/lib/option.js index 5a37d272e..8fa8a3f73 100644 --- a/lib/option.js +++ b/lib/option.js @@ -49,7 +49,8 @@ class Option { }; /** - * environment variable to check for option value + * Set environment variable to check for option value. + * Priority order of option values is default < env < cli * * @param {string} [name] * @return {Option} diff --git a/typings/index.d.ts b/typings/index.d.ts index c25dc98bb..725764b6a 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -44,29 +44,29 @@ export class Argument { constructor(arg: string, description?: string); /** - * Return argument name. - */ + * Return argument name. + */ name(): string; /** * Set the default value, and optionally supply the description to be displayed in the help. */ - default(value: unknown, description?: string): this; + default(value: unknown, description?: string): this; /** * Set the custom handler for processing CLI command arguments into argument values. */ - argParser(fn: (value: string, previous: T) => T): this; + argParser(fn: (value: string, previous: T) => T): this; /** * Only allow argument value to be one of choices. */ - choices(values: string[]): this; + choices(values: string[]): this; /** * Make option-argument required. */ - argRequired(): this; + argRequired(): this; /** * Make option-argument optional. @@ -100,6 +100,8 @@ export class Option { default(value: unknown, description?: string): this; /** + * Set environment variable to check for option value. + * Priority order of option values is default < env < cli */ env(name: string): this; diff --git a/typings/index.test-d.ts b/typings/index.test-d.ts index 237fb37ee..d37a0e386 100644 --- a/typings/index.test-d.ts +++ b/typings/index.test-d.ts @@ -345,6 +345,9 @@ const baseOption = new commander.Option('-f,--foo', 'foo description'); expectType(baseOption.default(3)); expectType(baseOption.default(60, 'one minute')); +// env +expectType(baseOption.env('PORT')); + // fullDescription expectType(baseOption.fullDescription()); From 30f694a146400b8286198d46a1c33cf55006164c Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 21 Aug 2021 17:01:55 +1200 Subject: [PATCH 09/19] Add help test for env --- tests/help.optionDescription.test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/help.optionDescription.test.js b/tests/help.optionDescription.test.js index e849cf196..2653d1d99 100644 --- a/tests/help.optionDescription.test.js +++ b/tests/help.optionDescription.test.js @@ -24,6 +24,13 @@ describe('optionDescription', () => { expect(helper.optionDescription(option)).toEqual('description (default: "default")'); }); + test('when option has env then return description and env name', () => { + const description = 'description'; + const option = new commander.Option('-a', description).env('ENV'); + const helper = new commander.Help(); + expect(helper.optionDescription(option)).toEqual('description (env: ENV)'); + }); + test('when option has default value description then return description and custom default description', () => { const description = 'description'; const defaultValueDescription = 'custom'; From 4c5fa9e7bfff092b2401c47deabcb27e1058613c Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 21 Aug 2021 18:15:48 +1200 Subject: [PATCH 10/19] Add env event tests --- tests/options.env.test.js | 103 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/tests/options.env.test.js b/tests/options.env.test.js index 40f68a5ee..5dc53cd37 100644 --- a/tests/options.env.test.js +++ b/tests/options.env.test.js @@ -1,5 +1,6 @@ const commander = require('../'); +// treating optional same as required, treat as option taking value rather than as boolean describe.each(['-f, --foo ', '-f, --foo [optional-arg]'])('option declared as: %s', (fooFlags) => { test('when env undefined and no cli then option undefined', () => { const program = new commander.Command(); @@ -203,3 +204,105 @@ describe('variadic', () => { delete process.env.BAR; }); }); + +describe('env only processed when applies', () => { + test('when env defined on another subcommand then env not applied', () => { + // Doing selective processing. Not processing env at addOption time. + const program = new commander.Command(); + process.env.BAR = 'env'; + program.command('one') + .action(() => {}); + const two = program.command('two') + .addOption(new commander.Option('-f, --foo ').env('BAR').default('default')) + .action(() => {}); + program.parse(['one'], { from: 'user' }); + expect(two.opts().foo).toBe('default'); + delete process.env.BAR; + }); + + test('when env and cli defined then only emit option event for cli', () => { + const program = new commander.Command(); + const optionEventMock = jest.fn(); + const optionEnvEventMock = jest.fn(); + program.on('option:foo', optionEventMock); + program.on('optionEnv:foo', optionEnvEventMock); + process.env.BAR = 'env'; + program.addOption(new commander.Option('-f, --foo ').env('BAR')); + program.parse(['--foo', 'cli'], { from: 'user' }); + expect(optionEventMock).toHaveBeenCalledWith('cli'); + expect(optionEventMock).toHaveBeenCalledTimes(1); + expect(optionEnvEventMock).toHaveBeenCalledTimes(0); + delete process.env.BAR; + }); + + test('when env and cli defined then only parse value for cli', () => { + const program = new commander.Command(); + const parseMock = jest.fn(); + process.env.BAR = 'env'; + program.addOption(new commander.Option('-f, --foo ').env('BAR').argParser(parseMock)); + program.parse(['--foo', 'cli'], { from: 'user' }); + expect(parseMock).toHaveBeenCalledWith('cli', undefined); + expect(parseMock).toHaveBeenCalledTimes(1); + delete process.env.BAR; + }); +}); + +describe('events dispatched for env', () => { + const optionEnvEventMock = jest.fn(); + + afterEach(() => { + optionEnvEventMock.mockClear(); + delete process.env.BAR; + }); + + test('when env defined then emit "optionEnv" and not "option"', () => { + // Decided to do separate events, so test stays that way. + const program = new commander.Command(); + const optionEventMock = jest.fn(); + program.on('option:foo', optionEventMock); + program.on('optionEnv:foo', optionEnvEventMock); + process.env.BAR = 'env'; + program.addOption(new commander.Option('-f, --foo ').env('BAR')); + program.parse([], { from: 'user' }); + expect(optionEventMock).toHaveBeenCalledTimes(0); + expect(optionEnvEventMock).toHaveBeenCalledTimes(1); + }); + + test('when env defined for required then emit "optionEnv" with value', () => { + const program = new commander.Command(); + program.on('optionEnv:foo', optionEnvEventMock); + process.env.BAR = 'env'; + program.addOption(new commander.Option('-f, --foo ').env('BAR')); + program.parse([], { from: 'user' }); + expect(optionEnvEventMock).toHaveBeenCalledWith('env'); + }); + + test('when env defined for optional then emit "optionEnv" with value', () => { + const program = new commander.Command(); + program.on('optionEnv:foo', optionEnvEventMock); + process.env.BAR = 'env'; + program.addOption(new commander.Option('-f, --foo [optional]').env('BAR')); + program.parse([], { from: 'user' }); + expect(optionEnvEventMock).toHaveBeenCalledWith('env'); + }); + + test('when env defined for boolean then emit "optionEnv" with no param', () => { + // check matches historical boolean action event + const program = new commander.Command(); + program.on('optionEnv:foo', optionEnvEventMock); + process.env.BAR = 'anything'; + program.addOption(new commander.Option('-f, --foo').env('BAR')); + program.parse([], { from: 'user' }); + expect(optionEnvEventMock).toHaveBeenCalledWith(); + }); + + test('when env defined for negated boolean then emit "optionEnv" with no param', () => { + // check matches historical boolean action event + const program = new commander.Command(); + program.on('optionEnv:no-foo', optionEnvEventMock); + process.env.BAR = 'anything'; + program.addOption(new commander.Option('-F, --no-foo').env('BAR')); + program.parse([], { from: 'user' }); + expect(optionEnvEventMock).toHaveBeenCalledWith(); + }); +}); From 661d51d5db299b2f3fa78ac5e9c8e01c722bfcdc Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 22 Aug 2021 12:59:01 +1200 Subject: [PATCH 11/19] Add env to help for negated option --- lib/help.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/help.js b/lib/help.js index c19c3238f..2187f2b97 100644 --- a/lib/help.js +++ b/lib/help.js @@ -234,16 +234,15 @@ class Help { */ optionDescription(option) { - if (option.negate) { - return option.description; - } const extraInfo = []; - if (option.argChoices) { + // Some of these do not make sense for negated boolean and suppress for backwards compatibility. + + if (option.argChoices && !option.negate) { 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) { + if (option.defaultValue !== undefined && !option.negate) { extraInfo.push(`default: ${option.defaultValueDescription || JSON.stringify(option.defaultValue)}`); } if (option.envVar !== undefined) { @@ -252,6 +251,7 @@ class Help { if (extraInfo.length > 0) { return `${option.description} (${extraInfo.join(', ')})`; } + return option.description; }; From fafedc4ecb6c12a88d972a1ad7bc9d68768eea49 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 22 Aug 2021 13:49:57 +1200 Subject: [PATCH 12/19] Make env error message more explicit --- lib/command.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/command.js b/lib/command.js index 97ea97193..cf2c96b29 100644 --- a/lib/command.js +++ b/lib/command.js @@ -561,7 +561,7 @@ Expecting one of '${allowedValues.join("', '")}'`); if (option.envVar) { this.on('optionEnv:' + oname, (val) => { - const invalidValueMessage = `error: option '${option.flags}' value '${val}' from ${option.envVar} is invalid.`; + const invalidValueMessage = `error: option '${option.flags}' value '${val}' from env '${option.envVar}' is invalid.`; handleOptionValue(val, invalidValueMessage, 'env'); }); } From d72a49a8f963948ea075ab945ca864d4afb6e859 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 22 Aug 2021 15:10:02 +1200 Subject: [PATCH 13/19] Add example file --- examples/options-env.js | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 examples/options-env.js diff --git a/examples/options-env.js b/examples/options-env.js new file mode 100644 index 000000000..8889f666e --- /dev/null +++ b/examples/options-env.js @@ -0,0 +1,38 @@ +#!/usr/bin/env node +// const { Command, Option } = require('commander'); // (normal include) +const { Command, Option } = require('../'); // include commander in git clone of commander repo +const program = new Command(); + +program.addOption(new Option('-p, --port ', 'specify port number') + .default(80) + .env('PORT') +); +program.addOption(new Option('-c, --colour', 'turn on colour output') + .env('COLOUR') +); +program.addOption(new Option('-C, --no-colour', 'turn off colour output') + .env('NO_COLOUR') +); +program.addOption(new Option('-s, --size ', 'specify size of drink') + .choices(['small', 'medium', 'large']) + .env('SIZE') +); + +program.parse(); +console.log(program.opts()); + +// Try the following: +// node options-env.js --help +// +// node options-env.js +// PORT=9001 node options-env.js +// PORT=9001 node options-env.js --port 123 +// +// COLOUR= node options-env.js +// COLOUR= node options-env.js --no-colour +// NO_COLOUR= node options-env.js +// NO_COLOUR= node options-env.js --colour +// +// SIZE=small node options-env.js +// SIZE=enormous node options-env.js +// SIZE=enormous node options-env.js --size=large From 9840d7f790673b4c4ee0cd8a96332856b7567f29 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 22 Aug 2021 15:22:48 +1200 Subject: [PATCH 14/19] Add env to README --- Readme.md | 9 +++++++-- examples/options-extra.js | 19 +++++++++++-------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/Readme.md b/Readme.md index 4cc640b94..02688c591 100644 --- a/Readme.md +++ b/Readme.md @@ -308,13 +308,14 @@ program.version('0.0.1', '-v, --vers', 'output the current version'); You can add most options using the `.option()` method, but there are some additional features available by constructing an `Option` explicitly for less common cases. -Example file: [options-extra.js](./examples/options-extra.js) +Example files: [options-extra.js](./examples/options-extra.js), [options-env.js](./examples/options-env.js) ```js program .addOption(new Option('-s, --secret').hideHelp()) .addOption(new Option('-t, --timeout ', 'timeout in seconds').default(60, 'one minute')) - .addOption(new Option('-d, --drink ', 'drink size').choices(['small', 'medium', 'large'])); + .addOption(new Option('-d, --drink ', 'drink size').choices(['small', 'medium', 'large'])) + .addOption(new Option('-p, --port ', 'port number').env('PORT')); ``` ```bash @@ -324,10 +325,14 @@ Usage: help [options] Options: -t, --timeout timeout in seconds (default: one minute) -d, --drink drink cup size (choices: "small", "medium", "large") + -p, --port port number (env: PORT) -h, --help display help for command $ extra --drink huge error: option '-d, --drink ' argument 'huge' is invalid. Allowed choices are small, medium, large. + +$ PORT=80 extra +Options: { timeout: 60, port: '80' } ``` ### Custom option processing diff --git a/examples/options-extra.js b/examples/options-extra.js index 404b3e86a..41c26fead 100644 --- a/examples/options-extra.js +++ b/examples/options-extra.js @@ -1,20 +1,23 @@ #!/usr/bin/env node // This is used as an example in the README for extra option features. +// See also options-env.js for more extensive env examples. -// const commander = require('commander'); // (normal include) -const commander = require('../'); // include commander in git clone of commander repo -const program = new commander.Command(); +// const { Command, Option } = require('commander'); // (normal include) +const { Command, Option } = require('../'); // include commander in git clone of commander repo +const program = new Command(); program - .addOption(new commander.Option('-s, --secret').hideHelp()) - .addOption(new commander.Option('-t, --timeout ', 'timeout in seconds').default(60, 'one minute')) - .addOption(new commander.Option('-d, --drink ', 'drink cup size').choices(['small', 'medium', 'large'])); + .addOption(new Option('-s, --secret').hideHelp()) + .addOption(new Option('-t, --timeout ', 'timeout in seconds').default(60, 'one minute')) + .addOption(new Option('-d, --drink ', 'drink cup size').choices(['small', 'medium', 'large'])) + .addOption(new Option('-p, --port ', 'port number').env('PORT')); program.parse(); console.log('Options: ', program.opts()); // Try the following: -// node options-extra.js --help -// node options-extra.js --drink huge +// node options-extra.js --help +// node options-extra.js --drink huge +// PORT=80 node options-extra.js From fc8d0f02f40e34a4476a9e2b5fb41a87c6faee75 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 22 Aug 2021 15:29:57 +1200 Subject: [PATCH 15/19] name param for .env is not optional --- lib/option.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/option.js b/lib/option.js index 8fa8a3f73..2ec204401 100644 --- a/lib/option.js +++ b/lib/option.js @@ -52,7 +52,7 @@ class Option { * Set environment variable to check for option value. * Priority order of option values is default < env < cli * - * @param {string} [name] + * @param {string} name * @return {Option} */ From 6096f064f9feb4e8b2246d7cf11eefeca1304168 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 22 Aug 2021 17:45:36 +1200 Subject: [PATCH 16/19] Add test that env counts for mandatory options --- tests/options.mandatory.test.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/options.mandatory.test.js b/tests/options.mandatory.test.js index 11221b1ec..ae2c42d26 100644 --- a/tests/options.mandatory.test.js +++ b/tests/options.mandatory.test.js @@ -166,7 +166,7 @@ describe('required program option with mandatory value not specified', () => { }); describe('required command option with mandatory value specified', () => { - test('when command has required value specified then specified value', () => { + test('when command has required value specified then no error and option has specified value', () => { const program = new commander.Command(); let cmdOptions; program @@ -181,6 +181,20 @@ describe('required command option with mandatory value specified', () => { expect(cmdOptions.subby).toBe('blue'); }); + + test('when command has required value specified using env then no error and option has specified value', () => { + const program = new commander.Command(); + program + .exitOverride() + .addOption(new commander.Option('-p, --port ', 'port number').makeOptionMandatory().env('FOO')); + + process.env.FOO = 'bar'; + program.parse([], { from: 'user' }); + delete process.env.FOO; + + expect(program.opts().port).toBe('bar'); + }); + }); describe('required command option with mandatory value not specified', () => { From be1e39cb1f66f4805f84a2d421e549e3d5c10ab2 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 22 Aug 2021 17:45:58 +1200 Subject: [PATCH 17/19] Lint --- tests/options.mandatory.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/options.mandatory.test.js b/tests/options.mandatory.test.js index ae2c42d26..0d06fbf27 100644 --- a/tests/options.mandatory.test.js +++ b/tests/options.mandatory.test.js @@ -194,7 +194,6 @@ describe('required command option with mandatory value specified', () => { expect(program.opts().port).toBe('bar'); }); - }); describe('required command option with mandatory value not specified', () => { From 83dd9f3f8f1c2a8bd6d30f91dbf1ce6a422978be Mon Sep 17 00:00:00 2001 From: John Gee Date: Tue, 24 Aug 2021 18:57:20 +1200 Subject: [PATCH 18/19] Remove stale parameter --- lib/command.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/command.js b/lib/command.js index cf2c96b29..4b44f4568 100644 --- a/lib/command.js +++ b/lib/command.js @@ -766,11 +766,10 @@ Expecting one of '${allowedValues.join("', '")}'`); * * @param {string} key * @param {Object} value - * @param {string} [source] * @return {Command} `this` command for chaining */ - setOptionValue(key, value, source) { + setOptionValue(key, value) { if (this._storeOptionsAsProperties) { this[key] = value; } else { From 82a6ef41d555978d48afd615eb0ab8ed448ff7f1 Mon Sep 17 00:00:00 2001 From: John Gee Date: Tue, 24 Aug 2021 18:58:12 +1200 Subject: [PATCH 19/19] Minor lint --- Readme.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/Readme.md b/Readme.md index 02688c591..6e84460ce 100644 --- a/Readme.md +++ b/Readme.md @@ -93,7 +93,6 @@ import { Command } from 'commander'; const program = new Command(); ``` - ## Options 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 ('|'). @@ -908,7 +907,6 @@ You can modify this behaviour for custom applications. In addition, you can modi Example file: [configure-output.js](./examples/configure-output.js) - ```js function errorColor(str) { // Add ANSI escape codes to display text in red.