Choices for arguments (#1525)

* Add Argument choices, and missing types and tests for default and argParse

* Only make arguments visible if some description, not just default or choices

* Improve format for partial argument descriptions

* Add argument choices to README

* Add test for edge case in argumentDescription

Author: shadowspawn

Readme.md

@@ -22,8 +22,9 @@ Read this in other languages: English | [简体中文](./Readme_zh-CN.md)
     - [More configuration](#more-configuration)
     - [Custom option processing](#custom-option-processing)
   - [Commands](#commands)
-    - [Specify the argument syntax](#specify-the-argument-syntax)
-    - [Custom argument processing](#custom-argument-processing)
+    - [Command-arguments](#command-arguments)
+      - [More configuration](#more-configuration-1)
+      - [Custom argument processing](#custom-argument-processing)
     - [Action handler](#action-handler)
     - [Stand-alone executable (sub)commands](#stand-alone-executable-subcommands)
     - [Life cycle hooks](#life-cycle-hooks)
@@ -33,7 +34,7 @@ Read this in other languages: English | [简体中文](./Readme_zh-CN.md)
     - [.usage and .name](#usage-and-name)
     - [.helpOption(flags, description)](#helpoptionflags-description)
     - [.addHelpCommand()](#addhelpcommand)
-    - [More configuration](#more-configuration-1)
+    - [More configuration](#more-configuration-2)
   - [Custom event listeners](#custom-event-listeners)
   - [Bits and pieces](#bits-and-pieces)
     - [.parse() and .parseAsync()](#parse-and-parseasync)
@@ -428,7 +429,7 @@ Configuration options can be passed with the call to `.command()` and `.addComma
 remove the command from the generated help output. Specifying `isDefault: true` will run the subcommand if no other
 subcommand is specified ([example](./examples/defaultCommand.js)).
 
-### Specify the argument syntax
+### Command-arguments
 
 For subcommands, you can specify the argument syntax in the call to `.command()` (as shown above). This
 is the only method usable for subcommands implemented using a stand-alone executable, but for other subcommands
@@ -438,7 +439,6 @@ To configure a command, you can use `.argument()` to specify each expected comma
 You supply the argument name and an optional description. The argument may be `<required>` or `[optional]`.
 You can specify a default value for an optional command-argument.
 
-
 Example file: [argument.js](./examples/argument.js)
 
 ```js
@@ -474,7 +474,19 @@ program
   .arguments('<username> <password>');
 ```
 
-### Custom argument processing
+#### More configuration
+
+There are some additional features available by constructing an `Argument` explicitly for less common cases.
+
+Example file: [arguments-extra.js](./examples/arguments-extra.js)
+
+```js
+program
+  .addArgument(new commander.Argument('<drink-size>', 'drink cup size').choices(['small', 'medium', 'large']))
+  .addArgument(new commander.Argument('[timeout]', 'timeout in seconds').default(60, 'one minute'))
+```
+
+#### Custom argument processing
 
 You may specify a function to do custom processing of command-arguments before they are passed to the action handler.
 The callback function receives two parameters, the user specified command-argument and the previous value for the argument.

examples/arguments-extra.js

@@ -0,0 +1,23 @@
+#!/usr/bin/env node
+
+// This is used as an example in the README for extra argument features.
+
+// const commander = require('commander'); // (normal include)
+const commander = require('../'); // include commander in git clone of commander repo
+const program = new commander.Command();
+
+program
+  .addArgument(new commander.Argument('<drink-size>', 'drink cup size').choices(['small', 'medium', 'large']))
+  .addArgument(new commander.Argument('[timeout]', 'timeout in seconds').default(60, 'one minute'))
+  .action((drinkSize, timeout) => {
+    console.log(`Drink size: ${drinkSize}`);
+    console.log(`Timeout (s): ${timeout}`);
+  });
+
+program.parse();
+
+// Try the following:
+//  node arguments-extra.js --help
+//  node arguments-extra.js huge
+//  node arguments-extra.js small
+//  node arguments-extra.js medium 30

lib/argument.js

@@ -1,3 +1,5 @@
+const { InvalidArgumentError } = require('./error.js');
+
 // @ts-check
 
 class Argument {
@@ -16,6 +18,7 @@ class Argument {
     this.parseArg = undefined;
     this.defaultValue = undefined;
     this.defaultValueDescription = undefined;
+    this.argChoices = undefined;
 
     switch (name[0]) {
       case '<': // e.g. <required>
@@ -48,6 +51,18 @@ class Argument {
     return this._name;
   };
 
+  /**
+   * @api private
+   */
+
+  _concatValue(value, previous) {
+    if (previous === this.defaultValue || !Array.isArray(previous)) {
+      return [value];
+    }
+
+    return previous.concat(value);
+  }
+
   /**
    * Set the default value, and optionally supply the description to be displayed in the help.
    *
@@ -73,6 +88,27 @@ class Argument {
     this.parseArg = fn;
     return this;
   };
+
+  /**
+   * Only allow option value to be one of choices.
+   *
+   * @param {string[]} values
+   * @return {Argument}
+   */
+
+  choices(values) {
+    this.argChoices = values;
+    this.parseArg = (arg, previous) => {
+      if (!values.includes(arg)) {
+        throw new InvalidArgumentError(`Allowed choices are ${values.join(', ')}.`);
+      }
+      if (this.variadic) {
+        return this._concatValue(arg, previous);
+      }
+      return arg;
+    };
+    return this;
+  };
 }
 
 /**

lib/help.js

@@ -260,11 +260,20 @@ class Help {
 
   argumentDescription(argument) {
     const extraInfo = [];
+    if (argument.argChoices) {
+      extraInfo.push(
+        // use stringify to match the display of the default value
+        `choices: ${argument.argChoices.map((choice) => JSON.stringify(choice)).join(', ')}`);
+    }
     if (argument.defaultValue !== undefined) {
       extraInfo.push(`default: ${argument.defaultValueDescription || JSON.stringify(argument.defaultValue)}`);
     }
     if (extraInfo.length > 0) {
-      return `${argument.description} (${extraInfo.join(', ')})`;
+      const extraDescripton = `(${extraInfo.join(', ')})`;
+      if (argument.description) {
+        return `${argument.description} ${extraDescripton}`;
+      }
+      return extraDescripton;
     }
     return argument.description;
   }

tests/argument.chain.test.js

@@ -0,0 +1,21 @@
+const { Argument } = require('../');
+
+describe('Argument methods that should return this for chaining', () => {
+  test('when call .default() then returns this', () => {
+    const argument = new Argument('<value>');
+    const result = argument.default(3);
+    expect(result).toBe(argument);
+  });
+
+  test('when call .argParser() then returns this', () => {
+    const argument = new Argument('<value>');
+    const result = argument.argParser(() => { });
+    expect(result).toBe(argument);
+  });
+
+  test('when call .choices() then returns this', () => {
+    const argument = new Argument('<value>');
+    const result = argument.choices(['a']);
+    expect(result).toBe(argument);
+  });
+});

tests/argument.custom-processing.test.js

@@ -163,3 +163,15 @@ test('when custom processing for argument throws plain error then not CommanderE
   expect(caughtErr).toBeInstanceOf(Error);
   expect(caughtErr).not.toBeInstanceOf(commander.CommanderError);
 });
+
+// this is the happy path, testing failure case in command.exitOverride.test.js
+test('when argument argument in choices then argument set', () => {
+  const program = new commander.Command();
+  let shade;
+  program
+    .exitOverride()
+    .addArgument(new commander.Argument('<shade>').choices(['red', 'blue']))
+    .action((shadeParam) => { shade = shadeParam; });
+  program.parse(['red'], { from: 'user' });
+  expect(shade).toBe('red');
+});

tests/argument.variadic.test.js

@@ -81,4 +81,26 @@ describe('variadic argument', () => {
 
     expect(program.usage()).toBe('[options] [args...]');
   });
+
+  test('when variadic used with choices and one value then set in array', () => {
+    const program = new commander.Command();
+    let passedArg;
+    program
+      .addArgument(new commander.Argument('<value...>').choices(['one', 'two']))
+      .action((value) => { passedArg = value; });
+
+    program.parse(['one'], { from: 'user' });
+    expect(passedArg).toEqual(['one']);
+  });
+
+  test('when variadic used with choices and two values then set in array', () => {
+    const program = new commander.Command();
+    let passedArg;
+    program
+      .addArgument(new commander.Argument('<value...>').choices(['one', 'two']))
+      .action((value) => { passedArg = value; });
+
+    program.parse(['one', 'two'], { from: 'user' });
+    expect(passedArg).toEqual(['one', 'two']);
+  });
 });

tests/command.exitOverride.test.js

@@ -275,6 +275,23 @@ describe('.exitOverride and error details', () => {
     expectCommanderError(caughtErr, 1, 'commander.invalidArgument', "error: option '--colour <shade>' argument 'green' is invalid. Allowed choices are red, blue.");
   });
 
+  test('when command argument not in choices then throw CommanderError', () => {
+    const program = new commander.Command();
+    program
+      .exitOverride()
+      .addArgument(new commander.Argument('<shade>').choices(['red', 'blue']))
+      .action(() => {});
+
+    let caughtErr;
+    try {
+      program.parse(['green'], { from: 'user' });
+    } catch (err) {
+      caughtErr = err;
+    }
+
+    expectCommanderError(caughtErr, 1, 'commander.invalidArgument', "error: command-argument value 'green' is invalid for argument 'shade'. Allowed choices are red, blue.");
+  });
+
   test('when custom processing for option throws InvalidArgumentError then catch CommanderError', () => {
     function justSayNo(value) {
       throw new commander.InvalidArgumentError('NO');

tests/command.help.test.js

@@ -281,3 +281,19 @@ test('when arguments described in deprecated way and empty description then argu
   const helpInformation = program.helpInformation();
   expect(helpInformation).toMatch(/Arguments:\n +file +input source/);
 });
+
+test('when argument has choices then choices included in helpInformation', () => {
+  const program = new commander.Command();
+  program
+    .addArgument(new commander.Argument('<colour>', 'preferred colour').choices(['red', 'blue']));
+  const helpInformation = program.helpInformation();
+  expect(helpInformation).toMatch('(choices: "red", "blue")');
+});
+
+test('when argument has choices and default then both included in helpInformation', () => {
+  const program = new commander.Command();
+  program
+    .addArgument(new commander.Argument('<colour>', 'preferred colour').choices(['red', 'blue']).default('red'));
+  const helpInformation = program.helpInformation();
+  expect(helpInformation).toMatch('(choices: "red", "blue", default: "red")');
+});

tests/help.argumentDescription.test.js

@@ -27,4 +27,10 @@ describe('argumentDescription', () => {
     const helper = new commander.Help();
     expect(helper.argumentDescription(argument)).toEqual('description (default: custom)');
   });
+
+  test('when an argument has default value and no description then still return default value', () => {
+    const argument = new commander.Argument('[n]').default('default');
+    const helper = new commander.Help();
+    expect(helper.argumentDescription(argument)).toEqual('(default: "default")');
+  });
 });