PATCH: _arguments (was: Re: odd diff completion)

PATCH: _arguments (was: Re: odd diff completion)

[Sven Wischnowsky]

Clint Adams wrote:

> Under 3.1.9:
> 
> diff pars<TAB>
> completes to diff parse.c
> 
> Under yesterday's CVS:
> 
> diff pars<TAB>
> completes to diff parsB

Ahem. Sorry, under certain conditons ca_get_sopt() returned an option
even for non-option strings.


In another message:

> > The `-A -*' doesn't mean that it complete ignores all unknown strings
> > starting with a hyphen, it just means that it doesn't stop completing
> > options if it finds undescribed `-foo's.
> 
> > So, you still have to give it the `--{install,...}' to make it ignore
> > it. Or you use the `= ' trick together with the `::' trick, as in:
> > 
> >            '(-i)--install:*::Debian packages:= ->install' \
> 
> I'm confused.  If I omit the '--{install,...}', doesn't that make
> --install an undescribed -foo, and thus it shouldn't stop completing
> options?

I've now had a look and am of the opinion, that the behaviour is the
right thing. The install case uses multiple sets. For multiple sets,
_arguments has to detect undescribed options and it has to stop trying 
to complete a set if there is a string not covered by the descriptions 
for that set on the line (otherwise it couldn't make the sets be
mutually exclusive).

We could make the -A pattern be used there, the patch below contains
the code for that, #if'ed out with a marker comment above it. However,
that would defeat the purpose of multiple sets, i.e. the sets are then 
not mutually exclusive if we use a simple pattern like -*. One would
then need a pattern that exactly matches all possible options -- not
very user-friendly.

So the solution for the current state of _arguments, which I don't
want to change is to include the --install etc. in the second call to
_arguments, that makes everything work as you want. BUT, I'd be
willing to give some more help for this. The only things I could think 
of are:

- an option, say -I, which gives an array of options which are to be
  ignored but not completed
- or, probably more more user-friendly, yet another optspec syntax
  describing options which are to be ignored but not completed; for
  example, if we find a syntax that starts with a special string for
  that, one could use <not>${^_dpkg_actions} in the second call to
  _arguments to make it skip all options described in $_dpkg_options

I think I prefer the latter (or both). Anyone got an idea for an
acceptable syntax? Or other suggestions?

Bye
 Sven

Index: Src/Zle/computil.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/computil.c,v
retrieving revision 1.29
diff -u -r1.29 computil.c
--- Src/Zle/computil.c	2000/06/13 12:14:32	1.29
+++ Src/Zle/computil.c	2000/06/15 07:20:53
@@ -1083,7 +1083,7 @@
     LinkList l = NULL;
 
     *lp = NULL;
-    for (p = NULL; *line; line++)
+    for (p = NULL; *line; line++) {
 	if ((p = d->single[STOUC(*line)]) && p->active &&
 	    p->args && p->name[0] == pre) {
 	    if (p->type == CAO_NEXT) {
@@ -1100,8 +1100,10 @@
 		}
 		break;
 	    }
-	} else if (!p || (!p->active && p->name[0] != pre))
+	} else if (p && !p->active)
 	    return NULL;
+	p = NULL;
+    }
     if (p && end)
 	*end = line;
     return p;
@@ -1444,7 +1446,14 @@
 		state.opt = 0;
 	    else
 		state.curopt = NULL;
-	} else if (multi && (*line == '-' || *line == '+') && cur != compcurrent)
+	} 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
+			 pattern be effectively ignored with multiple sets. */
+		   && (!napat || !pattry(napat, line))
+#endif
+		   )
 	    return 1;
 	else if (state.arg && (!napat || !pattry(napat, line))) {
 	    /* Otherwise it's a normal argument. */

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

PATCH: _arguments and _dpkg

[Clint Adams]

> - an option, say -I, which gives an array of options which are to be
>   ignored but not completed
> - or, probably more more user-friendly, yet another optspec syntax
>   describing options which are to be ignored but not completed; for
>   example, if we find a syntax that starts with a special string for
>   that, one could use <not>${^_dpkg_actions} in the second call to
>   _arguments to make it skip all options described in $_dpkg_options
> 
> I think I prefer the latter (or both). Anyone got an idea for an
> acceptable syntax? Or other suggestions?

I think I prefer the latter too.  In the meantime, I'm using the following
patch.

Unfortunately, with current CVS, this results in anything beginning
with -- followed by the first letter of any of the long options 
covered by the state engine to be not completed.

e.g. dpkg --p<TAB>, dpkg --pu<TAB>, dpkg --pur<TAB>, &c., will all
just insert a space.

Index: Completion/Debian/_dpkg
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Debian/_dpkg,v
retrieving revision 1.5
diff -u -r1.5 _dpkg
--- Completion/Debian/_dpkg	2000/06/13 17:45:56	1.5
+++ Completion/Debian/_dpkg	2000/06/16 15:09:48
@@ -1,7 +1,7 @@
 #compdef dpkg dpkg-deb
 
 local _dpkg_deb_actions _dpkg_common_actions _dpkg_actions _dpkg_options
-local _dpkg_options_recursive
+local _dpkg_options_recursive _dpkg_actions_install _dpkg_actions_record_avail
 local curcontext="$curcontext" context state line expl ret
 typeset -A opt_args
 
@@ -81,6 +81,8 @@
            --{force,refuse,no-force}'--[forcing options]:what:(auto-select downgrade configure-any hold bad-path not-root overwrite overwrite-diverted depends-version depends confnew confold confdef confmiss conflicts architecture overwrite-dir remove-reinstreq remove-essential)')
 
 _dpkg_options_recursive=('(--recursive)-R[recursive]' '(-R)--recursive')
