Re: matcher-list question

[Peter Stephenson <p.w.stephenson@ntlworld.com>]

On Tue, 27 Nov 2007 21:58:45 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> add_match_part() in compatch.c is responsible for assembling the bits of
> the match ("cline" structures), including prefixes, anchors, and
> suffixes.  In the case in question,  "-" is the anchor with "foo"
> as a prefix.  However, the code at line 352,
> 
>     if (m && (m->flags & CMF_LEFT)) {
> 	lp->flags |= CLF_SUF;
> 	lp->suffix = lp->prefix;
> 	lp->prefix = NULL;
>     }
> 
> kicks in because this is a left anchor, which swaps the suffix and the
> prefix.  I confirmed that commenting out this code gets the completion
> right in this particular case (with no obvious side effectis, though it
> presumably screws up in other completions or with other matchers).

No guarantees, but I think the following has a chance of fixing the
problem without side effects.

We're applying the code quoted above on the assumption that the last
part of the newly split cline (lp) is associated with the matcher (m)
passed down into add_match_part().  However, there are special cases, of
which this is one, where the last newly split part comes from one of the
right anchors in the global list bmatchers.  (I don't understand what
I'm saying, either.)  So in this case lp has nothing to do with m and we
shouldn't convert the prefix to a suffix.

I dare anybody to say "no, that can't be right".

Do let me know of any new failures in matching.

Index: Src/Zle/compcore.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compcore.c,v
retrieving revision 1.91
diff -u -r1.91 compcore.c
--- Src/Zle/compcore.c	27 Jun 2007 13:56:12 -0000	1.91
+++ Src/Zle/compcore.c	28 Nov 2007 20:37:58 -0000
@@ -2376,7 +2376,7 @@
 		    ms = dupstring(s);
 		else
 		    sl = strlen(ms = multiquote(s, 0));
-		lc = bld_parts(ms, sl, -1, NULL);
+		lc = bld_parts(ms, sl, -1, NULL, NULL);
 		isexact = 0;
 	    } else if (!(ms = comp_match(lpre, lsuf, s, cp, &lc,
 					 (!(dat->aflags & CAF_QUOTE) ?
@@ -2520,7 +2520,7 @@
 	for (pp = NULL, p = line; p->next; pp = p, p = p->next);
 
 	if (psl) {
-	    s = bld_parts(psuf, psl, psl, &sl);
+	    s = bld_parts(psuf, psl, psl, &sl, NULL);
 
 	    if (sline) {
 		Cline sp;
@@ -2554,7 +2554,7 @@
 	if (isl) {
 	    Cline tsl;
 
-	    s = bld_parts(isuf, isl, isl, &tsl);
+	    s = bld_parts(isuf, isl, isl, &tsl, NULL);
 
 	    if (sl)
 		sl->next = s;
@@ -2573,7 +2573,7 @@
 	    sl = tsl;
 	}
 	if (qisl) {
-	    Cline qsl = bld_parts(dupstring(qisuf), qisl, qisl, NULL);
+	    Cline qsl = bld_parts(dupstring(qisuf), qisl, qisl, NULL, NULL);
 
 	    qsl->flags |= CLF_SUF;
 	    qsl->suffix = qsl->prefix;
@@ -2616,7 +2616,7 @@
 	    if (pline)
 		for (p = cp_cline(pline, 1), lp = p; lp->next; lp = lp->next);
 	    else
-		p = bld_parts(ppre, ppl, ppl, &lp);
+		p = bld_parts(ppre, ppl, ppl, &lp, NULL);
 
 	    if (lp->prefix && !(line->flags & (CLF_SUF | CLF_MID)) &&
 		!lp->llen && !lp->wlen && !lp->olen) {
@@ -2644,19 +2644,19 @@
 	    }
 	}
 	if (pl) {
-	    Cline lp, p = bld_parts(pre, pl, pl, &lp);
+	    Cline lp, p = bld_parts(pre, pl, pl, &lp, NULL);
 
 	    lp->next = line;
 	    line = p;
 	}
 	if (ipl) {
-	    Cline lp, p = bld_parts(ipre, ipl, ipl, &lp);
+	    Cline lp, p = bld_parts(ipre, ipl, ipl, &lp, NULL);
 
 	    lp->next = line;
 	    line = p;
 	}
 	if (qipl) {
-	    Cline lp, p = bld_parts(dupstring(qipre), qipl, qipl, &lp);
+	    Cline lp, p = bld_parts(dupstring(qipre), qipl, qipl, &lp, NULL);
 
 	    lp->next = line;
 	    line = p;
@@ -2676,7 +2676,7 @@
 	    if (ppl)
 		memcpy(apre + qipl + ipl + pl, ppre, ppl);
 
-	    p = bld_parts(apre, palen, palen, &lp);
+	    p = bld_parts(apre, palen, palen, &lp, NULL);
 
 	    if (pline)
 		for (lp->next = cp_cline(pline, 1); lp->next; lp = lp->next);
Index: Src/Zle/compmatch.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compmatch.c,v
retrieving revision 1.51
diff -u -r1.51 compmatch.c
--- Src/Zle/compmatch.c	23 Jul 2007 13:24:45 -0000	1.51
+++ Src/Zle/compmatch.c	28 Nov 2007 20:37:58 -0000
@@ -337,22 +337,26 @@
 add_match_part(Cmatcher m, char *l, char *w, int wl,
 	       char *o, int ol, char *s, int sl, int osl, int sfx)
 {
-    Cline p, lp;
+    Cline p, lp, lprem;
 
     /* If the anchors are equal, we keep only one. */
 
     if (l && !strncmp(l, w, wl))
 	l = NULL;
 
-    /* Split the new part into parts and turn the last one into a
-     * `suffix' if we have a left anchor. */
-
-    p = bld_parts(s, sl, osl, &lp);
-
-    if (m && (m->flags & CMF_LEFT)) {
-	lp->flags |= CLF_SUF;
-	lp->suffix = lp->prefix;
-	lp->prefix = NULL;
+    /*
+     * Split the new part into parts and turn the last one into a
+     * `suffix' if we have a left anchor---don't do this if the last one
+     * came from a right anchor before the end of the part we're
+     * splitting.
+     */
+
+    p = bld_parts(s, sl, osl, &lp, &lprem);
+
+    if (lprem && m && (m->flags & CMF_LEFT)) {
+	lprem->flags |= CLF_SUF;
+	lprem->suffix = lprem->prefix;
+	lprem->prefix = NULL;
     }
     /* cline lists for suffixes are sorted from back to front, so we have
      * to revert the list we got. */
@@ -418,7 +422,7 @@
     if (wl || ll) {
 	Cline p, lp;
 
-	if ((p = n = bld_parts(w, wl, ll, &lp)) && n != lp) {
+	if ((p = n = bld_parts(w, wl, ll, &lp, NULL)) && n != lp) {
 	    for (; p->next != lp; p = p->next);
 
 	    if (matchsubs) {
@@ -1016,7 +1020,7 @@
 	 * cline list for these matches, too. */
 	w = dupstring(w);
 	wl = strlen(w);
-	*clp = bld_parts(w, wl, wl, NULL);
+	*clp = bld_parts(w, wl, wl, NULL, NULL);
 	*exact = 0;
     } else {
 	Cline pli, plil;
@@ -1074,7 +1078,7 @@
 	    add_match_str(NULL, NULL, wpfx, wpl, 1);
 
 	    mli = bld_parts(w + rpl, wl - rpl - rsl,
-			    (mpl - rpl) + (msl - rsl), &mlil);
+			    (mpl - rpl) + (msl - rsl), &mlil, NULL);
 	    mlil->flags |= CLF_MID;
 	    mlil->slen = msl - rsl;
 	    mlil->next = revert_cline(matchparts);
@@ -1161,11 +1165,19 @@
 /* This splits the given string into a list of cline structs, separated
  * at those places where one of the anchors of an `*' pattern was found.
  * plen gives the number of characters on the line that matched this
- * string. In lp we return a pointer to the last cline struct we build. */
+ * string.
+ *
+ * In *lp, if lp is not NULL, we return a pointer to the last cline struct we
+ * build.
+ *
+ * In *lprem, if lprem is not NULL, we return a pointer to the last
+ * cline struct we build if it came from the remainder of the
+ * line rather than from a right anchor match, else NULL.
+ */
 
 /**/
 Cline
-bld_parts(char *str, int len, int plen, Cline *lp)
+bld_parts(char *str, int len, int plen, Cline *lp, Cline *lprem)
 {
     Cline ret = NULL, *q = &ret, n = NULL;
     Cmlist ms;
@@ -1224,9 +1236,17 @@
 	if (llen > olen)
 	    llen = olen;
 	n->prefix = get_cline(NULL, llen, p, olen, NULL, 0, 0);
+	if (lprem)
+	    *lprem = n;
+    }
+    else if (!ret) {
+        *q = n =
+	    get_cline(NULL, 0, NULL, 0, NULL, 0, (plen <= 0 ? CLF_NEW : 0));
+	if (lprem)
+	    *lprem = n;
+    } else if (lprem) {
+	*lprem = NULL;
     }
-    else if (!ret)
-        *q = n = get_cline(NULL, 0, NULL, 0, NULL, 0, (plen <= 0 ? CLF_NEW : 0));
 
     n->next = NULL;
 

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/