Add command support to args.

pkg/args/lib/args.dart

Index: pkg/args/lib/args.dart
diff --git a/pkg/args/lib/args.dart b/pkg/args/lib/args.dart
index 0cb81961a3ec2c1743f09a9138c389eb3e3013c1..f308cb313778c30bb6f8cfe36333b64ad04a2b41 100644
--- a/pkg/args/lib/args.dart
+++ b/pkg/args/lib/args.dart
@@ -145,7 +145,29 @@
  *     var results = parser.parse(['--mode', 'on', '--mode', 'off']);
  *     print(results['mode']); // prints '[on, off]'
  *
- * ## Usage ##
+ * ## Defining commands ##
+ *
+ * In addition to *options*, you can also define *commands*. A command is a
+ * named argument that has its own set of options. For example, when you run:
+ *
+ *     $ git commit -a
+ *
+ * The executable is `git`, the command is `commit`, and the `-a` option is an
+ * option passed to the command. You can add a command like so:
+ *
+ *     var parser = new ArgParser();
+ *     var command = parser.addCommand("commit");
+ *     command.addFlag('all', abbr: 'a');
+ *
+ * It returns another [ArgParser] which you can use to define options and
+ * subcommands on that command. When an argument list is parsed, you can then
+ * determine which command was entered and what options were provided for it.
+ *
+ *     var results = parser.parse(['commit', '-a']);
+ *     print(results.command.name); // "commit"
+ *     print(results.command['a']); // true
+ *
+ * ## Displaying usage ##
  *
  * This library can also be used to automatically generate nice usage help
  * text like you get when you run a program with `--help`. To use this, you
@@ -179,7 +201,7 @@
  *
  *           [arm]       ARM Holding 32-bit chip
  *           [ia32]      Intel x86
- * 
+ *
  * To assist the formatting of the usage help, single line help text will
  * be followed by a single new line. Options with multi-line help text
  * will be followed by two new lines. This provides spatial diversity between
@@ -192,37 +214,45 @@ library args;
 
 import 'dart:math';
 
-// TODO(rnystrom): Use "package:" URL here when test.dart can handle pub.
 import 'src/utils.dart';
 
