40129: revert 39611, add code comments and test cases for _arguments

3 files changed, 298 insertions, 47 deletions

diff --git a/ChangeLog b/ChangeLog
index 43825d3cf..e76a775b9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2016-12-08  Oliver Kiddle  <opk@zsh.org>
 
+	* 40129: Src/Zle/computil.c, Test/Y03arguments.ztst:
+	revert 39611, add code comments and test cases for _arguments
+
 	* c.f. 40119: Src/zsh.h: correct typo in comment
 
 	* 40114: Completion/Zsh/Context/_brace_parameter: complete
diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index c78167329..192ddeab9 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -914,7 +914,7 @@ struct cadef {
     int lastt;			/* last time this was used */
     Caopt *single;		/* array of single-letter options */
     char *match;		/* -M spec to use */
-    int argsactive;		/* if arguments are still allowed */
+    int argsactive;		/* if normal arguments are still allowed */
 				/* used while parsing a command line */
     char *set;			/* set name prefix (<name>-), shared */
     char *sname;		/* set name */
@@ -922,7 +922,7 @@ struct cadef {
     char *nonarg;		/* pattern for non-args (-A argument) */
 };
 
-#define CDF_SEP 1
+#define CDF_SEP 1		/* -S was specified: -- terminates options */
 
 /* Description for an option. */
 
@@ -939,11 +939,11 @@ struct caopt {
     int not;			/* don't complete this option (`!...') */
 };
 
-#define CAO_NEXT    1
-#define CAO_DIRECT  2
-#define CAO_ODIRECT 3
-#define CAO_EQUAL   4
-#define CAO_OEQUAL  5
+#define CAO_NEXT    1		/* argument follows in next argument (`-opt:...') */
+#define CAO_DIRECT  2		/* argument follows option directly (`-opt-:...') */
+#define CAO_ODIRECT 3		/* argument may follow option directly (`-opt+:...') */
+#define CAO_EQUAL   4		/* argument follows mandatory equals (`-opt=-:...') */
+#define CAO_OEQUAL  5		/* argument follows optional equals (`-opt=:...') */
 
 /* Description for an argument */
 
@@ -957,7 +957,7 @@ struct caarg {
     char *opt;			/* option name if for an option */
     int num;			/* it's the num'th argument */
     int min;			/* it's also this argument, using opt. args */
-    int direct;			/* number was given directly */
+    int direct;			/* true if argument number was given explicitly */
     int active;			/* still allowed on command line */
     char *set;			/* set name, shared */
 };
@@ -1772,7 +1772,12 @@ ca_get_arg(Cadef d, int n)
     return NULL;
 }
 
-/* Use a xor list, marking options as inactive. */
+/* Mark options as inactive.
+ *   d: option definitions for a set
+ *   pass either:
+ *     xor: a list if exclusions
+ *     opts: if set, all options excluded leaving only nornal/rest arguments
+ * if ca_xor list initialised, exclusions are added to it */
 
 static LinkList ca_xor;
 
@@ -1848,22 +1853,37 @@ ca_inactive(Cadef d, char **xor, int cur, int opts, char *optname)
 
 typedef struct castate *Castate;
 
-/*
- *           **** DOCUMENT ME ****
- *
- * This structure and its use are a nightmare.
- */
+/* Encapsulates details from parsing the current line against a particular set,
+ * Covers positions of options and normal arguments. Used as a linked list
+ * with one state for each set. */
 
 struct castate {
-    Castate snext;
-    Cadef d;
-    int nopts;
-    Caarg def, ddef;
-    Caopt curopt, dopt;
-    int opt, arg, argbeg, optbeg, nargbeg, restbeg, curpos, argend;
-    int inopt, inrest, inarg, nth, doff, singles, oopt, actopts;
-    LinkList args;
-    LinkList *oargs;
+    Castate snext;	/* state for next set */
+    Cadef d;		/* parsed _arguments specs for the set */
+    int nopts;		/* number of specified options (size of oargs) */
+    Caarg def;		/* definition for the current set */
+    Caarg ddef;
+    Caopt curopt;	/* option description corresponding to option found on the command-line */
+    Caopt dopt;
+    int opt;		/* the length of the option up to a maximum of 2 */
+    int arg;		/* completing arguments to an option or rest args */
+    int argbeg;         /* position of first rest argument (+1) */
+    int optbeg;		/* first word after the last option to the left of the cursor:
+			 * in effect the start of any arguments to the current option */
+    int nargbeg;	/* same as optbeg but used during parse */
+    int restbeg;	/* same as argbeg but used during parse */
+    int curpos;		/* current word position */
+    int argend;         /* total number of words */
+    int inopt;		/* set to current word pos if word is a recognised option */
+    int inrest;		/* unused */
+    int inarg;          /* in a normal argument */
+    int nth;		/* number of current normal arg */
+    int doff;		/* length of current option */
+    int singles;	/* argument consists of clumped options */
+    int oopt;
+    int actopts;	/* count of active options */
+    LinkList args;	/* list of non-option args used for populating $line */
+    LinkList *oargs;	/* list of lists used for populating $opt_args */
 };
 
 static struct castate ca_laststate;
