address comments

Author: Danny McClanahan

src/optparse.coffee

@@ -15,8 +15,8 @@ exports.OptionParser = class OptionParser
   #     [short-flag, long-flag, description]
   #
   # Along with an an optional banner for the usage help.
-  constructor: (rules, @banner) ->
-    @rules = buildRules rules
+  constructor: (ruleDecls, @banner) ->
+    @rules = buildRules ruleDecls
 
   # Parse the list of arguments, populating an `options` object with all of the
   # specified options, and return it. Options after the first non-option
@@ -25,26 +25,42 @@ exports.OptionParser = class OptionParser
   # parsers that allow you to attach callback actions for every flag. Instead,
   # you're responsible for interpreting the options object.
   parse: (args) ->
-    state =
-      argsLeft: args[..]
-      options: {}
-    while (arg = state.argsLeft.shift())?
-      if (arg.match(LONG_FLAG) ? arg.match(SHORT_FLAG))?
-        tryMatchOptionalArgument(arg, state, @rules)
-      else if (multiMatch = arg.match(MULTI_FLAG))?
-        # Normalize arguments by expanding merged flags into multiple
-        # flags. This allows you to have `-wl` be the same as `--watch --lint`.
-        normalized = "-#{multiArg}" for multiArg in multiMatch[1].split ''
-        state.argsLeft.unshift(normalized...)
-      else
-        # the CS option parser is a little odd; options after the first
-        # non-option argument are treated as non-option arguments themselves.
-        # executable scripts do not need to have a `--` at the end of the
-        # shebang ("#!") line, and if they do, they won't work on Linux
-        state.argsLeft.unshift(arg) unless arg is '--'
+
+    options = arguments: []
+    for cmdLineArg, i in args
+      # The CS option parser is a little odd; options after the first
+      # non-option argument are treated as non-option arguments themselves.
+      # Executable scripts do not need to have a `--` at the end of the
+      # shebang ("#!") line, and if they do, they won't work on Linux.
+      multi = trySplitMultiFlag(cmdLineArg)
+      toProcess = multi or trySingleFlag(cmdLineArg)
+      # If the current argument could not be parsed as one or more arguments.
+      unless toProcess?
+        ++i if cmdLineArg is '--'
+        options.arguments = args[i..]
         break
-    state.options.arguments = state.argsLeft[..]
-    state.options
+
+      # Normalize arguments by expanding merged flags into multiple
+      # flags. This allows you to have `-wl` be the same as `--watch --lint`.
+      for argFlag, j in toProcess
+        rule = @rules.flagDict[argFlag]
+        unless rule?
+          context = if multi? then " (in multi-flag '#{cmdLineArg}')" else ''
+          throw new Error "unrecognized option: #{argFlag}#{context}"
+
+        {hasArgument, isList, name} = rule
+
+        options[name] = switch hasArgument
+          when no then true
+          else
+            next = toProcess[++j] or args[++i]
+            unless next?
+              throw new Error "value required for '#{argFlag}':
+                was the last argument provided"
+            switch isList
+              when no then next
+              else (options[name] or []).concat next
+    options
 
   # Return the help text for this **OptionParser**, listing and describing all
   # of the valid options, for `--help` and such.
@@ -61,23 +77,37 @@ exports.OptionParser = class OptionParser
 # Helpers
 # -------
 
-# Regex matchers for option flags.
+# Regex matchers for option flags on the command line and their rules.
 LONG_FLAG  = /^(--\w[\w\-]*)/
 SHORT_FLAG = /^(-\w)$/
 MULTI_FLAG = /^-(\w{2,})/
+# Matches the long flag part of a rule for an option with an argument. Not
+# applied to anything in process.argv.
 OPTIONAL   = /\[(\w+(\*?))\]/
 
 # Build and return the list of option rules. If the optional *short-flag* is
 # unspecified, leave it out by padding with `null`.
-buildRules = (rules) ->
-  for tuple in rules
+buildRules = (ruleDecls) ->
+  ruleList = for tuple in ruleDecls
     tuple.unshift null if tuple.length < 3
     buildRule tuple...
+  flagDict = {}
+  for rule in ruleList
+    # shortFlag is null if not provided in the rule.
+    for flag in [rule.shortFlag, rule.longFlag] when flag?
+      prevRule = flagDict[flag]
+      if prevRule?
+        throw new Error "flag #{flag} for switch #{rule.name}
+          was already declared for switch #{prevRule.name}"
+      flagDict[flag] = rule
+
+  {ruleList, flagDict}
 
 # Build a rule from a `-o` short flag, a `--output [DIR]` long flag, and the
 # description of what the option does.
-buildRule = (shortFlag, longFlag, description, options = {}) ->
+buildRule = (shortFlag, longFlag, description) ->
   match     = longFlag.match(OPTIONAL)
+  shortFlag = shortFlag?.match(SHORT_FLAG)[1]
   longFlag  = longFlag.match(LONG_FLAG)[1]
   {
     name:         longFlag.substr 2
@@ -88,22 +118,11 @@ buildRule = (shortFlag, longFlag, description, options = {}) ->
     isList:       !!(match and match[2])
   }
 
-addArgument = (rule, options, value) ->
-  options[rule.name] = if rule.isList
-    (options[rule.name] ? []).concat value
-  else value
-
-tryMatchOptionalArgument = (arg, state, rules) ->
-  for rule in rules
-    if arg in [rule.shortFlag, rule.longFlag]
-      if rule.hasArgument
-        value = state.argsLeft.shift()
-        if not value?
-          throw new Error "#{arg} requires a value, but was the last argument"
-      else
-        value = true
-
-      addArgument(rule, state.options, value)
-      return
+trySingleFlag = (arg) ->
+  if ([LONG_FLAG, SHORT_FLAG].some (pat) -> arg.match(pat)?) then [arg]
+  else null
 
-  throw new Error "unrecognized option: #{arg}"
+trySplitMultiFlag = (arg) ->
+  arg.match(MULTI_FLAG)?[1]
+    .split('')
+    .map (flagName) -> "-#{flagName}"