magicant / yash

Yet another shell
http://magicant.github.io/yash/
GNU General Public License v2.0
301 stars 28 forks source link

b$ is printed when resizing kitty #44

Open belka-ew opened 4 months ago

belka-ew commented 4 months ago

Describe the bug

kitty is a terminal emulator supporting window split and layouts (horizontal and vertical splits). When the size of a single pane is changed, a new line with b$ is printed on another one. Interestingly this happens only if I have some input in the prompt. Kitty has integrations with some shells, but I tried the same with some other shells without integration and it doesn't seem to happen.

To Reproduce Steps to reproduce the behavior:

  1. Open kitty with yash
  2. Enter something into the prompt
  3. Split the view (Ctrl-Shift-Enter by default)
  4. Go to the other pane for example by clicking on it
  5. Start resizing (Ctrl-Shift-R by default)
  6. Make the pane shorter or taller (s or t).

Expected behavior Resizing works without superfluous characters being printed.

Screenshots Screenshot_20240222_093353 Screenshot_20240222_095306

Environment (please complete the following information):

magicant commented 4 months ago

It seems kitty moves the cursor 1 column to the right for unknown reasons when the vertical window size is changed. ksh and mksh also seem to have trouble about the cursor position after being resized in kitty.

When the window is resized, yash tries to clear and re-display the entire prompt and command line. It seems the cursor is positioned on the second column (rather than the first) of the line just after the line is cleared, which is why the first character of the prompt is left behind. The dollar sign is coming from the effect of the lepromptsp option, which makes sure the new prompt starts from the first column of the line by printing dummy characters. You should not see the dollar if the prompt starts from the beginning of the line as expected.

belka-ew commented 4 months ago

Thanks for explaining where it comes from.

I looked into the kitty's issues and found a few closed issues describing problems with text and cursor position after resizing a window. I think it has something to do with the logic how kitty reflows the text after the view changes. For example here is one such issue: https://github.com/kovidgoyal/kitty/issues/5635. If you look into the commit fixing this issue there is a change of "some" X coordinate in rewrap.h:

-                    t->x = dest_x + (t->x - src_x + 1);
+                    t->x = dest_x + (t->x - src_x + (t->x > 0));

So 1 at the end of the line is replaced with t->x > 0. For the sake of experiment without looking deeper into it I replaced t->x > 0 with 0 and it fixed this paticular problem.

Do you think its a kitty problem or some edge case that can be fixed in yash?

magicant commented 4 months ago

The patch below may work, but since it is impossible for the shell to keep exact track of the cursor position moved by the terminal, especially when it is resized horizontally, it would be best that the terminal takes care of the cursor's movement.

diff --git a/lineedit/display.c b/lineedit/display.c
index 0e8f938a..7a4bd20e 100644
--- a/lineedit/display.c
+++ b/lineedit/display.c
@@ -490,8 +490,11 @@ void le_display_clear(bool clear)
         lebuf_print_sgr0();
         if (clear)
             lebuf_print_clear();
-        else
+        else {
             go_to((le_pos_T) { 0, 0 });
+            if (shopt_le_promptsp)
+                lebuf_print_cr();
+        }
         finish();
     }
 }
belka-ew commented 4 months ago

Yes, it works.

Some terminals support additional codes for shell integration, so kitty: https://sw.kovidgoyal.net/kitty/shell-integration/.

I tried to add the following sequence:

            lebuf_putchar('\x1b');
            lebuf_putchar('\x5d');
            lebuf_putchar('1');
            lebuf_putchar('3');
            lebuf_putchar('3');
            lebuf_putchar(';');
            lebuf_putchar('A');
            lebuf_putchar('\x1b');
            lebuf_putchar('\x5c');

at the beginning of le_display_flush() and it fixes the problem as well. But I suppose it is not the right place for it. Maybe it is a more correct approach.

By the way I noticed that I have this issue only if YASH_PS1R is set.

It marks the position of the prompt as far as I understand.

Brixy commented 1 month ago

Something similar happens in foot.

Without resizing some obsolete $ are show before the prompt:

$                                                              $                                      $ 

But it's OK because the prompt always starts with a new line (yet, apparently without a line break). Resizing adds more $.

Yet, when I add this line to .yashrc the problem disappears, even if /etc/profile was loaded by another login shell:

[ -f "/etc/profile" ] && . "/etc/profile"