Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add setOptionValueWithSource and getOptionValueSource #1613

Merged
merged 2 commits into from
Sep 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ 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.
You can also use `.getOptionValue()` and `.setOptionValue()` to work with a single option value.
(You can also use `.getOptionValue()` and `.setOptionValue()` to work with a single option value,
and `.getOptionValueSource()` and `.setOptionValueWithSource()` when it matters where the option value came from.)

Multi-word options such as "--template-engine" are camel-cased, becoming `program.opts().templateEngine` etc.

Expand Down
40 changes: 30 additions & 10 deletions lib/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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');
}
}

Expand Down Expand Up @@ -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);
}
};

Expand Down Expand Up @@ -793,13 +793,32 @@ Expecting one of '${allowedValues.join("', '")}'`);
};

/**
* @api private
*/
_setOptionValueWithSource(key, value, source) {
* Store option value and where the value came from.
*
* @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) {
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.
Expand Down Expand Up @@ -1112,6 +1131,7 @@ Expecting one of '${allowedValues.join("', '")}'`);
* @param {Promise|undefined} promise
* @param {Function} fn
* @return {Promise|undefined}
* @api private
*/

_chainOrCall(promise, fn) {
Expand Down Expand Up @@ -1456,8 +1476,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. Do not overwrite cli or options from unknown source (client-code).
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]);
Expand Down
8 changes: 7 additions & 1 deletion tests/command.chain.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,13 @@ describe('Command methods that should return this for chaining', () => {

test('when call .setOptionValue() then returns this', () => {
const program = new Command();
const result = program.setOptionValue();
const result = program.setOptionValue('foo', 'bar');
expect(result).toBe(program);
});

test('when call .setOptionValueWithSource() then returns this', () => {
const program = new Command();
const result = program.setOptionValueWithSource('foo', 'bar', 'cli');
expect(result).toBe(program);
});

Expand Down
20 changes: 20 additions & 0 deletions tests/options.env.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,26 @@ describe.each(['-f, --foo <required-arg>', '-f, --foo [optional-arg]'])('option
delete process.env.BAR;
});

test('when env defined and value source is config then option from env', () => {
const program = new commander.Command();
process.env.BAR = 'env';
program.addOption(new commander.Option(fooFlags).env('BAR'));
program.setOptionValueWithSource('foo', 'config', 'config');
program.parse([], { from: 'user' });
expect(program.opts().foo).toBe('env');
delete process.env.BAR;
});

test('when env defined and value source is unspecified then option unchanged', () => {
const program = new commander.Command();
process.env.BAR = 'env';
program.addOption(new commander.Option(fooFlags).env('BAR'));
program.setOptionValue('foo', 'client');
program.parse([], { from: 'user' });
expect(program.opts().foo).toBe('client');
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'));
Expand Down
35 changes: 35 additions & 0 deletions tests/options.getset.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,38 @@ describe.each([true, false])('storeOptionsAsProperties is %s', (storeOptionsAsPr
expect(program.opts().cheese).toBe(cheeseType);
});
});

test('when setOptionValueWithSource then value returned by opts', () => {
const program = new commander.Command();
const cheeseValue = 'blue';
program
.option('--cheese [type]', 'cheese type')
.setOptionValue('cheese', cheeseValue);
expect(program.opts().cheese).toBe(cheeseValue);
});

test('when setOptionValueWithSource then source returned by getOptionValueSource', () => {
const program = new commander.Command();
program
.option('--cheese [type]', 'cheese type')
.setOptionValueWithSource('cheese', 'blue', 'config');
expect(program.getOptionValueSource('cheese')).toBe('config');
});

test('when option value parsed from env then option source is 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.getOptionValueSource('foo')).toBe('env');
delete process.env.BAR;
});

test('when option value parsed from cli then option source is cli', () => {
const program = new commander.Command();
program
.addOption(new commander.Option('-f, --foo').env('BAR'));
program.parse(['--foo'], { from: 'user' });
expect(program.getOptionValueSource('foo')).toBe('cli');
});
17 changes: 14 additions & 3 deletions typings/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,9 @@ export interface OutputConfiguration {

}

type AddHelpTextPosition = 'beforeAll' | 'before' | 'after' | 'afterAll';
type HookEvent = 'preAction' | 'postAction';
export type AddHelpTextPosition = 'beforeAll' | 'before' | 'after' | 'afterAll';
export type HookEvent = 'preAction' | 'postAction';
export type OptionValueSource = 'default' | 'env' | 'config' | 'cli';

export interface OptionValues {
[key: string]: any;
Expand Down Expand Up @@ -526,12 +527,22 @@ export class Command {
*/
getOptionValue(key: string): any;

/**
/**
* Store option value.
*/
setOptionValue(key: string, value: unknown): this;

/**
* Store option value and where the value came from.
*/
setOptionValueWithSource(key: string, value: unknown, source: OptionValueSource): this;

/**
* Retrieve option value source.
*/
getOptionValueSource(key: string): OptionValueSource;

/**
* Alter parsing of short flags with optional values.
*
* @example
Expand Down
6 changes: 6 additions & 0 deletions typings/index.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,12 @@ void program.getOptionValue('example');
expectType<commander.Command>(program.setOptionValue('example', 'value'));
expectType<commander.Command>(program.setOptionValue('example', true));

// setOptionValueWithSource
expectType<commander.Command>(program.setOptionValueWithSource('example', [], 'cli'));

// getOptionValueSource
expectType<commander.OptionValueSource>(program.getOptionValueSource('example'));

// combineFlagAndOptionalValue
expectType<commander.Command>(program.combineFlagAndOptionalValue());
expectType<commander.Command>(program.combineFlagAndOptionalValue(false));
Expand Down