Re: One last _values / compvalues nit, and a nasty crash bug

[Sven Wischnowsky <wischnow@xxxxxxxxxxxxxxxxxxxxxxx>]

Oliver Kiddle wrote:

> ...
> 
> But we use _values in a number of places with and empty string as a
> separator so this isn't good, for example:
> wget -n<tab>
> : $fpath[(<tab>
> both now produce the invalid separator error where they previously
> worked. I don't suppose it is only the -S separator (as opposed to -s)
> that compvalues doesn't like an empty string?

Yes.  Until now.

Below is the patch to change the parsing or the current word for
_values.  I've tried it with empty value and argument separators and it
seems to work.  I'm going to commit this on the development branch. 
When it has got some testing, it should probably be applied to the other
branch as well (please someone remind me...).

> Is there any chance that compvalues, comparguments etc could be given
> some sort of documentation. I know this was mentioned recently but it
> could go in either the zsh-development-guide or completion-style-guide
> file.

Hrm.  I don't like either place for it.  These builtins are so
specialised that they won't be of any use outside _values and
_arguments.  Would it be enough if I add some comments in computil.c in
the bin_* functions?


Bye
  Sven

Index: Completion/Base/Utility/_values
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Base/Utility/_values,v
retrieving revision 1.4
diff -u -r1.4 _values
--- Completion/Base/Utility/_values	2001/06/01 14:29:11	1.4
+++ Completion/Base/Utility/_values	2001/06/11 11:39:06
@@ -27,7 +27,7 @@
 
     compvalues -V noargs args opts
 
-    if [[ "$PREFIX" = *${argsep}${~test} ]]; then
+    if [[ -n "$argsep" && "$PREFIX" = *${argsep}${~test} ]]; then
       local name
 
       name="${PREFIX%%${argsep}*}"
Index: Src/Zle/computil.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/computil.c,v
retrieving revision 1.58
diff -u -r1.58 computil.c
--- Src/Zle/computil.c	2001/06/01 08:53:55	1.58
+++ Src/Zle/computil.c	2001/06/11 11:39:07
@@ -2103,10 +2103,7 @@
 
     while (args[0][0] == '-' && (args[0][1] == 's' || args[0][1] == 'S') &&
            !args[0][2]) {
-	if (!args[1][0] || (args[1][0] && args[1][1])) {
-	    zwarnnam(nam, "invalid separator: %s", args[1], 0);
-	    return NULL;
-	}
+
         if (args[0][1] == 's') {
             hassep = 1;
             sep = args[1][0];
@@ -2314,20 +2311,122 @@
 static struct cvstate cv_laststate;
 static int cv_parsed = 0, cv_alloced = 0;
 
+/* Get the next value in the string.  Return it's definition and update the
+ * sp pointer to point to the end of the value (plus argument, if any).
+ * If there is no next value, the string pointer is set to null.  In any
+ * case ap will point to the beginning of the argument or will be a null
+ * pointer if there is no argument.
+ */
+
+static Cvval
+cv_next(Cvdef d, char **sp, char **ap)
+{
+    Cvval r = NULL;
+    char *s = *sp;
+
+    if (!*s) {
+        *sp = *ap = NULL;
+
+        return NULL;
+    }
+    if ((d->hassep && !d->sep) || !d->argsep) {
+        char sav, ec, *v = s, *os;
+
+        ec = ((d->hassep && d->sep) ? d->sep : d->argsep);
+
+        do {
+            sav = *++s;
+            *s = '\0';
+            if ((r = cv_get_val(d, v))) {
+                *s = sav;
+
+                break;
+            }
+            *s = sav;
+        } while (*s && *s != ec);
+
+        os = s;
+
+        if (d->hassep && d->sep) {
+            if ((s = strchr(s, d->sep)))
+                *sp = s + 1;
+            else
+                *sp = NULL;
+        } else
+            *sp = s;
+        if (d->argsep && *os == d->argsep) {
+            *ap = os + 1;
+            *sp = NULL;
+        } else if (r && r->type != CVV_NOARG)
+            *ap = os;
+        else
+            *ap = NULL;
+
+        return r;
+
+    } else if (d->hassep) {
+        char *ns = strchr(s, d->sep), *as, *sap, sav;
+        int skip = 0;
+
+        if (d->argsep && (as = strchr(s, d->argsep)) && (!ns || as <= ns)) {
+            *ap = as + 1;
+            ns = strchr(as + 1, d->sep);
+            skip = 1;
+            sap = as;
+        } else {
+            *ap = NULL;
+            sap = ns;
+        }
+        if (sap) {
+            sav = *sap;
+            *sap = '\0';
+        }
+        if ((!(r = cv_get_val(d, s)) || r->type == CVV_NOARG) && skip)
+            ns = as;
+
+        if (sap)
+            *sap = sav;
+
+        *sp = ((!ns || (ns == as && r && r->type != CVV_NOARG)) ? NULL : ns + 1);
+
+        return r;
+    } else {
+        char *as = strchr(s, d->argsep), *sap, sav;
+
+        *sp = NULL;
+
+        if (as) {
+            *ap = as + 1;
+            sap = as;
+            sav = *as;
+            *sap = '\0';
+        } else
+            *ap = sap = NULL;
+
+        r = cv_get_val(d, s);
+
+        if (sap)
+            *sap = sav;
+
+        return r;
+    }
+}
+
 /* Parse the current word. */
 
 static void
 cv_parse_word(Cvdef d)
 {
-    Cvval ptr;
+    Cvval val;
     struct cvstate state;
-    char *str, *eq;
+    char *str, *arg = NULL, *pign = compprefix;
+    int nosfx = 0;
 
     if (cv_alloced)
 	freelinklist(cv_laststate.vals, freestr);
 
-    for (ptr = d->vals; ptr; ptr = ptr->next)
-	ptr->active = 1;
+    for (val = d->vals; val; val = val->next)
+	val->active = 1;
 
     state.d = d;
     state.def = NULL;
@@ -2335,104 +2434,91 @@
     state.vals = (LinkList) znewlinklist();
 
     cv_alloced = 1;
-
-    if (d->hassep) {
-	if (d->sep) {
-	    char *end;
-	    int heq;
-
-	    for (str = compprefix, end = strchr(str, d->sep); end;) {
-		*end = '\0';
-
-		if ((heq = !!(eq = strchr(str, d->argsep))))
-		    *eq++ = '\0';
-		else
-		    eq = "";
-
-		if ((ptr = cv_get_val(d, str))) {
-		    zaddlinknode(state.vals, ztrdup(str));
-		    zaddlinknode(state.vals, ztrdup(eq));
-
-		    cv_inactive(d, ptr->xor);
-		}
-		if (heq)
-		    eq[-1] = d->argsep;
-
-		*end = d->sep;
-		str = end + 1;
-		end = strchr(str, d->sep);
-	    }
-	    ignore_prefix(str - compprefix);
-
-	    if ((str = strchr(compsuffix, d->sep))) {
-		char *beg = str;
-
-		for (str++; str; str = end) {
-		    if ((end = strchr(str, d->sep)))
-			*end = '\0';
-
-		    if ((heq = !!(eq = strchr(str, d->argsep))))
-			*eq++ = '\0';
-		    else
-			eq = "";
-
-		    if ((ptr = cv_get_val(d, str))) {
-			zaddlinknode(state.vals, ztrdup(str));
-			zaddlinknode(state.vals, ztrdup(eq));
-
-			cv_inactive(d, ptr->xor);
-		    }
-		    if (heq)
-			eq[-1] = d->argsep;
-		    if (end)
-			*end++ = d->sep;
-		}
-		ignore_suffix(strlen(beg));
-	    }
-	} else {
-	    char tmp[2];
-
-	    tmp[1] = '\0';
 
-	    for (str = compprefix; *str; str++) {
-		tmp[0] = *str;
-		if ((ptr = cv_get_val(d, tmp))) {
-		    zaddlinknode(state.vals, ztrdup(tmp));
-		    zaddlinknode(state.vals, ztrdup(""));
-
-		    cv_inactive(d, ptr->xor);
-		}
-	    }
-	    for (str = compsuffix; *str; str++) {
-		tmp[0] = *str;
-		if ((ptr = cv_get_val(d, tmp))) {
-		    zaddlinknode(state.vals, ztrdup(tmp));
-		    zaddlinknode(state.vals, ztrdup(""));
-
-		    cv_inactive(d, ptr->xor);
-		}
-	    }
-	    ignore_prefix(strlen(compprefix));
-	    ignore_suffix(strlen(compsuffix));
-	}
-    }
-    str = tricat(compprefix, compsuffix, "");
-    zsfree(compprefix);
-    zsfree(compsuffix);
-    compprefix = str;
-    compsuffix = ztrdup("");
-
-    if ((eq = strchr(str, d->argsep))) {
-	*eq++ = '\0';
-
-	if ((ptr = cv_get_val(d, str)) && ptr->type != CVV_NOARG) {
-	    eq[-1] = d->argsep;
-	    ignore_prefix(eq - str);
-	    state.def = ptr->arg;
-	    state.val = ptr;
-	} else
-	    eq[-1] = d->argsep;
+    for (str = compprefix; str && *str; ) {
+        if ((val = cv_next(d, &str, &arg))) {
+            zaddlinknode(state.vals, ztrdup(val->name));
+            if (arg) {
+                if (str) {
+                    char sav = str[-1];
+
+                    str[-1] = '\0';
+                    zaddlinknode(state.vals, ztrdup(arg));
+                    str[-1] = sav;
+                } else {
+                    zaddlinknode(state.vals, tricat(arg, compsuffix, ""));
+                    nosfx = 1;
+                }
+            } else
+                zaddlinknode(state.vals, ztrdup(""));
+
+            cv_inactive(d, val->xor);
+
+            if (str)
+                pign = str;
+            else
+                val->active = 1;
+        }
     }
+    state.val = val;
+    if (val && arg && !str)
+        state.def = val->arg;
+
+    if (!nosfx && d->hassep) {
+        int ign = 0;
+        char *more = NULL;
+
+        ignore_prefix(pign - compprefix);
+
+        if (!d->sep && (!val || val->type == CVV_NOARG)) {
+            ign = strlen(compsuffix);
+            more = compsuffix;
+        } else {
+            if (d->sep) {
+                char *ns = strchr(compsuffix, d->sep), *as;
+
+                if (d->argsep && (as = strchr(compsuffix, d->argsep)) &&
+                    (!ns || as <= ns)) {
+                    ign = strlen(as);
+                } else
+                    ign = (ns ? strlen(ns) : 0);
+
+                more = (ns ? ns + 1 : NULL);
+            } else if (d->argsep) {
+                char *as;
+
+                if ((as = strchr(compsuffix, d->argsep)))
+                    ign = strlen(as);
+            }
+        }
+        if (ign)
+            ignore_suffix(ign);
+
+        while (more && *more) {
+            if ((val = cv_next(d, &str, &arg))) {
+                zaddlinknode(state.vals, ztrdup(val->name));
+                if (arg) {
+                    if (str) {
+                        char sav = str[-1];
+
+                        str[-1] = '\0';
+                        zaddlinknode(state.vals, ztrdup(arg));
+                        str[-1] = sav;
+                    } else {
+                        zaddlinknode(state.vals, tricat(arg, compsuffix, ""));
+                        nosfx = 1;
+                    }
+                } else
+                    zaddlinknode(state.vals, ztrdup(""));
+
+                cv_inactive(d, val->xor);
+            }
+        }
+    } else if (arg)
+        ignore_prefix(arg - compprefix);
+    else
+        ignore_prefix(pign - compprefix);
+
     memcpy(&cv_laststate, &state, sizeof(state));
 }
 

-- 
Sven Wischnowsky                         wischnow@xxxxxxxxxxxxxxxxxxxxxxx