fix: normalize git patch prefixes for parser compatibility by aliou · Pull Request #106 · modem-dev/hunk

aliou · 2026-03-24T16:03:16Z

Hello!

I tried to use hunk but kept having the No files match the current filter error no matter what args I passed. After some digging, it is because my git configuration includes the following:

[diff]
  noprefix = true

So that git diff doesn't include the a/ and b/ prefixes:

diff --git src/core/git.ts src/core/git.ts
index f3f3ffa..e428042 100644
--- src/core/git.ts
+++ src/core/git.ts

Instead of

diff --git a/src/core/git.ts b/src/core/git.ts
index f3f3ffa..e428042 100644
--- a/src/core/git.ts
+++ b/src/core/git.ts

_{^{For context, this is to not have to remove the prefix when copying the filename from tmux or via double click/tap.}}

This PR forces the necessary option so that @pierre/diff doesn't struggle to parse the diff.

Let me know if you need some additional info/changes!

summary from gpt 5.3

Summary

force canonical git diff prefixes (a/ and b/) for git-backed inputs by passing command-scoped git config in:
- hunk diff
- hunk show
- hunk stash show
keep behavior local to each git invocation (no persistent config changes)
add parser-compatibility regression tests for config-sensitive cases

Why

@pierre/diffs currently expects a//b/-style headers. Repos/environments with settings like diff.noprefix=true or diff.mnemonicPrefix=true can emit non-canonical prefixes and lead to empty changesets in the UI.

Tests

bun run typecheck
bun test test/loaders.test.ts -t "loads git working tree changes when diff.noprefix is enabled|loads git working tree changes when diff.mnemonicPrefix is enabled|loads staged-only git diffs when diff.noprefix is enabled|loads stash show output when diff.noprefix is enabled"

Notes

intentionally overrides user/repo diff prefix formatting for these commands to preserve parser compatibility
does not include investigation doc in commit

greptile-apps · 2026-03-24T16:06:09Z

Greptile Summary

This PR fixes a parser-compatibility bug where users with non-standard git diff prefix settings would see an empty "No files match the current filter" changeset in hunk. The issue is that the upstream diff parser expects canonical prefixed headers, which certain git config options suppress.

The fix is clean and correctly scoped: a DIFF_PREFIX_NORMALIZATION_ARGS constant holding four -c config overrides is prepended to the args arrays returned by buildGitDiffArgs, buildGitShowArgs, and buildGitStashShowArgs. Since runGitText spawns [gitExecutable, ...args], the -c flags land before the subcommand name in every invocation — exactly the position git requires for command-scoped config overrides. The watch/reload path in watch.ts benefits automatically since it calls the same builder functions.

Key changes:

Adds withNormalizedDiffPrefixes() helper that prepends normalization flags to any git args array, forcing canonical a/ and b/ prefixes regardless of user or repo git config
Applies the helper consistently to all three git-backed command builders (diff, show, stash show)
Adds four integration tests covering working-tree, staged, and stash-show code paths with the problematic config settings enabled
The hunk show commit review path is the only builder that currently lacks a corresponding regression test for these config scenarios

Confidence Score: 5/5

This PR is safe to merge — the fix is minimal, correctly scoped to git spawn calls, and backed by integration tests.
The change prepends well-understood git -c flags in the correct position (before the subcommand), covers all three command builders consistently, and introduces no new dependencies or side effects on other code paths. The only gap is a missing test for hunk show with noprefix, which is a style suggestion rather than a functional risk.
No files require special attention

Important Files Changed

Filename	Overview
src/core/git.ts	Adds `DIFF_PREFIX_NORMALIZATION_ARGS` and `withNormalizedDiffPrefixes` to prepend git-level `-c` overrides before all three subcommands (`diff`, `show`, `stash show`). The placement is correct — `-c` flags land before the subcommand name in the final `spawnSync` call, which is exactly what git expects. All three builder functions are updated consistently.
test/loaders.test.ts	Adds four integration tests covering `diff.noprefix` and `diff.mnemonicPrefix` for the working-tree, staged, and stash-show paths. Missing a parallel test for the `hunk show` (commit review) path, which is the only builder function that now lacks a noprefix regression test.

Sequence Diagram

sequenceDiagram
    participant CLI as hunk CLI
    participant Builder as buildGit*Args()
    participant Normalizer as withNormalizedDiffPrefixes()
    participant RunGit as runGitText()
    participant Git as git process

    CLI->>Builder: input (kind, staged, ref, pathspecs…)
    Builder->>Builder: build subcommand args<br/>["diff"/"show"/"stash show", flags…]
    Builder->>Normalizer: args
    Normalizer->>Normalizer: prepend -c diff.noprefix=false<br/>-c diff.mnemonicPrefix=false<br/>-c diff.srcPrefix=a/<br/>-c diff.dstPrefix=b/
    Normalizer-->>Builder: [-c …flags…, "diff"/"show"/"stash show", …]
    Builder-->>CLI: normalized args
    CLI->>RunGit: { input, args }
    RunGit->>Git: spawnSync(["git", "-c", "diff.noprefix=false", …, "diff", …])
    Git-->>RunGit: stdout patch with canonical a/ b/ prefixes
    RunGit-->>CLI: patch text

