Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nix repl rendering breaks after resizing terminal #3500

Open
notpeelz opened this issue Apr 16, 2020 · 7 comments
Open

nix repl rendering breaks after resizing terminal #3500

notpeelz opened this issue Apr 16, 2020 · 7 comments
Labels
Milestone

Comments

@notpeelz
Copy link

notpeelz commented Apr 16, 2020

When typing long expressions in the REPL, resizing the terminal will cause the prompt to render/reflow incorrectly.

gif (this specific case can be tricky to reproduce -- depends on the columns and the length of the expression):
output
expression: lib.concatStrings (lib.attrValues (lib.mapAttrs (n: v: "${n}=${v}") { name = "blah"; }))
NixOS: 19.09.2399.5fa2612ca27
nix: 2.3.3 and 2.3.4
terminal: kitty 0.17.2

The issue can be avoided by pressing CTRL-L or CTRL-C after resizing the terminal.
However, it can also occur without prior resizing (might be a separate issue): https://asciinema.org/a/CDKIBLGUDU6NPQSZBsfVEEGY1 (couldn't reproduce on nix 2.3.4; possibly patched with NixOS/nixpkgs@0285a20)

@edolstra edolstra added the bug label Feb 12, 2021
@edolstra edolstra added this to the nix-2.4 milestone Feb 12, 2021
@thufschmitt
Copy link
Member

It looks like the editline library we’re using (iirc because GNU readline is GPLV3 while Nix is GPLV2) doesn’t implement anything to handle terminal resize. I’ve no idea whether that’s something that can easily be implemented on top of it.

Maybe we could switch to something a bit bigger like BSD editline (or its autotools port) 🤷🏻‍♂️

@Mic92
Copy link
Member

Mic92 commented Jul 27, 2021

There seems to be already some sigwhinch handler in place according to strace:

