Skip to content

Commit

Permalink
explicitly handle ctrl-c in confirmation prompt instead of signal han…
Browse files Browse the repository at this point in the history
…dler (#11897)

<!--
Thank you for contributing to uv! To help us out with reviewing, please
consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## 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 console-rs/console#235, which resolves
the issue, so now we can get rid of the call.

<!-- What's the purpose of the change? What does it do, and why? -->

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.

<!-- How was it tested? -->
  • Loading branch information
ericmarkmartin authored Mar 3, 2025
1 parent a141818 commit d57bb90
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 28 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
1 change: 0 additions & 1 deletion crates/uv-console/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,4 @@ doctest = false
workspace = true

[dependencies]
ctrlc = { workspace = true }
console = { workspace = true }
39 changes: 14 additions & 25 deletions crates/uv-console/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> {
// 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(),
Expand All @@ -48,11 +24,24 @@ pub fn confirm(message: &str, term: &Term, default: bool) -> std::io::Result<boo
// Match continuously on every keystroke, and do not wait for user to hit the
// `Enter` key.
let response = loop {
let input = term.read_key()?;
let input = term.read_key_raw()?;
match input {
Key::Char('y' | 'Y') => 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
});
}
_ => {}
};
};
Expand Down

0 comments on commit d57bb90

Please sign in to comment.