Re: PATCH: use argument sets style in _compdef and _zpty; Re: FD_CLOEXEC

[Sven Wischnowsky <wischnow@informatik.hu-berlin.de>]

[ We had a serious power failure on Friday, so I couldn't send the
  patch I had prepared... I can only hope I can remember all the
  things I wanted to say. ]

Oliver Kiddle wrote:

> ...
> 
> Secondly, I have updated the compset and zpty completions to use the
> argument sets thing. This showed up a couple of possible bugs:
> 
> zpty -e<tab> doesn't exclude all the other options (apart from -b)
> zpty -e -<tab> does work though
> 
> and
> 
> % zpty -w l <tab>
> _arguments:shift:297: shift count must be <= $#

I've seen Bart's patch for this, and the patch below partly removes
it again, the problem was in the C-code: it stuffed a NULL pointer in
an array.

The patch also fixes the first problem, but...

Multiple sets should really only be used when the command syntax is
too complicated, because they can be slower, for example, the C-code
has to parse the command line once for each set (I've added a comment
for this in the docs). Also, when using sets, we should try to make
them really readable (that may be one reason to use sets), I've added
a (uncommented) version for this in _zpty.

Then there were some problems with the completion itself: the pty
command name when starting a command wasn't completed. Also, it didn't 
use `_command_names -e' for the first command. And because of the pty
command name (first argument), we can't use _normal, so I used
_precommand (nice trick, I hope I can remember it).

Ok, that's it, I think.

Bye
 Sven

Index: Completion/Base/_arguments
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Base/_arguments,v
retrieving revision 1.23
diff -u -r1.23 _arguments
--- Completion/Base/_arguments	2000/05/28 21:10:13	1.23
+++ Completion/Base/_arguments	2000/05/29 07:53:20
@@ -293,9 +293,9 @@
             fi
           fi
         fi
-        shift "${#descrs} ? 1 : 0" descrs
-        shift "${#actions} ? 1 : 0" actions
-        shift "${#subcs} ? 1 : 0" subcs
+        shift descrs
+        shift actions
+        shift subcs
       done
 
       if [[ -z "$matched$hasopts" ]] && _requested options &&
Index: Completion/Builtins/_zpty
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Builtins/_zpty,v
retrieving revision 1.6
diff -u -r1.6 _zpty
--- Completion/Builtins/_zpty	2000/05/25 15:09:04	1.6
+++ Completion/Builtins/_zpty	2000/05/29 07:53:20
@@ -3,19 +3,42 @@
 local state line list names expl curcontext="$curcontext"
 typeset -A opt_args
 
+
 _arguments -C -s \
-  '(-r)*::args:_normal' \
- - eb \
-  '-e[echo input characters]' \
-  '-b[io to pseudo-terminal blocking]' \
- - d \
-  '-d[delete command]:*:name:->name' \
- - w \
-  '-w[send string to command]:name:->name:*:strings to write' \
- - L \
-  '-L[list defined commands as calls]' \
- - r \
-  '(*)-r[read string from command]:name:->name:param:_parameters' && return 0
+  '(-r -w -L -d)-e[echo input characters]' \
+  '(-r -w -L -d)-b[io to pseudo-terminal blocking]' \
+  '(-r -w -L -e -b)-d[delete command]:*:name:->name' \
+  '(-r -L -e -b -d)-w[send string to command]:name:->name:*:strings to write' \
+  '(: -r -w -e -b -d)-L[list defined commands as calls]' \
+  '(: -w -L -e -b -d)-r[read string from command]:name:->name:param: _parameters:pattern:' \
+  '(-r -w -L -d):zpty command name:' \
+  '(-r -w -L -d):cmd: _command_names -e' \
+  '(-r -w -L -d)*::args:_precommand' && return 0
+
+# One could use sets, but that's more expensive and zpty is simple enough.
+#
+# _arguments -C -s \
+#   - read \
+#     '-r[read string from command]' \
+#     ':name:->name' \
+#     ':param: _parameters' \
+#     ':pattern:' \
+#   - write \
+#     '-w[send string to command]' \
+#     ':name:->name' \
+#     '*:strings to write' \
+#   - list \
+#     '-L[list defined commands as calls]' \
+#   - delete \
+#     '-d[delete command]' \
+#     '*:name:->name' \
+#   - start \
+#     '-e[echo input characters]' \
+#     '-b[io to pseudo-terminal blocking]' \
+#     ':zpty command name:' \
+#     ':cmd: _command_names -e' \
+#     '*::args:_precommand' && return 0
+
 
 if [[ $state = name ]]; then
   if ! zmodload -e zsh/zpty; then
@@ -26,8 +49,10 @@
   names=( ${list%%:*} )
   if zstyle -T ":completion:${curcontext}" verbose; then
     zformat -a list ' --' ${${(f)"$(zpty)"}#*\) }
-    _wanted names expl 'zpty command names' compadd -d list - "$names[@]"
+    _wanted names expl 'zpty command name' compadd -d list - "$names[@]"
   else
-    _wanted names expl 'zpty command names' compadd - "$names[@]"
+    _wanted names expl 'zpty command name' compadd - "$names[@]"
   fi
+else
+  return 1
 fi
Index: Doc/Zsh/compsys.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/compsys.yo,v
retrieving revision 1.56
diff -u -r1.56 compsys.yo
--- Doc/Zsh/compsys.yo	2000/05/24 09:14:35	1.56
+++ Doc/Zsh/compsys.yo	2000/05/29 07:53:21
@@ -2867,6 +2867,13 @@
 line. With tt(-S), no option will be completed after a `tt(-)tt(-)' on 
 the line and this argument will otherwise be ignored.
 
