zsh-workers
 help / Atom feed
* PATCH: draw prompt on the correct line after window change
@ 2019-07-15 17:20 Roman Perepelitsa
  2019-07-15 19:14 ` Mikael Magnusson
  2019-07-15 21:24 ` Bart Schaefer
  0 siblings, 2 replies; 10+ messages in thread
From: Roman Perepelitsa @ 2019-07-15 17:20 UTC (permalink / raw)
  To: Zsh hackers list

[-- 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);
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: draw prompt on the correct line after window change
  2019-07-15 17:20 PATCH: draw prompt on the correct line after window change Roman Perepelitsa
@ 2019-07-15 19:14 ` Mikael Magnusson
  2019-07-15 19:32   ` Roman Perepelitsa
  2019-07-15 21:24 ` Bart Schaefer
  1 sibling, 1 reply; 10+ messages in thread
From: Mikael Magnusson @ 2019-07-15 19:14 UTC (permalink / raw)
  To: Roman Perepelitsa; +Cc: Zsh hackers list

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: draw prompt on the correct line after window change
  2019-07-15 19:14 ` Mikael Magnusson
@ 2019-07-15 19:32   ` Roman Perepelitsa
  2019-07-15 19:41     ` Roman Perepelitsa
  0 siblings, 1 reply; 10+ messages in thread
From: Roman Perepelitsa @ 2019-07-15 19:32 UTC (permalink / raw)
  To: Mikael Magnusson; +Cc: Zsh hackers list

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: draw prompt on the correct line after window change
  2019-07-15 19:32   ` Roman Perepelitsa
@ 2019-07-15 19:41     ` Roman Perepelitsa
  0 siblings, 0 replies; 10+ messages in thread
From: Roman Perepelitsa @ 2019-07-15 19:41 UTC (permalink / raw)
  To: Mikael Magnusson; +Cc: Zsh hackers list

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: draw prompt on the correct line after window change
  2019-07-15 17:20 PATCH: draw prompt on the correct line after window change Roman Perepelitsa
  2019-07-15 19:14 ` Mikael Magnusson
@ 2019-07-15 21:24 ` Bart Schaefer
  2019-07-15 22:09   ` Roman Perepelitsa
  1 sibling, 1 reply; 10+ messages in thread
From: Bart Schaefer @ 2019-07-15 21:24 UTC (permalink / raw)
  To: Roman Perepelitsa; +Cc: Zsh hackers list

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: draw prompt on the correct line after window change
  2019-07-15 21:24 ` Bart Schaefer
@ 2019-07-15 22:09   ` Roman Perepelitsa
  2019-07-15 22:11     ` Roman Perepelitsa
                       ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Roman Perepelitsa @ 2019-07-15 22:09 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: draw prompt on the correct line after window change
  2019-07-15 22:09   ` Roman Perepelitsa
@ 2019-07-15 22:11     ` Roman Perepelitsa
  2019-07-15 23:28     ` Bart Schaefer
  2019-07-16  0:01     ` Bart Schaefer
  2 siblings, 0 replies; 10+ messages in thread
From: Roman Perepelitsa @ 2019-07-15 22:11 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: draw prompt on the correct line after window change
  2019-07-15 22:09   ` Roman Perepelitsa
  2019-07-15 22:11     ` Roman Perepelitsa
@ 2019-07-15 23:28     ` Bart Schaefer
  2019-07-16  8:16       ` Roman Perepelitsa
  2019-07-16  0:01     ` Bart Schaefer
  2 siblings, 1 reply; 10+ messages in thread
From: Bart Schaefer @ 2019-07-15 23:28 UTC (permalink / raw)
  To: Roman Perepelitsa; +Cc: Zsh hackers list

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: draw prompt on the correct line after window change
  2019-07-15 22:09   ` Roman Perepelitsa
  2019-07-15 22:11     ` Roman Perepelitsa
  2019-07-15 23:28     ` Bart Schaefer
@ 2019-07-16  0:01     ` Bart Schaefer
  2 siblings, 0 replies; 10+ messages in thread
From: Bart Schaefer @ 2019-07-16  0:01 UTC (permalink / raw)
  To: Roman Perepelitsa; +Cc: Zsh hackers list

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: draw prompt on the correct line after window change
  2019-07-15 23:28     ` Bart Schaefer
@ 2019-07-16  8:16       ` Roman Perepelitsa
  0 siblings, 0 replies; 10+ messages in thread
From: Roman Perepelitsa @ 2019-07-16  8:16 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-15 17:20 PATCH: draw prompt on the correct line after window change Roman Perepelitsa
2019-07-15 19:14 ` Mikael Magnusson
2019-07-15 19:32   ` Roman Perepelitsa
2019-07-15 19:41     ` Roman Perepelitsa
2019-07-15 21:24 ` Bart Schaefer
2019-07-15 22:09   ` Roman Perepelitsa
2019-07-15 22:11     ` Roman Perepelitsa
2019-07-15 23:28     ` Bart Schaefer
2019-07-16  8:16       ` Roman Perepelitsa
2019-07-16  0:01     ` Bart Schaefer

zsh-workers

Archives are clonable: git clone --mirror http://inbox.vuxu.org/zsh-workers

Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.zsh.workers


AGPL code for this site: git clone https://public-inbox.org/ public-inbox