Re: Problems with "comparguments" (Re: PATCH: broken _netscape)

Re: Problems with "comparguments" (Re: PATCH: broken _netscape)

[Sven Wischnowsky]

Bart Schaefer wrote:

> On Nov 22,  4:08pm, Oliver Kiddle wrote:
> } Subject: PATCH: broken _netscape
> }
> } _netscape in pws-9 seems to be broken. This patch fixes it to a point
> } where it atleast runs without errors but for some reason, completion of
> } the arguments doesn't work. I suspect that it is a bug somewhere in
> } _arguments.
> 
> I may be entirely wrong, but based on a scan through the "set -x" output
> when completing after "netscape -", I'd say this is the same bug that is
> causing "cvs" to complete --allow-root and the directories after it.  I
> suspect a parsing bug in "comparguments".

It was the same bug, but only in shell code: the test if the thing on
the line could be turned into an option name with an argument so that
we can immediately complete that was performed too early. In fact, the 
nesting of the loops was the wrong way round.

> While we're on the topic, I noticed this in the manual page for "rxvt" the
> other day:
> 
>        -pixmap: file[;geom]
> ...
> 
> Note that here we have an option whose name actually includes a colon.  As
> far as I can tell there's *no* way to represent this in the argument list
> for _arguments or comparguments; perhaps _regex_arguments could deal with
> it, but it doesn't appear so from a look at _apt.

The patch below also hacks computil.c to allow colons in option names
by preceding them with a backslash:

  local curcontext="$curcontext" line style ret=1
  typeset -A opt_args

  _arguments \
      ...
      '-pixmap\::rotten option name and argument style:->foo' && ret=0

  if [[ -n "$state" ]]; then
    if compset -P '*\;' ; then
      _message 'geometry'
    else
      _files -S '\;' && ret=0
    fi
  fi

  return ret

> The mini-language of _arg_compile could handle it:

I know. But I didn't feel like turning that into C. More precisely, I
didn't feel like re-writing all functions that use _arguments. And
again: I know, a function/builtin that turns a _argument-list into the 
mini-language-form... But still: when looking at functions like _mount 
this is rather hairy.

Bye
 Sven

