From 90ade7efc73a717414c5f1ab0ed57de600dc6a2e Mon Sep 17 00:00:00 2001 From: Nick Eddy Date: Fri, 15 May 2026 15:20:07 -0700 Subject: [PATCH] Fix git status parser to handle paths with spaces and renames (#271) Switch `files_for_staging` to `git status --short -z` so paths arrive NUL-separated and unquoted, and parse each record by taking everything after the 3-byte `XY ` status prefix instead of splitting on whitespace. This preserves filenames containing spaces, and the parser now skips the source record of rename (`R`) and copy (`C`) entries since only the destination exists on disk. Co-Authored-By: Claude Opus 4.7 (1M context) --- command-signatures/src/generators/git.rs | 177 ++++++++++++++++++----- 1 file changed, 139 insertions(+), 38 deletions(-) diff --git a/command-signatures/src/generators/git.rs b/command-signatures/src/generators/git.rs index 1057c0b3..693fce44 100644 --- a/command-signatures/src/generators/git.rs +++ b/command-signatures/src/generators/git.rs @@ -440,16 +440,29 @@ fn post_process_tracked_files(output: &str) -> GeneratorResults { return GeneratorResults::default(); } - output - .lines() - // The first non-whitespace string is just a character indicating the type of indexed file. - .filter_map(|file| file.split_whitespace().nth(1)) - .map(|file| { - Suggestion::with_description(file, "Changed file") + // `git status --short -z` emits NUL-separated records of the form `XY ` + // (two-byte status code + space + raw pathname, no C-style quoting). Renames + // and copies are emitted as two records: `R \0\0` — the source + // path follows the destination and we skip it because that file no longer + // exists on disk. + let mut suggestions: Vec = Vec::new(); + let mut records = output.split('\0').filter(|r| !r.is_empty()); + while let Some(record) = records.next() { + let Some(path) = record.get(3..) else { continue }; + if matches!(record.as_bytes().first(), Some(b'R') | Some(b'C')) { + records.next(); + } + suggestions.push( + Suggestion::with_description(path, "Changed file") .with_priority(Priority::Global(Importance::More(Order(100)))) - .with_icon(IconType::File) - }) - .collect_unordered_results() + .with_icon(IconType::File), + ); + } + + GeneratorResults { + suggestions, + is_ordered: false, + } } fn post_process_git_for_each_ref(output: &str) -> GeneratorResults { @@ -770,7 +783,7 @@ pub fn generator() -> CommandSignatureGenerators { .add_generator( "files_for_staging", Generator::script( - CommandBuilder::single_command("git --no-optional-locks status --short"), + CommandBuilder::single_command("git --no-optional-locks status --short -z"), post_process_tracked_files, ), ) @@ -965,47 +978,135 @@ mod tests { ); } + fn changed_file(path: &str) -> Suggestion { + Suggestion { + exact_string: path.to_owned(), + display_name: None, + description: Some("Changed file".to_owned()), + priority: Priority::Global(Importance::More(Order(100))), + icon: Some(IconType::File), + is_hidden: false, + } + } + #[test] fn test_post_process_tracked_files() { - let command_output = r" - M app/src/features.rs - M app/src/launch_config_palette.rs - M app/src/workspace/mod.rs"; + // `git status --short -z` output: NUL-separated records, each `XY `. + let command_output = + " M app/src/features.rs\0M app/src/launch_config_palette.rs\0 M app/src/workspace/mod.rs\0"; assert_eq!( post_process_tracked_files(command_output), GeneratorResults { suggestions: vec![ - Suggestion { - exact_string: "app/src/features.rs".to_owned(), - display_name: None, - description: Some("Changed file".to_owned()), - priority: Priority::Global(Importance::More(Order(100))), - icon: Some(IconType::File), - is_hidden: false, - }, - Suggestion { - exact_string: "app/src/launch_config_palette.rs".to_owned(), - display_name: None, - description: Some("Changed file".to_owned()), - priority: Priority::Global(Importance::More(Order(100))), - icon: Some(IconType::File), - is_hidden: false, - }, - Suggestion { - exact_string: "app/src/workspace/mod.rs".to_owned(), - display_name: None, - description: Some("Changed file".to_owned()), - priority: Priority::Global(Importance::More(Order(100))), - icon: Some(IconType::File), - is_hidden: false, - }, + changed_file("app/src/features.rs"), + changed_file("app/src/launch_config_palette.rs"), + changed_file("app/src/workspace/mod.rs"), ], is_ordered: false, } ); } + /// Filenames with spaces must be preserved intact. Under `-z` git emits raw + /// bytes with no C-style quoting, so the parser must take everything after + /// the 3-byte `XY ` prefix rather than splitting on whitespace. + #[test] + fn test_post_process_tracked_files_with_spaces_in_path() { + // Untracked file `new file test.csv` under `-z`: + let command_output = "?? new file test.csv\0"; + + assert_eq!( + post_process_tracked_files(command_output), + GeneratorResults { + suggestions: vec![changed_file("new file test.csv")], + is_ordered: false, + } + ); + } + + /// Renames under `-z` are emitted as two records: `R \0\0`. + /// We surface the destination only — the source no longer exists on disk. + #[test] + fn test_post_process_tracked_files_rename() { + let command_output = "R new name.txt\0old name.txt\0 M other.rs\0"; + + assert_eq!( + post_process_tracked_files(command_output), + GeneratorResults { + suggestions: vec![changed_file("new name.txt"), changed_file("other.rs")], + is_ordered: false, + } + ); + } + + /// Copies (`C`) are formatted the same way as renames (`\0\0`) + /// and must skip the source record just like renames. + #[test] + fn test_post_process_tracked_files_copy() { + let command_output = "C copied.txt\0source.txt\0"; + + assert_eq!( + post_process_tracked_files(command_output), + GeneratorResults { + suggestions: vec![changed_file("copied.txt")], + is_ordered: false, + } + ); + } + + /// Two renames in a row exercise the iterator-state interaction between + /// successive skip-source decisions. + #[test] + fn test_post_process_tracked_files_back_to_back_renames() { + let command_output = "R a.rs\0a-old.rs\0R b.rs\0b-old.rs\0"; + + assert_eq!( + post_process_tracked_files(command_output), + GeneratorResults { + suggestions: vec![changed_file("a.rs"), changed_file("b.rs")], + is_ordered: false, + } + ); + } + + /// `git status --short -z` emits untracked directories with a trailing slash. + #[test] + fn test_post_process_tracked_files_untracked_directory() { + let command_output = "?? dir with space/\0"; + + assert_eq!( + post_process_tracked_files(command_output), + GeneratorResults { + suggestions: vec![changed_file("dir with space/")], + is_ordered: false, + } + ); + } + + /// Empty output yields no suggestions. + #[test] + fn test_post_process_tracked_files_empty() { + assert_eq!( + post_process_tracked_files(""), + GeneratorResults { + suggestions: vec![], + is_ordered: false, + } + ); + } + + /// Fatal errors short-circuit to the default (empty, ordered) result. + #[test] + fn test_post_process_tracked_files_fatal_error() { + let command_output = "fatal: not a git repository\n"; + + assert_eq!( + post_process_tracked_files(command_output), + GeneratorResults::default() + ); + } + #[test] fn test_post_process_tags() { let command_output = "v1.0.0\nv2.0.0\nv0.1.0";