Skip to content

Fix git status parser to handle paths with spaces and renames #272

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.

Sign up for GitHub

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

Merged
acarl005 merged 1 commit into from
Jul 13, 2026
Merged

Fix git status parser to handle paths with spaces and renames #272

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 76 additions & 80 deletions command-signatures/src/generators/git.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,18 +434,19 @@ pub fn filter_messages(out: &str) -> &str {
}
}

fn post_process_tracked_files(output: &str) -> GeneratorResults {
/// Parses `git diff --name-only -z` output: bare NUL-separated pathnames, one
/// per changed file, with no status prefix or quoting.
fn post_process_diff_name_only(output: &str) -> GeneratorResults {
let output = filter_messages(output);
if output.starts_with("fatal:") {
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")
.split('\0')
.filter(|path| !path.is_empty())
.map(|path| {
Suggestion::with_description(path, "Changed file")
.with_priority(Priority::Global(Importance::More(Order(100))))
.with_icon(IconType::File)
})
Expand Down Expand Up @@ -653,6 +654,9 @@ fn post_process_push_refspec_tags(out: &str) -> GeneratorResults {
/// pathspec itself (`ls-files ... --directory -- 'some/nested/'` prints just
/// `some/nested/`), recreating the very stall this fixes. Without it git prints the
/// full nested paths, and the walk stays bounded by the typed subtree.
///
/// `-z` makes the paths NUL-separated and raw: without it, `core.quotePath` C-quotes
/// filenames containing non-ASCII or special characters, corrupting the suggestion.
fn files_for_staging_command(
tokens: &[&str],
trailing_whitespace: bool,
Expand All @@ -669,31 +673,30 @@ fn files_for_staging_command(
// correct and cheap.
let Some(dir_end) = current_token.rfind('/') else {
return CommandBuilder::single_command(
"git --no-optional-locks ls-files --exclude-standard --others --modified --directory --no-empty-directory",
"git --no-optional-locks ls-files -z --exclude-standard --others --modified --directory --no-empty-directory",
);
};
let dir_prefix = &current_token[..=dir_end];

// Escape single quotes for safe embedding in the single-quoted pathspec.
let escaped_prefix = dir_prefix.replace('\'', "'\\''");
CommandBuilder::single_command(format!(
"git --no-optional-locks ls-files --exclude-standard --others --modified -- '{escaped_prefix}'"
"git --no-optional-locks ls-files -z --exclude-standard --others --modified -- '{escaped_prefix}'"
))
}

/// Post-processes `ls-files` output for `files_for_staging`: clean one-per-line paths
/// (untracked directories carry a trailing slash), so unlike
/// `post_process_tracked_files` there is no status prefix to strip — stripping here
/// would corrupt the filenames.
/// Post-processes `ls-files -z` output for `files_for_staging`: raw NUL-separated
/// paths (untracked directories carry a trailing slash) with no status prefix and no
/// quoting, so each record passes through as-is.
fn post_process_files_for_staging(output: &str) -> GeneratorResults {
let output = filter_messages(output);
if output.starts_with("fatal:") {
return GeneratorResults::default();
}

output
.lines()
.filter(|line| !line.is_empty())
.split('\0')
.filter(|path| !path.is_empty())
.map(|file| {
Suggestion::with_description(file, "Changed file")
.with_priority(Priority::Global(Importance::More(Order(100))))
Expand Down Expand Up @@ -871,12 +874,16 @@ pub fn generator() -> CommandSignatureGenerators {
Generator::command_from_tokens(
|tokens, _, _| {
if tokens.contains(&"--staged") || tokens.contains(&"--cached") {
CommandBuilder::pipe( CommandBuilder::single_command(r#"git --no-optional-locks status --short"#), CommandBuilder::single_command(r#"sed -ne '/^M /p' -e '/A /p'"#))
CommandBuilder::single_command(
"git --no-optional-locks diff --cached --diff-filter=AM --name-only -z",
)
} else {
CommandBuilder::pipe(CommandBuilder::single_command(r#"git --no-optional-locks status --short"#), CommandBuilder::single_command(r#"sed -ne '/M /p' -e '/A /p'"#))
CommandBuilder::single_command(
"git --no-optional-locks diff --diff-filter=AM --name-only -z",
)
}
},
post_process_tracked_files,
post_process_diff_name_only,
),
)
.add_generator(
Expand Down Expand Up @@ -960,8 +967,8 @@ pub fn generator() -> CommandSignatureGenerators {
mod tests {
use crate::generators::git::{
detect_refspec_prefix, files_for_staging_command, post_process_branches,
post_process_files_for_staging, post_process_push_refspec_branches,
post_process_push_refspec_tags, post_process_tags, post_process_tracked_files,
post_process_diff_name_only, post_process_files_for_staging,
post_process_push_refspec_branches, post_process_push_refspec_tags, post_process_tags,
};
use warp_completion_metadata::{
GeneratorResults, IconType, Importance, Order, Priority, Shell, Suggestion,
Expand Down Expand Up @@ -1025,81 +1032,70 @@ 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,
}
}

/// NUL-separated paths, including one with spaces, each become a suggestion.
#[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";
fn test_post_process_diff_name_only() {
let command_output = "app/src/features.rs\0app/src/new.rs\0dir with space/some file.rs\0";

assert_eq!(
post_process_tracked_files(command_output),
post_process_diff_name_only(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/new.rs"),
changed_file("dir with space/some file.rs"),
],
is_ordered: false,
}
);
}

// `ls-files` output is one clean path per line — no status prefix to strip. Untracked
// directories arrive collapsed with a trailing slash and pass through unchanged.
/// Empty output (no changed files) yields no suggestions.
#[test]
fn test_post_process_diff_name_only_empty() {
assert_eq!(
post_process_diff_name_only(""),
GeneratorResults {
suggestions: vec![],
is_ordered: false,
}
);
}

/// Fatal errors short-circuit to the default (empty, ordered) result.
#[test]
fn test_post_process_diff_name_only_fatal_error() {
assert_eq!(
post_process_diff_name_only("fatal: not a git repository\n"),
GeneratorResults::default()
);
}

// `ls-files -z` output is raw NUL-separated paths — no status prefix and no quoting,
// so spaces survive intact. Untracked directories arrive collapsed with a trailing
// slash and pass through unchanged.
#[test]
fn test_post_process_files_for_staging() {
let command_output = "big/\nsome/nested/folder1/file1.txt\ntracked.txt";
let command_output = "big/\0some/nested/folder1/file1.txt\0dir with space/some file.rs\0";

assert_eq!(
post_process_files_for_staging(command_output),
GeneratorResults {
suggestions: vec![
Suggestion {
exact_string: "big/".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: "some/nested/folder1/file1.txt".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: "tracked.txt".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("big/"),
changed_file("some/nested/folder1/file1.txt"),
changed_file("dir with space/some file.rs"),
],
is_ordered: false,
}
Expand All @@ -1114,7 +1110,7 @@ mod tests {
let cmd = files_for_staging_command(&["git", "add"], true, &[]);
assert_eq!(
cmd.build(Shell::Posix),
"git --no-optional-locks ls-files --exclude-standard --others --modified --directory --no-empty-directory"
"git --no-optional-locks ls-files -z --exclude-standard --others --modified --directory --no-empty-directory"
);
}

Expand All @@ -1125,7 +1121,7 @@ mod tests {
let cmd = files_for_staging_command(&["git", "add", "fil"], false, &[]);
assert_eq!(
cmd.build(Shell::Posix),
"git --no-optional-locks ls-files --exclude-standard --others --modified --directory --no-empty-directory"
"git --no-optional-locks ls-files -z --exclude-standard --others --modified --directory --no-empty-directory"
);
}

Expand All @@ -1137,7 +1133,7 @@ mod tests {
let cmd = files_for_staging_command(&["git", "add", "some/nested/folder1/"], false, &[]);
assert_eq!(
cmd.build(Shell::Posix),
"git --no-optional-locks ls-files --exclude-standard --others --modified -- 'some/nested/folder1/'"
"git --no-optional-locks ls-files -z --exclude-standard --others --modified -- 'some/nested/folder1/'"
);
}

Expand All @@ -1148,7 +1144,7 @@ mod tests {
let cmd = files_for_staging_command(&["git", "add", "some/nested/fi"], false, &[]);
assert_eq!(
cmd.build(Shell::Posix),
"git --no-optional-locks ls-files --exclude-standard --others --modified -- 'some/nested/'"
"git --no-optional-locks ls-files -z --exclude-standard --others --modified -- 'some/nested/'"
);
}

Expand Down
Loading