_{Reviews (1): Last reviewed commit: "fix: normalize git patch prefixes for pa..." | Re-trigger Greptile}

greptile-apps · 2026-03-24T16:06:13Z

test/loaders.test.ts

+  test("loads stash show output when diff.noprefix is enabled", async () => {
+    const dir = createTempRepo("hunk-stash-noprefix-");
+
+    writeFileSync(join(dir, "alpha.ts"), "export const alpha = 1;\n");
+    git(dir, "add", "alpha.ts");
+    git(dir, "commit", "-m", "initial");
+
+    git(dir, "config", "--local", "diff.noprefix", "true");
+    writeFileSync(join(dir, "alpha.ts"), "export const alpha = 2;\n");
+    git(dir, "stash", "push", "-m", "update alpha");
+
+    const bootstrap = await loadFromRepo(dir, {
+      kind: "stash-show",
+      options: { mode: "auto" },
+    });
+
+    expect(bootstrap.changeset.files.map((file) => file.path)).toEqual(["alpha.ts"]);
+  });


Missing test coverage for hunk show with noprefix config

The PR adds regression tests for git diff (unstaged + staged) and stash show with diff.noprefix=true, but buildGitShowArgs (used by hunk show) applies the same withNormalizedDiffPrefixes call and has no corresponding test. A user with diff.noprefix=true reviewing a commit via hunk show would hit the same original bug, and there's no guard against a future regression there.

Consider adding a parallel test:

test("loads show output when diff.noprefix is enabled", async () => { const dir = createTempRepo("hunk-show-noprefix-"); writeFileSync(join(dir, "alpha.ts"), "export const alpha = 1;\n"); git(dir, "add", "alpha.ts"); git(dir, "commit", "-m", "initial"); git(dir, "config", "--local", "diff.noprefix", "true"); writeFileSync(join(dir, "alpha.ts"), "export const alpha = 2;\n"); git(dir, "add", "alpha.ts"); git(dir, "commit", "-m", "update alpha"); const bootstrap = await loadFromRepo(dir, { kind: "show", options: { mode: "auto" }, }); expect(bootstrap.changeset.files.map((file) => file.path)).toEqual(["alpha.ts"]); });

benvinegar · 2026-03-24T21:10:54Z

@aliou Thanks for this excellent PR! I've reviewed the changes and they're solid.

The only feedback from the review was adding a test for hunk show with diff.noprefix=true (same pattern as the other tests you already added). I've added that test:

Commit: a3fd1bd - "test: add missing noprefix test for hunk show command"

The test passes and all 5 related noprefix/mnemonicPrefix tests now pass together:

bun test test/loaders.test.ts -t "diff.noprefix is enabled|diff.mnemonicPrefix is enabled"
5 pass, 15 filtered out, 0 fail

To incorporate this change, you can cherry-pick the commit if you're tracking the upstream remote:

git fetch origin
git cherry-pick a3fd1bd

Or I can push it to your fork directly if you add me as a collaborator. Otherwise, feel free to add the test manually - it's identical to the other noprefix tests, just using kind: "show" instead of stash-show or git.

All the existing tests and typecheck pass. Great work on this fix!

benvinegar · 2026-03-24T21:17:18Z

^Ignore my agent slop.

Actually I'll just merge this and do the followup.

fix: normalize git patch prefixes for parser compatibility

55ad40c

aliou force-pushed the fix/git-diff-force-prefix branch from 1be1fda to 55ad40c Compare March 24, 2026 16:03

greptile-apps bot reviewed Mar 24, 2026

View reviewed changes

benvinegar merged commit 6fface2 into modem-dev:main Mar 24, 2026
3 checks passed

benvinegar mentioned this pull request Mar 24, 2026

test: add missing noprefix test for hunk show command #117

Merged

aliou deleted the fix/git-diff-force-prefix branch March 25, 2026 13:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: normalize git patch prefixes for parser compatibility#106

fix: normalize git patch prefixes for parser compatibility#106
benvinegar merged 1 commit intomodem-dev:mainfrom
aliou:fix/git-diff-force-prefix

aliou commented Mar 24, 2026

Uh oh!

greptile-apps bot commented Mar 24, 2026

Uh oh!

greptile-apps bot Mar 24, 2026

Uh oh!

benvinegar commented Mar 24, 2026

Uh oh!

benvinegar commented Mar 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

aliou commented Mar 24, 2026

Summary

Why

Tests

Notes

Uh oh!

greptile-apps bot commented Mar 24, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

benvinegar commented Mar 24, 2026

Uh oh!

benvinegar commented Mar 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants