helix
helix copied to clipboard
helix-term: send the STOP signal to all processes in the process group
From kill(3p):
If pid is 0, sig shall be sent to all processes (excluding an unspecified set
of system processes) whose process group ID is equal to the process group ID
of the sender, and for which the process has permission to send a signal.
This fixes the issue of running git commit, attempting to suspend
helix with ^Z, and then not regaining control over the terminal and
having to press ^Z again.
Fixes https://github.com/helix-editor/helix/issues/3522.
I opted to add the nix crate because it has nice Rust wrappers around e.g. kill, handling error cases and other stuff for us. However, I could easily port it to the libc crate directly, if desired.
I tested this with:
$ cargo b
$ VISUAL=$PWD/target/debug/hx git commit --allow-empty
# ^Z
$ echo "tada I'm back at my terminal"
tada I'm back at my terminal
$ fg
# finish git commit
Yeah I think it would be preferrable to use libc directly. Pulling in nix for a handful of small functions doesn't seem worth it.
Latest commit switches from nix to libc.
It might be worth adding a comment explaining why it's safe.
See https://github.com/helix-editor/helix/pull/1843 and https://github.com/vorner/signal-hook/issues/132 we were hoping to fix this in signal-hook instead
I'll gladly revert this once that does get handled upstream, but seeing the upstream issue closed with no further activity does not inspire confidence it will be fixed anytime soon. This also resolves one of my longstanding annoyances when using helix is my general purpose editor, so I would really love if we could accept this as "good for now" until signal-hook decides to fix this case.
Maybe you could just upstream the fix? @pickfire closed the issue since he couldn't track it down.
Unfortunately, I don't have the time or energy to attempt to design this in a way upstream might consider palatable at the moment. Sorry.
I closed the issue previously because I feel like it's a hack. I was thinking of checking this with tokio since I think the issue originates there (or maybe it really is signal-hook) but I don't have the energy to check with them back then.
But thinking about it now again I still think it is good to have this, quite a few times I wanted to reopen my previous pull request.
Rebased.
Before merging this, I think we need an investigation in https://github.com/vorner/signal-hook/issues/132#issuecomment-1079897156, as well as looking to see if the issue lies somewhere within tokio. Both @cole-h and I probably won't be able to it at this moment of time.
Not sure why but when I tested with crossterm example again, it seemed to work.
I cloned crossterm and modified the example event-stream-tokio.
code
//! Demonstrates how to read events asynchronously with tokio.
//!
//! cargo run --features="event-stream" --example event-stream-tokio
//!
//! Add to dev-dependencies
//!
//! signal-hook-tokio = { version = "0.3", features = ["futures-v0_3"] }
use libc::c_int;
use signal_hook::consts::signal::*;
use signal_hook::low_level;
use signal_hook_tokio::Signals;
use std::{io::stdout, time::Duration};
use futures::{future::FutureExt, select, StreamExt};
use futures_timer::Delay;
use crossterm::{
cursor::position,
event::{
DisableMouseCapture, EnableMouseCapture, Event, EventStream, KeyCode, KeyEvent,
KeyModifiers,
},
execute,
terminal::{disable_raw_mode, enable_raw_mode},
Result,
};
const HELP: &str = r#"EventStream based on futures_util::Stream with tokio
- Keyboard, mouse and terminal resize events enabled
- Prints "." every second if there's no event
- Hit "c" to print current cursor position
- Use Esc to quit
"#;
async fn print_events() {
const SIGNALS: &[c_int] = &[
SIGTERM, SIGQUIT, SIGINT, SIGTSTP, SIGWINCH, SIGHUP, SIGCHLD, SIGCONT,
];
let mut reader = EventStream::new();
let mut sigs = Signals::new(SIGNALS).unwrap();
loop {
let mut delay = Delay::new(Duration::from_millis(1_000)).fuse();
let mut event = reader.next().fuse();
select! {
_ = delay => { println!(".\r"); },
maybe_event = event => {
match maybe_event {
Some(Ok(event)) => {
println!("Event::{:?}\r", event);
if event == Event::Key(KeyCode::Char('c').into()) {
println!("Cursor position: {:?}\r", position());
}
if event == Event::Key(KeyCode::Esc.into()) {
break;
}
if matches!(event, Event::Key(KeyEvent {
code: KeyCode::Char('z'),
modifiers: KeyModifiers::CONTROL,
..
})) {
low_level::raise(SIGTSTP).unwrap();
}
}
Some(Err(e)) => println!("Error: {:?}\r", e),
None => break,
}
}
maybe_signal = sigs.next().fuse() => {
if let Some(signal) = maybe_signal {
println!("Received signal {:?}\r", signal);
// After printing it, do whatever the signal was supposed to do in the first place
if let SIGTSTP = signal {
disable_raw_mode().unwrap();
low_level::emulate_default_handler(signal).unwrap();
enable_raw_mode().unwrap();
} else {
low_level::emulate_default_handler(signal).unwrap();
}
}
}
};
}
}
#[tokio::main]
async fn main() -> Result<()> {
println!("{}", HELP);
enable_raw_mode()?;
let mut stdout = stdout();
execute!(stdout, EnableMouseCapture)?;
print_events().await;
execute!(stdout, DisableMouseCapture)?;
disable_raw_mode()
}