Add support for variadic to choices (#1454)

tj · Jan 31, 2021 · 09dfb67 · 09dfb67
1 parent 02b40ec
commit 09dfb67
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 6 deletions.
diff --git a/index.js b/index.js
@@ -425,6 +425,18 @@ class Option {
     return this;
   };
 
+  /**
+   * @api private
+   */
+
+  _concatValue(value, previous) {
+    if (previous === this.defaultValue || !Array.isArray(previous)) {
+      return [value];
+    }
+
+    return previous.concat(value);
+  }
+
   /**
    * Only allow option value to be one of choices.
    *
@@ -434,10 +446,13 @@ class Option {
 
   choices(values) {
     this.argChoices = values;
-    this.parseArg = (arg) => {
+    this.parseArg = (arg, previous) => {
       if (!values.includes(arg)) {
         throw new InvalidOptionArgumentError(`Allowed choices are ${values.join(', ')}.`);
       }
+      if (this.variadic) {
+        return this._concatValue(arg, previous);
+      }
       return arg;
     };
     return this;
@@ -976,11 +991,7 @@ class Command extends EventEmitter {
           throw err;
         }
       } else if (val !== null && option.variadic) {
-        if (oldValue === defaultValue || !Array.isArray(oldValue)) {
-          val = [val];
-        } else {
-          val = oldValue.concat(val);
-        }
+        val = option._concatValue(val, oldValue);
       }
 
       // unassigned or boolean value

diff --git a/tests/options.variadic.test.js b/tests/options.variadic.test.js
@@ -40,6 +40,24 @@ describe('variadic option with required value', () => {
     expect(program.opts().required).toEqual(['one', 'two']);
   });
 
+  test('when variadic used with choices and one value then set in array', () => {
+    const program = new commander.Command();
+    program
+      .addOption(new commander.Option('-r,--required <value...>').choices(['one', 'two']));
+
+    program.parse(['--required', 'one'], { from: 'user' });
+    expect(program.opts().required).toEqual(['one']);
+  });
+
+  test('when variadic used with choices and two values then set in array', () => {
+    const program = new commander.Command();
+    program
+      .addOption(new commander.Option('-r,--required <value...>').choices(['one', 'two']));
+
+    program.parse(['--required', 'one', 'two'], { from: 'user' });
+    expect(program.opts().required).toEqual(['one', 'two']);
+  });
+
   test('when variadic with short combined argument then not variadic', () => {
     const program = new commander.Command();
     program