* Re: 'zle redisplay' bug in 5.3? [not found] ` <170105010914.ZM1529@torch.brasslantern.com> @ 2017-01-05 15:01 ` Peter Stephenson 2017-01-05 17:08 ` Bart Schaefer 0 siblings, 1 reply; 7+ messages in thread From: Peter Stephenson @ 2017-01-05 15:01 UTC (permalink / raw) To: Zsh Hackers' List On Thu, 5 Jan 2017 01:09:14 -0800 Bart Schaefer <schaefer@brasslantern.com> wrote: > On Jan 5, 4:01pm, Jan Larres wrote: > } > } expand-or-complete-with-dots() { > } echo -ne "\e[31m......\e[0m" > } zle expand-or-complete > } zle redisplay > } } > > Hmm. Indeed, with multi-line prompts, invoking redisplay immediately > after a completion menu is displayed will move the cursor upward as > many extra lines as the prompt is tall but then does not finish the > repainting of the prompt, leaving the cursor in the wrong place. > > You don't even need compinit, just do: > > % PS1=$':first line\n'"$PS1" > :first line > % ls <TAB> > :first line > % ls <M-x redisplay><RET> > > and you'll observe zle get confused. > > This is from workers/38048z ... where I asked for additional feedback > and got none ... discussion starts in users/21315. > > I suspect something is different about the complist module vs. plain > completion menu and the change in 38048 does not account for the > latter. I don't know anything about this beyond the fact that showinglist has three values: off, an extra special clever value (-1), and an extra extra special very very clever value (-2), and no normal values (> 0) because that wouldn't be clever enough. The documentation says /* Most lines of the buffer we've shown at once with the current list * * showing. == 0 if there is no list. == -1 if a new list has just * * been put on the screen. == -2 if zrefresh() needs to put up a new * * list. */ /**/ mod_export int showinglist; Is it a case of the two requiring different handling? You might have thought the different behaviour was the one where the new list was needed as in the other cases the screen is already in a consistent state (perhaps?) Something like the following? pws diff --git a/Src/Zle/zle_refresh.c b/Src/Zle/zle_refresh.c index 8d173cd..49ce154 100644 --- a/Src/Zle/zle_refresh.c +++ b/Src/Zle/zle_refresh.c @@ -2434,8 +2434,13 @@ redisplay(UNUSED(char **args)) moveto(0, 0); zputc(&zr_cr); /* extra care */ tc_upcurs(lprompth - 1); - resetneeded = !showinglist; - clearflag = showinglist; + if (showinglist == -2) { + resetneeded = 0; + clearflag = 1; + } else { + resetneeded = 1; + clearflag = 0; + } return 0; } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 'zle redisplay' bug in 5.3? 2017-01-05 15:01 ` 'zle redisplay' bug in 5.3? Peter Stephenson @ 2017-01-05 17:08 ` Bart Schaefer 2017-01-07 18:05 ` Bart Schaefer 0 siblings, 1 reply; 7+ messages in thread From: Bart Schaefer @ 2017-01-05 17:08 UTC (permalink / raw) To: Zsh Hackers' List On Jan 5, 3:01pm, Peter Stephenson wrote: } } The documentation says } } /* Most lines of the buffer we've shown at once with the current list * } * showing. == 0 if there is no list. == -1 if a new list has just * } * been put on the screen. == -2 if zrefresh() needs to put up a new * } * list. */ } } Is it a case of the two requiring different handling? Possibly, but the patch testing for -2 doesn't change anything. If I change it to test showinglist == -1, then the case in this thread works but TRAPINT handlers break it again (the original problem from the thread ending with 38048). So I begin to suspect that testing only showinglist isn't sufficient, but I'm not sure what else to look at. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 'zle redisplay' bug in 5.3? 2017-01-05 17:08 ` Bart Schaefer @ 2017-01-07 18:05 ` Bart Schaefer 2017-01-07 18:53 ` Peter Stephenson 0 siblings, 1 reply; 7+ messages in thread From: Bart Schaefer @ 2017-01-07 18:05 UTC (permalink / raw) To: Zsh Hackers' List } On Jan 5, 3:01pm, Peter Stephenson wrote: } } } } The documentation says } } } } /* Most lines of the buffer we've shown at once with the current list * } } * showing. == 0 if there is no list. == -1 if a new list has just * } } * been put on the screen. == -2 if zrefresh() needs to put up a new * } } * list. */ } } } } Is it a case of the two requiring different handling? That comment is incomplete. showinglist may also have a positive value which is tied to the value of "nlnct" -- which appears to be the number of newlines in the prompt string. Something is repeating the "move up nlnct lines" action twice. This is similar to the description in the thread from 38048. Not sure what to do with this information yet, and out of time to look at it this morning, but maybe it's a clue for someone else. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 'zle redisplay' bug in 5.3? 2017-01-07 18:05 ` Bart Schaefer @ 2017-01-07 18:53 ` Peter Stephenson 2017-01-07 20:04 ` Peter Stephenson 0 siblings, 1 reply; 7+ messages in thread From: Peter Stephenson @ 2017-01-07 18:53 UTC (permalink / raw) To: Zsh Hackers' List On Sat, 7 Jan 2017 10:05:05 -0800 Bart Schaefer <schaefer@brasslantern.com> wrote: > Something is repeating the "move up nlnct lines" action twice. This is > similar to the description in the thread from 38048. I can see two bits in zrefresh() that look like they have to do with this One is to do with listshown, rather than showinglist, that triggers what I take to be the key bit of the refresh code, together with clearlist. The interaction between showinglist and listshown appears to be utterly opaque. The other uses showinglist and then calls listmatches(). Furthermore, it then calls zrefresh() recursively. I modified that in 36416, commit 32f5d3d8, only to get called if there was no error, but the recursive call has always been there. This might have something to do with it, particularly now errflag signals both error and interrupt. Could propagation of ERRFLAG_INT or the lack of it or the fact that it affects the code that calls zrefresh() recursively have something to do with the interrupt problem that caused the changed to redisplaying? pws ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 'zle redisplay' bug in 5.3? 2017-01-07 18:53 ` Peter Stephenson @ 2017-01-07 20:04 ` Peter Stephenson 2017-01-08 4:46 ` Bart Schaefer 0 siblings, 1 reply; 7+ messages in thread From: Peter Stephenson @ 2017-01-07 20:04 UTC (permalink / raw) To: Zsh Hackers' List On Sat, 7 Jan 2017 18:53:25 +0000 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote: > The other uses showinglist and then calls listmatches(). Furthermore, > it then calls zrefresh() recursively. I modified that in 36416, commit > 32f5d3d8, only to get called if there was no error, but the recursive > call has always been there. This might have something to do with > it, particularly now errflag signals both error and interrupt. Could > propagation of ERRFLAG_INT or the lack of it or the fact that it affects > the code that calls zrefresh() recursively have something to do with > the interrupt problem that caused the changed to redisplaying? OK, how about this as an alternative to the previous change to the refresh code? If I'm following properly, this was a problem when TRAPINT () { zle reset-prompt return 127 } was in effect during interruption at a completion list. If we have been interrupted don't try to list matches at all; abort as if there ws no list. Obviously, I can't be sure this doesn't have side effects of its own but as far as I can see it seems to remove the TRAPINT problem without resorting to tweaking bits I haven't the faintest clue about, and it also seems OK without the trap. An additional fix is that at the zle call from the TRAPINT in this case we don't know where the command line has been, so should take account of both possibilities. In general allowing the command line to be modified here is dangerous but stopping it without forbidding all zle calls is difficult, so assume the user is only doing basic stuff here (like redisplay). Note that if the discussion returns nonetheless to showlinglist etc. I shall once again be unable to provide useful replies. pws 1diff --git a/Src/Zle/complist.c b/Src/Zle/complist.c index 2edaf6e..0350388 100644 --- a/Src/Zle/complist.c +++ b/Src/Zle/complist.c @@ -1993,7 +1993,8 @@ complistmatches(UNUSED(Hookdef dummy), Chdata dat) if (noselect > 0) noselect = 0; - if ((minfo.asked == 2 && mselect < 0) || nlnct >= zterm_lines) { + if ((minfo.asked == 2 && mselect < 0) || nlnct >= zterm_lines || + errflag) { showinglist = 0; amatches = oamatches; return (noselect = 1); diff --git a/Src/Zle/zle_refresh.c b/Src/Zle/zle_refresh.c index 8d173cd..8391739 100644 --- a/Src/Zle/zle_refresh.c +++ b/Src/Zle/zle_refresh.c @@ -2434,8 +2434,8 @@ redisplay(UNUSED(char **args)) moveto(0, 0); zputc(&zr_cr); /* extra care */ tc_upcurs(lprompth - 1); - resetneeded = !showinglist; - clearflag = showinglist; + resetneeded = 1; + clearflag = 0; return 0; } diff --git a/Src/Zle/zle_thingy.c b/Src/Zle/zle_thingy.c index c709285..c003148 100644 --- a/Src/Zle/zle_thingy.c +++ b/Src/Zle/zle_thingy.c @@ -703,7 +703,7 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func)) { Thingy t; struct modifier modsave = zmod; - int ret, saveflag = 0, setbindk = 0; + int ret, saveflag = 0, setbindk = 0, remetafy; char *wname = *args++, *keymap_restore = NULL, *keymap_tmp; if (!wname) @@ -714,7 +714,15 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func)) return 1; } - UNMETACHECK(); + /* + * zle is callable in traps, so we can't be sure the line is + * in its normal state. + */ + if (zlemetaline) { + unmetafy_line(); + remetafy = 1; + } else + remetafy = 0; while (*args && **args == '-') { char *num; @@ -728,6 +736,8 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func)) num = args[0][1] ? args[0]+1 : args[1]; if (!num) { zwarnnam(name, "number expected after -%c", **args); + if (remetafy) + metafy_line(); return 1; } if (!args[0][1]) @@ -745,19 +755,26 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func)) keymap_tmp = args[0][1] ? args[0]+1 : args[1]; if (!keymap_tmp) { zwarnnam(name, "keymap expected after -%c", **args); + if (remetafy) + metafy_line(); return 1; } if (!args[0][1]) *++args = "" - 1; keymap_restore = dupstring(curkeymapname); - if (selectkeymap(keymap_tmp, 0)) + if (selectkeymap(keymap_tmp, 0)) { + if (remetafy) + metafy_line(); return 1; + } break; case 'w': setbindk = 1; break; default: zwarnnam(name, "unknown option: %s", *args); + if (remetafy) + metafy_line(); return 1; } } @@ -775,6 +792,8 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func)) zmod = modsave; if (keymap_restore) selectkeymap(keymap_restore, 0); + if (remetafy) + metafy_line(); return ret; } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 'zle redisplay' bug in 5.3? 2017-01-07 20:04 ` Peter Stephenson @ 2017-01-08 4:46 ` Bart Schaefer 2017-01-08 17:59 ` Peter Stephenson 0 siblings, 1 reply; 7+ messages in thread From: Bart Schaefer @ 2017-01-08 4:46 UTC (permalink / raw) To: Peter Stephenson, Zsh Hackers' List On Jan 7, 8:04pm, Peter Stephenson wrote: } Subject: Re: 'zle redisplay' bug in 5.3? } } OK, how about this as an alternative to the previous change to the } refresh code? If I'm following properly, this was a problem when } } TRAPINT () { } zle reset-prompt } return 127 } } } } was in effect during interruption at a completion list. That's correct. However, this patch (checking errflag) does not fix the issue with interrupts. Here's how to reproduce: zsh -f autoload compinit compinit -u -D TRAPINT () { zle reset-prompt return 127 } _delay() { sleep 10 } zstyle \* completer _complete _delay Now complete after e.g. "ls " and press ^C within 10 seconds. The complist module does not need to be involved, so a change only there does not help. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 'zle redisplay' bug in 5.3? 2017-01-08 4:46 ` Bart Schaefer @ 2017-01-08 17:59 ` Peter Stephenson 0 siblings, 0 replies; 7+ messages in thread From: Peter Stephenson @ 2017-01-08 17:59 UTC (permalink / raw) To: Zsh Hackers' List On Sat, 7 Jan 2017 20:46:40 -0800 Bart Schaefer <schaefer@brasslantern.com> wrote: > On Jan 7, 8:04pm, Peter Stephenson wrote: > } Subject: Re: 'zle redisplay' bug in 5.3? > } > } OK, how about this as an alternative to the previous change to the > } refresh code? If I'm following properly, this was a problem when > } > } TRAPINT () { > } zle reset-prompt > } return 127 > } } > } > } was in effect during interruption at a completion list. > > That's correct. However, this patch (checking errflag) does not fix > the issue with interrupts. There isn't a *the* issue, is there? There can't be; the interrupt can go off absolutely anywhere. I've fixed the one with complist, which is what was originally reported, now you're saying there's something else... > autoload compinit > compinit -u -D > TRAPINT () { > zle reset-prompt > return 127 > } > _delay() { sleep 10 } > zstyle \* completer _complete _delay > > Now complete after e.g. "ls " and press ^C within 10 seconds. The > complist module does not need to be involved, so a change only there > does not help. That is indeed a completely different problem, at least the problem that shows up when I try this certainly is. I'm not sure the _delay has got much to do with it, though, so it's possible there's yet more. We're not actually in or under completion code here --- though the hook mechanism makes that statement less useful than it might otherwise be. However, that does mean that in this case we're not aborting a display --- we're aborting to a new command line. (That's the same as if you just do the straight compinit then the same key sequence, but the modification of the completer means that you ought to look at the internal state when the interrupt happens to confirm, which I've done.) It turns out the prompt doesn't get redrawn on that line because clearflag isn't initialised to zero, even though it's value is meaningless when we restart ZLE for the new line as we should redraw everything from scratch --- that's the residual effect of the completion code's hook using dolastprompt, I think. It looks like the line we're aborting isn't completely tidied up, either, there's a bit of stray output from the reset-prompt, but I consider that very minor as we're aborting it completely, and separate again. I intend to commit this. Please we can take problems one at a time? diff --git a/Src/Zle/complist.c b/Src/Zle/complist.c index 2edaf6e..0350388 100644 --- a/Src/Zle/complist.c +++ b/Src/Zle/complist.c @@ -1993,7 +1993,8 @@ complistmatches(UNUSED(Hookdef dummy), Chdata dat) if (noselect > 0) noselect = 0; - if ((minfo.asked == 2 && mselect < 0) || nlnct >= zterm_lines) { + if ((minfo.asked == 2 && mselect < 0) || nlnct >= zterm_lines || + errflag) { showinglist = 0; amatches = oamatches; return (noselect = 1); diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c index 15ea796..6c271b5 100644 --- a/Src/Zle/zle_main.c +++ b/Src/Zle/zle_main.c @@ -1245,6 +1245,7 @@ zleread(char **lp, char **rp, int flags, int context, char *init, char *finish) resetneeded = 0; fetchttyinfo = 0; trashedzle = 0; + clearflag = 0; raw_lp = lp; lpromptbuf = promptexpand(lp ? *lp : NULL, 1, NULL, NULL, &pmpt_attr); raw_rp = rp; diff --git a/Src/Zle/zle_refresh.c b/Src/Zle/zle_refresh.c index 8d173cd..8391739 100644 --- a/Src/Zle/zle_refresh.c +++ b/Src/Zle/zle_refresh.c @@ -2434,8 +2434,8 @@ redisplay(UNUSED(char **args)) moveto(0, 0); zputc(&zr_cr); /* extra care */ tc_upcurs(lprompth - 1); - resetneeded = !showinglist; - clearflag = showinglist; + resetneeded = 1; + clearflag = 0; return 0; } diff --git a/Src/Zle/zle_thingy.c b/Src/Zle/zle_thingy.c index c709285..c003148 100644 --- a/Src/Zle/zle_thingy.c +++ b/Src/Zle/zle_thingy.c @@ -703,7 +703,7 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func)) { Thingy t; struct modifier modsave = zmod; - int ret, saveflag = 0, setbindk = 0; + int ret, saveflag = 0, setbindk = 0, remetafy; char *wname = *args++, *keymap_restore = NULL, *keymap_tmp; if (!wname) @@ -714,7 +714,15 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func)) return 1; } - UNMETACHECK(); + /* + * zle is callable in traps, so we can't be sure the line is + * in its normal state. + */ + if (zlemetaline) { + unmetafy_line(); + remetafy = 1; + } else + remetafy = 0; while (*args && **args == '-') { char *num; @@ -728,6 +736,8 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func)) num = args[0][1] ? args[0]+1 : args[1]; if (!num) { zwarnnam(name, "number expected after -%c", **args); + if (remetafy) + metafy_line(); return 1; } if (!args[0][1]) @@ -745,19 +755,26 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func)) keymap_tmp = args[0][1] ? args[0]+1 : args[1]; if (!keymap_tmp) { zwarnnam(name, "keymap expected after -%c", **args); + if (remetafy) + metafy_line(); return 1; } if (!args[0][1]) *++args = "" - 1; keymap_restore = dupstring(curkeymapname); - if (selectkeymap(keymap_tmp, 0)) + if (selectkeymap(keymap_tmp, 0)) { + if (remetafy) + metafy_line(); return 1; + } break; case 'w': setbindk = 1; break; default: zwarnnam(name, "unknown option: %s", *args); + if (remetafy) + metafy_line(); return 1; } } @@ -775,6 +792,8 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func)) zmod = modsave; if (keymap_restore) selectkeymap(keymap_restore, 0); + if (remetafy) + metafy_line(); return ret; } ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-01-08 17:59 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20170105030137.v4tzweda6pxyqnrq@majutsushi.net> [not found] ` <CGME20170105091042epcas1p3ebc6c107d3121c8f6b980bbcbc59bc60@epcas1p3.samsung.com> [not found] ` <170105010914.ZM1529@torch.brasslantern.com> 2017-01-05 15:01 ` 'zle redisplay' bug in 5.3? Peter Stephenson 2017-01-05 17:08 ` Bart Schaefer 2017-01-07 18:05 ` Bart Schaefer 2017-01-07 18:53 ` Peter Stephenson 2017-01-07 20:04 ` Peter Stephenson 2017-01-08 4:46 ` Bart Schaefer 2017-01-08 17:59 ` 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).