40321: _arguments option groups

4 files changed, 248 insertions, 106 deletions

diff --git a/ChangeLog b/ChangeLog
index da1ece42a..78f590c87 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2017-01-11  Oliver Kiddle  <opk@zsh.org>
+
+	* 40321: Doc/Zsh/compsys.yo, Src/Zle/computil.c,
+	Test/Y03arguments.ztst: _arguments option groups
+
 2017-01-11  Peter Stephenson  <p.stephenson@samsung.com>
 
 	* unposted: Src/builtin.c, Src/exec.c: be more careful to free
diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index 953d51c4c..2a112ed15 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -3573,13 +3573,11 @@ This function can be used to give a complete specification for completion
 for a command whose arguments follow standard UNIX option and argument
 conventions.
 
-em(Options overview)
+em(Options Overview)
 
 Options to tt(_arguments) itself must be in separate words, i.e. tt(-s -w),
 not tt(-sw).  The options are followed by var(spec)s that describe options and
-arguments of the analyzed command.  var(spec)s that describe option flags must
-precede var(spec)s that describe non-option ("positional" or "normal")
-arguments of the analyzed line.  To avoid ambiguity, all
+arguments of the analyzed command.  To avoid ambiguity, all
 options to tt(_arguments) itself may be separated from the var(spec) forms
 by a single colon.
 
@@ -3997,18 +3995,48 @@ example(local curcontext="$curcontext")
 This is useful where it is not possible for multiple states to be valid
 together.
 
-em(Specifying multiple sets of options)
+em(Grouping Options)
 
-It is possible to specify multiple sets of options and
-arguments with the sets separated by single hyphens.  The specifications
-before the first hyphen (if any) are shared by all the remaining sets.
-The first word in every other set provides a name for the
-set which may appear in exclusion lists in specifications,
-either alone or before one of the possible values described above.
-In the second case a `tt(-)' should appear between this name and the
-remainder.
+Options can be grouped to simplify exclusion lists. A group is
+introduced with `tt(PLUS())' followed by a name for the group in the
+subsequent word. Whole groups can then be referenced in an exclusion
+list or a group name can be used to disambiguate between two forms of
+the same option. For example:
 
