Fix git status parser to handle paths with spaces and renames (#271) by nickeddy · Pull Request #272 · warpdotdev/command-signatures

nickeddy · 2026-05-15T22:47:57Z

Switch files_for_staging to git status --short -z so paths arrive NUL-separated
and unquoted; parse each record by taking everything after the 3-byte XY prefix
instead of splitting on whitespace, preserving filenames with spaces. Rename (R)
and copy (C) entries span two records — we keep the <to> path and skip the origin.

get_changed_or_tracked_files shared that post-processor but still emitted
newline-separated git status … | sed output, which the NUL parser collapsed into one
garbage suggestion. It now uses git diff [--cached] --diff-filter=AM --name-only -z
(git does the filtering — portable, unlike GNU-only sed -z), with its own NUL
path-splitting post-processor.

Tests added: test_post_process_diff_name_only (NUL paths incl. one with spaces),
_empty, _fatal_error. Existing files_for_staging tests already cover renames,
copies, spaces, and untracked dirs.

lucieleblanc

Hi @nickeddy , thank you for the fix. Since this change touches generators, could you provide screenshots of the generators working in a local build of Warp? The patch process is explained in the test-local-warp skill. If you're testing on MacOS, you can also use cmd-shift-5 to start a screen recording of the app.

I'll be happy to review this again once a screen capture or a set of screenshots showing the changes are uploaded and displayed in the PR description or a comment. Thanks!

…tdev#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) <noreply@anthropic.com>

nickeddy · 2026-05-27T17:26:36Z

Here is a screenshot of the fix in action:

Running on latest warp with Cargo.toml bumped to my command-signatures version:

source = "git+https://github.com/nickeddy/command-signatures.git?rev=506d3e0b07031f81044a3f8661ae9a83c6633f51#506d3e0b07031f81044a3f8661ae9a83c6633f51"

lucieleblanc

I agree with the general direction of these changes, but there are a few details we should clarify and another generator we should avoid regressing.

lucieleblanc · 2026-05-29T16:51:27Z

    }
 }

 fn post_process_tracked_files(output: &str) -> GeneratorResults {


This function is also used by get_changed_or_tracked_files, which still uses a newline-separated generator. We might need update that generator, or split this post-processing function into its own path to avoid regressing that generator.

In any case, please add a test covering get_changed_or_tracked_files so we can avoid regressing it too.

I've confirmed there was a regression here due to my changes. I've added a test for this and updated get_changed_or_tracked_files to drop the sed requirement for mac compatibility. All tests are passing and I've verified the fix works still for files with spaces and didn't break changed/tracked files.

lucieleblanc · 2026-05-29T16:57:12Z

+    // `git status --short -z` emits NUL-separated records of the form `XY <path>`
+    // (two-byte status code + space + raw pathname, no C-style quoting). Renames
+    // and copies are emitted as two records: `R  <to>\0<from>\0` — the source
+    // path follows the destination and we skip it because that file no longer
+    // exists on disk.


This comment seems specific to renamed files. "The source path [...] no longer exists on disk" is a little misleading. Is that always true? What does the second path represent for copied files?

Please update the PR description as well to clarify this.

Updated the description.

…andling Switching files_for_staging to git status --short -z made post_process_tracked_files a NUL-record parser, but get_changed_or_tracked_files still fed it newline-separated output (git status --short | sed). split('\0') then yields a single record, collapsing the whole blob into one garbage suggestion with embedded newlines. Drop the sed pipe (its -z flag is GNU-only and breaks on macOS) and let git filter directly: git diff [--cached] --diff-filter=AM --name-only -z. --cached inspects the index, without it the working tree; --diff-filter=AM keeps the original M/A-only behavior; --name-only -z emits bare NUL-separated paths handled by a dedicated post_process_diff_name_only. Also correct the post_process_tracked_files comment: for copies the rename/copy origin still exists on disk, so we skip it because it is unchanged, not because it is gone. Add tests covering the new get_changed_or_tracked_files path (incl. spaces, empty, fatal-error) so it cannot silently regress again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

lucieleblanc

A few more changes. In general, please prefer concise comments and clearer code, and comment inline in GitHub instead of adding comments about the diff in the diff itself.

Also, the PR description should not be this long. Please shorten it to explain the core of the change. The "verification" section should not include lints/fmt/clippy. Those are prerequisites; describe newly added/modified test cases instead.

Once again, please include screenshots of all affected generators in a comment to show they work as intended with all these edge cases. Thanks!

lucieleblanc · 2026-05-29T17:53:46Z

+    /// It can't share the `git status --short -z` path used by
+    /// `files_for_staging`, because it needs to distinguish staged from
+    /// working-tree changes and that requires filtering NUL records — for which
+    /// the only portable tool is git itself (`sed -z`/`grep -z` are GNU-only and
+    /// break on macOS). It therefore runs `git diff [--cached] --diff-filter=AM
+    /// --name-only -z`, which emits bare NUL-separated pathnames with no `XY `
+    /// status prefix. The post-processor must yield one suggestion per record
+    /// and preserve spaces in pathnames.


Unnecessary comment

lucieleblanc · 2026-05-29T17:57:40Z

+/// for the `get_changed_or_tracked_files` generator.
+///
+/// That generator can't share the `git status --short -z` path used by
+/// `files_for_staging`: it needs to distinguish staged (index) changes from
+/// working-tree changes, and the only place that distinction is visible is the
+/// command itself (the post-processor receives stdout alone). Doing the
+/// distinction with `git status` would mean filtering NUL records in the shell,
+/// which requires GNU-only `sed -z`/`grep -z` and breaks on macOS. Letting git
+/// filter via `--diff-filter` keeps everything portable.
+///
+/// `--name-only -z` emits bare NUL-separated pathnames (no `XY ` status prefix,
+/// no C-style quoting), so each non-empty record is already a complete path.


Delete "for the get_changed_or_tracked_files generator". It's brittle and will go out of date too easily if we add another generator that uses this post-processing.

Most of this comment explains why get_changed_or_tracked_files uses git diff, which doesn't belong here. Keep comments minimal, concise and put them on the relevant functions only.

Cut the verbose doc/inline comments down to concise descriptions on the relevant functions; the git-diff rationale moves to a PR review comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

nickeddy · 2026-05-29T19:27:12Z

Testing on nickeddy/warp@becc1fd + nickeddy/command-signatures@208a88b after setting up the following git folder:

git init
echo orig > "old name.txt"
echo base > tracked.rs
mkdir "dir with space" && echo x > "dir with space/some file.rs"
git add -A && git commit -qm base
git mv "old name.txt" "new name.txt"
printf 'staged\n' >> tracked.rs && git add tracked.rs
printf 'unstaged\n' >> "dir with space/some file.rs"
echo new > "brand new.txt"

cla-bot Bot added the cla-signed label May 15, 2026

This was referenced May 15, 2026

git add <tab> truncates filenames containing spaces #271

Open

Generator-produced suggestions are not shell-escaped before insertion warpdotdev/warp#11072

Open

lucieleblanc requested changes May 27, 2026

View reviewed changes

nickeddy force-pushed the fix-command-signatures-271 branch from 90ade7e to 506d3e0 Compare May 27, 2026 17:17

cargo fmt

36cdf5e

nickeddy mentioned this pull request May 28, 2026

fix: shell-escape generator-produced suggestion replacements warpdotdev/warp#11796

Draft

4 tasks

nickeddy requested a review from lucieleblanc May 28, 2026 17:23

lucieleblanc requested changes May 29, 2026

View reviewed changes

Trim comments per review

208a88b

Cut the verbose doc/inline comments down to concise descriptions on the relevant functions; the git-diff rationale moves to a PR review comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

Fix git status parser to handle paths with spaces and renames (#271)#272
nickeddy wants to merge 4 commits into
warpdotdev:mainfrom
nickeddy:fix-command-signatures-271

nickeddy commented May 15, 2026 •

edited

Loading

Uh oh!

lucieleblanc left a comment

Uh oh!

nickeddy commented May 27, 2026

Uh oh!

lucieleblanc left a comment

Uh oh!

lucieleblanc May 29, 2026

Uh oh!

nickeddy May 29, 2026

Uh oh!

lucieleblanc May 29, 2026

Uh oh!

nickeddy May 29, 2026

Uh oh!

lucieleblanc left a comment •

edited

Loading

Uh oh!

lucieleblanc May 29, 2026

Uh oh!

lucieleblanc May 29, 2026

Uh oh!

Uh oh!

nickeddy commented May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

nickeddy commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucieleblanc left a comment

Choose a reason for hiding this comment

Uh oh!

nickeddy commented May 27, 2026

Uh oh!

lucieleblanc left a comment

Choose a reason for hiding this comment

Uh oh!

lucieleblanc May 29, 2026

Choose a reason for hiding this comment

Uh oh!

nickeddy May 29, 2026

Choose a reason for hiding this comment

Uh oh!

lucieleblanc May 29, 2026

Choose a reason for hiding this comment

Uh oh!

nickeddy May 29, 2026

Choose a reason for hiding this comment

Uh oh!

lucieleblanc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucieleblanc May 29, 2026

Choose a reason for hiding this comment

Uh oh!

lucieleblanc May 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nickeddy commented May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nickeddy commented May 15, 2026 •

edited

Loading

lucieleblanc left a comment •

edited

Loading