+Note that using multiple sets will be slower than using only one set
+because the completion code has to parse the command line once for
+every set. So more than one set should only be used if the command
+syntax is too complicated. Note also that a option specification with
+rest-arguments (as in `tt(-foo:*:...)' often allows to avoid the use
+of multiple sets.
+
 Another option supported is `tt(-O) var(name)'. The var(name) will be
 taken as the name of an array and its elements will be given to
 functions called to generate matches when executing the
Index: Src/Zle/computil.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/computil.c,v
retrieving revision 1.23
diff -u -r1.23 computil.c
--- Src/Zle/computil.c	2000/05/24 08:32:13	1.23
+++ Src/Zle/computil.c	2000/05/29 07:53:22
@@ -510,7 +510,8 @@
 		*p = ':';
 	} else
 	    ret->action = ztrdup(rembslashcolon(p + 1));
-    }
+    } else
+	ret->action = ztrdup("");
     *def = p;
 
     return ret;
@@ -588,7 +589,7 @@
     /* Now get the -s, -A, -S and -M options. */
 
     args++;
-    while ((p = *args) && *p == '-') {
+    while ((p = *args) && *p == '-' && p[1]) {
 	for (q = ++p; *q; q++)
 	    if (*q == 'M') {
 		q = "";
@@ -1226,7 +1227,7 @@
     Caopt ptr, wasopt, dopt;
     struct castate state;
     char *line, *pe, **argxor = NULL;
-    int cur, doff, argend;
+    int cur, doff, argend, arglast;
     Patprog endpat = NULL;
 
     /* Free old state. */
@@ -1284,7 +1285,7 @@
 	 line; line = compwords[cur++]) {
 	ddef = adef = NULL;
 	dopt = NULL;
-	doff = state.singles = 0;
+	doff = state.singles = arglast = 0;
 
 	if (ca_inactive(d, argxor, cur, 0) ||
 	    ((d->flags & CDF_SEP) && !strcmp(line, "--"))) {
@@ -1423,6 +1424,7 @@
 	    if ((d->flags & CDF_ARG) && ca_inactive(d, NULL, cur + 1, 1))
 		return 1;
 
+	    arglast = 1;
 	    if (state.inopt) {
 		state.inopt = 0;
 		state.nargbeg = cur - 1;
@@ -1434,7 +1436,11 @@
 		(state.def->type == CAA_RREST ||
 		 state.def->type == CAA_RARGS)) {
 		state.inrest = 0;
-		state.opt = (cur == state.nargbeg + 1);
+		state.opt = (cur == state.nargbeg + 1 &&
+			     (!*line || 
+			      ((*line == '-' || *line == '+') &&
+			       (!line[1] ||
+				(*line == '-' && line[1] == '-' && !line[2])))));
 		state.optbeg = state.nargbeg;
 		state.argbeg = cur - 1;
 		state.argend = argend;
@@ -1510,6 +1516,10 @@
 		}
 	    } else {
 		ca_laststate.def = adef;
+		ca_laststate.opt = (!arglast || !*line || 
+				    ((*line == '-' || *line == '+') &&
+				     (!line[1] ||
+				      (*line == '-' && line[1] == '-' && !line[2]))));
 		ca_laststate.ddef = NULL;
 		ca_laststate.dopt = NULL;
 		ca_laststate.optbeg = state.nargbeg;
@@ -1801,7 +1811,7 @@
 	    for (; lstate; lstate = lstate->snext) {
 		if (lstate->actopts &&
 		    (lstate->opt || (lstate->doff && lstate->def) ||
-		     (lstate->def &&
+		     (lstate->def && lstate->def->opt &&
 		      (lstate->def->type == CAA_OPT ||
 		       (lstate->def->type >= CAA_RARGS &&
 			lstate->def->num < 0)))) &&

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