From d6609677a5ff6f256298fe6e06c888fbe997edb2 Mon Sep 17 00:00:00 2001 From: John Gee Date: Wed, 11 May 2022 18:20:35 +1200 Subject: [PATCH] Add Option.implies() (#1724) --- Readme.md | 48 ++++---- examples/options-extra.js | 4 +- examples/options-implies.js | 22 ++++ lib/command.js | 26 ++++- lib/option.js | 67 +++++++++++ tests/options.dual-options.test.js | 101 +++++++++++++++++ tests/options.implies.test.js | 172 +++++++++++++++++++++++++++++ typings/index.d.ts | 12 ++ typings/index.test-d.ts | 3 + 9 files changed, 430 insertions(+), 25 deletions(-) create mode 100644 examples/options-implies.js create mode 100644 tests/options.dual-options.test.js create mode 100644 tests/options.implies.test.js diff --git a/Readme.md b/Readme.md index fa173e53e..c2b765350 100644 --- a/Readme.md +++ b/Readme.md @@ -57,7 +57,7 @@ For information about terms used in this document see: [terminology](./docs/term ## Installation -```bash +```sh npm install commander ``` @@ -86,7 +86,7 @@ const limit = options.first ? 1 : undefined; console.log(program.args[0].split(options.separator, limit)); ``` -```sh +```console $ node split.js -s / --fits a/b/c error: unknown option '--fits' (Did you mean --first?) @@ -120,7 +120,7 @@ program.command('split') program.parse(); ``` -```sh +```console $ node string-util.js help split Usage: string-util split [options] @@ -180,7 +180,7 @@ Multi-word options such as "--template-engine" are camel-cased, becoming `progra An option and its option-argument can be separated by a space, or combined into the same argument. The option-argument can follow the short option directly or follow an `=` for a long option. -```bash +```sh serve -p 80 serve -p80 serve --port 80 @@ -219,7 +219,7 @@ if (options.small) console.log('- small pizza size'); if (options.pizzaType) console.log(`- ${options.pizzaType}`); ``` -```bash +```console $ pizza-options -p error: option '-p, --pizza-type ' argument missing $ pizza-options -d -s -p vegetarian @@ -255,7 +255,7 @@ program.parse(); console.log(`cheese: ${program.opts().cheese}`); ``` -```bash +```console $ pizza-options cheese: blue $ pizza-options --cheese stilton @@ -285,7 +285,7 @@ const cheeseStr = (options.cheese === false) ? 'no cheese' : `${options.cheese} console.log(`You ordered a pizza with ${sauceStr} and ${cheeseStr}`); ``` -```bash +```console $ pizza-options You ordered a pizza with sauce and mozzarella cheese $ pizza-options --sauce @@ -313,7 +313,7 @@ else if (options.cheese === true) console.log('add cheese'); else console.log(`add cheese type ${options.cheese}`); ``` -```bash +```console $ pizza-options no cheese $ pizza-options --cheese @@ -340,7 +340,7 @@ program program.parse(); ``` -```bash +```console $ pizza error: required option '-c, --cheese ' not specified ``` @@ -365,7 +365,7 @@ console.log('Options: ', program.opts()); console.log('Remaining arguments: ', program.args); ``` -```bash +```console $ collect -n 1 2 3 --letter a b c Options: { number: [ '1', '2', '3' ], letter: [ 'a', 'b', 'c' ] } Remaining arguments: [] @@ -387,7 +387,7 @@ The optional `version` method adds handling for displaying the command version. program.version('0.0.1'); ``` -```bash +```console $ ./examples/pizza -V 0.0.1 ``` @@ -404,7 +404,7 @@ 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 files: [options-extra.js](./examples/options-extra.js), [options-env.js](./examples/options-env.js), [options-conflicts.js](./examples/options-conflicts.js) +Example files: [options-extra.js](./examples/options-extra.js), [options-env.js](./examples/options-env.js), [options-conflicts.js](./examples/options-conflicts.js), [options-implies.js](./examples/options-implies.js) ```js program @@ -413,10 +413,11 @@ program .addOption(new Option('-d, --drink ', 'drink size').choices(['small', 'medium', 'large'])) .addOption(new Option('-p, --port ', 'port number').env('PORT')) .addOption(new Option('--donate [amount]', 'optional donation in dollars').preset('20').argParser(parseFloat)) - .addOption(new Option('--disable-server', 'disables the server').conflicts('port')); + .addOption(new Option('--disable-server', 'disables the server').conflicts('port')) + .addOption(new Option('--free-drink', 'small drink included free ').implies({ drink: 'small' })); ``` -```bash +```console $ extra --help Usage: help [options] @@ -424,15 +425,16 @@ Options: -t, --timeout timeout in seconds (default: one minute) -d, --drink drink cup size (choices: "small", "medium", "large") -p, --port port number (env: PORT) - --donate [amount] optional donation in dollars (preset: 20) + --donate [amount] optional donation in dollars (preset: "20") --disable-server disables the server + --free-drink small drink included free -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 --donate -Options: { timeout: 60, donate: 20, port: '80' } +$ PORT=80 extra --donate --free-drink +Options: { timeout: 60, donate: 20, port: '80', freeDrink: true, drink: 'small' } $ extra --disable-server --port 8000 error: option '--disable-server' cannot be used with option '-p, --port ' @@ -489,7 +491,7 @@ if (options.collect.length > 0) console.log(options.collect); if (options.list !== undefined) console.log(options.list); ``` -```bash +```console $ custom -f 1e2 float: 100 $ custom --integer 2 @@ -728,7 +730,7 @@ help option is `-h,--help`. Example file: [pizza](./examples/pizza) -```bash +```console $ node ./examples/pizza --help Usage: pizza [options] @@ -744,7 +746,7 @@ Options: A `help` command is added by default if your command has subcommands. It can be used alone, or with a subcommand name to show further help for the subcommand. These are effectively the same if the `shell` program has implicit help: -```bash +```sh shell help shell --help @@ -806,7 +808,7 @@ program.showHelpAfterError(); program.showHelpAfterError('(add --help for additional information)'); ``` -```sh +```console $ pizza --unknown error: unknown option '--unknown' (add --help for additional information) @@ -818,7 +820,7 @@ You can also show suggestions after an error for an unknown command or option. program.showSuggestionAfterError(); ``` -```sh +```console $ pizza --hepl error: unknown option '--hepl' (Did you mean --help?) @@ -991,7 +993,7 @@ program If you use `ts-node` and stand-alone executable subcommands written as `.ts` files, you need to call your program through node to get the subcommands called correctly. e.g. -```bash +```sh node -r ts-node/register pm.ts ``` diff --git a/examples/options-extra.js b/examples/options-extra.js index c3bc00ed0..1ce1b11ca 100644 --- a/examples/options-extra.js +++ b/examples/options-extra.js @@ -14,7 +14,8 @@ program .addOption(new Option('-d, --drink ', 'drink cup size').choices(['small', 'medium', 'large'])) .addOption(new Option('-p, --port ', 'port number').env('PORT')) .addOption(new Option('--donate [amount]', 'optional donation in dollars').preset('20').argParser(parseFloat)) - .addOption(new Option('--disable-server', 'disables the server').conflicts('port')); + .addOption(new Option('--disable-server', 'disables the server').conflicts('port')) + .addOption(new Option('--free-drink', 'small drink included free ').implies({ drink: 'small' })); program.parse(); @@ -23,6 +24,7 @@ console.log('Options: ', program.opts()); // Try the following: // node options-extra.js --help // node options-extra.js --drink huge +// node options-extra.js --free-drink // PORT=80 node options-extra.js // node options-extra.js --donate // node options-extra.js --donate 30.50 diff --git a/examples/options-implies.js b/examples/options-implies.js new file mode 100644 index 000000000..af1912817 --- /dev/null +++ b/examples/options-implies.js @@ -0,0 +1,22 @@ +#!/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(); + +// You can use .conflicts() with a single string, which is the camel-case name of the conflicting option. +program + .addOption(new Option('--quiet').implies({ logLevel: 'off' })) + .addOption(new Option('--log-level ').choices(['info', 'warning', 'error', 'off']).default('info')) + .addOption(new Option('-c, --cheese ', 'Add the specified type of cheese').implies({ dairy: true })) + .addOption(new Option('--no-cheese', 'You do not want any cheese').implies({ dairy: false })) + .addOption(new Option('--dairy', 'May contain dairy')); + +program.parse(); +console.log(program.opts()); + +// Try the following: +// node options-implies.js +// node options-implies.js --quiet +// node options-implies.js --log-level=warning --quiet +// node options-implies.js --cheese=cheddar +// node options-implies.js --no-cheese diff --git a/lib/command.js b/lib/command.js index 1a1424c5e..4c5089a3f 100644 --- a/lib/command.js +++ b/lib/command.js @@ -7,7 +7,7 @@ const process = require('process'); const { Argument, humanReadableArgName } = require('./argument.js'); const { CommanderError } = require('./error.js'); const { Help } = require('./help.js'); -const { Option, splitOptionFlags } = require('./option.js'); +const { Option, splitOptionFlags, DualOptions } = require('./option.js'); const { suggestSimilar } = require('./suggestSimilar'); // @ts-check @@ -1194,6 +1194,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 + this._parseOptionsImplied(); operands = operands.concat(parsed.operands); unknown = parsed.unknown; this.args = operands.concat(unknown); @@ -1569,6 +1570,29 @@ Expecting one of '${allowedValues.join("', '")}'`); }); } + /** + * Apply any implied option values, if option is undefined or default value. + * + * @api private + */ + _parseOptionsImplied() { + const dualHelper = new DualOptions(this.options); + const hasCustomOptionValue = (optionKey) => { + return this.getOptionValue(optionKey) !== undefined && !['default', 'implied'].includes(this.getOptionValueSource(optionKey)); + }; + this.options + .filter(option => (option.implied !== undefined) && + hasCustomOptionValue(option.attributeName()) && + dualHelper.valueFromOption(this.getOptionValue(option.attributeName()), option)) + .forEach((option) => { + Object.keys(option.implied) + .filter(impliedKey => !hasCustomOptionValue(impliedKey)) + .forEach(impliedKey => { + this.setOptionValueWithSource(impliedKey, option.implied[impliedKey], 'implied'); + }); + }); + } + /** * Argument `name` is missing. * diff --git a/lib/option.js b/lib/option.js index 437a1268a..2a3bf3fd7 100644 --- a/lib/option.js +++ b/lib/option.js @@ -34,6 +34,7 @@ class Option { this.hidden = false; this.argChoices = undefined; this.conflictsWith = []; + this.implied = undefined; } /** @@ -84,6 +85,24 @@ class Option { return this; } + /** + * Specify implied option values for when this option is set and the implied options are not. + * + * The custom processing (parseArg) is not called on the implied values. + * + * @example + * program + * .addOption(new Option('--log', 'write logging information to file')) + * .addOption(new Option('--trace', 'log extra details').implies({ log: 'trace.txt' })); + * + * @param {Object} impliedOptionValues + * @return {Option} + */ + implies(impliedOptionValues) { + this.implied = Object.assign(this.implied || {}, impliedOptionValues); + return this; + } + /** * Set environment variable to check for option value. * Priority order of option values is default < env < cli @@ -217,6 +236,53 @@ class Option { } } +/** + * This class is to make it easier to work with dual options, without changing the existing + * implementation. We support separate dual options for separate positive and negative options, + * like `--build` and `--no-build`, which share a single option value. This works nicely for some + * use cases, but is tricky for others where we want separate behaviours despite + * the single shared option value. + */ +class DualOptions { + /** + * @param {Option[]} options + */ + constructor(options) { + this.positiveOptions = new Map(); + this.negativeOptions = new Map(); + this.dualOptions = new Set(); + options.forEach(option => { + if (option.negate) { + this.negativeOptions.set(option.attributeName(), option); + } else { + this.positiveOptions.set(option.attributeName(), option); + } + }); + this.negativeOptions.forEach((value, key) => { + if (this.positiveOptions.has(key)) { + this.dualOptions.add(key); + } + }); + } + + /** + * Did the value come from the option, and not from possible matching dual option? + * + * @param {any} value + * @param {Option} option + * @returns {boolean} + */ + valueFromOption(value, option) { + const optionKey = option.attributeName(); + if (!this.dualOptions.has(optionKey)) return true; + + // Use the value to deduce if (probably) came from the option. + const preset = this.negativeOptions.get(optionKey).presetArg; + const negativeValue = (preset !== undefined) ? preset : false; + return option.negate === (negativeValue === value); + } +} + /** * Convert string from kebab-case to camelCase. * @@ -255,3 +321,4 @@ function splitOptionFlags(flags) { exports.Option = Option; exports.splitOptionFlags = splitOptionFlags; +exports.DualOptions = DualOptions; diff --git a/tests/options.dual-options.test.js b/tests/options.dual-options.test.js new file mode 100644 index 000000000..a74926188 --- /dev/null +++ b/tests/options.dual-options.test.js @@ -0,0 +1,101 @@ +const { Option, DualOptions } = require('../lib/option.js'); +const { Command } = require('../'); + +// This tests an internal helper class which is not currently exposed on the package. + +test('when positive option then stored in positiveOptions', () => { + const program = new Command(); + program.option('--one'); + const helper = new DualOptions(program.options); + expect(helper.positiveOptions.size).toEqual(1); + expect(helper.negativeOptions.size).toEqual(0); + expect(helper.dualOptions.size).toEqual(0); +}); + +test('when negative option then stored in negativeOptions', () => { + const program = new Command(); + program.option('--no-one'); + const helper = new DualOptions(program.options); + expect(helper.positiveOptions.size).toEqual(0); + expect(helper.negativeOptions.size).toEqual(1); + expect(helper.dualOptions.size).toEqual(0); +}); + +test('when unrelated positive and negative options then no dual options', () => { + const program = new Command(); + program + .option('--one') + .option('--no-two'); + const helper = new DualOptions(program.options); + expect(helper.dualOptions.size).toEqual(0); +}); + +test('when related positive and negative options then stored as dual option', () => { + const program = new Command(); + program + .option('--one') + .option('--no-one'); + const helper = new DualOptions(program.options); + expect(helper.dualOptions.size).toEqual(1); +}); + +test('when related negative and positive options then stored as dual option', () => { + const program = new Command(); + program + .option('--no-one') + .option('--one'); + const helper = new DualOptions(program.options); + expect(helper.dualOptions.size).toEqual(1); +}); + +describe('valueFromOption with boolean option', () => { + const positiveOption = new Option('--one'); + const negativeOption = new Option('--no-one'); + const options = [positiveOption, negativeOption]; + + test('when negativeOption with false then return true', () => { + const helper = new DualOptions(options); + expect(helper.valueFromOption(false, negativeOption)).toBe(true); + }); + + test('when negativeOption with true then return false', () => { + const helper = new DualOptions(options); + expect(helper.valueFromOption(true, negativeOption)).toBe(false); + }); + + test('when positiveOption with false then return false', () => { + const helper = new DualOptions(options); + expect(helper.valueFromOption(false, positiveOption)).toBe(false); + }); + + test('when positiveOption with true then return true', () => { + const helper = new DualOptions(options); + expect(helper.valueFromOption(true, positiveOption)).toBe(true); + }); +}); + +describe('valueFromOption with option expecting value and negative with preset', () => { + const positiveOption = new Option('--one '); + const negativeOption = new Option('--no-one').preset('FALSE'); + const options = [positiveOption, negativeOption]; + + test('when negativeOption with FALSE then return true', () => { + const helper = new DualOptions(options); + expect(helper.valueFromOption('FALSE', negativeOption)).toBe(true); + }); + + test('when negativeOption with string then return false', () => { + const helper = new DualOptions(options); + expect(helper.valueFromOption('foo', negativeOption)).toBe(false); + }); + + test('when positiveOption with FALSE then return false', () => { + const helper = new DualOptions(options); + expect(helper.valueFromOption('FALSE', positiveOption)).toBe(false); + }); + + test('when positiveOption with true then return true', () => { + const helper = new DualOptions(options); + expect(helper.valueFromOption('foo', positiveOption)).toBe(true); + }); +}); diff --git a/tests/options.implies.test.js b/tests/options.implies.test.js new file mode 100644 index 000000000..6aae37051 --- /dev/null +++ b/tests/options.implies.test.js @@ -0,0 +1,172 @@ +const { Command, Option } = require('../'); + +describe('check priorities', () => { + test('when source undefined and implied undefined then implied is undefined', () => { + const program = new Command(); + program + .addOption(new Option('--foo').implies({ bar: 'implied' })) + .option('--bar'); + program.parse([], { from: 'user' }); + expect(program.opts()).toEqual({}); + }); + + test('when source default and implied undefined then implied is undefined', () => { + const program = new Command(); + program + .addOption(new Option('--foo').implies({ bar: 'implied' }).default('default')) + .option('--bar'); + program.parse([], { from: 'user' }); + expect(program.opts()).toEqual({ foo: 'default' }); + }); + + test('when source from env and implied undefined then implied is implied', () => { + const program = new Command(); + const envName = 'COMMANDER_TEST_DELETE_ME'; + process.env[envName] = 'env'; + program + .addOption(new Option('--foo').implies({ bar: 'implied' }).env(envName)) + .option('--bar'); + program.parse([], { from: 'user' }); + expect(program.opts()).toEqual({ foo: true, bar: 'implied' }); + delete process.env[envName]; + }); + + test('when source from cli and implied undefined then implied is implied', () => { + const program = new Command(); + program + .addOption(new Option('--foo').implies({ bar: 'implied' })) + .option('--bar'); + program.parse(['--foo'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: true, bar: 'implied' }); + }); + + test('when source cli and implied default then implied is implied', () => { + const program = new Command(); + program + .addOption(new Option('--foo').implies({ bar: 'implied' })) + .option('--bar', '', 'default'); + program.parse(['--foo'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: true, bar: 'implied' }); + }); + + test('when source cli and env default then implied is env', () => { + const program = new Command(); + const envName = 'COMMANDER_TEST_DELETE_ME'; + process.env[envName] = 'env'; + program + .addOption(new Option('--foo').implies({ bar: 'implied' })) + .addOption(new Option('--bar ').env(envName)); + program.parse(['--foo'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: true, bar: 'env' }); + delete process.env[envName]; + }); +}); + +test('when imply non-option then ok and stored', () => { + const program = new Command(); + program + .addOption(new Option('--foo').implies({ bar: 'implied' })); + program.parse(['--foo'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: true, bar: 'implied' }); +}); + +test('when imply multiple values then store multiple values', () => { + const program = new Command(); + program + .addOption(new Option('--foo').implies({ one: 'ONE', two: 'TWO' })) + .option('--one') + .option('--two'); + program.parse(['--foo'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: true, one: 'ONE', two: 'TWO' }); +}); + +test('when imply multiple times then store multiple values', () => { + const program = new Command(); + program + .addOption(new Option('--foo').implies({ one: 'ONE' }).implies({ two: 'TWO' })) + .option('--one') + .option('--two'); + program.parse(['--foo'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: true, one: 'ONE', two: 'TWO' }); +}); + +test('when imply from positive option then positive implied', () => { + const program = new Command(); + program + .addOption(new Option('--foo').implies({ implied: 'POSITIVE' })) + .addOption(new Option('--no-foo').implies({ implied: 'NEGATIVE' })); + program.parse(['--foo'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: true, implied: 'POSITIVE' }); +}); + +test('when imply from negative option then negative implied', () => { + const program = new Command(); + program + .addOption(new Option('--foo').implies({ implied: 'POSITIVE' })) + .addOption(new Option('--no-foo').implies({ implied: 'NEGATIVE' })); + program.parse(['--no-foo'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: false, implied: 'NEGATIVE' }); +}); + +test('when imply from lone negative option then negative implied', () => { + const program = new Command(); + program + .addOption(new Option('--no-foo').implies({ implied: 'NEGATIVE' })); + program.parse(['--no-foo'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: false, implied: 'NEGATIVE' }); +}); + +test('when imply from negative option with preset then negative implied', () => { + const program = new Command(); + program + .addOption(new Option('--foo').implies({ implied: 'POSITIVE' })) + .addOption(new Option('--no-foo').implies({ implied: 'NEGATIVE' }).preset('FALSE')); + program.parse(['--no-foo'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: 'FALSE', implied: 'NEGATIVE' }); +}); + +test('when chained implies then only explicitly trigger', () => { + const program = new Command(); + program + .addOption(new Option('--one').implies({ two: true })) + .addOption(new Option('--two').implies({ three: true })) + .addOption(new Option('--three')); + program.parse(['--one'], { from: 'user' }); + expect(program.opts()).toEqual({ one: true, two: true }); +}); + +test('when looped implies then no infinite loop', () => { + const program = new Command(); + program + .addOption(new Option('--ying').implies({ yang: true })) + .addOption(new Option('--yang').implies({ ying: true })); + program.parse(['--ying'], { from: 'user' }); + expect(program.opts()).toEqual({ ying: true, yang: true }); +}); + +test('when conflict with implied value then throw', () => { + const program = new Command(); + program + .exitOverride() + .configureOutput({ + writeErr: () => {} + }) + .addOption(new Option('--unary')) + .addOption(new Option('--binary').conflicts('unary')) + .addOption(new Option('--one').implies({ unary: true })); + + expect(() => { + program.parse(['--binary', '--one'], { from: 'user' }); + }).toThrow(); +}); + +test('when requiredOption with implied value then not throw', () => { + const program = new Command(); + program + .requiredOption('--target ') + .addOption(new Option('--default-target').implies({ target: 'default-file' })); + + expect(() => { + program.parse(['--default-target'], { from: 'user' }); + }).not.toThrow(); +}); diff --git a/typings/index.d.ts b/typings/index.d.ts index 98f460742..e1b80e3d2 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -130,6 +130,18 @@ export class Option { */ conflicts(names: string | string[]): this; + /** + * Specify implied option values for when this option is set and the implied options are not. + * + * The custom processing (parseArg) is not called on the implied values. + * + * @example + * program + * .addOption(new Option('--log', 'write logging information to file')) + * .addOption(new Option('--trace', 'log extra details').implies({ log: 'trace.txt' })); + */ + implies(optionValues: OptionValues): this; + /** * Set environment variable to check for option value. * Priority order of option values is default < env < cli diff --git a/typings/index.test-d.ts b/typings/index.test-d.ts index b1dc94ccf..5e8b8397d 100644 --- a/typings/index.test-d.ts +++ b/typings/index.test-d.ts @@ -418,6 +418,9 @@ expectType(baseOption.choices(['a', 'b'] as const)); expectType(baseOption.conflicts('a')); expectType(baseOption.conflicts(['a', 'b'])); +// implies +expectType(baseOption.implies({ option: 'VALUE', colour: false })); + // name expectType(baseOption.name());