Reverse the extended headers in ReverseFileDiff

[youdie006]

The inconsistency

ReverseFileDiff swaps every direction-bearing field except one:

reverse := FileDiff{
    OrigName: fd.NewName,   // diff/reverse.go:13
    OrigTime: fd.NewTime,   // :14
    NewName:  fd.OrigName,  // :15
    NewTime:  fd.OrigTime,  // :16
    Extended: fd.Extended,  // :17  <- copied verbatim
}

and the hunk ranges are swapped at :65-69. But Extended carries direction too,
and this package already knows it: handleEmpty reads deleted file mode vs
new file mode at diff/parse.go:533,536 to decide which side is /dev/null,
and diff/parse.go:584-585 reads rename from /rename to for the same purpose.

The doc comment at diff/reverse.go:9-10 says the result "undoes the edit of the
original". A diff that still says new file mode does not undo a creation.

What a caller sees

ParseFileDiff → ReverseFileDiff → PrintFileDiff on git diff's output for a
file creation:

diff --git a/added.txt b/added.txt
new file mode 100644          <- still says "new"
index 0000000..587be6b
--- b/added.txt
+++ /dev/null

$ git apply --check that.diff
error: git apply: bad git-diff - expected /dev/null on line 2

For a rename it is error: git apply: bad git-diff - inconsistent old filename on line 2.

For reference, git diff -R on the same commit range produces
deleted file mode 100644 and index 587be6b..0000000. After this change the
output matches and git apply --check accepts it.

And an empty new file is a silent no-op. PrintFileDiff short-circuits at
diff/print.go:48 when Hunks == nil, so nothing is written but the extended
headers — which still say new file mode. Round-tripping the "reversed" diff back
through ParseFileDiff gives OrigName == "/dev/null", i.e. byte-identical to the
forward diff. ReverseFileDiff did nothing at all.

Nothing downstream compensates: the only non-test callers are
ReverseMultiFileDiff → ReverseFileDiff (diff/reverse.go:33) and
ReverseFileDiff → reverseHunk (:20), neither touches Extended, and
PrintFileDiff writes d.Extended straight out at diff/print.go:32-36.

The change

Swap the direction-bearing headers using the same vocabulary parse.go already
reads: new file mode ↔deleted file mode , old mode ↔new mode ,
rename from ↔rename to , copy from ↔copy to , and the two hashes in
index <a>..<b>[ mode].

Gated on a leading diff --git line — the same gate handleEmpty uses at
diff/parse.go:511. That gate is load-bearing: without it the existing
diff/testdata/sample_multi_file.reversed golden churns, because it encodes a
diff --ruN header with old mode 0777/new mode 0755 that is not git-shaped.
There is a test row for exactly that.

Every rewrite is an involution, so the existing
ReverseMultiFileDiff(ReverseMultiFileDiff(x)) == x test at
diff/reverse_test.go:172-176 still passes unchanged. The helper copies the slice,
so the caller's FileDiff is not mutated — there is a test row for that too.

Deliberate limitation

git diff -R also swaps the two arguments of the diff --git line itself
(diff --git a/old b/new → diff --git b/new a/old). I left that line alone:
parseDiffGitArgs (diff/parse.go:455-505) shows it is genuinely ambiguous for
filenames containing spaces or quotes, and round-tripping it would need re-quoting
logic. git apply --check accepts the output without it, which I verified for both
the creation and the rename case. Happy to extend if you want it.

similarity index / dissimilarity index are left as-is, matching git diff -R.

Verification

• go test ./... green; gofmt and go vet clean.
• Reverting to Extended: fd.Extended fails the new tests; dropping the
  diff --git gate also fails them, on the diff --ruN row.
• The oracle is git diff -R from the same commit range in a scratch repo, with
  git apply --check as acceptance — not a hand-written expectation.
• The test uses only go-cmp and stdlib, both already imported, and builds on the
  Go 1.20 in go.mod. No testdata files added, so no line-ending or TZ exposure.

I did not benchmark this. ReverseFileDiff is called once per file diff and the
added work is a slice copy plus a linear scan over at most a handful of header
lines; git log -- diff/reverse.go shows no perf-motivated commits. Say the word
if you want numbers.

Note

Open PRs #87/#88 also touch diff/reverse_test.go, but only as a mechanical Go
1.26 modernize. My tests are appended at the end of the file, so a rebase should be
trivial whichever lands first. No open PR touches diff/reverse.go.

Disclosure

I used an AI assistant to help find and prepare this change. I reviewed and tested
it myself, and the outputs above are from runs I performed.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-10T05:06:25.094581Z	0da2c37	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0da2c37008

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Reject copy diffs instead of swapping their endpoints

When the input is a Git copy such as copy from old / copy to new, swapping these values produces a copy from new back to old, but the inverse of a copy must remove new while retaining old. After the forward patch has been applied, both files exist, so this reversed patch fails because old already exists rather than undoing the creation of new. Since a copy-only diff does not contain enough file content to synthesize a deletion patch, this case should be rejected or handled through a representation that can actually delete the destination.

Useful? React with 👍 / 👎.

[keegancsmith]

Thanks for the thorough report and implementation. I pushed a small maintainer follow-up.

Copy diffs need different treatment: the inverse of a copy is deleting the destination, not copying it back over the source. Because a copy-only diff does not contain enough content to construct that deletion, ReverseFileDiff now returns an error for Git copy diffs.

I also simplified index-header reversal with strings.Cut while preserving modes and CRLF input, and added coverage for both behaviors. go test -race ./... and go vet ./... pass.

[keegancsmith]

The extended-header reversal is appropriately scoped, and the copy case now fails explicitly rather than producing a patch that cannot undo the original operation.