PATCH: Re: _arguments problems

PATCH: Re: _arguments problems

[Sven Wischnowsky]

Tanaka Akira wrote:

> I re-read the item of _arguments in zshcompsys(1) and found some
> problems.  (and wrote BNF for a personal reference.)
> 
> Z(4):akr@serein% Src/zsh -f
> serein% bindkey -e; autoload -U compinit; compinit -D; compdef _tst tst
> 
> (1)
> serein% _tst () { _arguments '-o:*a:a:(a)' ':A:(A)' ':B:(B)' }
> serein% tst A -o a <TAB>
> ->
> serein% tst A -o a 
> _tags:comptags:83: no tags registered
> serein% tst A -o a
> _tags:comptags:83: no tags registered
> serein% tst A -o a

I don't get an error message, but I don't get the `B' inserted,
either. The code that calculates which arguments are to be completed
didn't take the `*pat' things into account.

> (2)
> serein% _tst () { _arguments '-e:*last:::b:{compadd "${(j:,:)words}"}' }
> serein% tst -e <TAB> last xxx
> serein% tst -e ,last,xxx  last xxx
> 
> $words shouldn't have `xxx'.

Oh, I never thought of this, it seems. You're right.

> (3)
> The explanation of the example `(-foo):...' is wrong.
> 
>   ... in the  second
>   example the argument described by the specification will
>   not be offered if the option -foo is on  the  line. 
> 
> I think it's reverted.

Ugh. Indeed.

> (4)
> serein% _tst () { _arguments '(*)-x' ':a:(a)' }
> serein% tst -x <TAB>
> ->
> serein% tst -x a 
> 
> Hm.  What's excluded by `*'?

