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

Immediately redraw the display when terminal window is resized #2

Open
PaulJuliusMartinez opened this issue Feb 9, 2022 · 6 comments
Labels
Bug Something isn't working Mac Something related to macOS

Comments

@PaulJuliusMartinez
Copy link
Owner

jless does not immediately redraw the screen when the terminal window is resized; you have to press to key first.

The relevant code is in src/input.rs, which is quite convoluted.

When I originally wrote this code, I had two goals: 1) immediately respond to both keypresses and SIGWINCH (clearly a failure), and 2) do so without busy-looping.

To accomplish this, I tried to setup a pipe to receive the signals and then use poll on that pipe and stdin to respond to the first change from either input.

Along the way there's added complexity because termion's parse_event doesn't detect pure escapes.

I think this is possible, because vim does it, but I'm not totally sure. vim might be doing some busy looping in the background.

@PaulJuliusMartinez PaulJuliusMartinez added Bug Something isn't working Linux Something related to Linux Mac Something related to macOS labels Feb 9, 2022
@PaulJuliusMartinez
Copy link
Owner Author

Hm, I tested this in an EC2 Ubuntu instance and it did work as intended, with a small delay, so maybe it's just a macOS thing. (I've tried both Terminal and iTerm2, inside of tmux and not, and it never works.)

@PaulJuliusMartinez PaulJuliusMartinez removed the Linux Something related to Linux label Feb 9, 2022
@leahneukirchen
Copy link

The bottom bar is not redrawn properly after resizing, even when a key was pressed.

@yshavit
Copy link

yshavit commented Jul 21, 2023

I just spent a bit of time looking into this. tl;dr, poll() doesn't work for stdin on macOS after the call to remap_dev_tty_to_stdin().

On macOS, redirecting tty to stdin closes the normal stdin fd (0). Before that, poll() seems to work fine for fd 0; but afterwards, it returns instantly, with the revents bit for POLLNVAL (invalid fd) set. What's more, you can't even directly open() on "/dev/tty" as a workaround: macOS doesn't support poll() on devices.

The upshot of this is that regardless of actual input, the loop in TuiInput::next always breaks after the first iteration. If the SIGWINCH socket has a write, the loop returns that event; if not, it always reads the next get_event_from_buffered_input(), which then blocks if there isn't actually any input available. When the user presses a key, we go back to the loop and it works.

The comment in remap_dev_tty_to_stdin() says it's a workaround for kkawakam/rustyline#599, which was fixed in 10.0.0. So, I can try to upgrade that dep and see if I can just remove the redirect, at which point things should work. jless currently depends on 9.0.0, and the latest is 12.0.0.


The bottom bar is not redrawn properly after resizing, even when a key was pressed.

👆 I wasn't able to reproduce this, at least on the latest version.

@yshavit
Copy link

yshavit commented Jul 21, 2023

Hm, actually that doesn't help at all; the fundamental problem remains, which is that poll() doesn't work on /dev/tty. So in the case where the json comes in through stdin (cat whatever.json | jless), jless needs to keep an eye on /dev/tty (whether directly or via stdin), and that doesn't work on macOS.

I'm a bit of a rust newbie, but what would you say to not polling on stdin/tty at all, and instead using a channel to combine tty and the SIGWINCH? Specifically, I'm thinking that there could be a channel with two transmitters, each in a separate thread: one reads tty just as fast as it can, and puts parsed TuiEvents into the channel; the other polls the SIGWINCH socket, and puts WinChEvent events into the channel. Then, next() just reads the channel.

@leahneukirchen
Copy link

Yes, redrawing works with 0.9.0, so this can be closed imo (I don't use macOS).

@yshavit
Copy link

yshavit commented Jul 21, 2023

I still need to do a keypress (or mouse action) before the refresh happens. I'm on macos 11.7.8.

yshavit added a commit to yshavit/jless that referenced this issue Jul 23, 2023
By using a channel, we can remove the need for calling libc::poll on
the stdin pipe. This doesn't work on the Mac: when we redirect tty to
stdin, it breaks the original stdin pipe (fd 0), such that libc::poll on
it returns instantly, with an revents of POLLNVAL (invalid pipe). macOS
also doesn't support libc::poll on devices, so we can't poll tty
directly.

Instead, we spawn two threads: one just reads stdin forever, and the
other uses the socket to listen for SIGWINCH events as before (using
libc::poll). These both write to a channel, and the TuiInput simply
reads from this channel.

This fixes PaulJuliusMartinez#2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Mac Something related to macOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants