From d57bb9049975403dcdb4dac98440ca136846a13c Mon Sep 17 00:00:00 2001 From: Eric Mark Martin Date: Mon, 3 Mar 2025 10:30:48 -0500 Subject: [PATCH] explicitly handle ctrl-c in confirmation prompt instead of signal handler (#11897) ## Summary Follow on to #11706. In the original PR, I tried to solve the issue by getting rid of the `ctrlc::set_handler` call. Unfortunately, this didn't work on windows due to an issue with the console crate. console 0.15.11 includes https://github.com/console-rs/console/pull/235, which resolves the issue, so now we can get rid of the call. This change is not super important but I still think it's worthwhile. For one, spinning up a background thread to handle `SIGINT`s when we're going to be raising the `SIGINT` from within the function is more technical complexity than needed, now that there's an easy way to explicitly catch the Ctrl-C from the terminal input. Secondly, `ctrlc::set_handler`'s [docs](https://docs.rs/ctrlc/3.4.5/ctrlc/fn.set_handler.html) advise that you set the handler just once, at the beginning of the program, so this use seems somewhat error prone. In fact, uv already has a second [callsite](https://github.com/astral-sh/uv/blob/461f4d9007160f7061a4fc0c4a5a84c613fdbff7/crates/uv/src/commands/project/add.rs#L596-L611) for this function (though I'm not sure if the two callsites could currently ever both occur on the same run of uv) ## Test Plan I've tested this manually on linux (WSL ubuntu) and windows, though not on aarch64-apple-darwin as I don't have a machine running that. I would appreciate if someone would double-check that it works on such machines. As discussed in the original PR, this change is pretty hard to test due to the fact that the behavior only occurs if stderr is connected to a tty. I experimented with using pseudoterminals to test this but it's still quite tricky due to the lack of x-platform non-blocking reads on the pty. --- Cargo.lock | 1 - Cargo.toml | 2 +- crates/uv-console/Cargo.toml | 1 - crates/uv-console/src/lib.rs | 39 +++++++++++++----------------------- 4 files changed, 15 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3ec077b97..3cb7d3b00 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4890,7 +4890,6 @@ name = "uv-console" version = "0.0.1" dependencies = [ "console", - "ctrlc", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 07c6e4a2a..900eb5bc2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -90,7 +90,7 @@ cargo-util = { version = "0.2.14" } clap = { version = "4.5.17", features = ["derive", "env", "string", "wrap_help"] } clap_complete_command = { version = "0.6.1" } configparser = { version = "3.1.0" } -console = { version = "0.15.8", default-features = false } +console = { version = "0.15.11", default-features = false } csv = { version = "1.3.0" } ctrlc = { version = "3.4.5" } dashmap = { version = "6.1.0" } diff --git a/crates/uv-console/Cargo.toml b/crates/uv-console/Cargo.toml index 597105ee9..fcfb81dd4 100644 --- a/crates/uv-console/Cargo.toml +++ b/crates/uv-console/Cargo.toml @@ -11,5 +11,4 @@ doctest = false workspace = true [dependencies] -ctrlc = { workspace = true } console = { workspace = true } diff --git a/crates/uv-console/src/lib.rs b/crates/uv-console/src/lib.rs index ea2e7a7f7..43eb677dc 100644 --- a/crates/uv-console/src/lib.rs +++ b/crates/uv-console/src/lib.rs @@ -6,30 +6,6 @@ use std::{cmp::Ordering, iter}; /// This is a slimmed-down version of `dialoguer::Confirm`, with the post-confirmation report /// enabled. pub fn confirm(message: &str, term: &Term, default: bool) -> std::io::Result { - // Set the Ctrl-C handler to exit the process. - let result = ctrlc::set_handler(move || { - let term = Term::stderr(); - term.show_cursor().ok(); - term.write_str("\n").ok(); - term.flush().ok(); - - #[allow(clippy::exit, clippy::cast_possible_wrap)] - std::process::exit(if cfg!(windows) { - 0xC000_013A_u32 as i32 - } else { - 130 - }); - }); - - match result { - Ok(()) => {} - Err(ctrlc::Error::MultipleHandlers) => { - // If multiple handlers were set, we assume that the existing handler is our - // confirmation handler, and continue. - } - Err(err) => return Err(std::io::Error::new(std::io::ErrorKind::Other, err)), - } - let prompt = format!( "{} {} {} {} {}", style("?".to_string()).for_stderr().yellow(), @@ -48,11 +24,24 @@ pub fn confirm(message: &str, term: &Term, default: bool) -> std::io::Result break true, Key::Char('n' | 'N') => break false, Key::Enter => break default, + Key::CtrlC => { + let term = Term::stderr(); + term.show_cursor()?; + term.write_str("\n")?; + term.flush()?; + + #[allow(clippy::exit, clippy::cast_possible_wrap)] + std::process::exit(if cfg!(windows) { + 0xC000_013A_u32 as i32 + } else { + 130 + }); + } _ => {} }; };