Parse comma-separated multiple values.

R=rnystrom@google.com

Review URL: https://codereview.chromium.org//975463004
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1fb1bb7..8c416d6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,9 @@
+## 0.13.0
+
+* **Breaking change**: An option that allows multiple values will now
+  automatically split apart comma-separated values. This can be controlled with
+  the `splitCommas` option.
+
 ## 0.12.2+6
 
 * Remove the dependency on the `collection` package.
diff --git a/README.md b/README.md
index fa635cb..96df5e6 100644
--- a/README.md
+++ b/README.md
@@ -189,6 +189,17 @@
 print(results['mode']); // prints '[on, off]'
 ```
 
+By default, values for a multi-valued option may also be separated with commas:
+
+```dart
+var parser = new ArgParser();
+parser.addOption('mode', allowMultiple: true);
+var results = parser.parse(['--mode', 'on,off']);
+print(results['mode']); // prints '[on, off]'
+```
+
+This can be disabled by passing `splitCommas: false`.
+
 ## Defining commands ##
 
 In addition to *options*, you can also define *commands*. A command is a named
diff --git a/lib/src/arg_parser.dart b/lib/src/arg_parser.dart
index e7f728c..49d6aa6 100644
--- a/lib/src/arg_parser.dart
+++ b/lib/src/arg_parser.dart
@@ -77,18 +77,25 @@
   ///
   /// * There is already an option with name [name].
   /// * There is already an option using abbreviation [abbr].
+  /// * [splitCommas] is passed but [allowMultiple] is `false`.
   void addOption(String name, {String abbr, String help, String valueHelp,
       List<String> allowed, Map<String, String> allowedHelp, String defaultsTo,
-      void callback(value), bool allowMultiple: false, bool hide: false}) {
+      void callback(value), bool allowMultiple: false, bool splitCommas,
+      bool hide: false}) {
+    if (!allowMultiple && splitCommas != null) {
+      throw new ArgumentError(
+          'splitCommas may not be set if allowMultiple is false.');
+    }
+
     _addOption(name, abbr, help, valueHelp, allowed, allowedHelp, defaultsTo,
         callback, allowMultiple ? OptionType.MULTIPLE : OptionType.SINGLE,
-        hide: hide);
+        splitCommas: splitCommas, hide: hide);
   }
 
   void _addOption(String name, String abbr, String help, String valueHelp,
       List<String> allowed, Map<String, String> allowedHelp, defaultsTo,
       void callback(value), OptionType type,
-      {bool negatable: false, bool hide: false}) {
+      {bool negatable: false, bool splitCommas, bool hide: false}) {
     // Make sure the name isn't in use.
     if (_options.containsKey(name)) {
       throw new ArgumentError('Duplicate option "$name".');
@@ -105,7 +112,7 @@
 
     _options[name] = newOption(name, abbr, help, valueHelp, allowed,
         allowedHelp, defaultsTo, callback, type,
-        negatable: negatable, hide: hide);
+        negatable: negatable, splitCommas: splitCommas, hide: hide);
   }
 
   /// Parses [args], a list of command-line arguments, matches them against the
diff --git a/lib/src/option.dart b/lib/src/option.dart
index 4045bce..07e87f6 100644
--- a/lib/src/option.dart
+++ b/lib/src/option.dart
@@ -13,9 +13,10 @@
 Option newOption(String name, String abbreviation, String help,
     String valueHelp, List<String> allowed, Map<String, String> allowedHelp,
     defaultValue, Function callback, OptionType type,
-    {bool negatable, bool hide: false}) {
+    {bool negatable, bool splitCommas, bool hide: false}) {
   return new Option._(name, abbreviation, help, valueHelp, allowed, allowedHelp,
-      defaultValue, callback, type, negatable: negatable, hide: hide);
+      defaultValue, callback, type, negatable: negatable,
+      splitCommas: splitCommas, hide: hide);
 }
 
 /// A command-line option. Includes both flags and options which take a value.
@@ -30,6 +31,7 @@
   final Map<String, String> allowedHelp;
   final OptionType type;
   final bool negatable;
+  final bool splitCommas;
   final bool hide;
 
   /// Whether the option is boolean-valued flag.
@@ -43,13 +45,20 @@
 
   Option._(this.name, this.abbreviation, this.help, this.valueHelp,
       List<String> allowed, Map<String, String> allowedHelp, this.defaultValue,
-      this.callback, this.type, {this.negatable, this.hide: false})
+      this.callback, OptionType type, {this.negatable, bool splitCommas,
+      this.hide: false})
       : this.allowed = allowed == null
           ? null
           : new UnmodifiableListView(allowed),
         this.allowedHelp = allowedHelp == null
             ? null
-            : new UnmodifiableMapView(allowedHelp) {
+            : new UnmodifiableMapView(allowedHelp),
+        this.type = type,
+        // If the user doesn't specify [splitCommas], it defaults to true for
+        // multiple options.
+        this.splitCommas = splitCommas == null
+            ? type == OptionType.MULTIPLE
+            : splitCommas {
     if (name.isEmpty) {
       throw new ArgumentError('Name cannot be empty.');
     } else if (name.startsWith('-')) {
diff --git a/lib/src/parser.dart b/lib/src/parser.dart
index 1e297b0..848bb9c 100644
--- a/lib/src/parser.dart
+++ b/lib/src/parser.dart
@@ -131,7 +131,7 @@
     args.removeAt(0);
 
     if (option.isFlag) {
-      setOption(results, option, true);
+      setFlag(results, option, true);
     } else {
       readNextArgAsValue(option);
     }
@@ -196,7 +196,7 @@
     validate(
         option.isFlag, 'Option "-$c" must be a flag to be in a collapsed "-".');
 
-    setOption(results, option, true);
+    setFlag(results, option, true);
   }
 
   /// Tries to parse the current argument as a long-form named option, which
@@ -213,7 +213,7 @@
         validate(longOpt[3] == null,
             'Flag option "$name" should not be given a value.');
 
-        setOption(results, option, true);
+        setFlag(results, option, true);
       } else if (longOpt[3] != null) {
         // We have a value like --foo=bar.
         setOption(results, option, longOpt[3]);
@@ -235,7 +235,7 @@
       validate(option.isFlag, 'Cannot negate non-flag option "$name".');
       validate(option.negatable, 'Cannot negate option "$name".');
 
-      setOption(results, option, false);
+      setFlag(results, option, false);
     } else {
       // Walk up to the parent command if possible.
       validate(parent != null, 'Could not find an option named "$name".');
@@ -252,19 +252,42 @@
     if (!condition) throw new FormatException(message);
   }
 
-  /// Validates and stores [value] as the value for [option].
-  void setOption(Map results, Option option, value) {
-    // See if it's one of the allowed values.
-    if (option.allowed != null) {
-      validate(option.allowed.any((allow) => allow == value),
-          '"$value" is not an allowed value for option "${option.name}".');
+  /// Validates and stores [value] as the value for [option], which must not be
+  /// a flag.
+  void setOption(Map results, Option option, String value) {
+    assert(!option.isFlag);
+
+    if (!option.isMultiple) {
+      _validateAllowed(option, value);
+      results[option.name] = value;
+      return;
     }
 
-    if (option.isMultiple) {
-      var list = results.putIfAbsent(option.name, () => []);
-      list.add(value);
+    var list = results.putIfAbsent(option.name, () => []);
+
+    if (option.splitCommas) {
+      for (var element in value.split(",")) {
+        _validateAllowed(option, element);
+        list.add(element);
+      }
     } else {
-      results[option.name] = value;
+      _validateAllowed(option, value);
+      list.add(value);
     }
   }
+
+  /// Validates and stores [value] as the value for [option], which must be a
+  /// flag.
+  void setFlag(Map results, Option option, bool value) {
+    assert(option.isFlag);
+    results[option.name] = value;
+  }
+
+  /// Validates that [value] is allowed as a value of [option].
+  void _validateAllowed(Option option, String value) {
+    if (option.allowed == null) return;
+
+    validate(option.allowed.contains(value),
+        '"$value" is not an allowed value for option "${option.name}".');
+  }
 }
diff --git a/pubspec.yaml b/pubspec.yaml
index 0af927d..4a3b1f6 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,5 +1,5 @@
 name: args
-version: 0.12.2+6
+version: 0.13.0
 author: "Dart Team <misc@dartlang.org>"
 homepage: https://github.com/dart-lang/args
 description: >
diff --git a/test/args_test.dart b/test/args_test.dart
index 4c23ba5..b22d0c9 100644
--- a/test/args_test.dart
+++ b/test/args_test.dart
@@ -109,6 +109,13 @@
       }
     });
 
+    test('throws ArgumentError if splitCommas is passed with allowMultiple: '
+        'false', () {
+      var parser = new ArgParser();
+      throwsIllegalArg(() => parser.addOption('flummox', splitCommas: true));
+      throwsIllegalArg(() => parser.addOption('flummox', splitCommas: false));
+    });
+
     test('accepts valid option names', () {
       var parser = new ArgParser();
 
diff --git a/test/parse_test.dart b/test/parse_test.dart
index 0dd00ba..502210a 100644
--- a/test/parse_test.dart
+++ b/test/parse_test.dart
@@ -199,6 +199,51 @@
         parser.parse([]);
         expect(a, isEmpty);
       });
+
+      test('allowMultiple parses comma-separated strings', () {
+        var a;
+        var parser = new ArgParser();
+        parser.addOption('a',
+            allowMultiple: true, callback: (value) => a = value);
+
+        parser.parse(['--a=v,w', '--a=x']);
+        expect(a, equals(['v', 'w', 'x']));
+      });
+
+      test("allowMultiple doesn't parses comma-separated strings with "
+          "splitCommas: false", () {
+        var a;
+        var parser = new ArgParser();
+        parser.addOption('a',
+            allowMultiple: true,
+            splitCommas: false,
+            callback: (value) => a = value);
+
+        parser.parse(['--a=v,w', '--a=x']);
+        expect(a, equals(['v,w', 'x']));
+      });
+
+      test('allowMultiple parses empty strings', () {
+        var a;
+        var parser = new ArgParser();
+        parser.addOption('a',
+            allowMultiple: true, callback: (value) => a = value);
+
+        parser.parse(['--a=,v', '--a=w,', '--a=,', '--a=x,,y', '--a', '']);
+        expect(a, equals(['', 'v', 'w', '', '', '', 'x', '', 'y', '']));
+      });
+
+      test('allowMultiple with allowed parses comma-separated strings', () {
+        var a;
+        var parser = new ArgParser();
+        parser.addOption('a',
+            allowMultiple: true,
+            allowed: ['v', 'w', 'x'],
+            callback: (value) => a = value);
+
+        parser.parse(['--a=v,w', '--a=x']);
+        expect(a, equals(['v', 'w', 'x']));
+      });
     });
 
     group('abbreviations', () {
@@ -287,6 +332,14 @@
         throwsFormat(parser, ['-mprofile']);
       });
 
+      test('throw if a comma-separated value is not allowed', () {
+        var parser = new ArgParser();
+        parser.addOption('mode', abbr: 'm', allowMultiple: true,
+            allowed: ['debug', 'release']);
+
+        throwsFormat(parser, ['-mdebug,profile']);
+      });
+
       test('throw if any but the first is not a flag', () {
         var parser = new ArgParser();
         parser.addFlag('apple', abbr: 'a');