+_dpkg_actions_install=('(--install)-i[install]' '(-i)--install')
+_dpkg_actions_record_avail=('(--record-avail)-A[record available]' '(-A)--record-avail')
 
 case "${words[1]:t}" in
 dpkg)
@@ -102,7 +104,8 @@
 case "$state" in
   install|record_avail)
         _funcall ret _dpkg_$state && return ret
-        _arguments -C -A "-*" -s "$_dpkg_options[@]" \
+        _arguments -C -s "$_dpkg_options[@]" \
+           "${(@e):-\$_dpkg_actions_${state}}" \
 	   - recur \
 	   '(--recursive)-R[recursive]' \
 	   '(-R)--recursive' \

Re: PATCH: _arguments and _dpkg

[Clint Adams]

> Clint, does _dpkg now do the right thing?

I'm incredibly sleep-deprived, but it seems to work like a dream.  No
pun intended.

Re: PATCH: _arguments and _dpkg

[Sven Wischnowsky]

Clint Adams wrote:

> > - an option, say -I, which gives an array of options which are to be
> >   ignored but not completed
> > - or, probably more more user-friendly, yet another optspec syntax
> >   describing options which are to be ignored but not completed; for
> >   example, if we find a syntax that starts with a special string for
> >   that, one could use <not>${^_dpkg_actions} in the second call to
> >   _arguments to make it skip all options described in $_dpkg_options
> > 
> > I think I prefer the latter (or both). Anyone got an idea for an
> > acceptable syntax? Or other suggestions?
> 
> I think I prefer the latter too.  In the meantime, I'm using the following
> patch.

The patch comments that out again and adds the syntax `!<spec>', only
really useful for option-specs. Those options will be understood and
skipped but not completed. I changed _dpkg to what I think is needed
there, knowing nothing about dpkg...

Clint, does _dpkg now do the right thing?

> Unfortunately, with current CVS, this results in anything beginning
> with -- followed by the first letter of any of the long options 
> covered by the state engine to be not completed.
> 
> e.g. dpkg --p<TAB>, dpkg --pu<TAB>, dpkg --pur<TAB>, &c., will all
> just insert a space.

Slightly wrong test. It thought it was completing a string with
multiple single-letter options because it didn't stumble over the
second `-'.


