Added the continueParsing option to ArgParser.

pkg/args/lib/src/parser.dart

Index: pkg/args/lib/src/parser.dart
diff --git a/pkg/args/lib/src/parser.dart b/pkg/args/lib/src/parser.dart
index d35f9ea4aff1bb67a1c20034b17574168d0c6502..9352bf47270637ce910e9689623f5cda687684d2 100644
--- a/pkg/args/lib/src/parser.dart
+++ b/pkg/args/lib/src/parser.dart
@@ -14,6 +14,17 @@ final _LONG_OPT = new RegExp(r'^--([a-zA-Z\-_0-9]+)(=(.*))?$');
  * The actual parsing class. Unlike [ArgParser] which is really more an "arg
  * grammar", this is the class that does the parsing and holds the mutable
  * state required during a parse.
+ *
+ * If [parseAllOptions] is set, the parser will continue parsing even after it
+ * finds an argument that is neither an option nor a command. This allows
+ * options to be specified after command parameters.
+ *
+ * [parseAllOptions] is false by default, so when a non-option, non-command
+ * argument is encountered, it and any remaining arguments are passed to the
+ * leafmost command.
+ *
+ * An argument that looks like an option, but does not match any option will
+ * throw a [FormatException], regardless of the value of [parseAllOptions].

[Bob Nystrom, 2013/06/20 00:33:36]

Redundant with ArgParser docs. Remove, I think.

[Andrei Mouravski, 2013/06/22 00:54:02]

Done.

  */
 class Parser {
   /**
@@ -28,16 +39,24 @@ class Parser {
    */
   final Parser parent;
 
+  /** If `true`, parser will continue after it sees a non-option argument. */
+  final bool parseAllOptions;
+
   /** The grammar being parsed. */
   final ArgParser grammar;
 
   /** The arguments being parsed. */
   final List<String> args;
 
+  /** The remaining non-option, non-command arguments. */
+  List<String> rest;
+
   /** The accumulated parsed options. */
   final Map results = {};
 
-  Parser(this.commandName, this.grammar, this.args, [this.parent]);
+  Parser(this.commandName, this.grammar, this.args,
+      [this.parseAllOptions = false, this.parent, rest])
+      : this.rest = rest == null ? [] : rest;

[Bob Nystrom, 2013/06/20 00:33:36]

How about having the field be:

final rest = <String>[];

and then in the constructor body, do if (rest != null) this.rest.addAll(rest);

That way the caller doesn't have to copy.

[Andrei Mouravski, 2013/06/22 00:54:02]

Done.

 
   /** The current argument being parsed. */
   String get current => args[0];
@@ -67,20 +86,31 @@ class Parser {
       // options so that commands can have option-like names.
       var command = grammar.commands[current];
       if (command != null) {
+        validate(rest.isEmpty, 'Cannot specify arguments before a command.');
         var commandName = args.removeAt(0);
-        var commandParser = new Parser(commandName, command, args, this);
+        var commandParser = new Parser(commandName, command, args,
+            parseAllOptions, this, rest.toList());
         commandResults = commandParser.parse();
-        continue;
+
+        // All remaining arguments were passed to command so clear them here.
+        rest.clear();
+        break;
       }
 
       // Try to parse the current argument as an option. Note that the order
       // here matters.
-      if (parseSoloOption()) continue;
-      if (parseAbbreviation(this)) continue;
-      if (parseLongOption()) continue;
+      if (isCurrentArgAnOption) {
+        if (parseSoloOption()) continue;
+        if (parseAbbreviation(this)) continue;
+        if (parseLongOption()) continue;
+        throw new FormatException(
+            'Could not find an option or flag "${args[0]}".');

[Bob Nystrom, 2013/06/20 00:33:36]

Why is this throw needed? Shouldn't it have already thrown?

[Andrei Mouravski, 2013/06/22 00:54:02]

Because I'm more lenient inside the parse functions and this catches things that
look like options but aren't.

[Bob Nystrom, 2013/06/24 15:53:20]

*Why* are you being more lenient? Those functions should immediately throw if
they match their regex but can't find an appropriate option.
Why aren't those already caught like they were before?

[Andrei Mouravski, 2013/06/24 20:41:03]

Done.

+      }
 
-      // If we got here, the argument doesn't look like an option, so stop.
-      break;
+      // This argument is neither option nor command, so stop parsing unless
+      // the [parseAllOptions] option is set.
+      if (!parseAllOptions) break;
+      rest.add(args.removeAt(0));
     }
 
     // Set unspecified multivalued arguments to their default value,
@@ -95,7 +125,7 @@ class Parser {
     });
 
     // Add in the leftover arguments we didn't parse to the innermost command.
