Add a separate addMultiOption() method
This allows us to be more specific in our types and to keep flags that
are specific to multi-options in the place where they make sense.
Closes #90
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7cc69c0..20eba1f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,12 @@
`Option.abbr` and `Option.defaultsTo`. This makes all of `Option`'s fields
match the corresponding parameters to `ArgParser.addOption()`.
+* Deprecated the `allowMultiple` and `splitCommas` arguments to
+ `ArgParser.addOption()` in favor of a separate `ArgParser.addMultiOption()`
+ method. This allows us to provide more accurate type information, and to avoid
+ adding flags that only make sense for multi-options in places where they might
+ be usable for single-value options.
+
## 1.3.0
* Type `Command.run()`'s return value as `FutureOr<T>`.
diff --git a/analysis_options.yaml b/analysis_options.yaml
index 092773b..32747ee 100644
--- a/analysis_options.yaml
+++ b/analysis_options.yaml
@@ -1,5 +1,11 @@
analyzer:
- strong-mode: true
+ strong-mode: true
+
+ errors:
+ # TODO(nweiz): Remove this ignore when sdk#30084 is fixed or when args no
+ # longer refers to its own deprecated members.
+ deprecated_member_use: ignore
+
linter:
rules:
# Errors
diff --git a/lib/src/allow_anything_parser.dart b/lib/src/allow_anything_parser.dart
index 58e4812..248a39a 100644
--- a/lib/src/allow_anything_parser.dart
+++ b/lib/src/allow_anything_parser.dart
@@ -45,6 +45,20 @@
"ArgParser.allowAnything().addOption() isn't supported.");
}
+ void addMultiOption(String name,
+ {String abbr,
+ String help,
+ String valueHelp,
+ Iterable<String> allowed,
+ Map<String, String> allowedHelp,
+ Iterable<String> defaultsTo,
+ void callback(List<String> values),
+ bool splitCommas: true,
+ bool hide: false}) {
+ throw new UnsupportedError(
+ "ArgParser.allowAnything().addMultiOption() isn't supported.");
+ }
+
void addSeparator(String text) {
throw new UnsupportedError(
"ArgParser.allowAnything().addSeparator() isn't supported.");
diff --git a/lib/src/arg_parser.dart b/lib/src/arg_parser.dart
index c115197..bcdb8df 100644
--- a/lib/src/arg_parser.dart
+++ b/lib/src/arg_parser.dart
@@ -156,14 +156,8 @@
/// that are often surprising, and its use is discouraged in favor of reading
/// values from the [ArgResult].
///
- /// If [allowMultiple] is `true`, the user may pass this option multiple times
- /// and its value will be a `List<String>` rather than a `String`. The default
- /// value will be `[]` rather than `null`, or `[defaultsTo]` if [defaultsTo]
- /// is passed.
- ///
- /// If [splitCommas] is `true`, multiple values may be passed by writing
- /// `--option a,b` in addition to `--option a --option b`. It defaults to
- /// `true` if [allowMultiple] is `true` and `false` otherwise.
+ /// The [allowMultiple] and [splitCommas] options are deprecated; the
+ /// [addMultiOption] method should be used instead.
///
/// If [hide] is `true`, this option won't be included in [usage].
///
@@ -180,17 +174,89 @@
Map<String, String> allowedHelp,
String defaultsTo,
Function callback,
- bool allowMultiple: false,
- bool splitCommas,
+ @Deprecated("Use addMultiOption() instead.") bool allowMultiple: false,
+ @Deprecated("Use addMultiOption() instead.") 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,
- splitCommas: splitCommas, hide: hide);
+ _addOption(
+ name,
+ abbr,
+ help,
+ valueHelp,
+ allowed,
+ allowedHelp,
+ allowMultiple
+ ? (defaultsTo == null ? <String>[] : [defaultsTo])
+ : defaultsTo,
+ callback,
+ allowMultiple ? OptionType.multiple : OptionType.single,
+ splitCommas: splitCommas,
+ hide: hide);
+ }
+
+ /// Defines an option that takes multiple values.
+ ///
+ /// The [abbr] argument is a single-character string that can be used as a
+ /// shorthand for this option. For example, `abbr: "a"` will allow the user to
+ /// pass `-a value` or `-avalue`.
+ ///
+ /// The [help] argument is used by [usage] to describe this option.
+ ///
+ /// The [valueHelp] argument is used by [usage] as a name for the value this
+ /// argument takes. For example, `valueHelp: "FOO"` will include
+ /// `--option=<FOO>` rather than just `--option` in the usage string.
+ ///
+ /// The [allowed] argument is a list of valid values for this argument. If
+ /// it's non-`null` and the user passes a value that's not included in the
+ /// list, [parse] will throw a [FormatException]. The allowed values will also
+ /// be included in [usage].
+ ///
+ /// The [allowedHelp] argument is a map from values in [allowed] to
+ /// documentation for those values that will be included in [usage].
+ ///
+ /// The [defaultsTo] argument indicates the values this option will have if
+ /// the user doesn't explicitly pass it in (or `[]` by default).
+ ///
+ /// The [callback] argument is invoked with the option's value when the option
+ /// is parsed. Note that this makes argument parsing order-dependent in ways
+ /// that are often surprising, and its use is discouraged in favor of reading
+ /// values from the [ArgResult].
+ ///
+ /// If [splitCommas] is `true` (the default), multiple options may be passed
+ /// by writing `--option a,b` in addition to `--option a --option b`.
+ ///
+ /// If [hide] is `true`, this option won't be included in [usage].
+ ///
+ /// Throws an [ArgumentError] if:
+ ///
+ /// * There is already an option with name [name].
+ /// * There is already an option using abbreviation [abbr].
+ void addMultiOption(String name,
+ {String abbr,
+ String help,
+ String valueHelp,
+ Iterable<String> allowed,
+ Map<String, String> allowedHelp,
+ Iterable<String> defaultsTo,
+ void callback(List<String> values),
+ bool splitCommas: true,
+ bool hide: false}) {
+ _addOption(
+ name,
+ abbr,
+ help,
+ valueHelp,
+ allowed,
+ allowedHelp,
+ defaultsTo?.toList() ?? <String>[],
+ callback == null ? null : (value) => callback(value as List<String>),
+ OptionType.multiple,
+ splitCommas: splitCommas,
+ hide: hide);
}
void _addOption(
diff --git a/lib/src/option.dart b/lib/src/option.dart
index 0902fe3..727ea52 100644
--- a/lib/src/option.dart
+++ b/lib/src/option.dart
@@ -141,10 +141,8 @@
/// list containing [defaultsTo] if set.
dynamic getOrDefault(value) {
if (value != null) return value;
-
- if (!isMultiple) return defaultsTo;
- if (defaultsTo != null) return [defaultsTo];
- return [];
+ if (isMultiple) return defaultsTo ?? <String>[];
+ return defaultsTo;
}
static final _invalidChars = new RegExp(r'''[ \t\r\n"'\\/]''');
diff --git a/pubspec.yaml b/pubspec.yaml
index 4985ea2..c476bc3 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,5 +1,5 @@
name: args
-version: 1.4.0-dev
+version: 1.4.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 ff968ad..8741ad8 100644
--- a/test/args_test.dart
+++ b/test/args_test.dart
@@ -274,8 +274,11 @@
parser.addFlag('flag-def', defaultsTo: true);
parser.addOption('single-no');
parser.addOption('single-def', defaultsTo: 'def');
- parser.addOption('multi-no', allowMultiple: true);
- parser.addOption('multi-def', allowMultiple: true, defaultsTo: 'def');
+ parser.addOption('allow-multi-no', allowMultiple: true);
+ parser.addOption('allow-multi-def',
+ allowMultiple: true, defaultsTo: 'def');
+ parser.addMultiOption('multi-no');
+ parser.addMultiOption('multi-def', defaultsTo: ['def']);
expect(parser.options['flag-no'].getOrDefault(null), equals(null));
expect(parser.options['flag-no'].getOrDefault(false), equals(false));
@@ -285,6 +288,13 @@
expect(parser.options['single-no'].getOrDefault('v'), equals('v'));
expect(parser.options['single-def'].getOrDefault(null), equals('def'));
expect(parser.options['single-def'].getOrDefault('v'), equals('v'));
+ expect(parser.options['allow-multi-no'].getOrDefault(null), equals([]));
+ expect(
+ parser.options['allow-multi-no'].getOrDefault(['v']), equals(['v']));
+ expect(parser.options['allow-multi-def'].getOrDefault(null),
+ equals(['def']));
+ expect(
+ parser.options['allow-multi-def'].getOrDefault(['v']), equals(['v']));
expect(parser.options['multi-no'].getOrDefault(null), equals([]));
expect(parser.options['multi-no'].getOrDefault(['v']), equals(['v']));
expect(parser.options['multi-def'].getOrDefault(null), equals(['def']));
diff --git a/test/parse_test.dart b/test/parse_test.dart
index 87f3d81..d212cf0 100644
--- a/test/parse_test.dart
+++ b/test/parse_test.dart
@@ -152,104 +152,186 @@
expect(a, isNull);
});
- test(
- 'for multiple present, allowMultiple, options are invoked with '
- 'value as a list', () {
- var a;
- var parser = new ArgParser();
- parser.addOption('a',
- allowMultiple: true, callback: (value) => a = value);
+ group("with allowMultiple", () {
+ test('for multiple present, options are invoked with value as a list',
+ () {
+ var a;
+ var parser = new ArgParser();
+ parser.addOption('a',
+ allowMultiple: true, callback: (value) => a = value);
- parser.parse(['--a=v', '--a=x']);
- expect(a, equals(['v', 'x']));
+ parser.parse(['--a=v', '--a=x']);
+ expect(a, equals(['v', 'x']));
- // This reified type is important in strong mode so that people can
- // safely write "as List<String>".
- expect(a, new isInstanceOf<List<String>>());
+ // This reified type is important in strong mode so that people can
+ // safely write "as List<String>".
+ expect(a, new isInstanceOf<List<String>>());
+ });
+
+ test(
+ 'for single present, options are invoked with value as a single '
+ 'element list', () {
+ var a;
+ var parser = new ArgParser();
+ parser.addOption('a',
+ allowMultiple: true, callback: (value) => a = value);
+
+ parser.parse(['--a=v']);
+ expect(a, equals(['v']));
+ });
+
+ test('for absent, options are invoked with default value as a list',
+ () {
+ var a;
+ var parser = new ArgParser();
+ parser.addOption('a',
+ allowMultiple: true,
+ defaultsTo: 'v',
+ callback: (value) => a = value);
+
+ parser.parse([]);
+ expect(a, equals(['v']));
+ });
+
+ test('for absent, options are invoked with value as an empty list', () {
+ var a;
+ var parser = new ArgParser();
+ parser.addOption('a',
+ allowMultiple: true, callback: (value) => a = value);
+
+ parser.parse([]);
+ expect(a, isEmpty);
+ });
+
+ test('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("doesn't parse 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('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('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']));
+ });
});
- test(
- 'for single present, allowMultiple, options are invoked with '
- ' value as a single element list', () {
- var a;
- var parser = new ArgParser();
- parser.addOption('a',
- allowMultiple: true, callback: (value) => a = value);
+ group("with addMultiOption", () {
+ test('for multiple present, options are invoked with value as a list',
+ () {
+ var a;
+ var parser = new ArgParser();
+ parser.addMultiOption('a', callback: (value) => a = value);
- parser.parse(['--a=v']);
- expect(a, equals(['v']));
- });
+ parser.parse(['--a=v', '--a=x']);
+ expect(a, equals(['v', 'x']));
- test(
- 'for absent, allowMultiple, options are invoked with default '
- 'value as a list.', () {
- var a;
- var parser = new ArgParser();
- parser.addOption('a',
- allowMultiple: true,
- defaultsTo: 'v',
- callback: (value) => a = value);
+ // This reified type is important in strong mode so that people can
+ // safely write "as List<String>".
+ expect(a, new isInstanceOf<List<String>>());
+ });
- parser.parse([]);
- expect(a, equals(['v']));
- });
+ test(
+ 'for single present, options are invoked with value as a single '
+ 'element list', () {
+ var a;
+ var parser = new ArgParser();
+ parser.addMultiOption('a', callback: (value) => a = value);
- test(
- 'for absent, allowMultiple, options are invoked with value '
- 'as an empty list.', () {
- var a;
- var parser = new ArgParser();
- parser.addOption('a',
- allowMultiple: true, callback: (value) => a = value);
+ parser.parse(['--a=v']);
+ expect(a, equals(['v']));
+ });
- parser.parse([]);
- expect(a, isEmpty);
- });
+ test('for absent, options are invoked with default value', () {
+ var a;
+ var parser = new ArgParser();
+ parser.addMultiOption('a',
+ defaultsTo: ['v', 'w'], callback: (value) => a = value);
- test('allowMultiple parses comma-separated strings', () {
- var a;
- var parser = new ArgParser();
- parser.addOption('a',
- allowMultiple: true, callback: (value) => a = value);
+ parser.parse([]);
+ expect(a, equals(['v', 'w']));
+ });
- parser.parse(['--a=v,w', '--a=x']);
- expect(a, equals(['v', 'w', 'x']));
- });
+ test('for absent, options are invoked with value as an empty list', () {
+ var a;
+ var parser = new ArgParser();
+ parser.addMultiOption('a', callback: (value) => a = value);
- 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([]);
+ expect(a, isEmpty);
+ });
- parser.parse(['--a=v,w', '--a=x']);
- expect(a, equals(['v,w', 'x']));
- });
+ test('parses comma-separated strings', () {
+ var a;
+ var parser = new ArgParser();
+ parser.addMultiOption('a', callback: (value) => a = value);
- test('allowMultiple parses empty 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']));
+ });
- parser.parse(['--a=,v', '--a=w,', '--a=,', '--a=x,,y', '--a', '']);
- expect(a, equals(['', 'v', 'w', '', '', '', 'x', '', 'y', '']));
- });
+ test("doesn't parse comma-separated strings with splitCommas: false",
+ () {
+ var a;
+ var parser = new ArgParser();
+ parser.addMultiOption('a',
+ splitCommas: false, callback: (value) => a = value);
- 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']));
+ });
- parser.parse(['--a=v,w', '--a=x']);
- expect(a, equals(['v', 'w', 'x']));
+ test('parses empty strings', () {
+ var a;
+ var parser = new ArgParser();
+ parser.addMultiOption('a', callback: (value) => a = value);
+
+ parser.parse(['--a=,v', '--a=w,', '--a=,', '--a=x,,y', '--a', '']);
+ expect(a, equals(['', 'v', 'w', '', '', '', 'x', '', 'y', '']));
+ });
+
+ test('with allowed parses comma-separated strings', () {
+ var a;
+ var parser = new ArgParser();
+ parser.addMultiOption('a',
+ allowed: ['v', 'w', 'x'], callback: (value) => a = value);
+
+ parser.parse(['--a=v,w', '--a=x']);
+ expect(a, equals(['v', 'w', 'x']));
+ });
});
});
@@ -340,12 +422,22 @@
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']);
+ group('throw if a comma-separated value is not allowed', () {
+ test("with allowMultiple", () {
+ var parser = new ArgParser();
+ parser.addOption('mode',
+ abbr: 'm', allowMultiple: true, allowed: ['debug', 'release']);
- throwsFormat(parser, ['-mdebug,profile']);
+ throwsFormat(parser, ['-mdebug,profile']);
+ });
+
+ test("with addMultiOption", () {
+ var parser = new ArgParser();
+ parser
+ .addMultiOption('mode', abbr: 'm', allowed: ['debug', 'release']);
+
+ throwsFormat(parser, ['-mdebug,profile']);
+ });
});
test('throw if any but the first is not a flag', () {
@@ -449,22 +541,40 @@
expect(args['define'], equals('2'));
});
- test('returns a List if multi-valued', () {
- var parser = new ArgParser();
- parser.addOption('define', allowMultiple: true);
- var args = parser.parse(['--define=1']);
- expect(args['define'], equals(['1']));
- args = parser.parse(['--define=1', '--define=2']);
- expect(args['define'], equals(['1', '2']));
+ group('returns a List', () {
+ test('with allowMultiple', () {
+ var parser = new ArgParser();
+ parser.addOption('define', allowMultiple: true);
+ var args = parser.parse(['--define=1']);
+ expect(args['define'], equals(['1']));
+ args = parser.parse(['--define=1', '--define=2']);
+ expect(args['define'], equals(['1', '2']));
+ });
+
+ test('with addMultiOption', () {
+ var parser = new ArgParser();
+ parser.addMultiOption('define');
+ var args = parser.parse(['--define=1']);
+ expect(args['define'], equals(['1']));
+ args = parser.parse(['--define=1', '--define=2']);
+ expect(args['define'], equals(['1', '2']));
+ });
});
- test(
- 'returns the default value for multi-valued arguments '
- 'if not explicitly set', () {
- var parser = new ArgParser();
- parser.addOption('define', defaultsTo: '0', allowMultiple: true);
- var args = parser.parse(['']);
- expect(args['define'], equals(['0']));
+ group('returns the default value if not explicitly set', () {
+ test('with allowMultiple', () {
+ var parser = new ArgParser();
+ parser.addOption('define', defaultsTo: '0', allowMultiple: true);
+ var args = parser.parse(['']);
+ expect(args['define'], equals(['0']));
+ });
+
+ test('with addMultiOption', () {
+ var parser = new ArgParser();
+ parser.addMultiOption('define', defaultsTo: ['0']);
+ var args = parser.parse(['']);
+ expect(args['define'], equals(['0']));
+ });
});
test('are case-sensitive', () {