@@ -1909,7 +1929,9 @@ ca_opt_arg(Caopt opt, char *line)
     return ztrdup(line);
 }
 
-/* Parse a command line. */
+/* Parse the command line for a particular argument set (d).
+ * Returns 1 if the set should be skipped because it doesn't match
+ * existing options on the line. */
 
 static int
 ca_parse_line(Cadef d, int multi, int first)
@@ -1971,7 +1993,7 @@ ca_parse_line(Cadef d, int multi, int first)
 
 	goto end;
     }
-    if (d->nonarg)
+    if (d->nonarg) /* argument to -A */
 	napat = patcompile(d->nonarg, 0, NULL);
 
     /* Loop over the words from the line. */
@@ -2009,8 +2031,9 @@ ca_parse_line(Cadef d, int multi, int first)
 		return 1;
 	    continue;
 	}
-	/* We've got a definition for an argument, skip to the next. */
 
+	/* We've got a definition for an option/rest argument. For an option,
+	 * this means that we're completing arguments to that option. */
 	if (state.def) {
 	    state.arg = 0;
 	    if (state.curopt)
@@ -2159,8 +2182,7 @@ ca_parse_line(Cadef d, int multi, int first)
 		state.opt = 0;
 	    else
 		state.curopt = NULL;
-	} else if (multi && (*line == '-' || *line == '+') && cur != compcurrent &&
-		ca_get_opt(d, line, 0, NULL)
+	} else if (multi && (*line == '-' || *line == '+') && cur != compcurrent
 #if 0
 		   /**** Ouch. Using this will disable the mutual exclusion
 			 of different sets. Not using it will make the -A
@@ -2177,6 +2199,8 @@ ca_parse_line(Cadef d, int multi, int first)
 		return 1;
 
 	    arglast = 1;
+	    /* if this is the first normal arg after an option, may have been
+	     * earlier normal arguments if they're intermixed with options */
 	    if (state.inopt) {
 		state.inopt = 0;
 		state.nargbeg = cur - 1;
@@ -2526,25 +2550,26 @@ bin_comparguments(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 	    if (!(def = get_cadef(nam, args + 1)))
 		return 1;
 
-	    multi = !!def->snext;
+	    multi = !!def->snext; /* if we have sets */
 	    ca_parsed = cap;
 	    ca_xor = (multi ? newlinklist() : NULL);
 
-	    while (def) {
+	    while (def) { /* for each set */
 		use = !ca_parse_line(def, multi, first);
 		nx = ca_xor;
-		ca_xor = NULL;
+		ca_xor = NULL; /* don't want to duplicate the xors in the list */
 		while ((def = def->snext)) {
 		    if (nx) {
 			for (node = firstnode(nx); node; incnode(node)) {
 			    xor[0] = (char *) getdata(node);
 			    if (!strcmp(xor[0], def->sname) ||
 				ca_inactive(def, xor, compcurrent, 0, NULL))
-				break;
+				break; /* exclude this whole set */
 			}
-			if (!node)
+			if (!node) /* continue with this set */
 			    break;
 		    }
+		    /* entire set was excluded, continue to next set */
 		}
 		ca_xor = nx;
 		if (use && def) {
diff --git a/Test/Y03arguments.ztst b/Test/Y03arguments.ztst
index d59ed5424..b5a5a4be9 100644
--- a/Test/Y03arguments.ztst
+++ b/Test/Y03arguments.ztst
@@ -40,6 +40,15 @@
 >NO:{a}
 >NO:{b}
 
+# it ought to be possible to include the quoted backslash here
+ tst_arguments ':desc2:((a\:a\ value b\:other\\value))'
+ comptest $'tst \t'
+0:a and b with descriptions
+>line: {tst }{}
+>DESCRIPTION:{desc2}
+>NO:{a  -- a value}
+>NO:{b  -- othervalue}
+
  tst_arguments ':desc1:(arg1)' ':desc2:(arg2)' ':desc3:(arg3)'
  comptest $'tst \t\t\t\C-w\C-w\C-w\C-d'
 0:three arguments
@@ -81,6 +90,74 @@
 >line: {tst -o a }{}
 >line: {tst -o a b }{}
 
+ tst_arguments '!-x:arg:(ok)'
+ comptest $'tst -x \t'
+0:option argument to ignored option
+>line: {tst -x ok }{}
+
+ tst_arguments '!-a' -b
+ comptest $'tst -\t'
+0:ignored option not completed
+>line: {tst -b }{}
+
+ tst_arguments +x +y '!+z' ':arg:(x)'
+ comptest $'tst +z \t'
+0:ignored option is not taken to be the normal argument
+>line: {tst +z x }{}
+
+ tst_arguments --known --other
+ comptest $'tst --unknown -\t'
+0:unrecognised option has no effect on proceedings with no normal arguments
+>line: {tst --unknown --}{}
+
+ tst_arguments +x +y ':arg:(x)'
+ comptest $'tst +z \t'
+0:unrecognised option is taken to be the normal argument
+>line: {tst +z +}{}
+
+ tst_arguments '*-a:value:(1)'
+ comptest $'tst -a\t\t -a=\t'
+0:option argument follows in next argument
+>line: {tst -a }{}
+>line: {tst -a 1 }{}
+>line: {tst -a 1 -a=}{}
+>MESSAGE:{no arguments}
+
+ tst_arguments '*-a+:value:(1)'
+ comptest $'tst -a\t -a \t -a=\t'
+0:option argument either direct or in following argument
+>line: {tst -a1 }{}
+>line: {tst -a1 -a 1 }{}
+>line: {tst -a1 -a 1 -a=}{}
+
+ tst_arguments '*-a-:value:(1)'
+ comptest $'tst -a\t -a \t=\t'
+0:option argument follows directly
+>line: {tst -a1 }{}
+>line: {tst -a1 -a -a}{}
+>line: {tst -a1 -a -a=}{}
+
+ tst_arguments '*-a=:value:(1)'
+ comptest $'tst -a\t\t -a \t'
+0:option argument follows optional equals
+>line: {tst -a=}{}
+>line: {tst -a=1 }{}
+>line: {tst -a=1 -a 1 }{}
+
+ tst_arguments -s '*-a=:value:(1)'
+ comptest $'tst -a\t-a=\t -a \t'
+0:option argument follows optional equals, with -s
+>line: {tst -a1 }{}
+>line: {tst -a1 -a=1 }{}
+>line: {tst -a1 -a=1 -a 1 }{}
+
+ tst_arguments '*-a=-:value:(1)'
+ comptest $'tst -a\t\t-a \t'
+0:option argument follows mandatory equals
+>line: {tst -a=}{}
+>line: {tst -a=1 }{}
+>line: {tst -a=1 -a -a=}{}
+
  tst_arguments '-x:arg:'
  comptest $'tst -x\t'
 0:sticky option argument
@@ -99,6 +176,11 @@
 >DESCRIPTION:{option}
 >NO:{-x}
 
+ tst_arguments '-x' ": :_guard '[0-9]#' number"
+ comptest $'tst -\t'
+0:argument beginning with minus, guard on rest argument
+>line: {tst -x }{}
+
  tst_arguments '-o::optarg:(oa)' ':arg1:(a1)'
  comptest $'tst -o\t\t'
 0:optional option argument
@@ -110,9 +192,10 @@
 >NO:{a1}
 
  tst_arguments '-o:*a:a:(a)' ':A:(A)' ':B:(B)'
- comptest $'tst A -o a \t'
+ comptest $'tst A -o a \t\C-W\C-w-a -b -c a \t'
 0:variable length option arguments
 >line: {tst A -o a B }{}
+>line: {tst A -o -a -b -c a B }{}
 
  tst_arguments -s '-a' '-b' ':descr:{compadd - $+opt_args[-a]}'
  comptest $'tst -ab \t'
@@ -129,6 +212,40 @@
 0:rest arguments
 >line: {tst arg -b }{}
 
+ tst_arguments -a :more '*:rest:{ compadd - $words }'
+ comptest $'tst x -a rest \t'
+0:rest arguments with single colon
+>line: {tst x -a rest }{}
+>NO:{-a}
+>NO:{rest}
+>NO:{tst}
+>NO:{x}
+
+ tst_arguments -a :more '*::rest:{ compadd - $words }'
+ comptest $'tst x -a rest \t\eb\eb\eb\et\C-E \t'
+0:rest arguments with two colons
+>line: {tst x -a rest rest }{}
+>line: {tst -a x rest rest }{}
+>NO:{rest}
+>NO:{x}
+
+ tst_arguments -a -b :more '*:::rest:{ compadd - $words }'
+ comptest $'tst -b x -a -x rest \t'
+0:rest arguments with three colons
+>line: {tst -b x -a -x rest }{}
+>NO:{-x}
+>NO:{rest}
+
+ tst_arguments -a ::more '*:::rest:{ compadd - $words }'
+ comptest $'tst -a opt rest \t'
+0:rest arguments with three colons following optional argument
+>line: {tst -a opt rest rest }{}
+
+ tst_arguments -a::arg '*:::rest:{ compadd - $words }'
+ comptest $'tst -a opt rest \t'
+0:rest arguments with three colons following optional argument to an option
+>line: {tst -a opt rest rest }{}
+
  tst_arguments '-e:*last:::b:{compadd "${(j:,:)words}"}' ':arg1:(arg1)'
  comptest $'tst -\t\tla\t\C-hst\t\t\eb\eb\C-b\t\t'
 0:words array in rest arguments
@@ -140,14 +257,14 @@
 >line: {tst -e ,last }{ last arg1}
 >line: {tst -e ,last ,last,,last }{ last arg1}
 
- tst_arguments -s '-d+:msg1:' '*::msg2:{compadd $CURRENT}' 
+ tst_arguments -s '-d+:msg1:' '*::msg2:{compadd $CURRENT}'
  comptest $'tst add \t\t\t'
 0:opt_args
 >line: {tst add 2 }{}
 >line: {tst add 2 3 }{}
 >line: {tst add 2 3 4 }{}
 
- tst_arguments -s '-a' '-b' '-c' ':words:compadd - abyyy abzzz' 
+ tst_arguments -s '-a' '-b' '-c' ':words:compadd - abyyy abzzz'
  comptest $'tst ab\t'
 0:options and words (zsh-workers:12257)
 >line: {tst ab}{}
@@ -155,6 +272,12 @@
 >NO:{abyyy}
 >NO:{abzzz}
 
+ tst_arguments -M 'm:{j}={y}' -y -n ':yes/no:(y n)'
+ comptest $'tst j\t\eb-\C-e\t'
+0:matcher applies to options but not rest arguments
+>line: {tst j}{}
+>line: {tst -y }{}
+
  tst_arguments -x :word
  comptest $'tst -- -\t'
 0:option after --
@@ -175,15 +298,15 @@
 0:option after a word
 >line: {tst word -x }{}
 
- tst_arguments -A '-*' -x :word
+ tst_arguments -A'-*' -x :word
  comptest $'tst word -\t'
-0:option after word that doesn't match -A pattern
+0:option after word that doesn't match -A pattern, no space before pattern
 >line: {tst word -}{}
 >MESSAGE:{no more arguments}
 
  tst_arguments -A '-*' -x ':word:(-word)'
  comptest $'tst  word\eB\C-b-\t'
-0:option before a word that doesn't match -A pattern
+0:option before a word that doesn't match -A pattern, separate -A from pattern
 >line: {tst -}{ word}
 >DESCRIPTION:{word}
 >NO:{-word}
@@ -195,12 +318,68 @@
 0:continue completion after rest argument that looks like an option
 >line: {tst -a -x more }{}
 
+ tst_arguments '*-v'
+ comptest $'tst -v -\t'
+0:repeatable options
+>line: {tst -v -v }{}
+
+# necessary to exclude the rest arguments for the other set because
+# it is currently any unknown option rather than options from another
+# set that causes a set to be excluded
  tst_arguments -A '-*' - help -h -V - other -a '*: :(-x more)'
  comptest $'tst -a -x m\t'
 0:continue completion after rest argument that looks like an option (with sets)
->line: {tst -a -x more }{}
+>line: {tst -a -x m}{}
+#>line: {tst -a -x more }{}
+
+  tst_arguments - '(help)' -h -V - other -a '*:rest:(1 2 3)'
+  comptest $'tst -h \t'
+0:unknown option disables whole set (without -A)
+>line: {tst -h }{}
+>MESSAGE:{no arguments}
+
+  tst_arguments -A "-*" - '(help)' -h -V - other -a '*:rest:(1 2 3)'
+  comptest $'tst -h \t'
+0:unknown option disables whole set (with -A)
+>line: {tst -h }{}
+>MESSAGE:{no arguments}
 
- tst_arguments '(-v)-a' '(set1--m -a)-b' - '(set1)' -m -n - set2 -v -w
+ tst_arguments '(-C)-a' - set1 -C -v - set2 '(-a)-C' -w
+ comptest $'tst -a -\t' $'\C-w\C-w-C -\t'
+0:exclude option common to two sets and from one common option
+>line: {tst -a -}{}
+>DESCRIPTION:{option}
+>NO:{-v}
+>NO:{-w}
+>line: {tst -C -}{}
+>DESCRIPTION:{option}
+>NO:{-a}
+>NO:{-v}
+>NO:{-w}
+
+# _arguments doesn't know what rest arguments might be so any non-option
+# might apply: for the one set, it accepts "a"
+ tst_arguments -e - one -o '*:number:(1 2)' - two '(-e)*:letter:(a b)'
+ comptest $'tst \t' $'a -\t'
+0:rest argument rule in two sets
+>line: {tst }{}
+>DESCRIPTION:{letter}
+>NO:{a}
+>NO:{b}
+>DESCRIPTION:{number}
+>NO:{1}
+>NO:{2}
+>line: {tst  a -}{}
+>DESCRIPTION:{option}
+>NO:{-e}
+>NO:{-o}
+
+ tst_arguments '(set1-: set2-* set3-1)-a' - set1 '1: :(1)' '2: :(2)' - set2 '*:rest:(rest)' - set3 '1:num:(num)' '*: :(allowable)' - set4 ': :(allowed)'
+ comptest $'tst -a \t'
+0:exclude various forms of rest argument in set specific form
+>line: {tst -a allow}{}
+
+ tst_arguments '(-v)-a' '(set1--m -a)-b' - '(set1)' '( -a )-m' '( )-n' - set2 '(-w)*-v' -w
  comptest $'tst -a -\t' $'\C-w\C-w-'{b,m,v}$' -\t'
 0:exclusion lists
 >line: {tst -a -}{}
@@ -214,19 +393,36 @@
 >NO:{-n}
 >NO:{-v}
 >NO:{-w}
->line: {tst -m -}{}
+>line: {tst -m -b }{}
+>line: {tst -v -}{}
 >DESCRIPTION:{option}
 >NO:{-a}
 >NO:{-b}
 >NO:{-v}
->NO:{-w}
->line: {tst -v -}{}
+
+ tst_arguments -a - set1 -d - set2 '(set2)-m' -n -o ':arg:(x)' - set2 -x
+ comptest $'tst -m \t'
+0:exclude own set from an option
+>line: {tst -m -a }{}
+
+# the following two tests only verify the current questionable behaviour
+ tst_arguments - set1 '(set2)-a' -m -n - set2 -a -t -u
+ comptest $'tst -a -\t'
+0:exclude later set from an option common to both
+>line: {tst -a -}{}
 >DESCRIPTION:{option}
->NO:{-a}
->NO:{-b}
 >NO:{-m}
 >NO:{-n}
->NO:{-w}
+
+ tst_arguments - set2 -a -t -u - set1 '(set2)-a' -m -n
+ comptest $'tst -a -\t'
+0:exclude later set from an option common to both
+>line: {tst -a -}{}
+>DESCRIPTION:{option}
+>NO:{-m}
+>NO:{-n}
+>NO:{-t}
+>NO:{-u}
 
  tst_arguments '(-)-h' -a -b -c
  comptest $'tst -h -\t'
@@ -243,6 +439,33 @@
 >NO:{-a}
 >NO:{-b}
 
+# ideally, would handle exclusion within the current word
+ tst_arguments -s : '(-d)-a' -b -c -d
+ comptest $'tst -ab\t -\t\eb\eb \C-b-\t'
+0:exclusion with clumped options, in, after and before
+>line: {tst -ab}{}
+>DESCRIPTION:{option}
+>NO:{-c}
+>NO:{-d}
+>line: {tst -ab -c }{}
+>line: {tst -}{ -ab -c}
+>DESCRIPTION:{option}
+>NO:{-a}
+>NO:{-b}
+>NO:{-c}
+>NO:{-d}
+
+ tst_arguments '-a:arg' -b '(-b)-c'
+ comptest $'tst -a -c -\t'
+0:exclusion with option argument that looks like an option
+>line: {tst -a -c -}{}
+>MESSAGE:{no arguments}
+# seems we don't handle this case, ideal result would be as follows
+#>line: {tst -a -c -}{}
+#>DESCRIPTION:{option}
+#>NO:{-b}
+#>NO:{-c}
+
  tst_arguments --abc --aah :arg:
  comptesteval 'setopt bashautolist automenu'
  comptest $'tst --a\t\t\t'