[PATCH] zparseopts: Add -F option, completion, tests; improve documentation

[PATCH] zparseopts: Add -F option, completion, tests; improve documentation

[dana]

This adds an -F option to zparseopts which causes it to immediately abort and
print an error when it encounters an unrecognised option-like parameter,
similar to what it does when a required optarg isn't found. The desire for
this feature has come up in users/9361 and users/23776, as well as on IRC once
or twice, and it seems easy enough to do. (It's possible to do it reliably in
'user land', too, but it's slow and boiler-plate-y.)

Since i was in there, i also added a completion function and a bunch of tests,
and i clarified several things in the documentation. I went through the
mailing list looking for edge cases and other stuff that people had asked
about; most notably, i made the handling of ambiguous (overlapping) option
specs 'official', as suggested by Bart in users/17001. To be clear, this is
setting previously unspecified behaviour in stone; please let me know if you'd
rather i keep that bit out after all (or if you see any other issues, obv)

dana


diff --git a/Completion/Zsh/Command/_zparseopts b/Completion/Zsh/Command/_zparseopts
new file mode 100644
index 000000000..e13a91081
--- /dev/null
+++ b/Completion/Zsh/Command/_zparseopts
@@ -0,0 +1,37 @@
+#compdef zparseopts
+
+local ret=1
+local -a context line state state_descr alts opts
+local -A opt_args
+
+_arguments -A '-*' : \
+  '-a+[specify array in which to store parsed options]:array:_parameters -g "*array*~*readonly*"' \
+  '-A+[specify association in which to store parsed options]:association:_parameters -g "*association*~*readonly*"' \
+  '-D[remove parsed options from positional parameters]' \
+  "-E[don't stop parsing at first parameter not described by specs]" \
+  '-F[abort parsing and print error at first option-like parameter not described by specs]' \
+  '-K[preserve contents of arrays/associations when specs are not matched]' \
+  '-M[enable mapping among equivalent options with opt1=opt2 spec form]' \
+  '(-)-[end zparseopts options; specs follow]' \
+  '*: :->spec' \
+&& ret=0
+
+[[ $state == spec ]] &&
+if compset -P '*='; then
+  alts=()
+  (( $+opt_args[-M] )) && {
+    opts=( $line )
+    [[ $opts[1] == (-|--) ]] && shift opts
+    opts=( ${(@)opts%%(+|)(:|:-|::|)(=*|)} )
+    opts=( ${(@)opts:#${words[CURRENT]%%=*}} )
+    alts+=( "spec-opt-names:spec option name:(${(j< >)${(@q+)opts}})" )
+  }
+  alts+=( 'parameters:array:_parameters -g "*array*~*readonly*"' )
+  _alternative $alts && ret=0
+else
+  # Not great, but close enough for now
+  compset -S '=*'
+  _message -e spec-opts 'spec option (name[+][:|:-|::])' && ret=0
+fi
+
+return ret
diff --git a/Doc/Zsh/mod_zutil.yo b/Doc/Zsh/mod_zutil.yo
index 15f6ed365..fa1f7b3ea 100644
--- a/Doc/Zsh/mod_zutil.yo
+++ b/Doc/Zsh/mod_zutil.yo
@@ -180,7 +180,7 @@ item(tt(zregexparse))(
 This implements some internals of the tt(_regex_arguments) function.
 )
 findex(zparseopts)
-item(tt(zparseopts) [ tt(-D) tt(-K) tt(-M) tt(-E) ] [ tt(-a) var(array) ] [ tt(-A) var(assoc) ] [ tt(-) ] var(spec) ...)(
+item(tt(zparseopts) [ tt(-D) tt(-E) tt(-F) tt(-K) tt(-M) ] [ tt(-a) var(array) ] [ tt(-A) var(assoc) ] [ tt(-) ] var(spec) ...)(
 This builtin simplifies the parsing of options in positional parameters,
 i.e. the set of arguments given by tt($*).  Each var(spec) describes one
 option and must be of the form `var(opt)[tt(=)var(array)]'.  If an option
@@ -195,7 +195,7 @@ Note that it is an error to give any var(spec) without an
 Unless the tt(-E) option is given, parsing stops at the first string
 that isn't described by one of the var(spec)s.  Even with tt(-E),
 parsing always stops at a positional parameter equal to `tt(-)' or
-`tt(-)tt(-)'.
+`tt(-)tt(-)'. See also tt(-F).
 
 The var(opt) description must be one of the following.  Any of the special
 characters can appear in the option name provided it is preceded by a
@@ -234,9 +234,23 @@ first colon.
 )
 enditem()
 
+In all cases, option-arguments must appear either immediately following the
+option in the same positional parameter or in the next one. Even an optional
+argument may appear in the next parameter, unless it begins with a `tt(-)'.
+There is no special handling of `tt(=)' as with GNU-style argument parsers;
+given the var(spec) `tt(-foo:)', the positional parameter `tt(-)tt(-foo=bar)'
+is parsed as `tt(-)tt(-foo)' with an argument of `tt(=bar)'.
+
+When the names of two options that take no arguments overlap, the longest one
+wins, so that parsing for the var(spec)s `tt(-foo -foobar)' (for example) is
+unambiguous. However, due to the aforementioned handling of option-arguments,
+ambiguities may arise when at least one overlapping var(spec) takes an
+argument, as in `tt(-foo: -foobar)'. In that case, the last matching
+var(spec) wins.
+
 The options of tt(zparseopts) itself cannot be stacked because, for
 example, the stack `tt(-DEK)' is indistinguishable from a var(spec) for
-the GNU-style long option `tt(--DEK)'.  The options of tt(zparseopts)
+the GNU-style long option `tt(-)tt(-DEK)'.  The options of tt(zparseopts)
 itself are:
 
 startitem()
@@ -252,8 +266,29 @@ as the values.
 item(tt(-D))(
 If this option is given, all options found are removed from the positional
 parameters of the calling shell or shell function, up to but not including
-any not described by the var(spec)s.  This is similar to using the tt(shift)
-builtin.
+any not described by the var(spec)s.  If the first such parameter is `tt(-)'
+or `tt(-)tt(-)', it is removed as well.  This is similar to using the
+tt(shift) builtin.
+)
+item(tt(-E))(
+This changes the parsing rules to em(not) stop at the first string
+that isn't described by one of the var(spec)s.  It can be used to test
+for or (if used together with tt(-D)) extract options and their
+arguments, ignoring all other options and arguments that may be in the
+positional parameters.  As indicated above, parsing still stops at the
+first `tt(-)' or `tt(-)tt(-)' not described by a var(spec), but it is not
+removed when used with tt(-D).
+)
+item(tt(-F))(
+If this option is given, tt(zparseopts) immediately stops at the first
+option-like parameter not described by one of the var(spec)s, prints an
+error message, and returns status 1.  Removal (tt(-D)) and extraction
+(tt(-E)) are not performed, and option arrays are not updated.  This
+provides basic validation for the given options.
+
+Note that the appearance in the positional parameters of an option without
+its required argument always aborts parsing and returns an error as described
+above regardless of whether this option is used.
 )
 item(tt(-K))(
 With this option, the arrays specified with the tt(-a) option and with the
@@ -272,13 +307,6 @@ is found, the values are stored as usual.  This changes only the way the
 values are stored, not the way tt($*) is parsed, so results may be
 unpredictable if the `var(name)tt(+)' specifier is used inconsistently.
 )
-item(tt(-E))(
-This changes the parsing rules to em(not) stop at the first string
-that isn't described by one of the var(spec)s.  It can be used to test
-for or (if used together with tt(-D)) extract options and their
-arguments, ignoring all other options and arguments that may be in the
-positional parameters.
-)
 enditem()
 
 For example,
diff --git a/Src/Modules/zutil.c b/Src/Modules/zutil.c
index 19a8306b5..c4fe4a15e 100644
--- a/Src/Modules/zutil.c
+++ b/Src/Modules/zutil.c
@@ -1644,7 +1644,7 @@ static int
 bin_zparseopts(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 {
     char *o, *p, *n, **pp, **aval, **ap, *assoc = NULL, **cp, **np;
-    int del = 0, flags = 0, extract = 0, keep = 0;
+    int del = 0, flags = 0, extract = 0, fail = 0, keep = 0;
     Zoptdesc sopts[256], d;
     Zoptarr a, defarr = NULL;
     Zoptval v;
@@ -1681,6 +1681,14 @@ bin_zparseopts(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 		}
 		extract = 1;
 		break;
+	    case 'F':
+		if (o[2]) {
+		    args--;
+		    o = NULL;
+		    break;
+		}
+		fail = 1;
+		break;
 	    case 'K':
 		if (o[2]) {
 		    args--;
@@ -1843,6 +1851,10 @@ bin_zparseopts(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 	if (!(d = lookup_opt(o + 1))) {
 	    while (*++o) {
 		if (!(d = sopts[STOUC(*o)])) {
+		    if (fail) {
+			zwarnnam(nam, "bad option: %c", *o);
+			return 1;
+		    }
 		    o = NULL;
 		    break;
 		}
diff --git a/Test/V12zparseopts.ztst b/Test/V12zparseopts.ztst
new file mode 100644
index 000000000..d7fc33f72
--- /dev/null
+++ b/Test/V12zparseopts.ztst
@@ -0,0 +1,172 @@
+# Test zparseopts from the zsh/zutil module
+
+%prep
+
+  if zmodload zsh/zutil 2> /dev/null; then
+    # Produce a string representing an associative array ordered by its keys
+    order_assoc() {
+      local -a _arr
+      for 2 in "${(@kP)1}"; do
+        _arr+=( "${(q-)2} ${(q-)${(P)1}[$2]}" )
+      done
+      print -r - ${(j< >)${(@o)_arr}}
+    }
+  else
+    ZTST_unimplemented="can't load the zsh/zutil module for testing"
+  fi
+
+%test
+
+  () {
+    local -a optv
+    zparseopts -a optv - a b: c:- z
+    print -r - ret: $?, optv: $optv, argv: $argv
+  } -ab1 -c -d -e -z
+0:zparseopts -a
+>ret: 0, optv: -a -b 1 -c-d, argv: -ab1 -c -d -e -z
+
+  () {
+    local -A opts
+    zparseopts -A opts - a b: c:- z
+    print -r - ret: $?, opts: "$( order_assoc opts )", argv: $argv
+  } -ab1 -c -d -e -z
+0:zparseopts -A
+>ret: 0, opts: -a '' -b 1 -c -d, argv: -ab1 -c -d -e -z
+
+  () {
+    local -a optv
+    zparseopts -D -a optv - a b: c:- z
+    print -r - ret: $?, optv: $optv, argv: $argv
+  } -ab1 -c -d -e -z
+0:zparseopts -D
+>ret: 0, optv: -a -b 1 -c-d, argv: -e -z
+
+  () {
+    local -a optv
+    zparseopts -E -a optv - a b: c:- z
+    print -r - ret: $?, optv: $optv, argv: $argv
+  } -ab1 -c -d -e -z
+0:zparseopts -E
+>ret: 0, optv: -a -b 1 -c-d -z, argv: -ab1 -c -d -e -z
+
+  () {
+    local -a optv
+    zparseopts -D -E -a optv - a b: c:- z
+    print -r - ret: $?, optv: $optv, argv: $argv
+  } -ab1 -c -d -e -z
+0:zparseopts -D -E
+>ret: 0, optv: -a -b 1 -c-d -z, argv: -e
+
+  for 1 in '-a -x -z' '-ax -z' '-a --x -z'; do
+    () {
+      local -a optv
+      zparseopts -D -E -F -a optv - a b: c:- z
+      print -r - ret: $?, optv: $optv, argv: $argv
+    } $=1
+  done
+0:zparseopts -F
+?(anon):zparseopts:2: bad option: x
+>ret: 1, optv: , argv: -a -x -z
+?(anon):zparseopts:2: bad option: x
+>ret: 1, optv: , argv: -ax -z
+?(anon):zparseopts:2: bad option: -
+>ret: 1, optv: , argv: -a --x -z
+
+  for 1 in '-a 1 2 3' '1 2 3'; do
+    () {
+      local -a optv=( -x -y -z )
+      zparseopts -D -K -a optv - a b: c:- z
+      print -r - ret: $?, optv: $optv, argv: $argv
+    } $=1
+  done
+0:zparseopts -K -a
+>ret: 0, optv: -a, argv: 1 2 3
+>ret: 0, optv: -x -y -z, argv: 1 2 3
+
+  for 1 in '-a 1 2 3' '1 2 3'; do
+    () {
+      local -A opts=( -b 1 -z '' )
+      zparseopts -D -K -A opts - a b: c:- z
+      print -r - ret: $?, opts: "$( order_assoc opts )", argv: $argv
+    } $=1
+  done
+0:zparseopts -K -A
+>ret: 0, opts: -a '' -b 1 -z '', argv: 1 2 3
+>ret: 0, opts: -b 1 -z '', argv: 1 2 3
+
+  () {
+    local -a optv
+    local -A opts
+    zparseopts -D -M -a optv -A opts - a:=-aaa -aaa:
+    print -r - ret: $?, optv: $optv, opts: "$( order_assoc opts )", argv: $argv
+  } --aaa foo -a bar 1 2 3
+0:zparseopts -M
+>ret: 0, optv: --aaa bar, opts: --aaa bar, argv: 1 2 3
+
+  () {
+    local -a optv aa ab
+    zparseopts -a optv - a=aa b:=ab c:- z
+    print -r - ret: $?, optv: $optv, aa: $aa, ab: $ab, argv: $argv
+  } -ab1 -c -d
+0:multiple arrays
+>ret: 0, optv: -c-d, aa: -a, ab: -b 1, argv: -ab1 -c -d
+
+  for 1 in '-a - -b - - -b' '-a -- -b -- -- -b' '-a 1 -b - - -b'; do
+    # -D alone strips - out
+    () {
+      local -a optv
+      zparseopts -D -F -a optv - a b: c:- z
+      print -r - '(-D   )' ret: $?, optv: $optv, argv: $argv
+    } $=1
+    # -D -E leaves - in
+    () {
+      local -a optv
+      zparseopts -D -E -F -a optv - a b: c:- z
+      print -r - '(-D -E)' ret: $?, optv: $optv, argv: $argv
+    } $=1
+  done
+0:-/-- handling
+>(-D   ) ret: 0, optv: -a, argv: -b - - -b
+>(-D -E) ret: 0, optv: -a, argv: - -b - - -b
+>(-D   ) ret: 0, optv: -a, argv: -b -- -- -b
+>(-D -E) ret: 0, optv: -a, argv: -- -b -- -- -b
+>(-D   ) ret: 0, optv: -a, argv: 1 -b - - -b
+>(-D -E) ret: 0, optv: -a -b -, argv: 1 - -b
+
+  # Escaping should always work, but it's optional on the first character
+  for specs in '\+ \: \= \\' '+ : = \'; do
+    () {
+      local -a optv
+      zparseopts -D -a optv - $=specs
+      print -r - ret: $?, optv: $optv, argv: $argv
+    } -+:=\\ 1 2 3
+  done
+  () {
+    local -a optv
+    zparseopts -D -a optv - '-\:\:\::'
+    print -r - ret: $?, optv: $optv, argv: $argv
+  } --:::foo 1 2 3
+0:special characters in option names
+>ret: 0, optv: -+ -: -= -\, argv: 1 2 3
+>ret: 0, optv: -+ -: -= -\, argv: 1 2 3
+>ret: 0, optv: --::: foo, argv: 1 2 3
+
+  for specs in '-foo: -foobar' '-foobar -foo:'; do
+    () {
+      local -a optv
+      zparseopts -a optv - $=specs
+      print -r - ret: $?, optv: $optv, argv: $argv
+    } --foobar 1 2 3
+  done
+0:overlapping option specs (scan order)
+>ret: 0, optv: --foobar, argv: --foobar 1 2 3
+>ret: 0, optv: --foo bar, argv: --foobar 1 2 3
+
+  () {
+    local -a optv
+    zparseopts -a optv - a b: c:- z
+    print -r - ret: $?, optv: $optv, argv: $argv
+  } -ab1 -c
+0:missing optarg
+?(anon):zparseopts:2: missing argument for option: c
+>ret: 1, optv: , argv: -ab1 -c