Only the rest-arguments (`*:...'), for all arguments use a colon. I've 
made the docs mention the `*:...' explicitly.

> (5)
> serein% _tst () { _arguments '*-z:o:(o)' ':a:{compadd $opt_args[-z]}' }
> serein% tst -z a -z b <TAB>
> ->
> serein% tst -z a -z b b                
> 
> It should be `a:b'

Yup.

> (6)
> serein% _tst () { _arguments '*-o:1:(1):*:rest:{compadd $curcontext}' }
> serein% tst -o 1 <TAB><TAB>
> serein% tst -o 1 :complete:tst:option-o-2 :complete:tst:option-o-2 
> 
> This is the behaviour which is described in zshcompsys(1).  But I
> think it should be :complete:tst:option-o-rest

Hm, yes, looks better.

> (7)
> serein% _tst () { _arguments '*-o:1:(1):*:rest:{compadd $context}' }
> serein% tst -o 1 <TAB>
> 
> It completes nothing.  context parameter is not set?

Yes. Which part of the docs made you think it would be set? $context
is only mentioned for the `->state' actions and only then will it be
set.


Bye
 Sven

Index: Doc/Zsh/compsys.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/compsys.yo,v
retrieving revision 1.40
diff -u -r1.40 compsys.yo
--- Doc/Zsh/compsys.yo	2000/05/09 11:04:45	1.40
+++ Doc/Zsh/compsys.yo	2000/05/10 09:19:30
@@ -2991,10 +2991,11 @@
 example, the options `tt(-two)' and `tt(-three)' and the first
 argument will not be offered as possible completions if the option
 `tt(-one)' is on the line before the cursor and in the second example
-the argument described by the specification will not be offered if the
-option tt(-foo) is on the line. Also, the list may contain a single
+the option `tt(-foo)' will not be offered if the argument described by
+the specification is on the line. Also, the list may contain a single
 star as one of its elements to specify that the description for the
-rest arguments should not be used, a colon to
+rest arguments (i.e. a specification of the form `tt(*:...)') should
+not be used, a colon to
 specify that the descriptions for all normal (non-option-) arguments
 should not be used and a hyphen to specify that the descriptions for
 all options should not be used.
Index: Src/Zle/computil.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/computil.c,v
retrieving revision 1.16
diff -u -r1.16 computil.c
--- Src/Zle/computil.c	2000/05/10 08:20:41	1.16
+++ Src/Zle/computil.c	2000/05/10 09:19:31
@@ -1105,7 +1105,7 @@
     int nopts;
     Caarg def, ddef;
     Caopt curopt;
-    int opt, arg, argbeg, optbeg, nargbeg, restbeg, curpos;
+    int opt, arg, argbeg, optbeg, nargbeg, restbeg, curpos, argend;
     int inopt, inrest, inarg, nth, doff, singles, oopt, actopts;
     LinkList args;
     LinkList *oargs;
@@ -1123,7 +1123,7 @@
     Caopt ptr, wasopt;
     struct castate state;
     char *line, *pe, **argxor = NULL;
-    int cur, doff;
+    int cur, doff, argend;
     Patprog endpat = NULL;
 
     /* Free old state. */
@@ -1157,6 +1157,7 @@
     state.curopt = NULL;
     state.argbeg = state.optbeg = state.nargbeg = state.restbeg = state.actopts =
 	state.nth = state.inopt = state.inarg = state.opt = state.arg = 1;
+    state.argend = argend = arrlen(compwords) - 1;
     state.inrest = state.doff = state.singles = state.doff = state.oopt = 0;
     state.curpos = compcurrent;
     state.args = znewlinklist();
@@ -1197,15 +1198,19 @@
 
 	    if (state.def->type == CAA_REST || state.def->type == CAA_RARGS ||
 		state.def->type == CAA_RREST) {
+		if (state.curopt)
+		    state.oopt++;
 		if (state.def->end && pattry(endpat, line)) {
 		    state.def = NULL;
 		    state.curopt = NULL;
 		    state.opt = state.arg = 1;
+		    state.argend = ca_laststate.argend = cur - 1;
 		    continue;
 		}
-	    } else if ((state.def = state.def->next))
+	    } else if ((state.def = state.def->next)) {
 		state.argbeg = cur;
-	    else {
+		state.argend = argend;
+	    } else {
 		state.curopt = NULL;
 		state.opt = 1;
 	    }
@@ -1233,10 +1238,12 @@
 				state.curopt->args : NULL);
 	    doff = pe - line;
 	    state.optbeg = state.argbeg = state.inopt = cur;
+	    state.argend = argend;
 	    state.singles = (d->single && (!pe || !*pe) &&
 			     state.curopt->name[1] && !state.curopt->name[2]);
 
-	    state.oargs[state.curopt->num] = znewlinklist();
+	    if (!state.oargs[state.curopt->num])
+		state.oargs[state.curopt->num] = znewlinklist();
 
 	    if (ca_inactive(d, state.curopt->xor, cur, 0))
 		return 1;
@@ -1273,11 +1280,13 @@
 	    ddef = state.def = state.curopt->args;
 	    doff = pe - line;
 	    state.optbeg = state.argbeg = state.inopt = cur;
+	    state.argend = argend;
 	    state.singles = (!pe || !*pe);
 
 	    for (p = line + 1; p < pe; p++) {
 		if ((tmpopt = d->single[STOUC(*p)])) {
-		    state.oargs[tmpopt->num] = znewlinklist();
+		    if (!state.oargs[tmpopt->num])
+			state.oargs[tmpopt->num] = znewlinklist();
 
 		    if (ca_inactive(d, tmpopt->xor, cur, 0))
 			return 1;
@@ -1310,6 +1319,7 @@
 	    if (state.inopt) {
 		state.inopt = 0;
 		state.nargbeg = cur - 1;
+		state.argend = argend;
 	    }
 	    if (!d->args && !d->rest)
 		return 1;
@@ -1320,6 +1330,7 @@
 		state.opt = (cur == state.nargbeg + 1);
 		state.optbeg = state.nargbeg;
 		state.argbeg = cur - 1;
+		state.argend = argend;
 
 		for (; line; line = compwords[cur++])
 		    zaddlinknode(state.args, ztrdup(line));
@@ -1389,7 +1400,9 @@
 		ca_laststate.ddef = NULL;
 		ca_laststate.optbeg = state.nargbeg;
 		ca_laststate.argbeg = state.restbeg;
+		ca_laststate.argend = state.argend;
 		ca_laststate.singles = state.singles;
+		ca_laststate.oopt = state.oopt;
 		if (wasopt)
 		    wasopt->active = 1;
 	    }
@@ -1466,14 +1479,14 @@
 
 	if (!restr) {
 	    if ((restr = (arg->type == CAA_RARGS)))
-		restrict_range(ca_laststate.optbeg, arrlen(compwords) - 1);
+		restrict_range(ca_laststate.optbeg, ca_laststate.argend);
 	    else if ((restr = (arg->type == CAA_RREST)))
-		restrict_range(ca_laststate.argbeg, arrlen(compwords) - 1);
+		restrict_range(ca_laststate.argbeg, ca_laststate.argend);
 	}
 	if (arg->opt) {
 	    buf = (char *) zhalloc((arg->set ? strlen(arg->set) : 0) +
 				   strlen(arg->opt) + 40);
-	    if (arg->num > 0)
+	    if (arg->num > 0 && arg->type < CAA_REST)
 		sprintf(buf, "%soption%s-%d",
 			(arg->set ? arg->set : ""), arg->opt, arg->num);
 	    else

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: Re: _arguments problems

[Tanaka Akira]

In article <200005100920.LAA13270@beta.informatik.hu-berlin.de>,
  Sven Wischnowsky <wischnow@informatik.hu-berlin.de> writes:

> I don't get an error message, but I don't get the `B' inserted,
> either. The code that calculates which arguments are to be completed
> didn't take the `*pat' things into account.

Z(4):akr@serein% Src/zsh -f  
serein% bindkey -e; autoload -U compinit; compinit -D; compdef _tst tst
serein% _tst () { _arguments '-o:*a:a:(a)' ':A:(A)' ':B:(B)' }
serein% tst -o a <TAB>
A   B

It shouln't complete `B'.

> > It completes nothing.  context parameter is not set?
> 
> Yes. Which part of the docs made you think it would be set? $context
> is only mentioned for the `->state' actions and only then will it be
> set.

I didn't think following paragraph is applied only when `->state' is
used.  Maybe, this is my fault.

       The parameter `context' will be set to  the  automatically
       created  context  names.  These  are either strings of the
       form `option-opt-n' for the n'th argument  of  the  option
       -opt,  or  strings  of  the form `argument-n' for the n'th
       argument (for rest arguments the n is the string  `rest').
       For  example,  when  completing  the  argument  of  the -o
       option, the name is `option-o-1' and for the second normal
       (non-option-) argument it is `argument-2'.
-- 
Tanaka Akira

Re: PATCH: Re: _arguments problems

[Sven Wischnowsky]

Tanaka Akira wrote:

> In article <200005100920.LAA13270@beta.informatik.hu-berlin.de>,
>   Sven Wischnowsky <wischnow@informatik.hu-berlin.de> writes:
> 
> > I don't get an error message, but I don't get the `B' inserted,
> > either. The code that calculates which arguments are to be completed
> > didn't take the `*pat' things into account.
> 
> Z(4):akr@serein% Src/zsh -f  
> serein% bindkey -e; autoload -U compinit; compinit -D; compdef _tst tst
> serein% _tst () { _arguments '-o:*a:a:(a)' ':A:(A)' ':B:(B)' }
> serein% tst -o a <TAB>
> A   B
> 
> It shouln't complete `B'.

Ouch. Yes.

> > > It completes nothing.  context parameter is not set?
> > 
> > Yes. Which part of the docs made you think it would be set? $context
> > is only mentioned for the `->state' actions and only then will it be
> > set.
> 
> I didn't think following paragraph is applied only when `->state' is
> used.  Maybe, this is my fault.
> 
>        The parameter `context' will be set to  the  automatically
>        created  context  names.  These  are either strings of the
>        form `option-opt-n' for the n'th argument  of  the  option
>        -opt,  or  strings  of  the form `argument-n' for the n'th
>        argument (for rest arguments the n is the string  `rest').
>        For  example,  when  completing  the  argument  of  the -o
>        option, the name is `option-o-1' and for the second normal
>        (non-option-) argument it is `argument-2'.

Hmhm. $context is only mentioned for the `->state' actions above this, 
but we should probably make this clearer.

Bye
 Sven

Index: Doc/Zsh/compsys.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/compsys.yo,v
retrieving revision 1.41
diff -u -r1.41 compsys.yo
--- Doc/Zsh/compsys.yo	2000/05/10 09:22:49	1.41
+++ Doc/Zsh/compsys.yo	2000/05/10 11:51:09
@@ -3083,8 +3083,10 @@
 given as one string, separated by colons. All colons in the original
 arguments are preceded with backslashes.
 
-The parameter `tt(context)' will be set to the automatically created
-context names. These are either strings of the form
+The parameter `tt(context)' (only set in the calling function when
+using an action of the form `tt(->)var(string)', not during the
+evaluation of other var(action)s) will be set to the automatically
+created context names. These are either strings of the form
 `tt(option)var(-opt)tt(-)var(n)' for the var(n)'th argument of the
 option var(-opt), or strings of the form `tt(argument-)var(n)' for
 the var(n)'th argument (for rest arguments the var(n) is the string
Index: Src/Zle/computil.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/computil.c,v
retrieving revision 1.17
diff -u -r1.17 computil.c
--- Src/Zle/computil.c	2000/05/10 09:22:49	1.17
+++ Src/Zle/computil.c	2000/05/10 11:51:10
@@ -1198,14 +1198,12 @@
 
 	    if (state.def->type == CAA_REST || state.def->type == CAA_RARGS ||
 		state.def->type == CAA_RREST) {
-		if (state.curopt)
-		    state.oopt++;
 		if (state.def->end && pattry(endpat, line)) {
 		    state.def = NULL;
 		    state.curopt = NULL;
 		    state.opt = state.arg = 1;
 		    state.argend = ca_laststate.argend = cur - 1;
-		    continue;
+		    goto cont;
 		}
 	    } else if ((state.def = state.def->next)) {
 		state.argbeg = cur;
@@ -1378,6 +1376,8 @@
 	    endpat = patcompile(state.def->end, 0, NULL);
 
 	/* Copy the state into the global one. */
+
+    cont:
 
 	if (cur + 1 == compcurrent) {
 	    memcpy(&ca_laststate, &state, sizeof(state));

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de