From 1ce584804907f0925dc1d86d3799bc33e3817bff Mon Sep 17 00:00:00 2001 From: benbenbang Date: Fri, 11 Sep 2026 08:48:53 +0800 Subject: [PATCH] fix(worktree): honor early picker cancellation and selection Reap the picker before handling input errors, and ignore broken pipes caused by early picker exits. Add integration coverage for cancellation, no-match, and selection behavior. Closes #64 close #64 --- src/worktree.rs | 17 +++--- tests/support/fake_tool.rs | 21 +++++++- tests/worktree_picker.rs | 103 +++++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 7 deletions(-) create mode 100644 tests/worktree_picker.rs diff --git a/src/worktree.rs b/src/worktree.rs index c6e8d72..22c8899 100644 --- a/src/worktree.rs +++ b/src/worktree.rs @@ -98,12 +98,12 @@ pub fn pick(worktrees: &[Worktree], fzf: &Path) -> Result, Strin format!("cannot run fzf: {error}") } })?; - child - .stdin - .take() - .ok_or("cannot open fzf input")? - .write_all(&input) - .map_err(|error| format!("cannot write fzf input: {error}"))?; + // The picker may cancel or accept before consuming the complete list. + // Close stdin and reap it even when writing encounters a broken pipe. + let input_result = match child.stdin.take() { + Some(mut stdin) => stdin.write_all(&input), + None => Err(std::io::Error::other("cannot open fzf input")), + }; let output = child .wait_with_output() @@ -114,6 +114,11 @@ pub fn pick(worktrees: &[Worktree], fzf: &Path) -> Result, Strin if !output.status.success() { return Err(format!("fzf exited with {}", output.status)); } + if let Err(error) = input_result + && error.kind() != std::io::ErrorKind::BrokenPipe + { + return Err(format!("cannot write fzf input: {error}")); + } selected_path(worktrees, &output.stdout).map(Some) } diff --git a/tests/support/fake_tool.rs b/tests/support/fake_tool.rs index 8d9e190..1309901 100644 --- a/tests/support/fake_tool.rs +++ b/tests/support/fake_tool.rs @@ -4,7 +4,15 @@ use std::io::{self, Read, Write}; use std::process::ExitCode; fn main() -> ExitCode { - let tool = env::var("CJ_FAKE_TOOL").unwrap_or_default(); + let tool = env::var("CJ_FAKE_TOOL").unwrap_or_else(|_| { + env::current_exe() + .ok() + .and_then(|path| { + path.file_stem() + .map(|name| name.to_string_lossy().into_owned()) + }) + .unwrap_or_default() + }); let mode = env::var("CJ_FAKE_MODE").unwrap_or_default(); let args = env::args_os().skip(1).collect::>(); if let Some(path) = env::var_os("CJ_FAKE_ARGS") { @@ -17,6 +25,13 @@ fn main() -> ExitCode { } } match (tool.as_str(), mode.as_str()) { + ("git", _) => { + let path = env::var_os("CJ_FAKE_WORKTREE_LIST").expect("CJ_FAKE_WORKTREE_LIST"); + io::stdout() + .write_all(&fs::read(path).expect("read fake worktree list")) + .expect("write fake worktree list"); + ExitCode::SUCCESS + } ("zoxide", "success") => { println!("{}", env::var("CJ_FAKE_DEST").expect("CJ_FAKE_DEST")); ExitCode::SUCCESS @@ -31,6 +46,10 @@ fn main() -> ExitCode { print!("{index}\tselected\0"); ExitCode::SUCCESS } + ("fzf", "no-match") => { + record_stdin(); + ExitCode::from(1) + } ("fzf", "cancel") => { record_stdin(); ExitCode::from(130) diff --git a/tests/worktree_picker.rs b/tests/worktree_picker.rs new file mode 100644 index 0000000..b04b5bc --- /dev/null +++ b/tests/worktree_picker.rs @@ -0,0 +1,103 @@ +#![cfg(feature = "test-tools")] + +mod support; + +use std::env; +use std::fs; +use std::path::{Path, PathBuf}; + +use support::{TempDir, assert_success, cj, write_config}; + +#[test] +fn early_picker_exit_preserves_cancellation_selection_and_failure() { + let temp = TempDir::new("early-worktree-picker"); + let bin = temp.path().join("fake tools"); + fs::create_dir(&bin).expect("create fake tools directory"); + copy_tool(&bin, "git"); + let fzf = copy_tool(&bin, "fzf"); + let config = temp.path().join("config.toml"); + write_config(&config, Path::new("missing-zoxide"), &fzf); + + // Exceed pipe capacity so an exit without reading stdin reliably interrupts + // the parent's write, instead of accidentally fitting in the pipe buffer. + let mut records = Vec::new(); + for index in 0..4096 { + let path = temp + .path() + .join(format!("tree-{index}-{}", "x".repeat(192))); + records.extend_from_slice( + format!( + "worktree {}\0HEAD abc\0branch refs/heads/main\0\0", + path.display() + ) + .as_bytes(), + ); + } + assert!(records.len() > 1024 * 1024); + let listing = temp.path().join("worktree-list"); + fs::write(&listing, records).expect("write large worktree list"); + let mut paths = vec![bin]; + paths.extend(env::split_paths(&env::var_os("PATH").unwrap_or_default())); + let path = env::join_paths(paths).expect("join fake tools PATH"); + + for (mode, selection) in [ + ("cancel", "1"), + ("no-match", "1"), + ("success", "1"), + ("success", "invalid"), + ("failure", "1"), + ] { + let output = cj(temp.path(), temp.path()) + .arg("-C") + .arg(&config) + .arg("--jump-worktree") + .env("PATH", &path) + .env("CJ_FAKE_WORKTREE_LIST", &listing) + .env("CJ_FAKE_MODE", mode) + .env("CJ_FAKE_SELECTION", selection) + .env_remove("CJ_FAKE_TOOL") + .env_remove("CJ_FAKE_STDIN") // Exit immediately without reading. + .env_remove("CJ_FAKE_ARGS") + .output() + .expect("run early-exiting picker"); + match (mode, selection) { + ("cancel" | "no-match", _) => { + assert_success(&output); + assert_eq!(output.stdout, b"\n"); + assert!(output.stderr.is_empty()); + } + ("success", "1") => { + assert_success(&output); + let expected = temp.path().join(format!("tree-1-{}", "x".repeat(192))); + assert_eq!( + output.stdout, + format!("{}\n", expected.display()).as_bytes() + ); + assert!(output.stderr.is_empty()); + } + ("success", "invalid") => { + assert_eq!(output.status.code(), Some(2)); + assert!(output.stdout.is_empty()); + assert_eq!( + output.stderr, + b"cj: fzf returned an invalid worktree selection\n" + ); + } + ("failure", _) => { + assert_eq!(output.status.code(), Some(2)); + assert!(output.stdout.is_empty()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("fake fzf failure\ncj: fzf exited with")); + assert!(stderr.contains('7')); + assert!(!stderr.contains("cannot write fzf input")); + } + _ => unreachable!(), + } + } +} + +fn copy_tool(directory: &Path, name: &str) -> PathBuf { + let path = directory.join(format!("{name}{}", env::consts::EXE_SUFFIX)); + fs::copy(env!("CARGO_BIN_EXE_cj-test-tool"), &path).expect("copy fake tool"); + path +}