diff -u -r oldsrc/Zle/computil.c Src/Zle/computil.c
--- oldsrc/Zle/computil.c	Tue Nov 23 10:03:08 1999
+++ Src/Zle/computil.c	Tue Nov 23 11:38:34 1999
@@ -50,12 +50,15 @@
 cdisp_calc(Cdisp disp, char **args)
 {
     char *cp;
-    int i;
+    int i, nbc;
 
     for (; *args; args++) {
-	if ((cp = strchr(*args, ':')) && cp[1]) {
+	for (nbc = 0, cp = *args; *cp && *cp != ':'; cp++)
+	    if (*cp == '\\' && cp[1])
+		cp++, nbc++;
+	if (*cp == ':' && cp[1]) {
 	    disp->colon++;
-	    if ((i = cp - *args) > disp->pre)
+	    if ((i = cp - *args - nbc) > disp->pre)
 		disp->pre = i;
 	    if ((i = strlen(cp + 1)) > disp->suf)
 		disp->suf = i;
@@ -70,7 +73,7 @@
 {
     int sl = strlen(sep), pre = disp->pre, suf;
     VARARR(char, buf, disp->pre + disp->suf + sl + 1);
-    char **ret, **rp, *cp;
+    char **ret, **rp, *cp, *copy, *cpp, oldc;
 
     ret = (char **) zalloc((arrlen(args) + 1) * sizeof(char *));
 
@@ -78,18 +81,21 @@
     suf = pre + sl;
 
     for (rp = ret; *args; args++) {
-	if ((cp = strchr(*args, ':')) && cp[1]) {
+	copy = dupstring(*args);
+	for (cp = cpp = copy; *cp && *cp != ':'; cp++) {
+	    if (*cp == '\\' && cp[1])
+		cp++;
+	    *cpp++ = *cp;
+	}
+	oldc = *cpp;
+	*cpp = '\0';
+	if (((cpp == cp && oldc == ':') || *cp == ':') && cp[1]) {
 	    memset(buf, ' ', pre);
-	    memcpy(buf, *args, (cp - *args));
+	    memcpy(buf, copy, (cpp - copy));
 	    strcpy(buf + suf, cp + 1);
 	    *rp++ = ztrdup(buf);
-	} else {
-	    if (cp)
-		*cp = '\0';
-	    *rp++ = ztrdup(*args);
-	    if (cp)
-		*cp = ':';
-	}
+	} else
+	    *rp++ = ztrdup(copy);
     }
     *rp = NULL;
 
@@ -153,6 +159,7 @@
 }
 
 /* Initialisation. Store and calculate the string and matches and so on. */
+
 static int
 cd_init(char *nam, char *sep, char **args, int disp)
 {
@@ -218,15 +225,21 @@
 
     if ((set = cd_state.sets)) {
 	char **sd, **sdp, **md, **mdp, **ss, **ssp, **ms, **msp;
-	char **p, **mp, *cp;
+	char **p, **mp, *cp, *copy, *cpp, oldc;
 	int dl = 1, sl = 1, sepl = strlen(cd_state.sep);
 	int pre = cd_state.disp.pre, suf = cd_state.disp.suf;
 	VARARR(char, buf, pre + suf + sepl + 1);
 
 	for (p = set->strs; *p; p++)
-	    if (cd_state.showd && (cp = strchr(*p, ':')) && cp[1])
-		dl++;
-	    else
+	    if (cd_state.showd) {
+		for (cp = *p; *cp && *cp != ':'; cp++)
+		    if (*cp == '\\' && cp[1])
+			cp++;
+		if (*cp == ':' && cp[1])
+		    dl++;
+		else
+		    sl++;
+	    } else
 		sl++;
 
 	sd = (char **) zalloc(dl * sizeof(char *));
@@ -243,32 +256,34 @@
 
 	for (sdp = sd, ssp = ss, mdp = md, msp = ms,
 		 p = set->strs, mp = set->matches; *p; p++) {
-	    if ((cp = strchr(*p, ':')) && cp[1] && cd_state.showd) {
+	    copy = dupstring(*p);
+	    for (cp = cpp = copy; *cp && *cp != ':'; cp++) {
+		if (*cp == '\\' && cp[1])
+		    cp++;
+		*cpp++ = *cp;
+	    }
+	    oldc = *cpp;
+	    *cpp = '\0';
+	    if (((cpp == cp && oldc == ':') || *cp == ':') && cp[1] &&
+		cd_state.showd) {
 		memset(buf, ' ', pre);
-		memcpy(buf, *p, (cp - *p));
+		memcpy(buf, copy, (cpp - copy));
 		strcpy(buf + suf, cp + 1);
 		*sdp++ = ztrdup(buf);
 		if (mp) {
 		    *mdp++ = ztrdup(*mp);
 		    if (*mp)
 			mp++;
-		} else {
-		    *cp = '\0';
-		    *mdp++ = ztrdup(*p);
-		    *cp = ':';
-		}
+		} else
+		    *mdp++ = ztrdup(copy);
 	    } else {
-		if (cp)
-		    *cp = '\0';
-		*ssp++ = ztrdup(*p);
+		*ssp++ = ztrdup(copy);
 		if (mp) {
 		    *msp++ = ztrdup(*mp);
 		    if (*mp)
 			mp++;
 		} else
-		    *msp++ = ztrdup(*p);
-		if (cp)
-		    *cp = ':';
+		    *msp++ = ztrdup(copy);
 	    }
 	}
 	*sdp = *ssp = *mdp = *msp = NULL;
@@ -473,6 +488,25 @@
     return r;
 }
 
+/* Add backslashes before colons. */
+
+static char *
+bslashcolon(char *s)
+{
+    char *p, *r;
+
+    r = p = zhalloc((2 * strlen(s)) + 1);
+
+    while (*s) {
+	if (*s == ':')
+	    *p++ = '\\';
+	*p++ = *s++;
+    }
+    *p = '\0';
+
+    return r;
+}
+
 /* Parse an argument definition. */
 
 static Caarg
@@ -766,7 +800,7 @@
 		optp = &((*optp)->next);
 
 		opt->next = NULL;
-		opt->name = ztrdup(name);
+		opt->name = ztrdup(rembslashcolon(name));
 		if (descr)
 		    opt->descr = ztrdup(descr);
 		else if (adpre && oargs && !oargs->next &&
@@ -1414,14 +1448,15 @@
 		    default:          l = equal;   break;
 		    }
 		    if (p->descr) {
-			int len = strlen(p->name) + strlen(p->descr) + 2;
+			char *n = bslashcolon(p->name);
+			int len = strlen(n) + strlen(p->descr) + 2;
 
 			str = (char *) zhalloc(len);
-			strcpy(str, p->name);
+			strcpy(str, n);
 			strcat(str, ":");
 			strcat(str, p->descr);
 		    } else
-			str = p->name;
+			str = bslashcolon(p->name);
 		    addlinknode(l, str);
 		}
 	    }
diff -u -r oldcompletion/Base/_arguments Completion/Base/_arguments
--- oldcompletion/Base/_arguments	Tue Nov 23 10:05:36 1999
+++ Completion/Base/_arguments	Tue Nov 23 10:24:36 1999
@@ -190,8 +190,8 @@
     _tags options
   fi
 
-  while _tags; do
-    while true; do
+  while true; do
+    while _tags; do
       if [[ -n "$matched" ]] || _requested arguments; then
         _description expl "$descr"
 
@@ -286,30 +286,31 @@
             equal -QqS= -M "$match"
         fi
       fi
-      if [[ -n "$opts" && -z "$aret$matched" &&
-            nm -eq compstate[nmatches] ]]; then
+    done
+    if [[ -n "$opts" && -z "$aret$matched" &&
+          nm -eq compstate[nmatches] ]]; then
 
-        prefix="${PREFIX#*\=}"
-        suffix="$SUFFIX"
-        PREFIX="${PREFIX%%\=*}"
-        SUFFIX=''
-        compadd -M "$match" -D equal - "${(@)equal%%:*}"
-
-        if [[ $#equal -eq 1 ]]; then
-	  PREFIX="$prefix"
-	  SUFFIX="$suffix"
-	  IPREFIX="${IPREFIX}${equal[1]%%:*}="
-	  matched=yes
+      prefix="${PREFIX#*\=}"
+      suffix="$SUFFIX"
+      PREFIX="${PREFIX%%\=*}"
+      SUFFIX=''
+      compadd -M "$match" -D equal - "${(@)equal%%:*}"
+
+      if [[ $#equal -eq 1 ]]; then
+        PREFIX="$prefix"
+	SUFFIX="$suffix"
+	IPREFIX="${IPREFIX}${equal[1]%%:*}="
+	matched=yes
 
-	  comparguments -L "${equal[1]%%:*}" descr action subc
-	  curcontext="${oldcontext}:$subc"
+	comparguments -L "${equal[1]%%:*}" descr action subc
+	curcontext="${oldcontext}:$subc"
 
-	  continue
-        fi
+	_tags arguments
+
+	continue
       fi
-      break
-    done
-    [[ -n "$aret" || nm -ne compstate[nmatches] ]] && break
+    fi
+    break
   done
 
   [[ -z "$aret" || -z "$usecc" ]] && curcontext="$oldcontext"
diff -u olddoc/Zsh/compsys.yo Doc/Zsh/compsys.yo
--- olddoc/Zsh/compsys.yo	Mon Nov 22 14:29:17 1999
+++ Doc/Zsh/compsys.yo	Tue Nov 23 11:40:30 1999
@@ -1985,8 +1985,8 @@
 where it can be left is when neither a var(message), nor a var(action) 
 is given.
 
-To include a colon in the var(message) or the var(action), it has to
-be preceded by a backslash.
+To include a colon in the option name, the var(message) or the
+var(action), it has to be preceded by a backslash.
 
 During the evaluation or execution of the action the array `tt(line)'
 will be set to the command name and normal arguments from the command
@@ -2017,12 +2017,6 @@
 `tt(-)tt(-prefix)') are still considered to contain only one option
 name. This allows the use of the `tt(-s)' option to describe
 single-letter options together with such long option names.
-
-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
-var(actions). For example, this allows one to give options for the
-tt(compadd) builtin that should be used.
 
 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

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

Problems with "comparguments" (Re: PATCH: broken _netscape)

[Bart Schaefer]

On Nov 22,  4:08pm, Oliver Kiddle wrote:
} Subject: PATCH: broken _netscape
}
} _netscape in pws-9 seems to be broken. This patch fixes it to a point
} where it atleast runs without errors but for some reason, completion of
} the arguments doesn't work. I suspect that it is a bug somewhere in
} _arguments.

I may be entirely wrong, but based on a scan through the "set -x" output
when completing after "netscape -", I'd say this is the same bug that is
causing "cvs" to complete --allow-root and the directories after it.  I
suspect a parsing bug in "comparguments".

While we're on the topic, I noticed this in the manual page for "rxvt" the
other day:

       -pixmap: file[;geom]
              Specify  XPM  file  for  the  background  and  also
              optionally  specify  its  scaling  with  a geometry
              string.  Note you may need to add quotes  to  avoid
              special shell interpretation of the `;' in the com-
              mand-line; resource backgroundPixmap.

Note that here we have an option whose name actually includes a colon.  As
far as I can tell there's *no* way to represent this in the argument list
for _arguments or comparguments; perhaps _regex_arguments could deal with
it, but it doesn't appear so from a look at _apt.

The mini-language of _arg_compile could handle it:

_arg_compile args option -pixmap: follow next \
	explain "file[;scaling]" \
	means "XPM file for the background" \
	action etc.

(I'm not sure how to specify the "action" for "complete files before a
semicolon, with a quoted semicolon as the auto-suffix, and geometries
after the semicolon", and I don't have time to work it out right now.
I'd love to see it done, though.)  However, since _arg_compile presently
doesn't do anything but paste together things to pass to _arguments, it
isn't helpful that it can initially parse it ...

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