-For example:
+example(_arguments \ 
+    '(group2--x)-a' \ 
+  PLUS() group1 \ 
+    -m \ 
+    '(group2)-n' \ 
+  PLUS() group2 \ 
+    -x -y)
+
+If the name of a group is specified in the form
+`tt(LPAR())var(name)tt(RPAR())' then only one value from that group
+will ever be completed; more formally, all specifications are mutually
+exclusive to all other specifications in that group. This is useful for
+defining options that are aliases for each other. For example:
+
+example(_arguments \ 
+    -a -b \ 
+  PLUS() '(operation)' \ 
+    {-c,--compress}'[compress]' \ 
+    {-d,--decompress}'[decompress]' \ 
+    {-l,--list}'[list]')
+
+If an option in a group appears on the command line, it is stored in the
+associative array `tt(opt_args)' with 'var(group)tt(-)var(option)'
+as a key.  In the example above, a key `tt(operation--c)' is used if the option
+`tt(-c)' is present on the command line.
+
+em(Specifying Multiple Sets of Arguments)
+
+It is possible to specify multiple sets of options and arguments with
+the sets separated by single hyphens. This differs from groups in that
+sets are considered to be mutually exclusive of each other.
+
+Specifications before the first set and from any group are common to
+all sets. For example:
 
 example(_arguments \ 
     -a \ 
@@ -4024,28 +4052,11 @@ possible completions.  When it contains `tt(-d)' or an argument, the
 option `tt(-c)' will not be considered.  However, after `tt(-a)'
 both sets will still be considered valid.
 
-If an option in a set appears on the command line, it is stored in the
-associative array `tt(opt_args)' with 'var(set)tt(-)var(option)'
-as a key.  In the example above, a key `tt(set1--c)' is used if the option
-`tt(-c)' is on the command line.
-
-If the name given for one of the mutually exclusive sets is of the form
-`tt(LPAR())var(name)tt(RPAR())' then only one value from each set will ever
-be completed; more formally, all specifications are mutually
-exclusive to all other specifications in the same set.  This is
-useful for defining multiple sets of options which are mutually
-exclusive and in which the options are aliases for each other.  For
-example:
-
-example(_arguments \ 
-    -a -b \ 
-  - '(compress)' \ 
-    {-c,--compress}'[compress]' \ 
-  - '(uncompress)' \ 
-    {-d,--decompress}'[decompress]')
+As for groups, the name of a set may appear in exclusion lists, either
+alone or preceding a normal option or argument specification.
 
-As the completion code has to parse the command line separately for each
-set this form of argument is slow and should only be used when necessary.
+The completion code has to parse the command line separately for each
+set. This can be slow so sets should only be used when necessary.
 A useful alternative is often an option specification with rest-arguments
 (as in `tt(-foo:*:...)'); here the option tt(-foo) swallows up all
 remaining arguments as described by the var(optarg) definitions.
diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index 12aa8950f..7bf95351f 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -934,7 +934,7 @@ struct caopt {
     Caarg args;			/* option arguments */
     int active;			/* still allowed on command line */
     int num;			/* it's the num'th option */
-    char *set;			/* set name, shared */
+    char *gsname;		/* group or set name, shared */
     int not;			/* don't complete this option (`!...') */
 };
 
@@ -958,7 +958,7 @@ struct caarg {
     int min;			/* earliest possible arg pos, given optional args */
     int direct;			/* true if argument number was given explicitly */
     int active;			/* still allowed on command line */
-    char *set;			/* set name, shared */
+    char *gsname;		/* group or set name, shared */
 };
 
 #define CAA_NORMAL 1
@@ -1096,7 +1096,7 @@ parse_caarg(int mult, int type, int num, int opt, char *oname, char **def,
     ret->type = type;
     ret->opt = ztrdup(oname);
     ret->direct = 0;
-    ret->set = set;
+    ret->gsname = set;
 
     /* Get the description. */
 
@@ -1183,10 +1183,10 @@ parse_cadef(char *nam, char **args)
     Cadef all, ret;
     Caopt *optp;
     char **orig_args = args, *p, *q, *match = "r:|[_-]=* r:|=*", **xor, **sargs;
-    char *adpre, *adsuf, *axor = NULL, *doset = NULL, **setp = NULL;
+    char *adpre, *adsuf, *axor = NULL, *doset = NULL, **pendset = NULL, **curset = NULL;
     char *nonarg = NULL;
     int single = 0, anum = 1, xnum, flags = 0;
-    int state = 0, not = 0;
+    int foreignset = 0, not = 0;
 
     /* First string is the auto-description definition. */
 
@@ -1249,7 +1249,6 @@ parse_cadef(char *nam, char **args)
 
     if (nonarg)
 	tokenize(nonarg = dupstring(nonarg));
-
     /* Looks good. Optimistically allocate the cadef structure. */
 
     all = ret = alloc_cadef(orig_args, single, match, nonarg, flags);
@@ -1258,36 +1257,64 @@ parse_cadef(char *nam, char **args)
 
     /* Get the definitions. */
 
-    for (; *args; args++) {
+    for (; *args || pendset; args++) {
+	if (!*args) {
+	    /* start new set */
+	    args = sargs; /* go back and repeat parse of common options */
+	    doset = NULL;
+	    set_cadef_opts(ret);
+	    ret = ret->snext = alloc_cadef(NULL, single, match, nonarg, flags);
+	    optp = &(ret->opts);
+	    anum = 1;
+	    foreignset = 0;
+	    curset = pendset;
+	    pendset = 0;
+        }
         if (args[0][0] == '-' && !args[0][1] && args[1]) {
-	    if (!state) {
-		char *p;
-		int l;
-
-		if (setp) /* got to first set: skip to ours */
-		    args = setp;
-		/* else we were the first set so don't need to skip */
-		p = *++args;
-		l = strlen(p) - 1;
+	    if ((foreignset = curset && args != curset)) {
+		if (!pendset && args > curset)
+		    pendset = args; /* mark pointer to next pending set */
+		++args;
+	    } else {
+		/* Carrying on: this is the current set */
+		char *p = *++args;
+		int l = strlen(p) - 1;
+
 		if (*p == '(' && p[l] == ')') {
 		    axor = p = dupstring(p + 1);
 		    p[l - 1] = '\0';
 		} else
 		    axor = NULL;
+		if (!*p) {
+		    freecadef(all);
+		    zwarnnam(nam, "empty set name");
+		    return NULL;
+		}
 		ret->set = doset = tricat(p, "-", "");
-		state = 1;
-	    } else { /* starting new set */
-		setp = args;
-		state = 0;
-		args = sargs - 1;
-		doset = NULL;
-		set_cadef_opts(ret);
-		ret = ret->snext = alloc_cadef(NULL, single, match, nonarg, flags);
-		optp = &(ret->opts);
-		anum = 1;
+		curset = args; /* needed for the first set */
 	    }
 	    continue;
-	}
+	} else if (args[0][0] == '+' && !args[0][1] && args[1]) {
+	    char *p;
+	    int l;
+
+	    foreignset = 0; /* group not in any set, don't want to skip it */
+	    p = *++args;
+	    l = strlen(p) - 1;
+	    if (*p == '(' && p[l] == ')') {
+		axor = p = dupstring(p + 1);
+		p[l - 1] = '\0';
+	    } else
+		axor = NULL;
+	    if (!*p) {
+		freecadef(all);
+		zwarnnam(nam, "empty group name");
+		return NULL;
+	    }
+	    doset = tricat(p, "-", "");
+	    continue;
+	} else if (foreignset) /* skipping over a different set */
+	    continue;
 	p = dupstring(*args);
 	xnum = 0;
 	if ((not = (*p == '!')))
@@ -1499,7 +1526,7 @@ parse_cadef(char *nam, char **args)
 	    optp = &((*optp)->next);
 
 	    opt->next = NULL;
-	    opt->set = doset;
+	    opt->gsname = doset;
 	    opt->name = ztrdup(rembslashcolon(name));
 	    if (descr)
 		opt->descr = ztrdup(descr);
@@ -1795,63 +1822,85 @@ ca_inactive(Cadef d, char **xor, int cur, int opts)
     if ((xor || opts) && cur <= compcurrent) {
 	Caopt opt;
 	char *x;
-	int sl = (d->set ? (int)strlen(d->set) : -1), set = 0;
         /* current word could be a prefix of a longer one so only do
 	 * exclusions for single-letter options (for option clumping) */
 	int single = (cur == compcurrent);
 
 	for (; (x = (opts ? "-" : *xor)); xor++) {
-	    set = 0;
-	    if (sl > 0) {
-		if (strpfx(d->set, x)) {
-		    x += sl;
-		    set = 1;
-		} else if (!strncmp(d->set, x, sl - 1)) {
-		    Caopt p;
-
-		    for (p = d->opts; p; p = p->next)
-			if (p->set && !(single && *p->name && p->name[1] && p->name[2]))
-			    p->active = 0;
-			
-		    x = ":";
-		    set = 1;
+	    int excludeall = 0;
+	    char *grp = NULL;
+	    size_t grplen;
+	    char *next, *sep = x;
+
+	    while (*sep != '+' && *sep != '-' && *sep != ':' && *sep != '*' && !idigit(*sep)) {
+		if (!(next = strchr(sep, '-')) || !*++next) {
+		    /* exclusion is just the name of a set or group */
+		    excludeall = 1; /* excluding options and args */
+		    sep += strlen(sep);
+		    /* A trailing '-' is included in the various gsname fields but is not
+		     * there for this branch. This is why we add excludeall to grplen
+		     * when checking for the null in a few places below */
+		    break;
 		}
+		sep = next;
+	    }
+	    if (sep > x) { /* exclusion included a set or group name */
+		grp = x;
+		grplen = sep - grp;
+		x = sep;
 	    }
-	    if (x[0] == ':' && !x[1]) {
-		if (set) {
+
+	    if (excludeall || (x[0] == ':' && !x[1])) {
+		if (grp) {
 		    Caarg a;
 
 		    for (a = d->args; a; a = a->next)
-			if (a->set)
+			if (a->gsname && !strncmp(a->gsname, grp, grplen) &&
+				!a->gsname[grplen + excludeall])
 			    a->active = 0;
-		    if (d->rest && (!set || d->rest->set))
+		    if (d->rest && d->rest->gsname &&
+			    !strncmp(d->rest->gsname, grp, grplen) &&
+			    !d->rest->gsname[grplen + excludeall])
 			d->rest->active = 0;
 		} else
 		    d->argsactive = 0;
-	    } else if (x[0] == '-' && !x[1]) {
+	    }
+
+	    if (excludeall || (x[0] == '-' && !x[1])) {
 		Caopt p;
 
 		for (p = d->opts; p; p = p->next)
-		    if ((!set || p->set) && !(single && *p->name && p->name[1] && p->name[2]))
+		    if ((!grp || (p->gsname && !strncmp(p->gsname, grp, grplen) &&
+			    !p->gsname[grplen + excludeall])) &&
+			    !(single && *p->name && p->name[1] && p->name[2]))
 			p->active = 0;
-	    } else if (x[0] == '*' && !x[1]) {
-		if (d->rest && (!set || d->rest->set))
-		    d->rest->active = 0;
-	    } else if (idigit(x[0])) {
-		int n = atoi(x);
-		Caarg a = d->args;
-
-		while (a && a->num < n)
-		    a = a->next;
+	    }
 
-		if (a && a->num == n && (!set || a->set))
-		    a->active = 0;
-	    } else if ((opt = ca_get_opt(d, x, 1, NULL)) && (!set || opt->set) &&
-		    !(single && *opt->name && opt->name[1] && opt->name[2]))
-		opt->active = 0;
+	    if (excludeall || (x[0] == '*' && !x[1])) {
+		if (d->rest && (!grp || (d->rest->gsname &&
+			!strncmp(d->rest->gsname, grp, grplen) &&
+			!d->rest->gsname[grplen + excludeall])))
+		    d->rest->active = 0;
+            }
 
-	    if (opts)
-		break;
+	    if (!excludeall) {
+		if (idigit(x[0])) {
+		    int n = atoi(x);
+		    Caarg a = d->args;
+
+		    while (a && a->num < n)
+			a = a->next;
+
+		    if (a && a->num == n && (!grp || (a->gsname &&
+			    !strncmp(a->gsname, grp, grplen))))
+			a->active = 0;
+		} else if ((opt = ca_get_opt(d, x, 1, NULL)) &&
+			(!grp || (opt->gsname && !strncmp(opt->gsname, grp, grplen))) &&
+			!(single && *opt->name && opt->name[1] && opt->name[2]))
+		    opt->active = 0;
+		if (opts)
+		    break;
+	    }
 	}
     }
 }
@@ -2414,19 +2463,19 @@ ca_set_data(LinkList descr, LinkList act, LinkList subc,
 		    restrict_range(ca_laststate.argbeg, ca_laststate.argend);
 	    }
 	    if (arg->opt) {
-		buf = (char *) zhalloc((arg->set ? strlen(arg->set) : 0) +
+		buf = (char *) zhalloc((arg->gsname ? strlen(arg->gsname) : 0) +
 				       strlen(arg->opt) + 40);
 		if (arg->num > 0 && arg->type < CAA_REST)
 		    sprintf(buf, "%soption%s-%d",
-			    (arg->set ? arg->set : ""), arg->opt, arg->num);
+			    (arg->gsname ? arg->gsname : ""), arg->opt, arg->num);
 		else
 		    sprintf(buf, "%soption%s-rest",
-			    (arg->set ? arg->set : ""), arg->opt);
+			    (arg->gsname ? arg->gsname : ""), arg->opt);
 	    } else if (arg->num > 0) {
 		sprintf(nbuf, "argument-%d", arg->num);
-		buf = (arg->set ? dyncat(arg->set, nbuf) : dupstring(nbuf));
+		buf = (arg->gsname ? dyncat(arg->gsname, nbuf) : dupstring(nbuf));
 	    } else
-		buf = (arg->set ? dyncat(arg->set, "argument-rest") :
+		buf = (arg->gsname ? dyncat(arg->gsname, "argument-rest") :
 		       dupstring("argument-rest"));
 
 	    addlinknode(subc, buf);
@@ -2779,7 +2828,7 @@ bin_comparguments(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 	    for (s = lstate; s; s = s->snext)
 		for (o = s->d->opts, a = s->oargs; o; o = o->next, a++)
 		    if (*a) {
-			*p++ = (o->set ? tricat(o->set, o->name, "") :
+			*p++ = (o->gsname ? tricat(o->gsname, o->name, "") :
 				ztrdup(o->name));
 			*p++ = ca_colonlist(*a);
 		    }
diff --git a/Test/Y03arguments.ztst b/Test/Y03arguments.ztst
index 5957bc306..25bb96b84 100644
--- a/Test/Y03arguments.ztst
+++ b/Test/Y03arguments.ztst
@@ -463,6 +463,14 @@
 0:single option sets are still mutually exclusive
 >line: {tst -m -x }{}
 
+ tst_arguments '(set-c set-g)-a' '(set)-b' -c + grp -g - set -s
+ comptest $'tst -a -b -\t'
+0:excluding a set doesn't exclude common options as part of the set
+>line: {tst -a -b -}{}
+>DESCRIPTION:{option}
+>NO:{-c}
+>NO:{-g}
+
  tst_arguments '(-)-h' -a -b -c
  comptest $'tst -h -\t'
 0:exclude all other options
@@ -536,6 +544,75 @@ F:>DESCRIPTION:{option}
 F:>NO:{-b}
 F:>NO:{-c}
 
+ tst_arguments + grp1 -a -b - onlyset '(-a grp3--y grp2 grp4--)-m' -n + grp2 -u -v + grp3 -x -y + grp4 -0 ':rest'
+ comptest $'tst -m -\t'
+0:exclude group options
+>line: {tst -m -}{}
+>DESCRIPTION:{rest}
+>DESCRIPTION:{option}
+>NO:{-b}
+>NO:{-n}
+>NO:{-x}
+
+ tst_arguments -x + '(grp1)' -a -b + '(grp2)' -m -n
+ comptest $'tst -m -\t'
+0:single option groups are not mutually exclusive
+>line: {tst -m -}{}
+>DESCRIPTION:{option}
+>NO:{-a}
+>NO:{-b}
+>NO:{-x}
+
+ tst_arguments '(grp1 grp2-2)-a' '(grp1-*)-b' + grp1 ':first' '*:rest' + grp2 ':second'
+ comptest $'tst -a \t\eb\C-w\C-e x y \t'
+0:exclude rest args listed within a group
+>line: {tst -a -b }{}
+>line: {tst -b x y -a }{}
+
+ tst_arguments '(grp--m)-a' + grp '-m:value:(a b c)' + agrp '-m:other:(1 2 3)'
+ comptest $'tst -m \t\C-w-a -m \t'
+0:two forms of same option in different groups
+>line: {tst -m }{}
+>DESCRIPTION:{value}
+>NO:{a}
+>NO:{b}
+>NO:{c}
+>line: {tst -a -m }{}
+>DESCRIPTION:{other}
+>NO:{1}
+>NO:{2}
+>NO:{3}
+F:should offer both sets of arguments in first case
+
+ tst_arguments '(grp--m)-x' + '(grp)' -a -b -c ':val:(1 2 3)'
+ comptest $'tst 1 -\t'
+0:normal argument excludes options in internally mutually exclusive group
+>line: {tst 1 -x }{}
+
+ tst_arguments -s -a - set1 -c -s - set2 -c -t + grp -d
+ comptest $'tst -s\t -\t'
+0:mix sets, groups and option stacking
+>line: {tst -s}{}
+>DESCRIPTION:{option}
+>NO:{-a}
+>NO:{-c}
+>NO:{-d}
+>NO:{-t}
+>line: {tst -s -}{}
+>DESCRIPTION:{option}
+>NO:{-a}
+>NO:{-c}
+>NO:{-d}
+F:shouldn't offer -t in the first case (with stacked options)
+
+ tst_arguments -s '(set-a set--b grp-a grp--b grp-)-a' - set-a -s - set--b -t + grp-a -g + grp--b -h + grp- -i
+ comptest $'tst -a\t'
+0:sets and groups with - in their name
+>line: {tst -a}{}
+>DESCRIPTION:{option}
+>NO:{-h}
+>NO:{-t}
+
  tst_arguments --abc --aah :arg:
  comptesteval 'setopt bashautolist automenu'
  comptest $'tst --a\t\t\t'