-    var rest = args.toList();
+    rest.addAll(args.toList());
     args.clear();
     return new ArgResults(results, commandName, commandResults, rest);
   }
@@ -117,6 +147,10 @@ class Parser {
     args.removeAt(0);
   }
 
+  /** Returns `true` if the current argument looks like an option. */
+  bool get isCurrentArgAnOption => [_SOLO_OPT, _ABBR_OPT, _LONG_OPT].any(
+      (re) => re.firstMatch(current) != null);
+
   /**
    * Tries to parse the current argument as a "solo" option, which is a single
    * hyphen followed by a single letter. We treat this differently than
@@ -130,9 +164,8 @@ class Parser {
     var option = grammar.findByAbbreviation(soloOpt[1]);
     if (option == null) {
       // Walk up to the parent command if possible.
-      validate(parent != null,
+      return tryParseOnParent((p) => p.parseSoloOption(),
           'Could not find an option or flag "-${soloOpt[1]}".');
-      return parent.parseSoloOption();
     }
 
     args.removeAt(0);
@@ -161,9 +194,8 @@ class Parser {
     var first = grammar.findByAbbreviation(c);
     if (first == null) {
       // Walk up to the parent command if possible.
-      validate(parent != null,
+      return tryParseOnParent((p) => p.parseAbbreviation(innermostCommand),
           'Could not find an option with short name "-$c".');
-      return parent.parseAbbreviation(innermostCommand);
     } else if (!first.isFlag) {
       // The first character is a non-flag option, so the rest must be the
       // value.
@@ -194,9 +226,8 @@ class Parser {
     var option = grammar.findByAbbreviation(c);
     if (option == null) {
       // Walk up to the parent command if possible.
-      validate(parent != null,
+      tryParseOnParent((p) => p.parseShortFlag(c),
           'Could not find an option with short name "-$c".');
-      parent.parseShortFlag(c);
       return;
     }
 
@@ -238,8 +269,8 @@ class Parser {
       option = grammar.options[name];
       if (option == null) {
         // Walk up to the parent command if possible.
-        validate(parent != null, 'Could not find an option named "$name".');
-        return parent.parseLongOption();
+        return tryParseOnParent((p) => p.parseLongOption(),
+            'Could not find an option named "$name".');
       }
 
       args.removeAt(0);
@@ -249,8 +280,8 @@ class Parser {
       setOption(results, option, false);
     } else {
       // Walk up to the parent command if possible.
-      validate(parent != null, 'Could not find an option named "$name".');
-      return parent.parseLongOption();
+      return tryParseOnParent((p) => p.parseLongOption(),
+          'Could not find an option named "$name".');
     }
 
     return true;
@@ -264,6 +295,24 @@ class Parser {
     if (!condition) throw new FormatException(message);
   }
 
+  /**
+   * Tries to run parseFunc recursively on this parser's parent parser.
+   *
+   * Returns `true` if the parse succeeded on any ancestor of this parser.
+   * Returns `false` if no [Parser] accepted the [parseFunc] and
+   * [parseAllOptions] is true.
+   * Throws a [FormatException] exception otherwise.

[Bob Nystrom, 2013/06/20 00:33:36]

This documents *what* it does, but not *why* it does that.

[Andrei Mouravski, 2013/06/22 00:54:02]

Done.

+   */
+  bool tryParseOnParent(bool parseFunc(Parser p), String message) {
+    if (parent != null) {
+      return parseFunc(parent);
+    } else if (parseAllOptions) {
+      return false;

[Bob Nystrom, 2013/06/20 00:33:36]

What's this case for? If we can't parse it, shouldn't it always fail? We only
get here if it's an option.

[Andrei Mouravski, 2013/06/22 00:54:02]

It's to fall through when parsing.

+    } else {
+      throw new FormatException(message);
+    }
+  }
+
   /** Validates and stores [value] as the value for [option]. */
   setOption(Map results, Option option, value) {
     // See if it's one of the allowed values.