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

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

[Sven Wischnowsky]

[ 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

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

[Bart Schaefer]

On May 25,  4:03pm, Oliver Kiddle wrote:
} Subject: PATCH: use argument sets style in _compdef and _zpty; Re: FD_CLOE
}
} 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)

I can reproduce this, but

} % zpty -w l <tab>
} _arguments:shift:297: shift count must be <= $#

I can't reproduce that.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

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

[Oliver Kiddle]

Peter Stephenson wrote:

> I developed zftp under AIX, so it's just possible something funny is going
> on there.  Maybe Oliver could check it at least compiles?

Firstly, Peter: I have recompiled with your zftp patch on AIX without
any problems. Judging by the AIX man pages, your patch is correct and it
was wrong before. I did a few basic tests of zftp and it all worked
fine. If you can think of a specific test which will cover your changed
code, I can try that.

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'm not sure that the latter one isn't the syntax being wrong in the
_arguments call in _zpty. It occurs with the previous _zpty so is
not related to the argument sets stuff.

Oliver

Index: Completion/Builtins/_compdef
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Builtins/_compdef,v
retrieving revision 1.5
diff -u -c -r1.5 _compdef
*** Completion/Builtins/_compdef	2000/05/05 13:38:46	1.5
--- Completion/Builtins/_compdef	2000/05/25 14:56:00
***************
*** 6,18 ****
  _arguments -C -s \
    '(-d)-a[make function autoloadable]' \
    '(-d -p -P)-n[leave existing definitions intact]' \
-   '(-a -n -p -P -k -K)-d[delete]:*:completed command:->ccom' \
-   '(-n -d -P -k -K)-p[completion for command matching pattern]:completion function:->cfun:pattern' \
-   '(-n -d -p -k -K)-P[as -p for commands without own completion]:completion function:->cfun:pattern' \
-   '(-d -p -P -K)-k[define widget and key binding]:completion function:->cfun:style:->style:*:key' \
-   '(-d -p -P -k)-K[define multiple widgets based on function]:*::: :->multi' \
    ':completion function:->cfun' \
!   '*:commands: _command_names' && return 0
  
  if [[ $state = multi ]]; then
    case $(( CURRENT % 3 )) in
--- 6,23 ----
  _arguments -C -s \
    '(-d)-a[make function autoloadable]' \
    '(-d -p -P)-n[leave existing definitions intact]' \
    ':completion function:->cfun' \
!   '*:commands: _command_names' \
!  - d \
!   '(-a -n)-d[delete]:*:completed command:->ccom' \
!  - p \
!   '(-n)-p[completion for command matching pattern]:completion function:->cfun:pattern' \
!  - P \
!   '(-n)-P[as -p for commands without own completion]:completion function:->cfun:pattern' \
!  - k \
!   '-k[define widget and key binding]:completion function:->cfun:style:->style:*:key' \
!  - K \
!   '-K[define multiple widgets based on function]:*::: :->multi' && return 0
  
  if [[ $state = multi ]]; then
    case $(( CURRENT % 3 )) in
Index: Completion/Builtins/_zpty
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Builtins/_zpty,v
retrieving revision 1.5
diff -u -c -r1.5 _zpty
*** Completion/Builtins/_zpty	2000/05/05 13:38:46	1.5
--- Completion/Builtins/_zpty	2000/05/25 14:56:00
***************
*** 4,18 ****
  typeset -A opt_args
  
  _arguments -C -s \
!   '(-d -w -r -L)-e[echo input characters]' \
!   '(-d -w -r -L)-b[io to pseudo-terminal blocking]' \
!   '(-e -b -w -r -L)-d[delete command]:*:name:->name' \
!   '(-e -b -d -r -L)-w[send string to command]:name:->name:*:strings to write' \
!   '(-e -b -d -w -L *)-r[read string from command]:name:->name:param:_parameters' \
!   '(-e -b -d -w -r)-L[list defined commands as calls]' \
!   '(-r)*::args:_normal' && return 0
  
  if [[ $state = name ]]; then
    list=( ${${(f)"$(zpty)"}#*\) } )
    names=( ${list%%:*} )
    if zstyle -T ":completion:${curcontext}" verbose; then
--- 4,27 ----
  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
  
  if [[ $state = name ]]; then
+   if ! zmodload -e zsh/zpty; then
+     _message "zpty module not loaded"
+     return 1
+   fi
    list=( ${${(f)"$(zpty)"}#*\) } )
    names=( ${list%%:*} )
    if zstyle -T ":completion:${curcontext}" verbose; then