Re: PATCH: complist with long display lines

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Mon, 07 Aug 2006 10:19:46 -0700
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> Now, about zsh-workers/21842 ...

  There was only one catch in the completion code, catch 22, and that
  stated that a concern for one's sanity in the face of complexities that
  were both real and immediate was the product of a rational mind.

The main completion code doesn't count an extra line for the screen for
strings that exactly fit the screen width: see the chunk of code in
compresult.c where I've added the note that it should count character
widths properly, as well as elsewhere in calclist().  This appears to be
correctly reflected in the way the display appears.

The complist code, however, adds 1 to the count of lines (variously held
in ml and mlprinted) if the number of columns just reaches the screen
width.  As it only allocates the number of lines that the main
completion code tells it, this causes bad array addressing.  I've tried
to fix this consistently by doing the same every time the number of
columns printed is divided the screen width, however I haven't the first
clue what part of the process most of those apply to.

The same mistake seems to appear somewhere else, I think, and with some
trepidation I've changed an occurrence in printfmt() in zle_tricky.c;
you'll see that it does subtract the 1 further up in the bit where it's
found a newline.

*IMPORTANT*: please don't ask me if this is really correct as I'll crawl
into a hole in the ground.  However, do continue to report any
reproducible problems, which is the only way we're ever going to sort
this mess out.

The DPUTS() should check that the array accesses are in range.  If these
go off there's another counting problem.

On my xterm (Fedora Core 5, package xterm-213-1.FC5) a line that
fits the width exactly doesn't show the last character.  This doesn't
happen with either gnome-terminal or konsole, so I'm assuming it's
Somebody Else's Problem.  (It's triggered by a termcap CLEAREOL at the
end of the line; xterm moves the cursor back over the character it's
just printed to avoid moving onto the next line, but doesn't seem to
realise that's just cosmetic and so deletes the character when clearing
the line.  We *could* work around this but I'm hanged if I can be
bothered.)

Index: Src/Zle/complist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/complist.c,v
retrieving revision 1.87
diff -u -r1.87 complist.c
--- Src/Zle/complist.c	7 Aug 2006 15:58:44 -0000	1.87
+++ Src/Zle/complist.c	9 Aug 2006 22:03:26 -0000
@@ -398,6 +398,9 @@
 static int mtab_been_reallocated;
 static Cmgroup *mgtab, *mgtabp;
 static struct listcols mcolors;
+#ifdef DEBUG
+int mgtabsize;
+#endif
 
 /* Used in mtab/mgtab, for explanations. */
 
@@ -660,7 +663,7 @@
 	     * There might be problems with characters of printing width
 	     * greater than one here.
 	     */
-	    if (col >= columns) {
+	    if (col > columns) {
 		ml++;
 		if (mscroll && !--mrestlines && (ask = asklistscroll(ml))) {
 		    mlprinted = ml - oml;
@@ -698,7 +701,7 @@
 		return 0;
 	    }
 	    putc(nc, shout);
-	    if (++col == columns) {
+	    if (++col > columns) {
 		ml++;
 		if (mscroll && !--mrestlines && (ask = asklistscroll(ml))) {
 		    mlprinted = ml - oml;
@@ -1037,7 +1040,8 @@
 		    *stop = 1;
 		    if (stat && n)
 			mfirstl = -1;
-		    return (mlprinted = l + (cc / columns));
+		    mlprinted = l + (cc ? ((cc-1) / columns) : 0);
+		    return mlprinted;
 		}
 	    }
 	}
@@ -1051,7 +1055,8 @@
     if (stat && n)
 	mfirstl = -1;
 
-    return (mlprinted = l + (cc / columns));
+    mlprinted = l + (cc ? ((cc-1) / columns) : 0);
+    return mlprinted;	    
 }
 
 /* This is like zputs(), but allows scrolling. */
@@ -1158,6 +1163,7 @@
 			int mm = (mcols * ml), i;
 
 			for (i = mcols; i--; ) {
+			    DPUTS(mm+i >= mgtabsize, "BUG: invalid position");
 			    mtab[mm + i] = mtmark(NULL);
 			    mgtab[mm + i] = mgmark(NULL);
 			}
@@ -1479,22 +1485,29 @@
 
             if (m->flags & CMF_DUMMY) {
                 for (i = mcols; i--; ) {
+		    DPUTS(mm+i >= mgtabsize, "BUG: invalid position");
                     mtab[mm + i] = mtmark(mp);
                     mgtab[mm + i] = mgmark(g);
                 }
             } else {
                 for (i = mcols; i--; ) {
+		    DPUTS(mm+i >= mgtabsize, "BUG: invalid position");
                     mtab[mm + i] = mp;
                     mgtab[mm + i] = g;
                 }
             }
 	}
 	if (!dolist(ml)) {
-	    mlprinted = printfmt(m->disp, 0, 0, 0) / columns;
+	    int nc = printfmt(m->disp, 0, 0, 0);
+	    if (nc)
+		mlprinted = (nc - 1) / columns;
+	    else
+		mlprinted = 0;
 	    return 0;
 	}
 	if (m->gnum == mselect) {
 	    int mm = (mcols * ml);
+	    DPUTS(mm >= mgtabsize, "BUG: invalid position");
 	    mline = ml;
 	    mcol = 0;
 	    mmtabp = mtab + mm;
@@ -1533,22 +1546,29 @@
 
             if (m->flags & CMF_DUMMY) {
                 for (i = (width ? width : mcols); i--; ) {
+		    DPUTS(mx+mm+i >= mgtabsize, "BUG: invalid position");
                     mtab[mx + mm + i] = mtmark(mp);
                     mgtab[mx + mm + i] = mgmark(g);
                 }
             } else {
                 for (i = (width ? width : mcols); i--; ) {
+		    DPUTS(mx+mm+i >= mgtabsize, "BUG: invalid position");
                     mtab[mx + mm + i] = mp;
                     mgtab[mx + mm + i] = g;
                 }
             }
 	}
 	if (!dolist(ml)) {
-	    mlprinted = ZMB_nicewidth(m->disp ? m->disp : m->str) / columns;
+	    int nc = ZMB_nicewidth(m->disp ? m->disp : m->str);
+	    if (nc)
+		mlprinted = (nc-1) / columns;
+	    else
+		mlprinted = 0;
 	    return 0;
 	}
 	if (m->gnum == mselect) {
 	    int mm = mcols * ml;
+	    DPUTS(mx+mm >= mgtabsize, "BUG: invalid position");
 
 	    mcol = mx;
 	    mline = ml;
@@ -1572,7 +1592,7 @@
 	    return 1;
 	}
 	len = ZMB_nicewidth(m->disp ? m->disp : m->str);
