From 81b7128b99882cdba86a55ca5351af1dfef94584 Mon Sep 17 00:00:00 2001 From: Keegan Smith Date: Thu, 10 Sep 2026 07:21:29 +0000 Subject: [PATCH] fix/reverse: align Git headers with reverse direction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reversed patch should identify its paths in the same order as git diff -R. Preserve Git’s original argument spelling so quoted and escaped names remain exact, and decline to rewrite headers whose boundaries cannot be established safely. Amp-Thread-ID: https://ampcode.com/threads/T-01a08a2d-ba87-7779-89ec-92cdd2c8b8cf Co-authored-by: Amp --- diff/reverse.go | 63 ++++++++++++++++++++++++++++++++++++++-- diff/reverse_test.go | 68 ++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 124 insertions(+), 7 deletions(-) diff --git a/diff/reverse.go b/diff/reverse.go index edbd32d..74fd7dc 100644 --- a/diff/reverse.go +++ b/diff/reverse.go @@ -12,7 +12,7 @@ import ( // cannot be reversed because they do not contain enough information to delete // the copied file. func ReverseFileDiff(fd *FileDiff) (*FileDiff, error) { - extended, err := reverseExtendedHeaders(fd.Extended) + extended, err := reverseExtendedHeaders(fd.Extended, fd.OrigName, fd.NewName) if err != nil { return nil, err } @@ -34,13 +34,14 @@ func ReverseFileDiff(fd *FileDiff) (*FileDiff, error) { } // reverseExtendedHeaders reverses the direction encoded in git's extended headers. -func reverseExtendedHeaders(headers []string) ([]string, error) { +func reverseExtendedHeaders(headers []string, origName, newName string) ([]string, error) { // handleEmpty gates on the same prefix when it reads the direction back out. if len(headers) == 0 || !strings.HasPrefix(headers[0], "diff --git ") { return headers, nil } reversed := make([]string, len(headers)) copy(reversed, headers) + reversed[0] = reverseDiffGitHeader(reversed[0], origName, newName) for i, header := range reversed { switch { case strings.HasPrefix(header, "new file mode "): @@ -58,6 +59,64 @@ func reverseExtendedHeaders(headers []string) ([]string, error) { return reversed, nil } +// reverseDiffGitHeader swaps the two path arguments while preserving their +// original quoting. The parsed filenames are used only to reject malformed or +// ambiguous input; names recovered from other headers can disambiguate Git's +// unquoted paths containing spaces. +func reverseDiffGitHeader(header, origName, newName string) string { + const prefix = "diff --git " + args := header[len(prefix):] + lineEnding := "" + if strings.HasSuffix(args, "\r") { + args = strings.TrimSuffix(args, "\r") + lineEnding = "\r" + } + + first, second, valid := parseDiffGitArgs(args) + if !valid { + return header + } + + var rawFirst, rawSecond string + switch { + case first != "" && second != "": + var ok bool + rawFirst, rawSecond, ok = splitDiffGitArgs(args, first, second) + if !ok { + return header + } + case origName != "" && newName != "" && args == origName+" "+newName: + rawFirst, rawSecond = origName, newName + default: + return header + } + + return prefix + rawSecond + " " + rawFirst + lineEnding +} + +// splitDiffGitArgs locates the raw argument boundary after parseDiffGitArgs has +// validated and decoded both paths. +func splitDiffGitArgs(args, first, second string) (string, string, bool) { + if args[0] == '"' { + _, remainder, err := readQuotedFilename(args) + if err != nil || len(remainder) < 2 || remainder[0] != ' ' { + return "", "", false + } + return args[:len(args)-len(remainder)], remainder[1:], true + } + if args[len(args)-1] == '"' { + i := strings.IndexByte(args, '"') + if i < 2 || args[i-1] != ' ' { + return "", "", false + } + return args[:i-1], args[i:], true + } + if args != first+" "+second { + return "", "", false + } + return first, second, true +} + // swapHeaderValues exchanges the values of the first "from" header and the // first "to" header, leaving both prefixes where they are. func swapHeaderValues(headers []string, fromPrefix, toPrefix string) { diff --git a/diff/reverse_test.go b/diff/reverse_test.go index 3cc3962..1acadb8 100644 --- a/diff/reverse_test.go +++ b/diff/reverse_test.go @@ -233,27 +233,27 @@ func TestReverseFileDiffExtendedHeaders(t *testing.T) { { name: "new file", input: []string{"diff --git a/f b/f", "new file mode 100644", "index 0000000..587be6b"}, - want: []string{"diff --git a/f b/f", "deleted file mode 100644", "index 587be6b..0000000"}, + want: []string{"diff --git b/f a/f", "deleted file mode 100644", "index 587be6b..0000000"}, }, { name: "deleted file", input: []string{"diff --git a/f b/f", "deleted file mode 100644", "index 587be6b..0000000"}, - want: []string{"diff --git a/f b/f", "new file mode 100644", "index 0000000..587be6b"}, + want: []string{"diff --git b/f a/f", "new file mode 100644", "index 0000000..587be6b"}, }, { name: "rename", input: []string{"diff --git a/old b/new", "similarity index 70%", "rename from old", "rename to new", "index 94954ab..8b14c4f 100644"}, - want: []string{"diff --git a/old b/new", "similarity index 70%", "rename from new", "rename to old", "index 8b14c4f..94954ab 100644"}, + want: []string{"diff --git b/new a/old", "similarity index 70%", "rename from new", "rename to old", "index 8b14c4f..94954ab 100644"}, }, { name: "mode change", input: []string{"diff --git a/f b/f", "old mode 100644", "new mode 100755"}, - want: []string{"diff --git a/f b/f", "old mode 100755", "new mode 100644"}, + want: []string{"diff --git b/f a/f", "old mode 100755", "new mode 100644"}, }, { name: "CRLF index", input: []string{"diff --git a/f b/f\r", "index 94954ab..8b14c4f 100644\r"}, - want: []string{"diff --git a/f b/f\r", "index 8b14c4f..94954ab 100644\r"}, + want: []string{"diff --git b/f a/f\r", "index 8b14c4f..94954ab 100644\r"}, }, { name: "no extended headers", @@ -284,6 +284,64 @@ func TestReverseFileDiffExtendedHeaders(t *testing.T) { } } +func TestReverseFileDiffGitHeader(t *testing.T) { + // These expected lines were captured from git diff -R. Git is deliberately + // not invoked by the test. + tests := []struct { + name string + input, orig, new string + want string + }{ + { + name: "unquoted paths with spaces disambiguated by parsed names", + input: "diff --git a/old name.txt b/new name.txt", + orig: "a/old name.txt", + new: "b/new name.txt", + want: "diff --git b/new name.txt a/old name.txt", + }, + { + name: "quoted newline", + input: `diff --git "a/line\nbreak.txt" "b/other\nline.txt"`, + want: `diff --git "b/other\nline.txt" "a/line\nbreak.txt"`, + }, + { + name: "quoted escapes and non-ASCII", + input: `diff --git "a/quote\"slash\\\303\251.txt" "b/renamed \"path\"\\\303\270.txt"`, + want: `diff --git "b/renamed \"path\"\\\303\270.txt" "a/quote\"slash\\\303\251.txt"`, + }, + { + name: "CRLF", + input: "diff --git a/plain-old.txt b/plain-new.txt\r", + want: "diff --git b/plain-new.txt a/plain-old.txt\r", + }, + { + name: "ambiguous paths without corroborating names", + input: "diff --git a/old name.txt b/new name.txt", + want: "diff --git a/old name.txt b/new name.txt", + }, + { + name: "malformed quoting", + input: `diff --git "a/old b/new`, + orig: "a/old", + new: "b/new", + want: `diff --git "a/old b/new`, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fd := &FileDiff{OrigName: test.orig, NewName: test.new, Extended: []string{test.input}} + reversed, err := ReverseFileDiff(fd) + if err != nil { + t.Fatal(err) + } + if got := reversed.Extended[0]; got != test.want { + t.Errorf("diff --git header: got %q, want %q", got, test.want) + } + }) + } +} + func TestReverseFileDiffRejectsCopy(t *testing.T) { input := []byte("diff --git a/old b/new\nsimilarity index 100%\ncopy from old\ncopy to new\n") fd, err := ParseFileDiff(input)