[-- Attachment #1: Type: text/plain, Size: 1636 bytes --] Many terminals reflow text when the window is resized. When the height of the prompt changes as a result of this reflowing, ZSH draws the updated prompt on the wrong line. This can lead to some parts of the prompt not being erased, or to the disappearance of lines prior to the prompt. There are many ways to reproduce this issue. Here are a couple. Both require terminals that reflow text when the window is resized. 1. Run `zsh -df`, hit <enter> a few times, then type `xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx` (don't hit <enter>). OR 2. Run `PROMPT="${(pl.$COLUMNS..-.)}%f"$'\n> ' zsh -df` and hit <enter> a few times. Now try resizing the terminal window back and forth causing lines to wrap and unwrap. Terminal content before the last prompt will be erased one line at a time. This patch cannot handle the case when the terminal window is being resized while the first prompt line is outside the terminal window. The content of the viewport will be correct but scrolling the terminal window up will reveal some mess up there. ZSH before this patch also fails in this case although it creates a different mess. The change is conservative. The new code triggers only on window resize and not, for example, on redisplay. This reduces the chance that it'll break something that isn't currently broken. I've tested this code only on GNOME Terminal. Before I go testing on a dozen different terminals I'd like to get some feedback. Anything I'm missing? Anything tricky to look out for? The patch is attached to the email. You can also see it in https://github.com/zsh-users/zsh/compare/master...romkatv:fix-winchanged. Roman. [-- Attachment #2: window-resize-fix.patch --] [-- Type: text/x-patch, Size: 2390 bytes --] diff --git a/Src/Zle/zle_refresh.c b/Src/Zle/zle_refresh.c index 85e55e0d4..249d75d71 100644 --- a/Src/Zle/zle_refresh.c +++ b/Src/Zle/zle_refresh.c @@ -660,6 +660,7 @@ static int more_start, /* more text before start of screen? */ lpromptw, rpromptw, /* prompt widths on screen */ lpromptwof, /* left prompt width with real end position */ lprompth, /* lines taken up by the prompt */ + cursorsaved, /* whether prompt start position was saved */ rprompth, /* right prompt height */ vcs, vln, /* video cursor position column & line */ vmaxln, /* video maximum number of lines */ @@ -994,6 +995,7 @@ zrefresh(void) int remetafy; /* flag that zle line is metafied */ zattr txtchange; /* attributes set after prompts */ int rprompt_off = 1; /* Offset of rprompt from right of screen */ + int savecursorneeded = 0; /* prompt start position needs to be saved */ struct rparams rpms; #ifdef MULTIBYTE_SUPPORT int width; /* width of wide character */ @@ -1133,7 +1135,13 @@ zrefresh(void) zsetterm(); #ifdef TIOCGWINSZ if (winchanged) { - moveto(0, 0); + if (cursorsaved) { + tcout(TCRESTRCURSOR); + zputc(&zr_cr); + vln = vcs = 0; + } else { + moveto(0, 0); + } t0 = olnct; /* this is to clear extra lines even when */ winchanged = 0; /* the terminal cannot TCCLEAREOD */ listshown = 0; @@ -1164,7 +1172,16 @@ zrefresh(void) if (termflags & TERM_SHORT) vcs = 0; else if (!clearflag && lpromptbuf[0]) { - zputs(lpromptbuf, shout); + cursorsaved = 0; + if (tccan(TCSAVECURSOR) && tccan(TCRESTRCURSOR)) { + if (!(termflags & TERM_SHORT)) { + savecursorneeded = 1; + } else if (lprompth <= rwinh) { + tcout(TCSAVECURSOR); + cursorsaved = 1; + } + } + zputs(lpromptbuf, shout); if (lpromptwof == winw) zputs("\n", shout); /* works with both hasam and !hasam */ } else { @@ -1737,6 +1754,12 @@ individually */ clearf = 0; oput_rpmpt = put_rpmpt; + if (savecursorneeded && lprompth + nlnct <= rwinh) { + moveto(1 - lprompth, 0); + tcout(TCSAVECURSOR); + cursorsaved = 1; + } + /* move to the new cursor position */ moveto(rpms.nvln, rpms.nvcs);
On 7/15/19, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
> Many terminals reflow text when the window is resized. When the height
> of the prompt changes as a result of this reflowing, ZSH draws the
> updated prompt on the wrong line. This can lead to some parts of the
> prompt not being erased, or to the disappearance of lines prior to the
> prompt.
>
> There are many ways to reproduce this issue. Here are a couple. Both
> require terminals that reflow text when the window is resized.
>
> 1. Run `zsh -df`, hit <enter> a few times, then type
> `xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx` (don't hit <enter>).
>
> OR
>
> 2. Run `PROMPT="${(pl.$COLUMNS..-.)}%f"$'\n> ' zsh -df`
> and hit <enter> a few times.
>
> Now try resizing the terminal window back and forth causing lines to
> wrap and unwrap. Terminal content before the last prompt will be
> erased one line at a time.
>
> This patch cannot handle the case when the terminal window is being
> resized while the first prompt line is outside the terminal window.
> The content of the viewport will be correct but scrolling the terminal
> window up will reveal some mess up there. ZSH before this patch also
> fails in this case although it creates a different mess.
>
> The change is conservative. The new code triggers only on window
> resize and not, for example, on redisplay. This reduces the chance
> that it'll break something that isn't currently broken.
>
> I've tested this code only on GNOME Terminal. Before I go testing on a
> dozen different terminals I'd like to get some feedback. Anything I'm
> missing? Anything tricky to look out for?
I've tested it on urxvt and it makes things worse there, enlarging the
window always deletes the last output line even with an empty input
line.
--
Mikael Magnusson
On Mon, Jul 15, 2019 at 9:14 PM Mikael Magnusson <mikachu@gmail.com> wrote:
>
> I've tested it on urxvt and it makes things worse there, enlarging the
> window always deletes the last output line even with an empty input
> line.
Thanks for trying it out.
I cannot reproduce this with the following procedure:
1. Ubuntu 18.04, fresh install.
2. apt install rxvt-unicode (v9.22)
3. urxvt
4. PROMPT="${(pl.$COLUMNS..-.)}%f"$'\n> ' zsh -df
5. reset
6. Hit <enter> a few times.
7. Resize terminal window back and forth, both fast and slow.
What should I do to see lines being deleted when enlarging the window?
Roman.
On Mon, Jul 15, 2019 at 9:32 PM Roman Perepelitsa
<roman.perepelitsa@gmail.com> wrote:
>
> On Mon, Jul 15, 2019 at 9:14 PM Mikael Magnusson <mikachu@gmail.com> wrote:
> >
> > I've tested it on urxvt and it makes things worse there, enlarging the
> > window always deletes the last output line even with an empty input
> > line.
>
> Thanks for trying it out.
>
> I cannot reproduce this with the following procedure:
>
> 1. Ubuntu 18.04, fresh install.
> 2. apt install rxvt-unicode (v9.22)
> 3. urxvt
> 4. PROMPT="${(pl.$COLUMNS..-.)}%f"$'\n> ' zsh -df
> 5. reset
> 6. Hit <enter> a few times.
> 7. Resize terminal window back and forth, both fast and slow.
>
> What should I do to see lines being deleted when enlarging the window?
P.S.
I've tried the same steps on ZSH without my patch. The current prompt
moves one line up every time I enlarge the window, erasing the
previous line in the process. This is the sort of issue my patch is
meant to fix.
On Mon, Jul 15, 2019 at 10:21 AM Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote: > > I've tested this code only on GNOME Terminal. Before I go testing on a > dozen different terminals I'd like to get some feedback. Anything I'm > missing? Anything tricky to look out for? The most significant problem with window resizing is not the terminal itself, but window/session managers that try to do "live update" of the window contents as the cursor is rapidly dragged around. These tend to send a stream of WINCH signals which we have had to do various tricks to manage. I would be concerned that attempting to write a control sequence to the terminal and read back the response in the midst of a flood of such signals would fail as often as it succeeds. If this is already limited/controlled by the mechanisms in place, great. > The patch is attached to the email. You can also see it in > https://github.com/zsh-users/zsh/compare/master...romkatv:fix-winchanged. Thanks for the link, it's the only way I was able to see this on a mobile device. Please inline patches, or attach them as text/plain (not text/x-something), without base64 encoding if possible.
On Mon, Jul 15, 2019 at 11:24 PM Bart Schaefer <schaefer@brasslantern.com> wrote: > > On Mon, Jul 15, 2019 at 10:21 AM Roman Perepelitsa > <roman.perepelitsa@gmail.com> wrote: > > > > I've tested this code only on GNOME Terminal. Before I go testing on a > > dozen different terminals I'd like to get some feedback. Anything I'm > > missing? Anything tricky to look out for? > > The most significant problem with window resizing is not the terminal > itself, but window/session managers that try to do "live update" of > the window contents as the cursor is rapidly dragged around. These > tend to send a stream of WINCH signals which we have had to do various > tricks to manage. I would be concerned that attempting to write a > control sequence to the terminal and read back the response in the > midst of a flood of such signals would fail as often as it succeeds. > If this is already limited/controlled by the mechanisms in place, > great. This patch isn't only doing writes to the terminal without any reads. Whenever ZLE prints prompt, it issues TCSAVECURSOR which instruct the terminal to save the position of the cursor in a special stash. When window resizes, ZLE issues TCRESTRCURSOR to tell the terminal to move the cursor to the previously saved position. The nice thing about these commands is that the stashed cursor position will still be in the right place after the terminal has reflowed text. I _think_ this shouldn't window/session managers but I'll definitely need to test it first. Do you have suggestions on what I should test with? I have a battery of 13 terminals and 4 operating systems that I use for testing changes but I'm not sure any of my configurations qualify as high-risk for this patch. > > The patch is attached to the email. You can also see it in > > https://github.com/zsh-users/zsh/compare/master...romkatv:fix-winchanged. > > Thanks for the link, it's the only way I was able to see this on a > mobile device. Please inline patches, or attach them as text/plain > (not text/x-something), without base64 encoding if possible. Sorry about that. I don't think it's possible to specify content type for attachments in GMail. Perhaps if I attach *.txt instead of *.patch it'll become "text/plain"? I'll try it next time. Roman.
On Tue, Jul 16, 2019 at 12:09 AM Roman Perepelitsa
<roman.perepelitsa@gmail.com> wrote:
> This patch isn't only doing writes to the terminal without any reads.
Misleading typo ^. Should be: "this patch is only doing writes ...".
On Mon, Jul 15, 2019 at 3:09 PM Roman Perepelitsa
<roman.perepelitsa@gmail.com> wrote:
>
> I _think_ this shouldn't window/session managers but I'll definitely
> need to test it first. Do you have suggestions on what I should test
> with?
Install some of the alternate window managers provided with the
various OSs you have, rather than the default ones. There's a
plethora of them. The main thing is one that does live redraw while
resizing windows, rather than having you drag a window outline around
and then snapping the window to the outline when you release the
mouse.
On Mon, Jul 15, 2019 at 3:09 PM Roman Perepelitsa
<roman.perepelitsa@gmail.com> wrote:
>
> This patch [is] only doing writes to the terminal without any reads.
> Whenever ZLE prints prompt, it issues TCSAVECURSOR
OK, then there is a different problem: Nothing prevents the prompt
itself from printing TCSAVECURSOR/TCRESTRCURSOR, and in fact some of
the themed prompts provided with zsh do that. There's no way for zsh
to guarantee that the cursorsaved flag is accurate.
On Tue, Jul 16, 2019 at 1:28 AM Bart Schaefer <schaefer@brasslantern.com> wrote: > Install some of the alternate window managers provided with the > various OSs you have, rather than the default ones. There's a > plethora of them. The main thing is one that does live redraw while > resizing windows, rather than having you drag a window outline around > and then snapping the window to the outline when you release the > mouse. Got it. So far all tests I've done were with window managers that redraw the window continuously while it's being resized. I'll make sure to test with those that don't redraw until the mouse button is released. > > This patch [is] only doing writes to the terminal without any reads. > > Whenever ZLE prints prompt, it issues TCSAVECURSOR > > OK, then there is a different problem: Nothing prevents the prompt > itself from printing TCSAVECURSOR/TCRESTRCURSOR, and in fact some of > the themed prompts provided with zsh do that. There's no way for zsh > to guarantee that the cursorsaved flag is accurate. In my patch I attempted to not break code that stores/restores cursor. It'll work in all the following cases: 1. The cursor is saved and restored by a foreground process. 2. The cursor is saved and restored in precmd. 3. The cursor is saved and restored in preexec. 4. The cursor is saved and restored by PROMPT and/or RPROMPT. Cursor cannot be restored if the terminal window may have been scrolled due to extra lines added at the bottom. This limits the number of places where anyone can save/restore cursors. For example, saving the cursor in precmd and restoring it in preexec won't work because there can be new lines added in between (unless single_line_zle is set, but in this case my patch doesn't save/restore cursor). I _think_ my code doesn't break working code but I may be wrong. What should I test with? Which theme(s) save and restore cursor? It's worth reiterating that currently prompt refresh on window change is quite broken. Even if my patch isn't perfect, it still may be an improvement (it is so far in all my tests). Roman.
On Mon, 15 Jul 2019 at 19:21, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote: > > Many terminals reflow text when the window is resized. > (...) > This patch cannot handle the case when the terminal window is being > resized while the first prompt line is outside the terminal window. > The content of the viewport will be correct but scrolling the terminal > window up will reveal some mess up there. ZSH before this patch also > fails in this case although it creates a different mess. I thought that I'll bump this patch. The bug is very annoying, it constantly appears in the reports to the list starting probably from 2016 when I've joined (well, that was in 2015, actually). Take a look at a mess that it creates: https://www.reddit.com/r/zsh/comments/ejeeb1/. -- Sebastian Gniazdowski News: https://twitter.com/ZdharmaI IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin Blog: http://zdharma.org
On Mon, Jan 6, 2020 at 6:50 PM Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote: > I thought that I'll bump this patch. The bug is very annoying, it > constantly appears in the reports to the list starting probably from > 2016 when I've joined (well, that was in 2015, actually). As another data point, I received a total of 7 bug reports in powerlevel10k about this bug. Here's my most extensive explanation of what is going on: https://github.com/romkatv/powerlevel10k/issues/234#issuecomment-544387392. I keep my patch available for anyone who's brave enough to use a patched zsh. I use it myself, too. I should clarify that the ball is totally on my side. The patch received cool feedback but wasn't rejected outright. I estimated that effort required to turn the tide is likely higher than I can muster and chose to withdraw. Roman.
On Mon, 6 Jan 2020 at 19:07, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote: > I should clarify that the ball is totally on my side. The patch > received cool feedback but wasn't rejected outright. I estimated that > effort required to turn the tide is likely higher than I can muster > and chose to withdraw. From what I've read: 1. Bart said that using the terminals feature to save cursor position might collide with application's use of the the feature IMO, that's a valid point, however, it's mostly about being valid, not about making the fix happen. We need the terminal feature to fix the bug, to resolve the potential collision with an application we could provide an option. I recall how easy it was for Peter to add GLOBSTAR_SHORT option, so this shouldn't be a blockade. 2. Mikael wrote what you've verified to be an unfortunate run of unpatched Zsh instead of the patched Zsh. So, I think that there's not much to do on your side. -- Sebastian Gniazdowski News: https://twitter.com/ZdharmaI IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin Blog: http://zdharma.org
On Mon, Jan 6, 2020 at 7:31 PM Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote: > > On Mon, 6 Jan 2020 at 19:07, Roman Perepelitsa > <roman.perepelitsa@gmail.com> wrote: > > I should clarify that the ball is totally on my side. The patch > > received cool feedback but wasn't rejected outright. I estimated that > > effort required to turn the tide is likely higher than I can muster > > and chose to withdraw. > > From what I've read: > 1. Bart said that using the terminals feature to save cursor position > might collide with application's use of the the feature Yes, I saw that. I believe this patch doesn't break any existing useful code. If anyone can point me to a piece of code that could be broken by my patch, I'll test it. > So, I think that there's not much to do on your side. I gave up because because no one has expressed enthusiasm and two members in good standing expressed skepticism that I was unable to address. No one is obligated to accept my patches unless I put in the work to convince the community that the patch does something useful and doesn't break things. It didn't seem like I was making progress on either of these fronts, so I gave up. Roman.
On Mon, 6 Jan 2020 at 19:46, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote: > > I gave up because because no one has expressed enthusiasm and two > members in good standing expressed skepticism that I was unable to > address. No one is obligated to accept my patches unless I put in the > work to convince the community that the patch does something useful > and doesn't break things. It didn't seem like I was making progress on > either of these fronts, so I gave up. I hope that the patch will be accepted. It's an innovative approach to solving what appeared to be unsolvable (the inability for Zsh to track what's happening on the screen, in the terminal). -- Sebastian Gniazdowski News: https://twitter.com/ZdharmaI IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin Blog: http://zdharma.org
[-- Attachment #1: Type: text/plain, Size: 664 bytes --] On Mon, Jan 6, 2020, 11:03 AM Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote: > On Mon, 6 Jan 2020 at 19:46, Roman Perepelitsa > <roman.perepelitsa@gmail.com> wrote: > > > > I gave up because because no one has expressed enthusiasm and two > > members in good standing expressed skepticism that I was unable to > > address. > > I hope that the patch will be accepted. It's an innovative approach to > solving what appeared to be unsolvable > IIR the problem correctly, a possible approach to resolve my concern would be to have the terminal report the cursor position for the shell to read+save it, rather than rely on the terminal save/restore action. >
On Mon, Jan 6, 2020 at 8:28 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> IIR the problem correctly, a possible approach to resolve my concern would be to have the terminal report the cursor position for the shell to read+save it, rather than rely on the terminal save/restore action.
I implemented this prior to the patch I sent. Unfortunately, this
added enough prompt latency to make it noticeable. I suppose some
users might value correct WINCH behavior above fast prompt but I'm not
one of them. When I switched to save/restore cursor, I haven't looked
back. It looks to me like a strict improvement over the current state.
I think it doesn't break or slow anything down, while at the same time
it fixes WINCH handling in most cases (not in all cases though, see my
original email). I might be wrong, and there might be code that gets
broken. If you know any code that might be broken by my patch due to
its use of save/restore cursor, please point me to it and I'll test
my patch against it.
Roman.
On Mon, 6 Jan 2020 at 20:28, Bart Schaefer <schaefer@brasslantern.com> wrote: > > IIR the problem correctly, a possible approach to resolve my concern would be to have the terminal report the cursor position for the shell to read+save it, rather than rely on the terminal save/restore action. Ah, and that's doable by the \e[6n escape, like in the promptnl function: https://github.com/zsh-users/zsh/blob/master/Functions/Misc/promptnl#L25 -- Sebastian Gniazdowski News: https://twitter.com/ZdharmaI IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin Blog: http://zdharma.org
On Mon, Jan 6, 2020 at 8:38 PM Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote: > > On Mon, Jan 6, 2020 at 8:28 PM Bart Schaefer <schaefer@brasslantern.com> wrote: > > IIR the problem correctly, a possible approach to resolve my concern would be to have the terminal report the cursor position for the shell to read+save it, rather than rely on the terminal save/restore action. > > I implemented this prior to the patch I sent. Unfortunately, this > added enough prompt latency to make it noticeable. I've implemented this again to verify that my recollection of the reason I rejected this approach was correct. It was not. u7 is indeed slow but the primary reason I abandoned it in the first implementation is that it doesn't work here. The absolute position of prompt start changes when terminal content gets reflown upon resize. After a few more issues filed against powerlevel10k due to the resizing bug, I wrote this troubleshooting entry: https://github.com/romkatv/powerlevel10k/blob/master/README.md#horrific-mess-when-resizing-terminal-window. Perhaps it'll help others who stumble on this. Roman.