[pid 42184] ioctl(2, TIOCGWINSZ, {ws_row=42, ws_col=87, ws_xpixel=957, ws_ypixel=966}) = 0
[pid 42184] rt_sigtimedwait([HUP INT PIPE TERM WINCH], {si_signo=SIGWINCH, si_code=SI_KERNEL}, NULL, 8) = 28 (SIGWINCH)
[pid 42184] ioctl(2, TIOCGWINSZ, {ws_row=42, ws_col=174, ws_xpixel=1914, ws_ypixel=966}) = 0
[pid 42184] rt_sigtimedwait([HUP INT PIPE TERM WINCH], {si_signo=SIGWINCH, si_code=SI_KERNEL}, NULL, 8) = 28 (SIGWINCH)
[pid 42184] ioctl(2, TIOCGWINSZ, {ws_row=42, ws_col=87, ws_xpixel=957, ws_ypixel=966}) = 0
[pid 42184] rt_sigtimedwait([HUP INT PIPE TERM WINCH], {si_signo=SIGWINCH, si_code=SI_KERNEL}, NULL, 8) = 28 (SIGWINCH)
[pid 42184] ioctl(2, TIOCGWINSZ, {ws_row=42, ws_col=174, ws_xpixel=1914, ws_ypixel=966}) = 0
[pid 42184] rt_sigtimedwait([HUP INT PIPE TERM WINCH], {si_signo=SIGWINCH, si_code=SI_KERNEL}, NULL, 8) = 28 (SIGWINCH)
[pid 42184] ioctl(2, TIOCGWINSZ, {ws_row=42, ws_col=87, ws_xpixel=957, ws_ypixel=966}) = 0
[pid 42184] rt_sigtimedwait([HUP INT PIPE TERM WINCH], {si_signo=SIGWINCH, si_code=SI_KERNEL}, NULL, 8) = 28 (SIGWINCH)
[pid 42184] ioctl(2, TIOCGWINSZ, {ws_row=42, ws_col=174, ws_xpixel=1914, ws_ypixel=966}) = 0
[pid 42184] rt_sigtimedwait([HUP INT PIPE TERM WINCH], {si_signo=SIGWINCH, si_code=SI_KERNEL}, NULL, 8) = 28 (SIGWINCH)
[pid 42184] ioctl(2, TIOCGWINSZ, {ws_row=42, ws_col=87, ws_xpixel=957, ws_ypixel=966}) = 0
[pid 42184] rt_sigtimedwait([HUP INT PIPE TERM WINCH], {si_signo=SIGWINCH, si_code=SI_KERNEL}, NULL, 8) = 28 (SIGWINCH)
[pid 42184] ioctl(2, TIOCGWINSZ, {ws_row=42, ws_col=174, ws_xpixel=1914, ws_ypixel=966}) = 0
[pid 42184] rt_sigtimedwait([HUP INT PIPE TERM WINCH],

And there is also a rl_reset_terminal() function that can be used for this. So this looks like it is an uncovered edge case.

@edolstra edolstra modified the milestones: nix-2.4, nix-3.0 Aug 30, 2021
@edolstra edolstra modified the milestones: nix-2.5, nix-2.6 Dec 13, 2021
@edolstra edolstra modified the milestones: nix-2.6, nix-2.7 Jan 21, 2022
@edolstra edolstra modified the milestones: nix-2.7, nix-2.8 Mar 3, 2022
@edolstra edolstra modified the milestones: nix-2.8, nix-2.9 Apr 14, 2022
@edolstra edolstra modified the milestones: nix-2.9, nix-2.10 May 27, 2022
@edolstra edolstra modified the milestones: nix-2.10, nix-2.11 Jul 11, 2022
@edolstra edolstra modified the milestones: nix-2.11, nix-2.12 Aug 25, 2022
@xaverdh
Copy link

xaverdh commented Nov 26, 2022

This is a well known race condition between terminal reflow and the application doing reflow itself. The issue is that drawing is not atomic in the sense that when the application decides to draw something, it has to move the cursor for that. Now if the prompt spans multiple lines, it has to account for line breaks to compute the offsets. These depend on the terminal size, but the size could have asynchronously changed any time.

Something along the lines of https://gitlab.freedesktop.org/Per_Bothner/specifications/blob/master/proposals/semantic-prompts.md could solve it, but currently there is little interest to get it actually done.

@xaverdh
Copy link

xaverdh commented Nov 26, 2022

A workaround for applications I found, which somewhat improves the situation, is to not use the full buffer for displaying. You leave a small space (2 characters wide) to the right of the terminal buffer, so when the user resizes the terminal, terminal reflow will not happen immediately, since there is still some space left. This gives the application time to redraw itself.
Obviously still racy, but works in practice

@xaverdh
Copy link

xaverdh commented Nov 26, 2022

A workaround for applications I found, which somewhat improves the situation, is to not use the full buffer for displaying. You leave a small space (2 characters wide) to the right of the terminal buffer, so when the user resizes the terminal, terminal reflow will not happen immediately, since there is still some space left. This gives the application time to redraw itself. Obviously still racy, but works in practice

Looking at the readline docs, this could probably be implemented by hooking into rl_redisplay_function

@xaverdh
Copy link

xaverdh commented Nov 26, 2022

ah maybe a better approach is to install a signal handler for SIGWINCH and then rl_set_screen_size

@xaverdh
Copy link

xaverdh commented Nov 28, 2022

Ah ok editline doesn't support these, its very minimalistic indeed..

@edolstra edolstra modified the milestones: nix-2.12, nix-2.13 Dec 6, 2022
@edolstra edolstra modified the milestones: nix-2.13, nix-2.14 Jan 17, 2023
@edolstra edolstra removed this from the nix-2.14 milestone Feb 28, 2023
@edolstra edolstra added this to the nix-2.15 milestone Feb 28, 2023
@edolstra edolstra modified the milestones: nix-2.15, nix-2.16, nix-2.17 May 26, 2023
@edolstra edolstra modified the milestones: nix-2.17, nix-2.18 Jul 24, 2023
@edolstra edolstra modified the milestones: nix-2.18, nix-2.19 Sep 20, 2023
@edolstra edolstra modified the milestones: nix-2.19, nix-2.20 Nov 20, 2023
@edolstra edolstra modified the milestones: nix-2.20, nix-2.21 Mar 4, 2024
@edolstra edolstra modified the milestones: nix-2.21, nix-2.22 Mar 11, 2024
@edolstra edolstra modified the milestones: nix-2.22, nix-2.23 Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants