* PATCH: complist with long display lines @ 2006-08-07 15:40 Peter Stephenson 2006-08-07 17:19 ` Bart Schaefer 0 siblings, 1 reply; 7+ messages in thread From: Peter Stephenson @ 2006-08-07 15:40 UTC (permalink / raw) To: Zsh hackers list Amazingly, I think I may have fixed a bug in completion. With the following (note long line which might get messed up in the post): _testcomp() { compadd \ averylongbitoftextthatshouldcausethedisplaytowrapovertothenextlinewitheffectsthatremaintobeseen \ bach cach dach each fach gach hach } compdef _testcomp testcomp and menu selection in effect, type "testcomp <TAB>". The long line should appear and be highlighted. Now hit <TAB> again. The display is messed up: if you see what I see, everything is printed two lines too low. The following appears to fix it. It moves the cursor back to get the line calculation in singledraw() correct. Index: Src/Zle/complist.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/Zle/complist.c,v retrieving revision 1.85 diff -u -r1.85 complist.c --- Src/Zle/complist.c 3 Aug 2006 15:37:50 -0000 1.85 +++ Src/Zle/complist.c 7 Aug 2006 15:33:46 -0000 @@ -1649,6 +1649,8 @@ g = mgtab[ml1 * columns + mc1]; clprintm(g, mtab[ml1 * columns + mc1], mcc1, ml1, lc1, (g->widths ? g->widths[mcc1] : g->width)); + if (mlprinted) + (void) tcmultout(TCUP, TCMULTUP, mlprinted); putc('\r', shout); if (md2 != md1) @@ -1658,6 +1660,8 @@ g = mgtab[ml2 * columns + mc2]; clprintm(g, mtab[ml2 * columns + mc2], mcc2, ml2, lc2, (g->widths ? g->widths[mcc2] : g->width)); + if (mlprinted) + (void) tcmultout(TCUP, TCMULTUP, mlprinted); putc('\r', shout); if (mstatprinted) { -- Peter Stephenson <pws@csr.com> Software Engineer CSR PLC, Churchill House, Cambridge Business Park, Cowley Road Cambridge, CB4 0WZ, UK Tel: +44 (0)1223 692070 To access the latest news from CSR copy this link into a web browser: http://www.csr.com/email_sig.php ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: complist with long display lines 2006-08-07 15:40 PATCH: complist with long display lines Peter Stephenson @ 2006-08-07 17:19 ` Bart Schaefer 2006-08-07 21:56 ` DervishD 2006-08-09 22:04 ` Peter Stephenson 0 siblings, 2 replies; 7+ messages in thread From: Bart Schaefer @ 2006-08-07 17:19 UTC (permalink / raw) To: Zsh hackers list On Aug 7, 4:40pm, Peter Stephenson wrote: } } Amazingly, I think I may have fixed a bug in completion. Wow, that's cool. Now, about zsh-workers/21842 ... ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: complist with long display lines 2006-08-07 17:19 ` Bart Schaefer @ 2006-08-07 21:56 ` DervishD 2006-08-09 22:04 ` Peter Stephenson 1 sibling, 0 replies; 7+ messages in thread From: DervishD @ 2006-08-07 21:56 UTC (permalink / raw) To: Bart Schaefer; +Cc: Zsh hackers list Hi all :) * Bart Schaefer <schaefer@brasslantern.com> dixit: > On Aug 7, 4:40pm, Peter Stephenson wrote: > } > } Amazingly, I think I may have fixed a bug in completion. > > Wow, that's cool. > > Now, about zsh-workers/21842 ... That's exactly what I was going to write now ;))) Raúl Núñez de Arenas Coronado -- Linux Registered User 88736 | http://www.dervishd.net http://www.pleyades.net & http://www.gotesdelluna.net It's my PC and I'll cry if I want to... RAmen! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: complist with long display lines 2006-08-07 17:19 ` Bart Schaefer 2006-08-07 21:56 ` DervishD @ 2006-08-09 22:04 ` Peter Stephenson 2006-08-10 8:01 ` Bart Schaefer 2006-08-10 16:22 ` Peter Stephenson 1 sibling, 2 replies; 7+ messages in thread From: Peter Stephenson @ 2006-08-09 22:04 UTC (permalink / raw) To: zsh-workers On Mon, 07 Aug 2006 10:19:46 -0700 Bart Schaefer <schaefer@brasslantern.com> 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@ntlworld.com> Web page now at http://homepage.ntlworld.com/p.w.stephenson/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: complist with long display lines 2006-08-09 22:04 ` Peter Stephenson @ 2006-08-10 8:01 ` Bart Schaefer 2006-08-10 9:27 ` Peter Stephenson 2006-08-10 16:22 ` Peter Stephenson 1 sibling, 1 reply; 7+ messages in thread From: Bart Schaefer @ 2006-08-10 8:01 UTC (permalink / raw) To: zsh-workers On Aug 9, 11:04pm, Peter Stephenson wrote: } } The main completion code doesn't count an extra line for the screen for } strings that exactly fit the screen width } } 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. Brilliant, Peter. How did you ever track this down? It may be worthwhile (or at least satisfying to paranoid types) to add the following patch, which I've had sitting around uncommitted because it failed to resolve the addressing issues that you seem to finally have identified. This patch prevents the infinite loop alluded to in 21842, by catching cases where an array index starts out negative and decrements from there. It might be unnecessary now, but: Index: Src/Zle/complist.c --- Src/Zle/complist.c 2006-08-10 00:34:13.000000000 -0700 +++ Src/Zle/complist.c.defense 2006-08-10 21:19:26.000000000 -0700 @@ -841,7 +831,7 @@ selectlocalmap(NULL); settyinfo(&shttyinfo); putc('\r', shout); - for (i = columns - 1; i--; ) + for (i = columns - 1; i-- > 0; ) putc(' ', shout); putc('\r', shout); @@ -1162,7 +1148,7 @@ if (mselect >= 0) { int mm = (mcols * ml), i; - for (i = mcols; i--; ) { + for (i = mcols; i-- > 0; ) { DPUTS(mm+i >= mgtabsize, "BUG: invalid position"); mtab[mm + i] = mtmark(NULL); mgtab[mm + i] = mgmark(NULL); @@ -1484,13 +1469,13 @@ int mm = (mcols * ml), i; if (m->flags & CMF_DUMMY) { - for (i = mcols; i--; ) { + for (i = mcols; i-- > 0; ) { DPUTS(mm+i >= mgtabsize, "BUG: invalid position"); mtab[mm + i] = mtmark(mp); mgtab[mm + i] = mgmark(g); } } else { - for (i = mcols; i--; ) { + for (i = mcols; i-- > 0; ) { DPUTS(mm+i >= mgtabsize, "BUG: invalid position"); mtab[mm + i] = mp; mgtab[mm + i] = g; @@ -1545,13 +1523,13 @@ int mm = mcols * ml, i; if (m->flags & CMF_DUMMY) { - for (i = (width ? width : mcols); i--; ) { + for (i = (width ? width : mcols); i-- > 0; ) { 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--; ) { + for (i = (width ? width : mcols); i-- > 0; ) { DPUTS(mx+mm+i >= mgtabsize, "BUG: invalid position"); mtab[mx + mm + i] = mp; mgtab[mx + mm + i] = g; @@ -2184,7 +2142,7 @@ Cmatch **p = mtab; for (y = 0; y < mlines; y++) { - for (x = mcols; x; x--, p++) + for (x = mcols; x > 0; x--, p++) if (*p && !mmarked(*p) && **p && mselect == (**p)->gnum) break; if (x) { @@ -2195,7 +2153,7 @@ int c; while (mlbeg) { - for (q = p, c = columns; c; q++, c--) + for (q = p, c = columns; c > 0; q++, c--) if (*q && !mmarked(*q)) break; if (c) @@ -2228,7 +2180,7 @@ int c; while (mlbeg < mlines) { - for (q = p, c = columns; c; q++, c--) + for (q = p, c = columns; c > 0; q++, c--) if (*q) break; if (c) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: complist with long display lines 2006-08-10 8:01 ` Bart Schaefer @ 2006-08-10 9:27 ` Peter Stephenson 0 siblings, 0 replies; 7+ messages in thread From: Peter Stephenson @ 2006-08-10 9:27 UTC (permalink / raw) To: zsh-workers Bart Schaefer wrote: > How did you ever track this down? That's three evenings which will never come again. Once I'd found that array indexing for mgtab was off, which took a certain amount of digging since it's referenced in some odd ways, it wasn't so hard to find out how the limits were being set and how the assumption in the line limit was being violated. As I indicated, whether the assumption is present at all and only the places where I've applied the fix is another matter to be determined... > It may be worthwhile (or at least satisfying to paranoid types) to add > the following patch, which I've had sitting around uncommitted because > it failed to resolve the addressing issues that you seem to finally > have identified. This patch prevents the infinite loop alluded to in > 21842, by catching cases where an array index starts out negative and > decrements from there. It might be unnecessary now, but: I don't think I've done anything which would prevent this infinite loop. mcols comes from the global columns which comes from the environment, so if that's negative there are problems. So I think this would be useful. -- Peter Stephenson <pws@csr.com> Software Engineer CSR PLC, Churchill House, Cambridge Business Park, Cowley Road Cambridge, CB4 0WZ, UK Tel: +44 (0)1223 692070 To access the latest news from CSR copy this link into a web browser: http://www.csr.com/email_sig.php ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: complist with long display lines 2006-08-09 22:04 ` Peter Stephenson 2006-08-10 8:01 ` Bart Schaefer @ 2006-08-10 16:22 ` Peter Stephenson 1 sibling, 0 replies; 7+ messages in thread From: Peter Stephenson @ 2006-08-10 16:22 UTC (permalink / raw) To: zsh-workers Peter Stephenson wrote: > 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. This appears to be wrong both here and the corresponding change in complist.c, functions printfmt() and compprintfmt() respectively. This turned up when I was using the file-list zstyle with a description which (including the file name at the end) just happened to hit the right hand edge of the screen. In this case, unlike the others, you get a new line at the end of the display; I'm guessing this makes the difference. (However, obviously it isn't the calculation I'm modifying that produces the extra newline, so I don't know where that comes from.) The two changes affect the non-complist and complist display of the same output. This is different from the other cases because... er... oh look! There's Superman! Over there! Luckily, changing these doesn't undo the main point of the fix. Phew. I've subjected this to the combinations of tests I know about and they all seem to work. Index: Src/Zle/complist.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/Zle/complist.c,v retrieving revision 1.88 diff -u -r1.88 complist.c --- Src/Zle/complist.c 9 Aug 2006 22:08:38 -0000 1.88 +++ Src/Zle/complist.c 10 Aug 2006 16:15:47 -0000 @@ -1055,8 +1055,12 @@ if (stat && n) mfirstl = -1; - mlprinted = l + (cc ? ((cc-1) / columns) : 0); - return mlprinted; + /* + * *Not* subtracting 1 from cc at this point appears to be + * correct. C.f. printfmt in zle_tricky.c. + */ + mlprinted = l + (cc / columns); + return mlprinted; } /* This is like zputs(), but allows scrolling. */ Index: Src/Zle/zle_tricky.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_tricky.c,v retrieving revision 1.71 diff -u -r1.71 zle_tricky.c --- Src/Zle/zle_tricky.c 9 Aug 2006 22:08:39 -0000 1.71 +++ Src/Zle/zle_tricky.c 10 Aug 2006 16:15:49 -0000 @@ -2175,7 +2174,12 @@ putc(' ', shout); } } - return l + ((cc-1) / columns); + /* + * Experiments suggest that at this point not subtracting 1 from + * cc is correct, i.e. if just misses wrapping we still add 1. + * (Why?) + */ + return l + (cc / columns); } /* This is used to print expansions. */ -- Peter Stephenson <pws@csr.com> Software Engineer CSR PLC, Churchill House, Cambridge Business Park, Cowley Road Cambridge, CB4 0WZ, UK Tel: +44 (0)1223 692070 To access the latest news from CSR copy this link into a web browser: http://www.csr.com/email_sig.php ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2006-08-10 16:23 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2006-08-07 15:40 PATCH: complist with long display lines Peter Stephenson 2006-08-07 17:19 ` Bart Schaefer 2006-08-07 21:56 ` DervishD 2006-08-09 22:04 ` Peter Stephenson 2006-08-10 8:01 ` Bart Schaefer 2006-08-10 9:27 ` Peter Stephenson 2006-08-10 16:22 ` Peter Stephenson
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).