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,39 @@ 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
+
+    options = {}
+    for cmdArg, i in args
+      # Normalize arguments by expanding merged flags into multiple
+      # flags. This allows you to have `-wl` be the same as `--watch --lint`.
+      multi = splitMultiArg cmdArg
+      subArgs = multi or [cmdArg]
+      for sub, j in subArgs
+        # 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 '--'
-        break
-    state.options.arguments = state.argsLeft[..]
-    state.options
+        # 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.
+        unless looksLikeOption sub
+          ++j if sub is `--`
+          options.arguments = [subArgs[j..]..., args[++i..]...]
+          return options
+
+        rule = @rules.flagDict[sub]
+        unless rule?
+          context = if multi? then " (in multi-flag '#{cmdArg}')" else ''
+          throw new Error "unrecognized option: #{sub}#{context}"
+
+        {hasArgument, isList, name} = rule
+
+        value = if hasArgument
+          subArgs[++j] or args[++i] or
+            throw new Error "value required for '#{sub}':
+              was the last argument provided"
+        else true
+
+        value = (options[name] or []).concat value if isList
+        options[name] = value
+    options
 
   # Return the help text for this **OptionParser**, listing and describing all
   # of the valid options, for `--help` and such.
@@ -61,23 +74,38 @@ 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...
+  switches = {}
+  argFlags = {}
+  for rule in ruleList
+    flagDict = if rule.hasArgument then argFlags else switches
+    for flag in [rule.shortFlag, rule.longFlag]
+      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, switches, argFlags}
 
 # 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 +116,10 @@ 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
-
-  throw new Error "unrecognized option: #{arg}"
+looksLikeOption = (maybeOption) ->
+  [LONG_FLAG, SHORT_FLAG].some (pat) -> maybeOption.match(pat)?
+
+splitMultiArg = (maybeMulti) ->
+  maybeMulti.match(MULTI_FLAG)?[1]
+    .split('')
+    .map (arg) -> "-#{arg}"