From 96478f325d5bded8797efea9e6cd1dbccd78a639 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 16 Oct 2026 17:40:55 +0800 Subject: [PATCH] fix(ui): test that the Explorer argument reaches the process as written (#2579) Review of #2625: the test checked the argument reveal_command builds but not how it is passed, so it still passed with raw_arg replaced by args. The process is now built by reveal_process, and a Windows test runs it on `cmd /c echo`, which prints its command line back: with Rust's own quoting the argument comes back wrapped in quotes with the inner ones escaped, and the test fails. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RUNQSPVXx3dDBabRZHAnjz --- crates/ui-egui/src/tab_context.rs | 48 +++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/crates/ui-egui/src/tab_context.rs b/crates/ui-egui/src/tab_context.rs index 6f5319d637..7cc502722b 100644 --- a/crates/ui-egui/src/tab_context.rs +++ b/crates/ui-egui/src/tab_context.rs @@ -55,21 +55,8 @@ fn reveal(path: &std::path::Path) -> Result<(), String> { return Err("The file no longer exists".into()); } let platform = std::env::consts::OS; - let run = |program: &str, args: &[std::ffi::OsString]| { - let mut command = std::process::Command::new(program); - // Windows: `reveal_command` already quotes what Explorer needs quoted, and Rust's own - // quoting of an argument with a space breaks `/select,` (#2579), so pass them as written. - #[cfg(windows)] - { - use std::os::windows::process::CommandExt as _; - for arg in args { - command.raw_arg(arg); - } - } - #[cfg(not(windows))] - command.args(args); - command.output().map(|o| o.status.success()).map_err(|e| e.to_string()) - }; + let run = + |program: &str, args: &[std::ffi::OsString]| reveal_process(program, args).output().map(|o| o.status.success()).map_err(|e| e.to_string()); if matches!(platform, "linux" | "freebsd") { // `dbus-send --print-reply` waits up to 2 s for the file manager (then xdg-open may run), // so it runs off the UI thread; a failure there is only logged. @@ -83,6 +70,24 @@ fn reveal(path: &std::path::Path) -> Result<(), String> { reveal_via(platform, &path, run) } +/// The process that runs `program` with `args`. Windows: `reveal_command` already quotes what +/// Explorer needs quoted, and Rust's own quoting of an argument with a space breaks `/select,` +/// (#2579), so the arguments go on the command line as written. +#[cfg(not(target_arch = "wasm32"))] +fn reveal_process(program: &str, args: &[std::ffi::OsString]) -> std::process::Command { + let mut command = std::process::Command::new(program); + #[cfg(windows)] + { + use std::os::windows::process::CommandExt as _; + for arg in args { + command.raw_arg(arg); + } + } + #[cfg(not(windows))] + command.args(args); + command +} + #[cfg(not(target_arch = "wasm32"))] fn reveal_via( platform: &str, @@ -240,6 +245,19 @@ mod tests { assert!(reveal_via("linux", path, |_, _| Err("command missing".into())).is_err()); } + /// #2579 (review of #2625): the `/select,"…"` argument reaches the process exactly as written. + /// `cmd /c echo` prints its command line back; with Rust's own quoting (`args`) it would come + /// back wrapped in quotes with the inner ones escaped, which Explorer reads as no selection. + #[cfg(windows)] + #[test] + fn the_reveal_argument_reaches_the_process_as_written() { + let (_, args) = reveal_command("windows", std::path::Path::new(r"C:\Reports\annual report.pdf")).unwrap(); + let mut echo: Vec = ["/d", "/c", "echo"].map(std::ffi::OsString::from).to_vec(); + echo.extend(args); + let out = reveal_process("cmd", &echo).output().unwrap(); + assert_eq!(String::from_utf8_lossy(&out.stdout).trim_end(), r#"/select,"C:\Reports\annual report.pdf""#); + } + #[test] fn reveal_arguments_preserve_spaces_and_unicode() { let path = std::path::Path::new("/tmp/年度 report.pdf");