The patch also adds two `&& return 0' to _dpkg, something I wanted to
remind you of for quite some time now...

Bye
 Sven

Index: Completion/Debian/_dpkg
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Debian/_dpkg,v
retrieving revision 1.6
diff -u -r1.6 _dpkg
--- Completion/Debian/_dpkg	2000/06/16 15:22:26	1.6
+++ Completion/Debian/_dpkg	2000/06/19 13:06:14
@@ -81,21 +81,23 @@
            --{force,refuse,no-force}'--[forcing options]:what:(auto-select downgrade configure-any hold bad-path not-root overwrite overwrite-diverted depends-version depends confnew confold confdef confmiss conflicts architecture overwrite-dir remove-reinstreq remove-essential)')
 
 _dpkg_options_recursive=('(--recursive)-R[recursive]' '(-R)--recursive')
-_dpkg_actions_install=('(--install)-i[install]' '(-i)--install')
-_dpkg_actions_record_avail=('(--record-avail)-A[record available]' '(-A)--record-avail')
 
+# not needed anymore?
+#  _dpkg_actions_install=('(--install)-i[install]' '(-i)--install')
+#  _dpkg_actions_record_avail=('(--record-avail)-A[record available]' '(-A)--record-avail')
+
 case "${words[1]:t}" in
 dpkg)
 _arguments -C -s "$_dpkg_actions[@]" \
            "$_dpkg_deb_actions[@]" \
            "$_dpkg_common_actions[@]" \
            "$_dpkg_options[@]" \
-	   "$_dpkg_options_recursive[@]"
+	   "$_dpkg_options_recursive[@]" && return 0
 ;;
 
 dpkg-deb)
 _arguments "$_dpkg_deb_actions[@]" \
-           "$_dpkg_common_actions[@]"
+           "$_dpkg_common_actions[@]" && return 0
 
 ;;
 
@@ -104,14 +106,16 @@
 case "$state" in
   install|record_avail)
         _funcall ret _dpkg_$state && return ret
-        _arguments -C -s "$_dpkg_options[@]" \
-           "${(@e):-\$_dpkg_actions_${state}}" \
+# not needed anymore?
+#            "${(@e):-\$_dpkg_actions_${state}}" \
+        _arguments -C -A '-*' -s \
+             "$_dpkg_options[@]" \
+             \!${^_dpkg_actions%%:*} \
 	   - recur \
-	   '(--recursive)-R[recursive]' \
-	   '(-R)--recursive' \
-	   ':directory:_files -/' \
+             "$_dpkg_options_recursive[@]" \
+	     ':directory:_files -/' \
 	   - nonrecur \
-	   ':Debian package:_files -g \*.deb'
+	     ':Debian package:_files -g \*.deb'
   ;;
   remove|purge|status|listfiles)
         _funcall ret _dpkg_$state && return ret
Index: Doc/Zsh/compsys.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/compsys.yo,v
retrieving revision 1.70
diff -u -r1.70 compsys.yo
--- Doc/Zsh/compsys.yo	2000/06/19 09:55:31	1.70
+++ Doc/Zsh/compsys.yo	2000/06/19 13:06:16
@@ -2800,6 +2800,17 @@
 used and a hyphen (tt(-)) to specify that the descriptions for all options
 should not be used.  This paragraph desperately needs rewriting.
 
+To simplify writing writing functions that call tt(_arguments) more
+than once, the var(specs) may also start with the character `tt(!)'
+(exclamation mark) to make the spec em(not) be completed.  However, if
+this is used with one of the forms describing options, the option (and 
+its arguments, if it takes any) will be understood and skipped if they 
+appear on the command line.  It's just that the option itself will not
+be completed. This is intended to be used with an array containing the 
+options used in the first call to tt(arguments).  The second call can
+then use `tt(\!${^global_options})' to ignore those options and
+complete only the ones understood in the current context.
+
 In every case above, the var(action) determines how the possible
 completions should be generated.  In places where no sensible matches can
 be generated, the action should consist of only a space. This will make
Index: Src/Zle/computil.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/computil.c,v
retrieving revision 1.32
diff -u -r1.32 computil.c
--- Src/Zle/computil.c	2000/06/19 11:43:12	1.32
+++ Src/Zle/computil.c	2000/06/19 13:06:17
@@ -325,6 +325,7 @@
     int active;			/* still allowed on command line */
     int num;			/* it's the num'th option */
     char *set;			/* set name, shared */
+    int not;			/* don't complete this option (`!...') */
 };
 
 #define CAO_NEXT    1
@@ -573,7 +574,7 @@
     char *adpre, *adsuf, *axor = NULL, *doset = NULL, **setp = NULL;
     char *nonarg = NULL;
     int single = 0, anum = 1, xnum, nopts, ndopts, nodopts, flags = 0;
-    int state = 0;
+    int state = 0, not = 0;
 
     nopts = ndopts = nodopts = 0;
 
@@ -683,6 +684,8 @@
 	}
 	p = dupstring(*args);
 	xnum = 0;
+	if ((not = (*p == '!')))
+	    p++;
 	if (*p == '(') {
 	    /* There is a xor list, get it. */
 
@@ -910,6 +913,7 @@
 	    opt->type = otype;
 	    opt->args = oargs;
 	    opt->num = nopts++;
+	    opt->not = not;
 
 	    if (otype == CAO_DIRECT || otype == CAO_EQUAL)
 		ndopts++;
@@ -932,6 +936,9 @@
 
 	    int type = CAA_REST;
 
+	    if (not)
+		continue;
+
 	    if (*++p != ':') {
 		freecadef(all);
 		zwarnnam(nam, "invalid rest argument definition: %s", *args, 0);
@@ -957,6 +964,9 @@
 	    int type = CAA_NORMAL, direct;
 	    Caarg arg, tmp, pre;
 
+	    if (not)
+		continue;
+
 	    if ((direct = idigit(*p))) {
 		/* Argment number is given. */
 		int num = 0;
@@ -1100,7 +1110,7 @@
 		}
 		break;
 	    }
-	} else if (p && !p->active)
+	} else if (!p || (p && !p->active))
 	    return NULL;
 	p = NULL;
     }
@@ -1853,7 +1863,7 @@
 		      (compcurrent == 1)))) {
 		    ret = 0;
 		    for (p = lstate->d->opts; p; p = p->next) {
-			if (p->active) {
+			if (p->active && !p->not) {
 			    switch (p->type) {
 			    case CAO_NEXT:    l = next;    break;
 			    case CAO_DIRECT:  l = direct;  break;

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