Skip to content

Capture initial Enter key release#59

Merged
LegNeato merged 1 commit intoRust-GPU:mainfrom
bspeice:powershell_enter
Mar 30, 2025
Merged

Capture initial Enter key release#59
LegNeato merged 1 commit intoRust-GPU:mainfrom
bspeice:powershell_enter

Conversation

@bspeice
Copy link
Contributor

@bspeice bspeice commented Mar 26, 2025

Part of some minor issues I ran into in #58:

When installing with Powershell, cargo gpu build will be started with an Enter keydown, but has an opportunity to observe the corresponding Enter keyup event too (see crossterm-rs/crossterm#124). When this happens, it's impossible run cargo gpu build without --auto-install-rust-toolchain because I'm unable to press "y".

I don't think it's ever expected for the first interaction in a terminal to be a keyup event, so maybe we can just re-read the input if that happens?

Copy link

@LegNeato LegNeato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Small nit, looks like it is failing CI:

error: using `print!()` with a format string that ends in a single newline
   --> crates/cargo-gpu/src/spirv_cli.rs:219:9
    |
219 |         print!("\n");
    |         ^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#print_with_newline
    = note: `-D clippy::print-with-newline` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::print_with_newline)]`
help: use `println!` instead
    |
219 -         print!("\n");
219 +         println!();

@bspeice bspeice requested a review from LegNeato March 30, 2025 17:55
@bspeice
Copy link
Contributor Author

bspeice commented Mar 30, 2025

 error: use of `println!`
   --> crates/cargo-gpu/src/spirv_cli.rs:226:9
    |
226 |         println!();
    |         ^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#print_stdout
    = note: `-D clippy::print-stdout` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::print_stdout)]`

error: could not compile `cargo-gpu` (bin "cargo-gpu") due to 1 previous error

Hmm... I can remove the newline in the diff to get CI passing, but the output does look a little weird when there are no line breaks. Any suggestion on how to proceed here?

@LegNeato
Copy link

I think crate::user_output!() will work?

# Conflicts:
#	crates/cargo-gpu/src/spirv_cli.rs
@bspeice
Copy link
Contributor Author

bspeice commented Mar 30, 2025

user_output! adds a crab to the beginning of the output, and I was hoping to avoid multiple on the same line. I think we can do one better though - put the newline in the original prompt. Because the terminal is in raw mode, the key being entered into the terminal shouldn't show on the line below.

Appears that lints are now passing.

@LegNeato
Copy link

Wfm, thanks!

@LegNeato LegNeato merged commit 66896f8 into Rust-GPU:main Mar 30, 2025
6 checks passed
@bspeice bspeice deleted the powershell_enter branch March 30, 2025 23:17
@bspeice bspeice mentioned this pull request Mar 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants