crossterm-rs / crossterm

Cross platform terminal library rust
MIT License
3.19k stars 276 forks source link

[RfC] Support for backgrounding an application via `Ctrl+Z` + `fg` #494

Open Byron opened 3 years ago

Byron commented 3 years ago

Hi,

I am the maintainer of dua, a tool to visualize disk usage and act on it, using tui with either the termion or crossterm backend.

Recently an issue popped up indicating that backgrounding an application with Ctrl+Z does not work as expected, leaving the terminal in a messy state (independently of the application having to handle Ctrl+Z itself in RAW mode). The long version of what's happening there is in this comment, in short one would need capabilities to install signal handlers for SIGTSTP and SIGCONT, along with the ability to send oneself a signal to stop a process.

In theory, the crossterm project is in the best position to implement this in a cross-platform way, which in theory could work for other tui backends as well, like termion.

As the maintainer of the crosstermion crate, I could imagine implementing a proof of concept as well, probably without windows support.

What do you think about this, I am keen to hear your thoughts.

Thank you

TimonPost commented 3 years ago

Crossterm supports signal handling with resize events already. It looks like it is possible to integrate this behavior quite easily. I will take a look into your linked comment and see the details of this issue.

Byron commented 3 years ago

That's great, thank you! As the tui maintainer talked about the possibility to stop supporting the curses backend, this would make crossterm the only one that supports this feature. It would also sway me to stop building the termion backend, as by now the last advantage of slightly faster compile times might be neglectable.

TimonPost commented 3 years ago

Interesting, great work on your end! I wrote the terminal library which does a similar thing by abstracting terminal backends. Have you seen it? Maybe we can combine efforts there.

Byron commented 3 years ago

Thanks for the heads-up, actually I didn't know about terminal before! Great work, too, for sure. I am happy to strip crosstermion for parts if needed, it was really just born out of the desire to not put all eggs in one basket. After having tried to get a contribution into termion, I came to consider it as unresponsive enough to stop using it. Thus I would be happy to focus on getting crossterm to the point where it's the best of all worlds.

The only reason to keep termion might be to support redox (I think), but I'd be happy to focus on the here and now and not bet on it just yet.

In any case, if you have anything I can help with, just let me know. In one possible future of this we would put threaded async input handling (as in crosstermion) into crossterm (possibly behind a feature toggle) and then I could remove crosstermion from the picture alltogether. Once crossterm supports backgrounding out of the box, I would think it's feature complete (at least for my needs). And as new needs arise, for my work on gitoxide for instance, I am happy to provide the unix parts of necessary implementation.

Just as a note: Right now all my tools consume crossterm through tui, so as long as crossterm doesn't change its minor version there is issue with this at all. Otherwise tui also needs to upgrade and publish a new version - that can be a bit cumbersome.

TimonPost commented 3 years ago

The only reason to keep termion might be to support redox (I think),

Haha, yea, it could be possible to support all redox features, but I think it is a waste of time for a small userbase. The library should probably focus on solid support for 99.5% of the users.

future of this we would put threaded async input handling (as in crosstermion) into crossterm

Crossterm supports async input ontop of futures, why do you think crosstermion has something special to offer on top of this?

In any case, if you have anything I can help with, just let me know

Crossterm is quite to stable and should be almost ready for a 1.0, I don't have a lot of time to work on it, this signal issue might be interesting to get working. For the rest https://github.com/crossterm-rs/crossterm/pull/407 is hard to get fixed. I am not sure on why /dev/tty can not be opened on MacOs systems. Perhaps you have some knowledge in that area?

Byron commented 3 years ago

Crossterm supports async input ontop of futures, why do you think crosstermion has something special to offer on top of this?

You are right, probably I could put this chunk of code anywhere where it's needed without using a common crate or have that functionality in crossterm. Maybe I am missing something that (by now) is already there, too. Something this code avoids is to use any actual async, as it costs a lot in terms of dependencies and using threads + channels is absolutely sufficient in these cases. To me, the compile-time costs of async have to be offset by value which to me is provided only when one wants to use a lot of async combinators, have easily cancellable operations, and/or go for a lot of concurrent IO.

For the rest #407 is hard to get fixed. I am not sure on why /dev/tty can not be opened on MacOs systems. Perhaps you have some knowledge in that area?

I have no knowledge, unfortunately, but will be giving it a spin to see what happens. Maybe I can figure something out.

I don't have a lot of time to work on it

But it's great that you are as responsive as you are when dealing with issues and PRs - this makes a massive difference and enables others to fix up what they need! crossterm has a real chance of making looking at curses or termion unnecessary, which to me is a goal worth working for. So…, thanks for creating this project, and thanks x 1000 for maintaining it so well :)!

Byron commented 3 years ago

I did take a look at the issue, just to realize that it probably takes quite a while to truly understand how everything is working with async events from the ground up, especially given my generally lack of knowledge on how a (BSD) tty actually works, and what shortcomings there might be compared to Linux.

This is where I would be interested to know how curses does it, assuming it's the most complete and most portable implementation out there.

That said, a possible approach for me was to try to implement this myself in the simplest possible way - it would certainly be interesting to start to understand the machinery I use every day :D.

Andrew15-5 commented 1 month ago

I'm trying to combat this too. I haven't found anything on the Internet about Ctrl+Z handling! The only clue is using low_level::emulate_default_handler(SIGTSTP)? from https://docs.rs/signal-hook/latest/signal_hook/. I didn't look into what it does yet. But by using kill -TSTP PID I was able to successfully put the app in the background. Oh, and also the app didn't correctly/fully go to background when called from just's recipe, only when run directly from the shell. I don't know though if and how I can put the code from that example into this:

        if let Event::Key(key) = input {
            if let KeyCode::Char('z') = key.code {
                if key.modifiers.contains(KeyModifiers::CONTROL) {
               ....

Because it looks like the signal loop is endless, so I had to run it in a separate thread via tokio::spawn().

Andrew15-5 commented 4 weeks ago

I was able to mostly bring this feature back here. If the app is called directly from shell, then you're basically all set. But if called from another app (just), then you have to press the shortcut twice. Also it adds a weird (signal) label next to zsh: suspended X. It's definitely better than nothing.

The basic idea is to run this:

disable_raw_mode().unwrap();
std::io::stdout().execute(LeaveAlternateScreen).unwrap();
signal_hook::low_level::emulate_default_handler(SIGTSTP).unwrap();

The thread (and I think all children) will stop executing on the last line. And when you resume it will start from the line below (where signal was fired). Since the app uses tokio runtime, I also had to use the tokio adapter crate. And additionally, the ratatui screen must be cleared after the CONT signal is received as well as other stuff:

enable_raw_mode().unwrap();
std::io::stdout().execute(EnterAlternateScreen).unwrap();

That's it. But I think disabling the initial remapping of Ctrl+Z is the only right way to go (when crossterm-rs initializes its key press listener or whatever).