-	mlprinted = len / columns;
+	mlprinted = len ? (len-1) / columns : 0;
 
 	if ((g->flags & CGF_FILES) && m->modec) {
 	    if (m->gnum != mselect) {
@@ -1646,6 +1666,7 @@
         tc_downcurs(md1);
     if (mc1)
         tcmultout(TCRIGHT, TCMULTRIGHT, mc1);
+    DPUTS(ml1 * columns + mc1 >= mgtabsize, "BUG: invalid position");
     g = mgtab[ml1 * columns + mc1];
     clprintm(g, mtab[ml1 * columns + mc1], mcc1, ml1, lc1,
              (g->widths ? g->widths[mcc1] : g->width));
@@ -1657,6 +1678,7 @@
         tc_downcurs(md2 - md1);
     if (mc2)
         tcmultout(TCRIGHT, TCMULTRIGHT, mc2);
+    DPUTS(ml2 * columns + mc2 >= mgtabsize, "BUG: invalid position");
     g = mgtab[ml2 * columns + mc2];
     clprintm(g, mtab[ml2 * columns + mc2], mcc2, ml2, lc2,
              (g->widths ? g->widths[mcc2] : g->width));
@@ -1756,6 +1778,9 @@
 	memset(mtab, 0, i * sizeof(Cmatch **));
 	free(mgtab);
 	mgtab = (Cmgroup *) zalloc(i * sizeof(Cmgroup));
+#ifdef DEBUG
+	mgtabsize = i;
+#endif
 	memset(mgtab, 0, i * sizeof(Cmgroup));
 	mlastcols = mcols = columns;
 	mlastlines = mlines = listdat.nlines;
Index: Src/Zle/compresult.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compresult.c,v
retrieving revision 1.63
diff -u -r1.63 compresult.c
--- Src/Zle/compresult.c	1 Aug 2006 21:28:04 -0000	1.63
+++ Src/Zle/compresult.c	9 Aug 2006 22:03:28 -0000
@@ -1480,6 +1480,7 @@
 		hidden = 1;
 		while ((sptr = *pp)) {
 		    while (sptr && *sptr) {
+			/* TODO: we need to use wcwidth() here */
 			nlines += (nlptr = strchr(sptr, '\n'))
 			    ? 1 + (nlptr - sptr - 1) / columns
 			    : (ztrlen(sptr) - 1) / columns;
Index: Src/Zle/zle_tricky.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_tricky.c,v
retrieving revision 1.70
diff -u -r1.70 zle_tricky.c
--- Src/Zle/zle_tricky.c	3 Aug 2006 15:37:50 -0000	1.70
+++ Src/Zle/zle_tricky.c	9 Aug 2006 22:03:29 -0000
@@ -2074,6 +2074,7 @@
 
     for (; *p; p++) {
 	/* Handle the `%' stuff (%% == %, %n == <number of matches>). */
+	/* TODO: we need to use wcwidth() to count cc */
 	if (doesc && *p == '%') {
 	    if (*++p) {
 		m = 0;
@@ -2174,7 +2175,7 @@
 		putc(' ', shout);
 	}
     }
-    return l + (cc / columns);
+    return l + ((cc-1) / columns);
 }
 
 /* This is used to print expansions. */

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