diff --git a/lib/command.js b/lib/command.js index 184bc0b79..f6c9a24a8 100644 --- a/lib/command.js +++ b/lib/command.js @@ -36,7 +36,7 @@ class Command extends EventEmitter { this._scriptPath = null; this._name = name || ''; this._optionValues = {}; - this._optionValueSources = {}; // default < env < cli + this._optionValueSources = {}; // default < config < env < cli this._storeOptionsAsProperties = false; this._actionHandler = null; this._executableHandler = false; @@ -527,7 +527,7 @@ Expecting one of '${allowedValues.join("', '")}'`); } // preassign only if we have a default if (defaultValue !== undefined) { - this._setOptionValueWithSource(name, defaultValue, 'default'); + this.setOptionValueWithSource(name, defaultValue, 'default'); } } @@ -558,13 +558,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._setOptionValueWithSource(name, option.negate ? false : defaultValue || true, valueSource); + this.setOptionValueWithSource(name, option.negate ? false : defaultValue || true, valueSource); } else { - this._setOptionValueWithSource(name, val, valueSource); + this.setOptionValueWithSource(name, val, valueSource); } } else if (val !== null) { // reassign - this._setOptionValueWithSource(name, option.negate ? false : val, valueSource); + this.setOptionValueWithSource(name, option.negate ? false : val, valueSource); } }; @@ -580,6 +580,12 @@ Expecting one of '${allowedValues.join("', '")}'`); }); } + // This pattern is to avoid changing the existing behaviour while adding new feature! + this.on('optionConfig:' + oname, (val) => { + const invalidValueMessage = `error: option '${option.flags}' value '${val}' from config is invalid.`; + handleOptionValue(val, invalidValueMessage, 'config'); + }); + return this; } @@ -793,13 +799,31 @@ Expecting one of '${allowedValues.join("', '")}'`); }; /** - * @api private + * Set option value, and record source for later use. + * + * @param {string} key + * @param {Object} value + * @param {string} source - expected values are default/config/env/cli + * @return {Command} `this` command for chaining */ - _setOptionValueWithSource(key, value, source) { + setOptionValueWithSource(key, value, source) { this.setOptionValue(key, value); this._optionValueSources[key] = source; + return this; } + /** + * Get source of option value. + * Expected values are default/config/env/cli + * + * @param {string} key + * @return {string} + */ + + getOptionValueSource(key) { + return this._optionValueSources[key]; + }; + /** * Get user arguments implied or explicit arguments. * Side-effects: set _scriptPath if args included application, and use that to set implicit command name. @@ -1112,6 +1136,7 @@ Expecting one of '${allowedValues.join("', '")}'`); * @param {Promise|undefined} promise * @param {Function} fn * @return {Promise|undefined} + * @api private */ _chainOrCall(promise, fn) { @@ -1448,7 +1473,7 @@ Expecting one of '${allowedValues.join("', '")}'`); /** * Apply any option related environment variables, if option does - * not have a value from cli or client code. + * not already have a value from a higher priority source (cli). * * @api private */ @@ -1456,8 +1481,8 @@ Expecting one of '${allowedValues.join("', '")}'`); this.options.forEach((option) => { 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') { + // Priority check + if (this.getOptionValue(optionKey) === undefined || ['default', 'config', 'env'].includes(this.getOptionValueSource(optionKey))) { 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]); @@ -1470,6 +1495,71 @@ Expecting one of '${allowedValues.join("', '")}'`); }); } + /** + * Merge option values from config, if option does + * not already have a value from a higher priority source (env, cli). + * + * @param {object} config + * @param {string} [configDescription] + * @return {Command} `this` command for chaining + */ + mergeConfig(config, configDescription) { + const configError = (key, explanation) => { + // WIP + const extraDetail = configDescription ? ` in ${configDescription}` : ''; + const message = `error: invalid value for config key '${key}'${extraDetail}\n${explanation}`; + this._displayError('commander.configError', 1, message); + }; + + Object.keys(config).forEach((configKey) => { + const option = this.options.find(option => option.attributeName() === configKey || option.is(configKey)); + if (option) { + const optionKey = option.attributeName(); + const optionValueSource = this.getOptionValueSource(optionKey) || 'unknown'; + // Priority check + if (this.getOptionValue(optionKey) === undefined || ['default', 'config'].includes(optionValueSource)) { + const value = config[configKey]; + if (value === false) { + if (configKey.startsWith('-')) { + configError(configKey, 'Config value false can only be used with a property name, not a flag.'); + } + let negatedOption = option.negate ? option : null; + if (!option.negate && option.long) { + const negativeLongFlag = option.long.replace(/^--/, '--no-'); + negatedOption = this._findOption(negativeLongFlag); + } + if (!negatedOption) { + configError(configKey, 'Config value is false and no matching negated option.'); + } + this.emit(`optionConfig:${negatedOption.name()}`); + } else if (value === true) { + if (option.required) { + configError(configKey, 'Expecting string value.'); + } + if (option.negate && !configKey.startsWith('-')) { + configError(configKey, 'Use value false with property or true with flag for negated option.'); + } + this.emit(`optionConfig:${option.name()}`, option.optional ? null : undefined); + } else if (typeof value === 'string') { + if (!option.required && !option.optional) { + configError(configKey, 'Boolean and negated options require boolean values, not a string.'); + } + this.emit(`optionConfig:${option.name()}`, value); + } else if (Array.isArray(value)) { + value.forEach((item) => { + const configItem = {}; + configItem[configKey] = item; + this.mergeConfig(configItem); + }); + } else { + configError(configKey, `Unsupported config value type '${typeof value}'`); + } + } + } + }); + return this; + } + /** * Argument `name` is missing. * diff --git a/tests/command.chain.test.js b/tests/command.chain.test.js index 09fc6e5a3..2faeb987b 100644 --- a/tests/command.chain.test.js +++ b/tests/command.chain.test.js @@ -196,4 +196,10 @@ describe('Command methods that should return this for chaining', () => { const result = cmd.copyInheritedSettings(program); expect(result).toBe(cmd); }); + + test('when call .mergeConfig() then returns this', () => { + const program = new Command(); + const result = program.mergeConfig({}); + expect(result).toBe(program); + }); }); diff --git a/tests/command.mergeConfig.test.js b/tests/command.mergeConfig.test.js new file mode 100644 index 000000000..31f6eee2b --- /dev/null +++ b/tests/command.mergeConfig.test.js @@ -0,0 +1,209 @@ +const { Command, Option } = require('../'); + +// Some repetition in test implementation across different blocks. + +describe('check config keys', () => { + test('when use short flag then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('-d, --debug'); + program.mergeConfig({ '-d': true }); + expect(program.opts().debug).toBe(true); + }); + + test('when use long flag then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('-d, --debug'); + program.mergeConfig({ '--debug': true }); + expect(program.opts().debug).toBe(true); + }); + + test('when use property name then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('--dashed-words'); + program.mergeConfig({ dashedWords: true }); + expect(program.opts().dashedWords).toBe(true); + }); + + test('when use property name of negated then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('--no-colour'); + program.mergeConfig({ colour: false }); + expect(program.opts().colour).toBe(false); + }); +}); + +describe('check happy config values', () => { + test('when boolean option with true then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('-d, --debug'); + program.mergeConfig({ debug: true }); + expect(program.opts().debug).toBe(true); + }); + + test('when negated property name with false then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('--no-colour'); + program.mergeConfig({ colour: false }); + expect(program.opts().colour).toBe(false); + }); + + test('when negated flag with true then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('--no-colour'); + program.mergeConfig({ '--no-colour': true }); + expect(program.opts().colour).toBe(false); + }); + + test('when boolean option and negated with true then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('--colour'); + program.option('--no-colour'); + program.mergeConfig({ colour: true }); + expect(program.opts().colour).toBe(true); + }); + + test('when boolean option and negated with false then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('--colour'); + program.option('--no-colour'); + program.mergeConfig({ colour: false }); + expect(program.opts().colour).toBe(false); + }); + + test('when require with string then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('--port '); + program.mergeConfig({ port: '80' }); + expect(program.opts().port).toBe('80'); + }); + + test('when require with array of strings then merged', () => { + // Don't need variadic to use array. Treated same way as specifying multiple times on command-line. + const program = new Command(); + program.exitOverride(); + program.option('--port '); + program.mergeConfig({ port: ['1', '2'] }); + expect(program.opts().port).toBe('2'); + }); + + test('when require... with string then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('--port '); + program.mergeConfig({ port: '80' }); + expect(program.opts().port).toEqual(['80']); + }); + + test('when require... with array of strings then merged', () => { + // Don't need variadic to use array. Treated same way as specifying multiple times on command-line. + const program = new Command(); + program.exitOverride(); + program.option('--port '); + program.mergeConfig({ port: ['1', '2'] }); + expect(program.opts().port).toEqual(['1', '2']); + }); + + test('when optional with string then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('--port [number]'); + program.mergeConfig({ port: '80' }); + expect(program.opts().port).toBe('80'); + }); + + test('when optional with true then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('--port [number]'); + program.mergeConfig({ port: true }); + expect(program.opts().port).toBe(true); + }); +}); + +describe('check happy custom parsing', () => { + test('when boolean option with true then merged', () => { + const program = new Command(); + program.exitOverride(); + program.option('-f, --float ', 'description', parseFloat); + program.mergeConfig({ float: '1e3' }); + expect(program.opts().float).toBe(1000); + }); +}); + +describe('check priority', () => { + test('when default and config then option from config', () => { + const program = new Command(); + program.option('--port ', 'description', 'default'); + program.parse([], { from: 'user' }); + program.mergeConfig({ port: 'config' }); + expect(program.opts().port).toBe('config'); + }); + + test('when env and config then option from env', () => { + const program = new Command(); + process.env.PORT = 'env'; + program.addOption(new Option('-p, --port ').env('PORT')); + program.parse([], { from: 'user' }); + program.mergeConfig({ port: 'config' }); + expect(program.opts().port).toBe('env'); + delete process.env.PORT; + }); + + test('when cli and config then option from cli', () => { + const program = new Command(); + program.option('--port ', 'description', 'default'); + program.parse(['--port=cli'], { from: 'user' }); + program.mergeConfig({ port: 'config' }); + expect(program.opts().port).toBe('cli'); + }); +}); + +describe('merge of different priority with variadic does override not combine', () => { + test('when env and config then option from env', () => { + const program = new Command(); + process.env.PORT = 'env'; + program.addOption(new Option('-p, --port ').env('PORT')); + program.parse([], { from: 'user' }); + program.mergeConfig({ port: 'config' }); + expect(program.opts().port).toEqual(['env']); + delete process.env.PORT; + }); + + test('when cli and config then option from cli', () => { + const program = new Command(); + program.option('--port ', 'description'); + program.parse(['--port=cli'], { from: 'user' }); + program.mergeConfig({ port: 'config' }); + expect(program.opts().port).toEqual(['cli']); + }); +}); + +describe('double config', () => { + test('when merge with different keys then both in option values', () => { + const program = new Command(); + program + .option('-a ') + .option('-b '); + program.mergeConfig({ a: 'aa' }); + program.mergeConfig({ b: 'bb' }); + expect(program.opts()).toEqual({ a: 'aa', b: 'bb' }); + }); + + test('when merge with same keys then combined', () => { + const program = new Command(); + program.option('--port ', 'description'); + program.mergeConfig({ port: '1' }); + program.mergeConfig({ port: '2' }); + expect(program.opts().port).toEqual(['1', '2']); + }); +}); diff --git a/tests/options.env.test.js b/tests/options.env.test.js index 5dc53cd37..f103a1eb9 100644 --- a/tests/options.env.test.js +++ b/tests/options.env.test.js @@ -27,6 +27,16 @@ describe.each(['-f, --foo ', '-f, --foo [optional-arg]'])('option delete process.env.BAR; }); + test('when env defined and config 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' }); + program.mergeConfig({ }); + expect(program.opts().foo).toBe('env'); + 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'));