Repository navigation
fix(tui): restore native Windows GNU support #9980
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -956,6 +956,18 @@ struct ParsedTerminalProbe { | |
| pending_input: Vec<u8>, | ||
| } | ||
|
|
||
| #[cfg(unix)] | ||
| fn write_terminal_probe_queries(stdout: &mut impl Write, query_window_pixels: bool) { | ||
| if query_window_pixels { | ||
| let _ = write!(stdout, "\x1b[14t"); | ||
| } | ||
| let _ = write!(stdout, "\x1b_Gi=31,s=1,v=1,a=q,t=d,f=24;AAAA\x1b\\\x1b[c"); | ||
| let _ = stdout.flush(); | ||
| } | ||
|
|
||
| #[cfg(not(unix))] | ||
| fn write_terminal_probe_queries(_stdout: &mut impl Write, _query_window_pixels: bool) {} | ||
|
|
||
| /// Probe terminal capabilities in one exchange and return any user input read | ||
| /// alongside the replies. The final DA1 request acts as an ordering marker: | ||
| /// any preceding Kitty reply advertises support, while its absence does not | ||
|
|
@@ -982,11 +994,7 @@ pub fn probe_terminal(known_cell_pixels: Option<(u16, u16)>) -> StartupTerminalP | |
| ioctl_pixels.is_none() && terminal_size.is_some_and(|(cols, rows)| cols > 0 && rows > 0); | ||
|
|
||
| let mut stdout = std::io::stdout(); | ||
| if query_window_pixels { | ||
| let _ = write!(stdout, "\x1b[14t"); | ||
| } | ||
| let _ = write!(stdout, "\x1b_Gi=31,s=1,v=1,a=q,t=d,f=24;AAAA\x1b\\\x1b[c"); | ||
| let _ = stdout.flush(); | ||
| write_terminal_probe_queries(&mut stdout, query_window_pixels); | ||
|
|
||
| let bytes = read_stdin_until(TERMINAL_PROBE_TIMEOUT, terminal_probe_complete); | ||
| let parsed = parse_terminal_probe(&bytes); | ||
|
|
@@ -1292,6 +1300,16 @@ mod tests { | |
| assert_eq!(resolve_cell_pixels(None, None), FALLBACK_CELL_PIXELS); | ||
| } | ||
|
|
||
| #[cfg(not(unix))] | ||
| #[test] | ||
| fn terminal_probe_does_not_write_queries_without_a_reply_reader() { | ||
| let mut output = Vec::new(); | ||
|
|
||
| write_terminal_probe_queries(&mut output, true); | ||
|
|
||
| assert!(output.is_empty(), "unread terminal queries were written: {output:?}"); | ||
| } | ||
|
|
||
|
Comment on lines
+1303
to
+1312
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: rg -n 'probe_terminal|write_terminal_probe_queries' cmux-tui/crates/cmux-tui/src/ui/graphics.rs
sed -n '940,1030p' cmux-tui/crates/cmux-tui/src/ui/graphics.rs
sed -n '1280,1330p' cmux-tui/crates/cmux-tui/src/ui/graphics.rsRepository: manaflow-ai/cmux Length of output: 5973 🏁 Script executed: set -eu
printf '%s\n' '--- diff summary ---'
git diff --stat 579a979ba5eeff8ed564731f0418292a24d15132 9ff0934a9fb0054ded95a61a07e7c4f9f202b104 -- cmux-tui/crates/cmux-tui/src/ui/graphics.rs
printf '%s\n' '--- relevant diff ---'
git diff --unified=80 579a979ba5eeff8ed564731f0418292a24d15132 9ff0934a9fb0054ded95a61a07e7c4f9f202b104 -- cmux-tui/crates/cmux-tui/src/ui/graphics.rs
printf '%s\n' '--- probe-related definitions and tests ---'
rg -n -A35 -B12 'fn (read_stdin_until|terminal_probe_complete|ioctl_cell_pixels)|read_stdin_until\\(|probe_terminal\\(' cmux-tui/crates/cmux-tui/src/ui/graphics.rs
printf '%s\n' '--- graphics test inventory ---'
rg -n '^\\s*fn [A-Za-z0-9_]+\\(' cmux-tui/crates/cmux-tui/src/ui/graphics.rs | tail -40Repository: manaflow-ai/cmux Length of output: 15493 🏁 Script executed: set -eu
rg -n -A12 -B8 'probe_terminal|write_terminal_probe_queries' cmux-tui --glob '*.rs'Repository: manaflow-ai/cmux Length of output: 9165 Exercise the non-Unix The added test calls Add an entrypoint-level test that captures probe output, or provide an injectable writer for the probe so the test can call 🤖 Prompt for AI Agents |
||
| #[test] | ||
| fn newly_detected_metrics_replace_known_metrics() { | ||
| assert_eq!(resolve_cell_pixels(Some((8, 16)), Some((11, 23))), (11, 23)); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| pub fn zig_target_arg(target: &str, host: &str) -> Option<String> { | ||
| // Preserve Zig's native target selection for existing native builds. The | ||
| // GNU Windows host is the exception: Zig otherwise defaults to MSVC and | ||
| // requires a Windows SDK even when the Rust toolchain is MinGW-only. | ||
| if target == host && !target.ends_with("-windows-gnu") { | ||
| return None; | ||
| } | ||
| zig_target_for_rust_target(target).map(|zig_target| format!("-Dtarget={zig_target}")) | ||
| } | ||
|
|
||
| fn zig_target_for_rust_target(target: &str) -> Option<&'static str> { | ||
| match target { | ||
| "x86_64-pc-windows-gnu" => Some("x86_64-windows-gnu"), | ||
| "x86_64-pc-windows-msvc" => Some("x86_64-windows-msvc"), | ||
| "aarch64-pc-windows-msvc" => Some("aarch64-windows-msvc"), | ||
| // Cross-compiling libghostty-vt for the release distribution targets | ||
| // (npm/PyPI `cmux` binaries). Zig cross-compiles these cleanly and | ||
| // pairs with cargo-zigbuild for the Rust link step. | ||
| "x86_64-apple-darwin" => Some("x86_64-macos"), | ||
| "aarch64-apple-darwin" => Some("aarch64-macos"), | ||
| "x86_64-unknown-linux-gnu" => Some("x86_64-linux-gnu"), | ||
| "aarch64-unknown-linux-gnu" => Some("aarch64-linux-gnu"), | ||
| "x86_64-unknown-linux-musl" => Some("x86_64-linux-musl"), | ||
| "aarch64-unknown-linux-musl" => Some("aarch64-linux-musl"), | ||
| _ => None, | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn native_windows_gnu_keeps_the_explicit_gnu_abi() { | ||
| assert_eq!( | ||
| zig_target_arg("x86_64-pc-windows-gnu", "x86_64-pc-windows-gnu").as_deref(), | ||
| Some("-Dtarget=x86_64-windows-gnu") | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn native_non_windows_builds_keep_zigs_native_target() { | ||
| assert_eq!(zig_target_arg("aarch64-apple-darwin", "aarch64-apple-darwin"), None); | ||
| } | ||
|
|
||
| #[test] | ||
| fn cross_targets_keep_their_explicit_zig_abi() { | ||
| let cases = [ | ||
| ( | ||
| "x86_64-pc-windows-gnu", | ||
| "aarch64-unknown-linux-gnu", | ||
| "-Dtarget=x86_64-windows-gnu", | ||
| ), | ||
| ( | ||
| "x86_64-pc-windows-msvc", | ||
| "aarch64-unknown-linux-gnu", | ||
| "-Dtarget=x86_64-windows-msvc", | ||
| ), | ||
| ( | ||
| "aarch64-pc-windows-msvc", | ||
| "x86_64-unknown-linux-gnu", | ||
| "-Dtarget=aarch64-windows-msvc", | ||
| ), | ||
| ("x86_64-apple-darwin", "aarch64-unknown-linux-gnu", "-Dtarget=x86_64-macos"), | ||
| ("aarch64-apple-darwin", "x86_64-unknown-linux-gnu", "-Dtarget=aarch64-macos"), | ||
| ( | ||
| "x86_64-unknown-linux-gnu", | ||
| "aarch64-unknown-linux-gnu", | ||
| "-Dtarget=x86_64-linux-gnu", | ||
| ), | ||
| ( | ||
| "aarch64-unknown-linux-gnu", | ||
| "x86_64-unknown-linux-gnu", | ||
| "-Dtarget=aarch64-linux-gnu", | ||
| ), | ||
| ( | ||
| "x86_64-unknown-linux-musl", | ||
| "aarch64-unknown-linux-gnu", | ||
| "-Dtarget=x86_64-linux-musl", | ||
| ), | ||
| ( | ||
| "aarch64-unknown-linux-musl", | ||
| "x86_64-unknown-linux-gnu", | ||
| "-Dtarget=aarch64-linux-musl", | ||
| ), | ||
| ]; | ||
|
|
||
| for (target, host, expected) in cases { | ||
| assert_eq!(zig_target_arg(target, host).as_deref(), Some(expected), "{target}"); | ||
| } | ||
| } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 4354
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 45670
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 45670
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 41468
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 41493
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 41935
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 2936
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 40120
Reject or resolve drive-relative paths before building the file URL.
Navigationpreserves the supplied path, andlist_directorypreserves it in eachFileEntry. A drive-relative path such asC:foo.mdcan therefore reachfile_url. The Windows branch treats it as an absolute drive path and formats it asfile:///C:foo.md, which changes its meaning. Resolve the path to an absolute path or reject it before callingfile_url.🤖 Prompt for AI Agents