+final _SOLO_OPT = new RegExp(r'^-([a-zA-Z0-9])$');
+final _ABBR_OPT = new RegExp(r'^-([a-zA-Z0-9]+)(.*)$');
+final _LONG_OPT = new RegExp(r'^--([a-zA-Z\-_0-9]+)(=(.*))?$');
+
 /**
  * A class for taking a list of raw command line arguments and parsing out
  * options and flags from them.
  */
 class ArgParser {
-  static final _SOLO_OPT = new RegExp(r'^-([a-zA-Z0-9])$');
-  static final _ABBR_OPT = new RegExp(r'^-([a-zA-Z0-9]+)(.*)$');
-  static final _LONG_OPT = new RegExp(r'^--([a-zA-Z\-_0-9]+)(=(.*))?$');
-
-  final Map<String, _Option> _options;
+  final Map<String, _Option> _options = <String, _Option>{};
+  final Map<String, ArgParser> _commands = <String, ArgParser>{};
 
   /**
    * The names of the options, in the order that they were added. This way we
    * can generate usage information in the same order.
    */
   // TODO(rnystrom): Use an ordered map type, if one appears.
-  final List<String> _optionNames;
+  final List<String> _optionNames = <String>[];
 
-  /** The current argument list being parsed. Set by [parse()]. */
-  List<String> _args;
+  /** Creates a new ArgParser. */
+  ArgParser();
 
-  /** Index of the current argument being parsed in [_args]. */
-  int _current;
+  /**
+   * Defines a command. A command is a named argument which may in turn
+   * define its own options and subcommands. Returns an [ArgParser] that can
+   * be used to define the command's options.
+   */

[nweiz, 2013/01/11 00:06:06]

When you have a sec, can you run your comment-fixing script on this code?

[Bob Nystrom, 2013/01/11 17:59:49]

I thought about that, but I'd like to do that in a separate patch if it's OK.

[nweiz, 2013/01/11 21:43:09]

sgtm

+  ArgParser addCommand(String name) {
+    // Make sure the name isn't in use.
+    if (_commands.containsKey(name)) {
+      throw new ArgumentError('Duplicate command "$name".');
+    }
 
-  /** Creates a new ArgParser. */
-  ArgParser()
-    : _options = <String, _Option>{},
-      _optionNames = <String>[];
+    var command = new ArgParser();
+    _commands[name] = command;
+    return command;
+  }
 
   /**
    * Defines a flag. Throws an [ArgumentError] if:
@@ -283,12 +313,79 @@ class ArgParser {
    * flags and options defined by this parser, and returns the result.
    */
   ArgResults parse(List<String> args) {
-    _args = args;
-    _current = 0;
-    var results = {};
+    return new _ArgParser(null, this, args).parse();
+  }
+
+  /**
+   * Generates a string displaying usage information for the defined options.
+   * This is basically the help text shown on the command line.
+   */
+  String getUsage() {
+    return new _Usage(this).generate();
+  }
+
+  /**
+   * Get the default value for an option. Useful after parsing to test
+   * if the user specified something other than the default.
+   */
+  getDefault(String option) {
+    if (!_options.containsKey(option)) {
+      throw new ArgumentError('No option named $option');
+    }
+    return _options[option].defaultValue;
+  }
+
+  /**
+   * Finds the option whose abbreviation is [abbr], or `null` if no option has
+   * that abbreviation.
+   */
+  _Option _findByAbbr(String abbr) {
+    for (var option in _options.values) {
+      if (option.abbreviation == abbr) return option;
+    }
+
+    return null;
+  }
+}
+
+/**
+ * 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.
+ */
+class _ArgParser {

[nweiz, 2013/01/11 00:06:06]

There are getting to be a lot of classes in this file. Maybe time to put stuff
in src?

Also it might accentuate the distinction between this and ArgParser if you just
called it _Parser.

[Bob Nystrom, 2013/01/11 17:59:49]

Great idea. Done.
 
Also done. Well, just Parser since it's in a sublibrary.

+  /**
+   * If parser is parsing a command's options, this will be the name of the
+   * command. For top-level results, this returns `null`.
+   */
+  final String commandName;
+
+  /**
+   * The parser for the supercommand of this command parser, or `null` if this
+   * is the top-level parser.
+   */
+  final _ArgParser parent;
+
+  /** The grammar being parsed. */
+  final ArgParser parser;

[nweiz, 2013/01/11 00:06:06]

"grammar"? It's kind of confusing to read "parser" and have it not be an
instance of the class that you explicitly call out as the actual parser.

[Bob Nystrom, 2013/01/11 17:59:49]

Done.

+
+  /** The arguments being parsed. */
+  final List<String> args;
+
+  /** The accumulated parsed options. */
+  final Map results = {};
+
+  _ArgParser(this.commandName, this.parser, this.args, [this.parent]);
+
+  /** The current argument being parsed. */
+  String get current => args[0];
+
+  /** Parses the arguments. This can only be called once. */
+  ArgResults parse() {
+    var commandResults = null;
 
     // Initialize flags to their defaults.
-    _options.forEach((name, option) {
+    parser._options.forEach((name, option) {
       if (option.allowMultiple) {
         results[name] = [];
       } else {
@@ -297,20 +394,28 @@ class ArgParser {
     });
 
     // Parse the args.
-    for (_current = 0; _current < args.length; _current++) {
-      var arg = args[_current];
-
-      if (arg == '--') {
+    while (args.length > 0) {
+      if (current == '--') {
         // Reached the argument terminator, so stop here.
-        _current++;
+        args.removeAt(0);
         break;
       }
 
+      // Try to parse the current argument as a command. This happens before
+      // options so that commands can have option-like names.
+      var command = parser._commands[current];
+      if (command != null) {
+        var commandName = args.removeAt(0);
+        var commandParser = new _ArgParser(commandName, command, args, this);
+        commandResults = commandParser.parse();
+        continue;
+      }
+
       // Try to parse the current argument as an option. Note that the order
       // here matters.
-      if (_parseSoloOption(results)) continue;
-      if (_parseAbbreviation(results)) continue;
-      if (_parseLongOption(results)) continue;
+      if (parseSoloOption()) continue;
+      if (parseAbbreviation(this)) continue;
+      if (parseLongOption()) continue;
 
       // If we got here, the argument doesn't look like an option, so stop.
       break;
@@ -318,8 +423,8 @@ class ArgParser {
 
     // Set unspecified multivalued arguments to their default value,
     // if any, and invoke the callbacks.
-    for (var name in _optionNames) {
-      var option = _options[name];
+    for (var name in parser._optionNames) {
+      var option = parser._options[name];
       if (option.allowMultiple &&
           results[name].length == 0 &&
           option.defaultValue != null) {
@@ -329,58 +434,27 @@ class ArgParser {
     }
 
     // Add in the leftover arguments we didn't parse.
-    return new ArgResults(results,
-        _args.getRange(_current, _args.length - _current));
+    // TODO(bob): How should "rest" be handled with commands? Which command on
+    // the stack gets them?

[nweiz, 2013/01/11 00:06:06]

The deepest command should get the rest of the arguments.

[Bob Nystrom, 2013/01/11 17:59:49]

Done.

+    return new ArgResults(results, commandName, commandResults, args);
   }
 
   /**
-   * Generates a string displaying usage information for the defined options.
-   * This is basically the help text shown on the command line.
+   * Pulls the value for [option] from the next argument in [args] (where the
+   * current option is at [index]. Validates that there is a valid value there.

[nweiz, 2013/01/11 00:06:06]

"[index]" -> "[index])".

[Bob Nystrom, 2013/01/11 17:59:49]

Fixed. That doc was out of date. The old parser didn't destructively consume the
arg list.

    */
-  String getUsage() {
-    return new _Usage(this).generate();
-  }
-
-  /**
-   * Called during parsing to validate the arguments. Throws a
-   * [FormatException] if [condition] is `false`.
-   */
-  _validate(bool condition, String message) {
-    if (!condition) throw new FormatException(message);
-  }
+  void readNextArgAsValue(_Option option) {
+    args.removeAt(0);
 
-  /** Validates and stores [value] as the value for [option]. */
-  _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}".');
-    }
-
-    if (option.allowMultiple) {
-      results[option.name].add(value);
-    } else {
-      results[option.name] = value;
-    }
-  }
-
-  /**
-   * Pulls the value for [option] from the next argument in [_args] (where the
-   * current option is at index [_current]. Validates that there is a valid
-   * value there.
-   */
-  void _readNextArgAsValue(Map results, _Option option) {
-    _current++;
     // Take the option argument from the next command line arg.
-    _validate(_current < _args.length,
+    validate(args.length > 0,
         'Missing argument for "${option.name}".');
 
     // Make sure it isn't an option itself.
-    _validate(!_ABBR_OPT.hasMatch(_args[_current]) &&
-              !_LONG_OPT.hasMatch(_args[_current]),
+    validate(!_ABBR_OPT.hasMatch(current) && !_LONG_OPT.hasMatch(current),
         'Missing argument for "${option.name}".');

[nweiz, 2013/01/11 00:06:06]

You should run these validations before you remove the argument from the list.
Also, shouldn't this produce a test error?

[Bob Nystrom, 2013/01/11 17:59:49]

It's a little confusing. The first element in the list is the option. This is
reading the value after the option (the second element). So if your args are
"--mode debug", this consumes "--mode" and then reads the value.

This is because the calling code removes the option *after* calling this.
Except, after calling this, it ends up removing the option value instead, and
*this* function removes the option name.

But that's confusing, so I reorganized the code to be less crazy. Now the code
that calls this consumes the option first and then this consumes the value.

 
-    _setOption(results, option, _args[_current]);
+    setOption(results, option, current);
   }
 
   /**
@@ -389,20 +463,25 @@ class ArgParser {
    * collapsed abbreviations (like "-abc") to handle the possible value that
    * may follow it.
    */
-  bool _parseSoloOption(Map results) {
-    var soloOpt = _SOLO_OPT.firstMatch(_args[_current]);
+  bool parseSoloOption() {
+    var soloOpt = _SOLO_OPT.firstMatch(current);
     if (soloOpt == null) return false;
 
-    var option = _findByAbbr(soloOpt[1]);
-    _validate(option != null,
-        'Could not find an option or flag "-${soloOpt[1]}".');
+    var option = parser._findByAbbr(soloOpt[1]);
+    if (option == null) {
+      // Walk up to the parent command if possible.
+      validate(parent != null,
+          'Could not find an option or flag "-${soloOpt[1]}".');
+      return parent.parseSoloOption();
+    }
 
     if (option.isFlag) {
-      _setOption(results, option, true);
+      setOption(results, option, true);
     } else {
-      _readNextArgAsValue(results, option);
+      readNextArgAsValue(option);
     }
 
+    args.removeAt(0);
     return true;
   }
 
@@ -411,107 +490,131 @@ class ArgParser {
    * (like "-abc") or a single abbreviation with the value directly attached
    * to it (like "-mrelease").
    */
-  bool _parseAbbreviation(Map results) {
-    var abbrOpt = _ABBR_OPT.firstMatch(_args[_current]);
+  bool parseAbbreviation(_ArgParser innermostCommand) {
+    var abbrOpt = _ABBR_OPT.firstMatch(current);
     if (abbrOpt == null) return false;
 
     // If the first character is the abbreviation for a non-flag option, then
     // the rest is the value.
     var c = abbrOpt[1].substring(0, 1);
-    var first = _findByAbbr(c);
+    var first = parser._findByAbbr(c);
     if (first == null) {
-      _validate(false, 'Could not find an option with short name "-$c".');
+      // Walk up to the parent command if possible.
+      validate(parent != null,
+          '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.
       var value = '${abbrOpt[1].substring(1)}${abbrOpt[2]}';
-      _setOption(results, first, value);
+      setOption(results, first, value);
     } else {
       // If we got some non-flag characters, then it must be a value, but
       // if we got here, it's a flag, which is wrong.
-      _validate(abbrOpt[2] == '',
+      validate(abbrOpt[2] == '',
         'Option "-$c" is a flag and cannot handle value '
         '"${abbrOpt[1].substring(1)}${abbrOpt[2]}".');
 
       // Not an option, so all characters should be flags.
+      // We use "innermostCommand" here so that if a parent command parses the
+      // *first* letter, subcommands can still be found to parse the other
+      // letters.
       for (var i = 0; i < abbrOpt[1].length; i++) {
         var c = abbrOpt[1].substring(i, i + 1);
-        var option = _findByAbbr(c);
-        _validate(option != null,
-            'Could not find an option with short name "-$c".');
-
-        // In a list of short options, only the first can be a non-flag. If
-        // we get here we've checked that already.
-        _validate(option.isFlag,
-            'Option "-$c" must be a flag to be in a collapsed "-".');
-
-        _setOption(results, option, true);
+        innermostCommand.parseShortFlag(c);
       }
     }
 
+    args.removeAt(0);
     return true;
   }
 
+  void parseShortFlag(String c) {
+    var option = parser._findByAbbr(c);
+    if (option == null) {
+      // Walk up to the parent command if possible.
+      validate(parent != null,
+          'Could not find an option with short name "-$c".');
+      parent.parseShortFlag(c);
+      return;
+    }
+
+    // In a list of short options, only the first can be a non-flag. If
+    // we get here we've checked that already.
+    validate(option.isFlag,
+        'Option "-$c" must be a flag to be in a collapsed "-".');
+
+    setOption(results, option, true);
+  }
+
   /**
    * Tries to parse the current argument as a long-form named option, which
    * may include a value like "--mode=release" or "--mode release".
    */
-  bool _parseLongOption(Map results) {
-    var longOpt = _LONG_OPT.firstMatch(_args[_current]);
+  bool parseLongOption() {
+    var longOpt = _LONG_OPT.firstMatch(current);
     if (longOpt == null) return false;
 
     var name = longOpt[1];
-    var option = _options[name];
+    var option = parser._options[name];
     if (option != null) {
       if (option.isFlag) {
-        _validate(longOpt[3] == null,
+        validate(longOpt[3] == null,
             'Flag option "$name" should not be given a value.');
 
-        _setOption(results, option, true);
+        setOption(results, option, true);
       } else if (longOpt[3] != null) {
         // We have a value like --foo=bar.
-        _setOption(results, option, longOpt[3]);
+        setOption(results, option, longOpt[3]);
       } else {
         // Option like --foo, so look for the value as the next arg.
-        _readNextArgAsValue(results, option);
+        readNextArgAsValue(option);
       }
     } else if (name.startsWith('no-')) {
       // See if it's a negated flag.
       name = name.substring('no-'.length);
-      option = _options[name];
-      _validate(option != null, 'Could not find an option named "$name".');
-      _validate(option.isFlag, 'Cannot negate non-flag option "$name".');
-      _validate(option.negatable, 'Cannot negate option "$name".');
+      option = parser._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();
+      }
 
-      _setOption(results, option, false);
+      validate(option.isFlag, 'Cannot negate non-flag option "$name".');
+      validate(option.negatable, 'Cannot negate option "$name".');
+
+      setOption(results, option, false);
     } else {
-      _validate(option != null, 'Could not find an option named "$name".');
+      // Walk up to the parent command if possible.
+      validate(parent != null, 'Could not find an option named "$name".');
+      return parent.parseLongOption();
     }
 
+    args.removeAt(0);
     return true;
   }
 
   /**
-   * Finds the option whose abbreviation is [abbr], or `null` if no option has
-   * that abbreviation.
+   * Called during parsing to validate the arguments. Throws a
+   * [FormatException] if [condition] is `false`.
    */
-  _Option _findByAbbr(String abbr) {
-    for (var option in _options.values) {
-      if (option.abbreviation == abbr) return option;
-    }
-
-    return null;
+  validate(bool condition, String message) {
+    if (!condition) throw new FormatException(message);
   }
 
-  /**
-   * Get the default value for an option. Useful after parsing to test
-   * if the user specified something other than the default.
-   */
-  getDefault(String option) {
-    if (!_options.containsKey(option)) {
-      throw new ArgumentError('No option named $option');
+  /** Validates and stores [value] as the value for [option]. */
+  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}".');
+    }
+
+    if (option.allowMultiple) {
+      results[option.name].add(value);
+    } else {
+      results[option.name] = value;
     }
-    return _options[option].defaultValue;
   }
 }
 
@@ -524,6 +627,18 @@ class ArgResults {
   final Map _options;
 
   /**
+   * If these are the results for parsing a command's options, this will be
+   * the name of the command. For top-level results, this returns `null`.
+   */
+  final String name;
+
+  /**
+   * The command that was selected, or `null` if none was. This will contain
+   * the options that were selected for that command.
+   */
+  final ArgResults command;
+
+  /**
    * The remaining command-line arguments that were not parsed as options or
    * flags. If `--` was used to separate the options from the remaining
    * arguments, it will not be included in this list.
@@ -531,7 +646,7 @@ class ArgResults {
   final List<String> rest;
 
   /** Creates a new [ArgResults]. */
-  ArgResults(this._options, this.rest);
+  ArgResults(this._options, this.name, this.command, this.rest);
 
   /** Gets the parsed command-line option named [name]. */
   operator [](String name) {