Re: matcher-list question

Re: matcher-list question

[Peter Stephenson]

On Tue, 27 Nov 2007 09:56:53 +0000
Peter Stephenson <pws@csr.com> wrote:
> Andy Spiegl wrote:
> lama:~>zsh -f
> lama% autoload -U compinit; compinit -C
> lama% zstyle ":completion:*" matcher-list 'r:|[-./_]=* r:|=*' '+l:|=*'

Note that

  zstyle ":completion:*" matcher-list 'r:|[-./_]=* r:|=* l:|=*'

shows the same problem slightly more simply.

> lama% ls         
> foo-bar-abc  foo-bar-def  foo-bar-geh
> lama% ls bar<TAB>
> lama% ls -foobar-
> 
> There's a bug somewhere in the code that assembles partial lines using
> matching in this fashion (the code that generates the lists seems to be
> OK, or at least the list of matches is correct).

I've got at least somewhere...

This is mostly for my benefit, in case I forget.  I can't imagine
anyone else taking the slightest bit of interest.

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).

Still to understand: where left anchoring is actually coming in seeing
as the "-" in the matcher in use is in an expression which is marked as
a right anchor (yes, I checked that even in Svenland an 'l' in the
matcher is associated with CMF_LEFT).  The only left anchor is the
'l:|=*', which anchors to the start of the command line; it should be
this that's matching the "foo" because it's only matched by virtue of
the wildcard at the start of the line, but I don't see why it's handling
the first "-".  If I could understand why the "-" appeared here instead
of associated with a matcher with CMF_RIGHT, I might get further.

However, that's quite enough for one night.

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

Re: matcher-list question

[Peter Stephenson]

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/

Re: matcher-list question

[Peter Stephenson]

On Wed, 28 Nov 2007 20:47:09 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> No guarantees, but I think the following has a chance of fixing the
> problem without side effects.

Here's a test.

Index: Test/Y02compmatch.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/Y02compmatch.ztst,v
retrieving revision 1.3
diff -u -r1.3 Y02compmatch.ztst
--- Test/Y02compmatch.ztst	11 Mar 2004 19:14:42 -0000	1.3
+++ Test/Y02compmatch.ztst	28 Nov 2007 21:27:59 -0000
@@ -667,6 +667,14 @@
 >NO:{aA.bC.cB.dA}
 >NO:{aD.bC.cB.dA}
 
+ users_12227_matcher='r:|[-./_]=* r:|=* l:|=*'
+ users_12227_list=(foo-bar-abc foo-bar-def foo-bar-ghi)
+ test_code $users_12227_matcher users_12227_list
+ comptest $'tst bar\t'
+0:Test for wildcard left anchor with partial word right anchors
+>line: {tst foo-bar-}{}
+>COMPADD:{}
+>INSERT_POSITIONS:{12}
 
 %clean
 

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

Re: matcher-list question

[Andy Spiegl]

Peter Stephenson wrote:

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

Are these patches included in 4.3.4-dev-3 ?
If yes, then they didn't solve the problem. :-(

Thanks,
 Andy.

-- 
 System Error, hit any user to continue