First pass at a generic reusable command line option parser.

lib/args/args.dart

+// Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+/**
+ * This library lets you define parsers for parsing raw command-line arguments
+ * into a set of options and values using [GNU][] and [POSIX][] style options.
+ *
+ * ## Defining options ##
+ *
+ * To use this library, you create an [ArgParser] object which will contain
+ * the set of options you support:
+ *
+ *     var parser = new ArgParser();
+ *
+ * Then you define a set of options on that parser using [addOption()] and
+ * [addFlag()]. The minimal way to create an option is:
+ *
+ *     parser.addOption('name');
+ *
+ * This creates an option named "name". Options must be given a value on the
+ * command line. If you have a simple on/off option, you can instead use a

[nweiz, 2012/05/03 21:07:07]

Using "on/off option" here is confusing, since you just claimed that options
must be given a value. Maybe "If you have a simple on/off flag, you can instead
use:"

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+ * flag:
+ *
+ *     parser.addFlag('name');
+ *
+ * (From here on out "option" will refer to both "regular" options and flags.
+ * In cases where the distinction matters, we'll use "non-flag option".)
+ *
+ * Options may have an optional single-character abbreviation:
+ *
+ *     parser.addOption('mode', abbr: 'm');
+ *     parser.addFlag('verbose', abbr: 'v');
+ *
+ * They may also specify a default value. If provided, then when you later
+ * query for the option, in the parsed results, the default value will be
+ * returned if it wasn't provided by the arguments:

[nweiz, 2012/05/03 21:07:07]

Awkward sentence structure. I suggest "The default value will be used if the
option isn't provided".

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+ *
+ *     parser.addOption('mode', defaultsTo: 'debug');
+ *     parser.addFlag('verbose', defaultsTo: false);

[nweiz, 2012/05/03 21:07:07]

Don't flags default to false even without defaultsTo? A flag with a default
value of "true" would be a better example.

[Bob Nystrom, 2012/05/03 22:31:19]

They default to 'null', so you can detect whether or not it was provided at all.
Do you think that's a good idea?

[nweiz, 2012/05/03 23:47:52]

No, I don't. If there's a three-way behavioral difference between "--flag",
"--no-flag", and not passing the flag, they should just take a non-flag option
instead.

Users certainly won't expect there to be a difference between not passing the
flag and passing "--no-flag" if the default isn't "true".

[Bob Nystrom, 2012/05/04 18:26:03]

Done.

+ *
+ * The default value for non-flag options can be any [String]. For flags, it
+ * must be a [bool].
+ *
+ * To validate non-flag options, you may provide an allowed set of values. When
+ * you do, it will throw an [ArgFormatException] when you parse the arguments
+ * if the value for an option is not in the allowed set.

[nweiz, 2012/05/03 21:07:07]

Style nit: be consistent about whether you use a colon or a period before
examples.

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+ *
+ *     parser.addOption('mode', allowed: ['debug', 'release']);
+ *
+ * You can provide a callback when you define an option. When you later parse
+ * a set of arguments, the callback for that option will be invoked with the
+ * value provided for it.
+ *
+ *     parser.addOption('mode', callback: (mode) => print('Got mode $mode));
+ *     parser.addFlag('verbose', callback: (verbose) {
+ *       if (verbose) print('Verbose');
+ *     });
+ *
+ * The callback for each option will *always* be called

[nweiz, 2012/05/03 21:07:07]

Style nit: early line break.

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+ * when you parse a set of arguments. If the option isn't provided in the args,
+ * the callback will be passed the default value, or `null` if there is none
+ * set.
+ *
+ * ## Parsing arguments ##
+ *
+ * Once you have an [ArgParser] set up with some options and flags, you use it
+ * by calling [ArgParser.parse()] with a set of arguments:
+ *
+ *     var results = parser.parse(['some', 'command', 'line', 'args']);
+ *
+ * These will usually come from `new Options().arguments`, but you can pass in
+ * any list of strings. It returns an instance of [ArgResults]. This is a
+ * map-like object that will return the value of any parsed option.
+ *
+ *     var parser = new ArgParser();
+ *     parser.addOption('mode');
+ *     parser.addFlag('verbose', defaultsTo: true);
+ *     var results = parser.parser('['--mode', 'debug', 'something', 'else']);

[nweiz, 2012/05/03 21:07:07]

.parse

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+ *
+ *     print(results['mode']); // debug
+ *     print(results['verbose']); // true
+ *
+ * The [parse()] method will stop as soon as it reaches `--` or anything that
+ * it doesn't recognize as an option, flag, or option value. If there are still
+ * arguments left, they will be provided to you in
+ * [ArgResults.remainingArguments].

[nweiz, 2012/05/03 21:07:07]

The actual field is "remainingArgs".

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+ *
+ *     print(results.remainingArguments); // ['something', 'else']
+ *
+ * ## Specifying options ##
+ *
+ * To actually pass in options and flags on the command line, use GNU or POSIX
+ * style. If you define an option like:
+ *
+ *     parser.addOption('name', abbr: 'n');
+ *
+ * Then a value for it can be specified on the command line using any of:
+ *
+ *     --name=somevalue
+ *     --name somevalue
+ *     -nsomevalue
+ *     -n somevalue
+ *
+ * Given this flag:
+ *
+ *     parser.addFlag('name', abbr: 'n');
+ *
+ * You can set it on using one of:
+ *
+ *     --name
+ *     -n

[nweiz, 2012/05/03 21:07:07]

Mention here that you support abbreviation collapsing.

[Bob Nystrom, 2012/05/03 22:31:19]

Good call. Done.

+ *
+ * Or set it off using:
+ *
+ *     --no-name
+ *
+ * ## 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
+ * will also want to provide some help text when you create your options. To
+ * define help text for the entire option, do:
+ *
+ *     parser.addOption('mode', help: 'The compiler configuration',
+ *         allowed: ['debug', 'release']);
+ *     parser.addFlag('verbose', help: 'Show additional diagnostic info');
+ *
+ * For non-flag options, you can also provide detailed help for each expected
+ * value using a map:
+ *
+ *     parser.addOption('arch', help: 'The architecture to compile for',
+ *         allowedHelp: {

[nweiz, 2012/05/03 21:07:07]

allowedHelp sounds awkward. Why not just allow "allowed" to take a hash? We have
optional types after all.

[Bob Nystrom, 2012/05/03 22:31:19]

My motivation is that it doesn't require the two to be perfectly in sync.
test.dart has a couple of cases where the allowed help is a little different
from the actual allowed set of values.

[nweiz, 2012/05/03 23:47:52]

That seems like a pretty narrow edge case to make the general case worse. Why
not just allow null as a key in the hash to indicate an undocumented option?

[Bob Nystrom, 2012/05/04 18:26:03]

I'm still a little leery of mixing the help directly in the allowed set by using
a map. I kind of like help being split out separately (even though it is a bit
redundant), and I can see us extended the allowed API in ways that wouldn't play
nice. For example, test.dart's --runtime option supports both "ff" and "firefox"
to mean the same thing. It might be nice for args to know that and automatically
canonicalize them like:

allowed: ['ie', ['ff', 'firefox'], 'safari']

Or something like that. Extending the API like that gets harder if we commit to
using a map, I think.

[nweiz, 2012/05/04 18:30:51]

If that is a case we want to support in the future, we can add allowedHelp for
that specific case. I don't think we should enforce redundancy in the common
case in order to potentially eliminate redundancy in an uncommon edge case some
time in the future.

+ *           'ia32': 'Intel x86',
+ *           'arm': 'ARM Holding 32-bit chip'
+ *         });
+ *
+ * If you define a set of options like the above, then calling this:
+ *
+ *     print(parser.getUsage());
+ *
+ * Will display something like:
+ *
+ *     --mode            The compiler configuration
+ *                       [debug, release]
+ *
+ *     --[no-]verbose    Show additional diagnostic info
+ *     --arch            The architecture to compile for
+ *
+ *           [arm]       ARM Holding 32-bit chip
+ *           [ia32]      Intel x86
+ *
+ * [posix]: http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap12.html#tag_12_02
+ * [gnu]: http://www.gnu.org/prep/standards/standards.html#Command_002dLine-Interfaces
+ */
+#library('args');
+
+#import('utils.dart');
+
+/**
+ * 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 = const RegExp(@'^-([a-z0-9])$');
+  static final _ABBR_OPT = const RegExp(@'^-([a-z0-9]+)(.*)$');
+  static final _LONG_OPT = const RegExp(@'^--([a-z\-_0-9]+)(=(.*))?$');
+
+  final Map<String, _Option> _options;
+
+  /**
+   * The names of the options, in the order that they were added. This way we
+   * can generate usage information in the same order.
+   */
+  final List<String> _optionNames;

[nweiz, 2012/05/03 21:07:07]

Maybe add a TODO here about using ordered maps once they're available.

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+
+  /** The current argument list being parsed. Set by [parse()]. */
+  List<String> _args;
+
+  /** Index of the current argument being parsed in [_args]. */
+  int _current;
+
+  /** Creates a new ArgParser. */
+  ArgParser()
+    : _options = <_Option>{},
+      _optionNames = <String>[] {
+  }

[nweiz, 2012/05/03 21:07:07]

Use ";" for the empty body.

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+
+  /**
+   * Defines a flag. Throws an [IllegalArgumentException] if:
+   *
+   * * There is already an option with name [name].

[nweiz, 2012/05/03 21:07:07]

Style nit: s/with name/named/

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+   * * There is already an option using abbreviation [abbr].
+   */
+  void addFlag(String name, [String abbr, String help, bool defaultsTo,
+      void callback(bool value)]) {
+    _addOption(name, abbr, help, null, null, defaultsTo, callback,
+        isFlag: true);
+  }
+
+  /**
+   * Defines a value-taking option. Throws an [IllegalArgumentException] if:
+   *
+   * * There is already an option with name [name].
+   * * There is already an option using abbreviation [abbr].
+   */
+  void addOption(String name, [String abbr, String help, List<String> allowed,
+      Map<String, String> allowedHelp, String defaultsTo,
+      void callback(bool value)]) {
+    _addOption(name, abbr, help, allowed, allowedHelp, defaultsTo, callback,
+        isFlag: false);
+  }
+
+  void _addOption(String name, [String abbr, String help, List<String> allowed,

[nweiz, 2012/05/03 21:07:07]

No need to make these arguments optional.

[Bob Nystrom, 2012/05/03 22:31:19]

isFlag does so that it's named, but otherwise done.

+      Map<String, String> allowedHelp, defaultsTo,
+      void callback(bool value), bool isFlag]) {
+    // Make sure the name isn't in use.
+    if (_options.containsKey(name)) {
+      throw new IllegalArgumentException('Duplicate option "$name".');
+    }
+
+    // Make sure the abbreviation isn't too long or in use.
+    if (abbr != null) {
+      if (abbr.length > 1) {
+        throw new IllegalArgumentException(
+            'Abbreviation "$abbr" is longer than one character.');
+      }
+
+      var existing = _findByAbbr(abbr);
+      if (existing != null) {
+        throw new IllegalArgumentException(
+            'Abbreviation "$abbr" is already used by "${existing.name}".');
+      }
+    }
+
+    _options[name] = new _Option(name, abbr, help, allowed, allowedHelp,
+        defaultsTo, callback, isFlag: isFlag);
+    _optionNames.add(name);
+  }
+
+  /**
+   * Parses [args], a list of command-line arguments, matches them against the
+   * flags and options defined by this parser, and returns the result.
+   */
+  ArgResults parse(List<String> args) {
+    _args = args;
+    _current = 0;
+    var results = {};
+
+    // Initialize flags to their defaults.
+    _options.forEach((name, option) {
+      results[name] = option.defaultValue;
+    });
+
+    // Parse the args.
+    for (_current = 0; _current < args.length; _current++) {
+      var arg = args[_current];
+
+      if (arg == '--') {
+        // Reached the argument terminator, so stop here.
+        _current++;
+        break;
+      }
+
+      // 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 we got here, the argument doesn't look like an option, so stop.
+      break;
+    }
+
+    // Invoke the callbacks.
+    for (var name in _optionNames) {
+      var option = _options[name];
+      if (option.callback != null) option.callback(results[name]);
+    }
+
+    // Add in the leftover arguments we didn't parse.
+    return new ArgResults(results,
+        _args.getRange(_current, _args.length - _current));
+  }
+
+  /**
+   * Generates a string displaying usage imformation for the defined options.
+   * This is basically the help text shown on the command line.
+   */
+  String getUsage() {

[nweiz, 2012/05/03 21:07:07]

Consider making this a getter

[Bob Nystrom, 2012/05/03 22:31:19]

My thoughts here are:

1. It's slower than I like getters to be. I could give it a name that clarifies
that like "generateUsage".
2. At some point, it will likely have arguments (like whether or not --help
should be automatically added).

[nweiz, 2012/05/03 23:47:52]

It's very fast relative to the time it'll take a user to read it. Since that's
the only context it'll be called, I wouldn't worry about speed.
Wouldn't the --help thing be a property of the ArgParser itself, since it needs
to know whether to parse --help?

[Bob Nystrom, 2012/05/04 18:26:03]

Good point, but keeping it a method per our discussion.

+    return new _Usage(this).generate();
+  }
+
+  /**
+   * Called during parsing to validate the arguments. Throws an
+   * [ArgFormatException] if [condition] is `false`.
+   */
+  _validate(bool condition, String message) {
+    if (!condition) throw new ArgFormatException(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.
+    if (option.allowed != null) {
+      _validate(option.allowed.some((allow) => allow == value),

[nweiz, 2012/05/03 21:07:07]

It's dumb that Collection.contains doesn't exist.

[Bob Nystrom, 2012/05/03 22:31:19]

Yup.

+          '"$value" is not an allowed value for option "${option.name}".');
+    }
+
+    results[option.name] = value;
+  }
+
+  /**
+   * Pulls the value for [options] from the next argument in [args] (where the

[nweiz, 2012/05/03 21:07:07]

s/options/option/

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+   * current option is at index [i]. Validates that there is a valid value

[nweiz, 2012/05/03 21:07:07]

s/i/_current/

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+   * there.
+   */
+  void _readNextArgAsValue(Map results, _Option option) {
+    _current++;
+    // Take the option argument from the next command line arg.
+    _validate(_current < _args.length,
+        'Missing argument for "${option.name}".');
+
+    // Make sure it isn't an option itself.
+    _validate(!_ABBR_OPT.hasMatch(_args[_current]) &&
+              !_LONG_OPT.hasMatch(_args[_current]),

[nweiz, 2012/05/03 21:07:07]

Doesn't the style guide never want you to align something with the previous
line? Isn't the rule always some set number of spaces of indentation?

[Bob Nystrom, 2012/05/03 22:31:19]

I think for some languages at google at is. I believe ours is "at least 4". So
you can line things up deeper but you don't have to.

+        'Missing argument for "${option.name}".');
+
+    _setOption(results, option, _args[_current]);
+  }
+
+  /**
+   * Tries to parse the current argument as a "solo" option, which is a single
+   * hyphen followed by a single letter. We treat this specially from

[nweiz, 2012/05/03 21:07:07]

Style nit: s/specially from/differently than/.

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+   * collapsed abbreviations (like "-abc") to handle the possible value that
+   * may follow it.
+   */
+  bool _parseSoloOption(Map results) {
+    var soloOpt = _SOLO_OPT.firstMatch(_args[_current]);
+    if (soloOpt == null) return false;
+
+    var option = _findByAbbr(soloOpt[1]);
+    _validate(option != null,
+        'Could not find an option or flag "-${soloOpt[1]}".');
+
+    if (option.isFlag) {
+      _setOption(results, option, true);
+    } else {
+      _readNextArgAsValue(results, option);
+    }
+
+    return true;
+  }
+
+  /**
+   * Tries to parse the current argument as a series of collapsed abbreviations
+   * (like "-abc") or a single abbreviation with the value directly attached
+   * to it (like "-mrelease").
+   */
+  bool _parseAbbreviation(Map results) {

[nweiz, 2012/05/03 21:07:07]

I think programs that support abbreviation collapsing usually support passing
arguments to the final collapsed abbreviation. E.g. "-abc bar" and "-abcbar"
would both be equivalent to "-a -b -c bar".

[Bob Nystrom, 2012/05/03 22:31:19]

I find mixing collapsed args and option values to be nasty and potentially
ambiguous, so I'd like to not go too far down that road. If it wasn't common
practice, I personally wouldn't even want to support '-mdebug' since it's
seemingly ambiguous.

[nweiz, 2012/05/03 23:47:52]

That's fair, just wanted to make sure you were aware of it.

+    var abbrOpt = _ABBR_OPT.firstMatch(_args[_current]);
+    if (abbrOpt == null) return false;
+
+    // If the first character is the abbreviation for an option, then the

[nweiz, 2012/05/03 21:07:07]

s/option/non-flag option/, here and below.

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+    // rest is the value.
+    var c = abbrOpt[1].substring(0, 1);
+    var first = _findByAbbr(c);
+    if (first == null) {
+      _validate(false, 'Could not find an option with short name "-$c".');
+    } else if (!first.isFlag) {
+      // The first character is an option, so the rest must be the value.
+      var value = '${abbrOpt[1].substring(1)}${abbrOpt[2]}';
+      _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] == '',
+        'Option "-$c" is a flag and cannot handle value '
+        '"${abbrOpt[1].substring(1)}${abbrOpt[2]}".');
+
+      // Not an option, so all characters should be flags.
+      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);
+      }
+    }
+
+    return 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]);
+    if (longOpt == null) return false;
+
+    var name = longOpt[1];
+    var option = _options[name];
+    if (option != null) {
+      if (option.isFlag) {
+        _validate(longOpt[3] == null,
+            'Flag option "$name" should not be given a value.');
+
+        _setOption(results, option, true);
+      } else {
+        // Option. Find the argument value.
+        if (longOpt[3] != null) {

[nweiz, 2012/05/03 21:07:07]

Collapse this if statement into the previous else.

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+          // We have a value like --foo=bar.
+          _setOption(results, option, longOpt[3]);
+        } else {
+          _readNextArgAsValue(results, 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.');
+
+      _setOption(results, option, false);
+    } else {
+      _validate(option != null, 'Could not find an option named "$name".');
+    }
+
+    return true;
+  }
+
+  /**
+   * Finds the option whose abbreviation is [abbr], or `null` if no option has
+   * that abbreviation.
+   */
+  _Option _findByAbbr(String abbr) {
+    for (var option in _options.getValues()) {
+      if (option.abbreviation == abbr) return option;
+    }
+
+    return null;
+  }
+}
+
+/**
+ * The results of parsing a series of command line arguments using
+ * [ArgParser.parse()]. Includes the parsed options and any remaining unparsed
+ * command line arguments.
+ */
+class ArgResults {
+  final Map _options;
+
+  /**
+   * 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.
+   */
+  final List<String> remainingArgs;

[nweiz, 2012/05/03 21:07:07]

"rest" would be terser, which I think is valuable for a field that most clients
will be accessing.

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+
+  /** Creates a new [ArgResults]. */
+  ArgResults(this._options, this.remainingArgs);
+
+  /** Gets the parsed command-line option named [name]. */
+  operator [](String name) {
+    if (!_options.containsKey(name)) {
+      throw new IllegalArgumentException(
+          'Could not find an option named "$name".');
+    }
+
+    return _options[name];
+  }
+}
+
+/**
+ * Exception thrown by [ArgParser.parse()] when the argument list isn't valid.
+ */
+class ArgFormatException implements Exception {
+  final String message;
+  const ArgFormatException(this.message);
+}
+
+class _Option {
+  final String name;
+  final String abbreviation;
+  final List allowed;
+  final defaultValue;
+  final Function callback;
+  final String help;
+  final Map<String, String> allowedHelp;
+  final bool isFlag;
+
+  _Option(this.name, this.abbreviation, this.help, this.allowed,
+      this.allowedHelp, this.defaultValue, this.callback, [this.isFlag]);

[nweiz, 2012/05/03 21:07:07]

No reason for isFlag to be optional.

[Bob Nystrom, 2012/05/03 22:31:19]

bools are named so that the callsite is clear: new _Option(..., isFlag: true);

[nweiz, 2012/05/03 23:47:52]

Every place you're calling this, you're passing in "isFlag: isFlag".

+}
+
+class _Usage {

[nweiz, 2012/05/03 21:07:07]

I'd like to see more field/method documentation for this, even though it's
private. Currently it's kind of difficult to figure out the semantics of
everything that's going on.

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+  static final NUM_COLUMNS = 3; // Abbreviation, long name, help.
+
+  final ArgParser args;
+  StringBuffer buffer;
+  int currentColumn = 0;
+  List<int> columnWidths;
+  int numHelpLines = 0;
+  int newlinesNeeded = 0;
+
+  _Usage(this.args);
+
+  /**
+   * Generates a string displaying usage imformation for the defined options.

[nweiz, 2012/05/03 21:07:07]

information

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+   * This is basically the help text shown on the command line.
+   */
+  String generate() {
+    buffer = new StringBuffer();
+
+    calculateColumnWidths();
+
+    for (var name in args._optionNames) {
+      var option = args._options[name];
+      write(0, getAbbreviation(option));
+      write(1, getLongOption(option));
+
+      if (option.help != null) write(2, option.help);
+
+      if (option.allowedHelp != null) {
+        var allowedNames = option.allowedHelp.getKeys();
+        allowedNames.sort((a, b) => a.compareTo(b));
+        newline();
+        for (var name in allowedNames) {
+          write(1, getAllowedTitle(name));
+          write(2, option.allowedHelp[name]);
+        }
+        newline();
+      } else if (option.allowed != null) {
+        write(2, buildAllowedList(option));
+      } else if (option.defaultValue != null) {

[nweiz, 2012/05/03 21:07:07]

What happens if you specify the default and an allowed list?

[Bob Nystrom, 2012/05/03 22:31:19]

The allowed list text (the previous if branch) will mention it:

    -m, --mode    The configuration
                  [debug, release (default), profile]

+        if (option.isFlag) {
+          write(2, '(defaults to ${option.defaultValue ? "on" : "off"})');
+        } else {
+          write(2, '(defaults to "${option.defaultValue}")');
+        }
+      }
+
+      // If any given option displays more than one line of text on the right
+      // column (i.e. help, default value, allowed options, etc.) then put a
+      // blank line after it. This gives space where it's useful while still
+      // keeping simple one-line options clumped together.
+      if (numHelpLines > 1) newline();
+    }
+
+    return buffer.toString();
+  }
+
+  String getAbbreviation(_Option option) {
+    if (option.abbreviation != null) {
+      return '-${option.abbreviation}, ';
+    } else {
+      return '';
+    }
+  }
+
+  String getLongOption(_Option option) {
+    if (option.isFlag) {
+      return '--[no-]${option.name}';
+    } else {
+      return '--${option.name}';
+    }
+  }
+
+  String getAllowedTitle(String allowed) {
+    return '      [$allowed]';
+  }
+
+  void calculateColumnWidths() {
+    int abbr = 0;
+    int title = 0;
+    for (var name in args._optionNames) {
+      var option = args._options[name];
+
+      // Make room in the first column if there are abbreviations.
+      abbr = Math.max(abbr, getAbbreviation(option).length);
+
+      // Make room for the option.
+      title = Math.max(title, getLongOption(option).length);
+
+      // Make room for the allowed help affects it.

[nweiz, 2012/05/03 21:07:07]

Remove "affects it".

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+      if (option.allowedHelp != null) {
+        for (var allowed in option.allowedHelp.getKeys()) {
+          title = Math.max(title, getAllowedTitle(allowed).length);
+        }
+      }
+    }
+
+    // Leave a gutter between the columns.
+    title += 4;
+    columnWidths = [abbr, title];
+  }
+
+  newline() {
+    newlinesNeeded++;
+    currentColumn = 0;
+    numHelpLines = 0;
+  }
+
+  write(int column, String text) {
+    for (var line in text.split('\n')) {
+      writeLine(column, line);
+    }
+  }
+
+  writeLine(int column, String text) {
+    // Write any pending newlines. We do this lazily so that the last bit of
+    // usage doesn't have dangling newlines. We only write newlines right
+    // *before* we write some real content.
+    while (newlinesNeeded > 0) {
+      buffer.add('\n');
+      newlinesNeeded--;
+    }
+
+    // Advance until we are at the right column (which may mean wrapping around
+    // to the next line.
+    while (currentColumn != column) {
+      if (currentColumn < columnWidths.length) {

[nweiz, 2012/05/03 21:07:07]

This is pretty confusing. Without any context, I would guess that
columnWidths.length is 3, since there are three columns. It would be much
clearer to say currentColumn < NUM_COLUMNS - 1, especially since you use that
value elsewhere.

[Bob Nystrom, 2012/05/03 22:31:19]

Done.

+        buffer.add(padRight('', columnWidths[currentColumn]));
+      } else {
+        buffer.add('\n');
+      }
+      currentColumn = (currentColumn + 1) % NUM_COLUMNS;
+    }
+
+    if (column < columnWidths.length) {
+      // Fixed-size column, so pad it.
+      buffer.add(padRight(text, columnWidths[column]));
+    } else {
+      // The last column, so just write it.
+      buffer.add(text);
+    }
+
+    // Advance to the next column.
+    currentColumn = (currentColumn + 1) % NUM_COLUMNS;
+
+    // If we reached the last column, we need to wrap to the next line.
+    if (column == NUM_COLUMNS - 1) newlinesNeeded++;
+
+    // Keep track of how many consecutive lines we've written in the last
+    // column.
+    if (column == NUM_COLUMNS - 1) {
+      numHelpLines++;
+    } else {
+      numHelpLines = 0;
+    }
+  }
+
+  buildAllowedList(_Option option) {
+    var allowedBuffer = new StringBuffer();
+    allowedBuffer.add('[');
+    bool first = true;
+    for (var allowed in option.allowed) {

[nweiz, 2012/05/03 21:07:07]

I would use List.map and Strings.join here.

[Bob Nystrom, 2012/05/03 22:31:19]

With '[' and the lack of '+' on strings, I find it's a little simpler this way.
Sometimes an imperative solution is OK. :)

+      if (!first) allowedBuffer.add(', ');
+      allowedBuffer.add(allowed);
+      if (allowed == option.defaultValue) {
+        allowedBuffer.add(' (default)');
+      }
+      first = false;
+    }
+    allowedBuffer.add(']');
+    return allowedBuffer.toString();
+  }
+}