jesseduffield.lazygit/pkg/commands/patch/patch_test.go
Stefan Haller cf8e5fd27e Recover diff-line identity by parsing the buffer, behind a swappable seam
The focused main view's click/enter/e/G handlers all need the same thing: given
a rendered diff row, the patch-space line it corresponds to. Until now that came
solely from delta's lazygit-edit:// hyperlinks, which only carry a path and a
single line number — no side. That's lossy: for a deletion the number is the old
line, but the consumers fed it into new-file lookups, and two consecutive
deletions (which share a new-file line number) couldn't be told apart at all.

Replace GetFileAndLineForClickedDiffLine with GetDiffLineInfo, returning the
fuller (file, type, new-line, old-line) record from diff-line-metadata-notes.md.
This is mechanism #1: parse the decolorized view buffer — walk up to the file's
"diff --git" section, reuse patch.Parse on it (splitting multi-file commit diffs
on the "diff --git" boundaries), and read the type and line numbers off the patch
arithmetic. It serves the structure-preserving renderings — no pager, git diff
--color, and delta --color-only without line numbers — with no external
dependency.

To avoid trusting a mis-parse, the parser bails when a hunk's body no longer
matches its header (Patch.IsWellFormed). That's what happens when a pager keeps
the diff/hunk headers but restructures the body: delta's line-number gutters push
the +/- marker off the start of each line, so every body line reads as context.
Such renderings fall through to the next backend rather than yielding a confident
wrong answer. (diff-so-fancy goes further and rewrites the headers too, so it
fails even earlier, on the missing "diff --git".)

GetDiffLineInfo is a seam with swappable backends: the buffer parser first, then
the old hyperlink reader as a fallback for renderings the parser can't handle
(delta's default mode, or delta with line-number gutters). The future #2 OSC
per-cell metadata reader plugs in ahead of both, behind the same record shape.

Wire the consumers to the record per that doc's field mapping:
- dive into staging/patch building lands on the exact patch line, looking a
  deletion up by its old-file line number (PatchLineForOldLineNumber) so the
  two-deletions case resolves correctly;
- `e` edits at the new-file line;
- `G` anchors the PR link on the left (old) side for a deletion, the right (new)
  side otherwise.

The hyperlink fallback can't convey the side, so it reports DiffLineOther, which
the consumers treat as a non-deletion — i.e. exactly today's behavior.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-08-08 12:58:59 +02:00

932 lines
18 KiB
Go

package patch
import (
"testing"
"github.com/stretchr/testify/assert"
)
const simpleDiff = `diff --git a/filename b/filename
index dcd3485..1ba5540 100644
--- a/filename
+++ b/filename
@@ -1,5 +1,5 @@
apple
-orange
+grape
...
...
...
`
const renameWithModificationDiff = `diff --git a/oldname b/newname
similarity index 62%
rename from oldname
rename to newname
index dcd3485..1ba5540 100644
--- a/oldname
+++ b/newname
@@ -1,5 +1,5 @@
apple
-orange
+grape
...
...
...
`
const addNewlineToEndOfFile = `diff --git a/filename b/filename
index 80a73f1..e48a11c 100644
--- a/filename
+++ b/filename
@@ -60,4 +60,4 @@ grape
...
...
...
-last line
\ No newline at end of file
+last line
`
const removeNewlinefromEndOfFile = `diff --git a/filename b/filename
index e48a11c..80a73f1 100644
--- a/filename
+++ b/filename
@@ -60,4 +60,4 @@ grape
...
...
...
-last line
+last line
\ No newline at end of file
`
const twoHunks = `diff --git a/filename b/filename
index e48a11c..b2ab81b 100644
--- a/filename
+++ b/filename
@@ -1,5 +1,5 @@
apple
-grape
+orange
...
...
...
@@ -8,6 +8,8 @@ grape
...
...
...
+pear
+lemon
...
...
...
`
const twoHunksWithMoreAdditionsThanRemovals = `diff --git a/filename b/filename
index bac359d75..6e5b89f36 100644
--- a/filename
+++ b/filename
@@ -1,5 +1,6 @@
apple
-grape
+orange
+kiwi
...
...
...
@@ -8,6 +9,8 @@ grape
...
...
...
+pear
+lemon
...
...
...
`
const twoChangesInOneHunk = `diff --git a/filename b/filename
index 9320895..6d79956 100644
--- a/filename
+++ b/filename
@@ -1,5 +1,5 @@
apple
-grape
+kiwi
orange
-pear
+banana
lemon
`
// Two deletions with no line between them: they share a new-file line number
// (both sit at the same new-file position), so only their old-file line numbers
// distinguish them.
const consecutiveDeletions = `diff --git a/filename b/filename
index 9320895..6d79956 100644
--- a/filename
+++ b/filename
@@ -1,4 +1,2 @@
apple
-grape
-pear
lemon
`
const newFile = `diff --git a/newfile b/newfile
new file mode 100644
index 0000000..4e680cc
--- /dev/null
+++ b/newfile
@@ -0,0 +1,3 @@
+apple
+orange
+grape
`
const deletedFile = `diff --git a/newfile b/newfile
deleted file mode 100644
index 4e680cc1f..000000000
--- a/newfile
+++ /dev/null
@@ -1,3 +0,0 @@
-apple
-orange
-grape
`
const addNewlineToPreviouslyEmptyFile = `diff --git a/newfile b/newfile
index e69de29..c6568ea 100644
--- a/newfile
+++ b/newfile
@@ -0,0 +1 @@
+new line
\ No newline at end of file
`
const exampleHunk = `@@ -1,5 +1,5 @@
apple
-grape
+orange
...
...
...
`
func TestTransform(t *testing.T) {
type scenario struct {
testName string
filename string
diffText string
firstLineIndex int
lastLineIndex int
reverse bool
stripRename bool
expected string
}
scenarios := []scenario{
{
testName: "nothing selected",
filename: "filename",
firstLineIndex: -1,
lastLineIndex: -1,
diffText: simpleDiff,
expected: "",
},
{
testName: "only context selected",
filename: "filename",
firstLineIndex: 5,
lastLineIndex: 5,
diffText: simpleDiff,
expected: "",
},
{
testName: "whole range selected",
filename: "filename",
firstLineIndex: 0,
lastLineIndex: 11,
diffText: simpleDiff,
expected: `--- a/filename
+++ b/filename
@@ -1,5 +1,5 @@
apple
-orange
+grape
...
...
...
`,
},
{
testName: "only removal selected",
filename: "filename",
firstLineIndex: 6,
lastLineIndex: 6,
diffText: simpleDiff,
expected: `--- a/filename
+++ b/filename
@@ -1,5 +1,4 @@
apple
-orange
...
...
...
`,
},
{
testName: "only addition selected",
filename: "filename",
firstLineIndex: 7,
lastLineIndex: 7,
diffText: simpleDiff,
expected: `--- a/filename
+++ b/filename
@@ -1,5 +1,6 @@
apple
+grape
orange
...
...
...
`,
},
{
testName: "range that extends beyond diff bounds",
filename: "filename",
firstLineIndex: -100,
lastLineIndex: 100,
diffText: simpleDiff,
expected: `--- a/filename
+++ b/filename
@@ -1,5 +1,5 @@
apple
-orange
+grape
...
...
...
`,
},
{
testName: "add newline to end of file",
filename: "filename",
firstLineIndex: -100,
lastLineIndex: 100,
diffText: addNewlineToEndOfFile,
expected: `--- a/filename
+++ b/filename
@@ -60,4 +60,4 @@ grape
...
...
...
-last line
\ No newline at end of file
+last line
`,
},
{
testName: "add newline to end of file, reversed",
filename: "filename",
firstLineIndex: -100,
lastLineIndex: 100,
reverse: true,
diffText: addNewlineToEndOfFile,
expected: `--- a/filename
+++ b/filename
@@ -60,4 +60,4 @@ grape
...
...
...
-last line
\ No newline at end of file
+last line
`,
},
{
testName: "remove newline from end of file",
filename: "filename",
firstLineIndex: -100,
lastLineIndex: 100,
diffText: removeNewlinefromEndOfFile,
expected: `--- a/filename
+++ b/filename
@@ -60,4 +60,4 @@ grape
...
...
...
-last line
+last line
\ No newline at end of file
`,
},
{
testName: "remove newline from end of file, reversed",
filename: "filename",
firstLineIndex: -100,
lastLineIndex: 100,
reverse: true,
diffText: removeNewlinefromEndOfFile,
expected: `--- a/filename
+++ b/filename
@@ -60,4 +60,4 @@ grape
...
...
...
-last line
+last line
\ No newline at end of file
`,
},
{
testName: "remove newline from end of file, removal only",
filename: "filename",
firstLineIndex: 8,
lastLineIndex: 8,
diffText: removeNewlinefromEndOfFile,
expected: `--- a/filename
+++ b/filename
@@ -60,4 +60,3 @@ grape
...
...
...
-last line
`,
},
{
testName: "remove newline from end of file, removal only, reversed",
filename: "filename",
firstLineIndex: 8,
lastLineIndex: 8,
reverse: true,
diffText: removeNewlinefromEndOfFile,
expected: `--- a/filename
+++ b/filename
@@ -60,5 +60,4 @@ grape
...
...
...
-last line
last line
\ No newline at end of file
`,
},
{
testName: "remove newline from end of file, addition only",
filename: "filename",
firstLineIndex: 9,
lastLineIndex: 9,
diffText: removeNewlinefromEndOfFile,
expected: `--- a/filename
+++ b/filename
@@ -60,4 +60,5 @@ grape
...
...
...
+last line
last line
\ No newline at end of file
`,
},
{
testName: "remove newline from end of file, addition only, reversed",
filename: "filename",
firstLineIndex: 9,
lastLineIndex: 9,
reverse: true,
diffText: removeNewlinefromEndOfFile,
expected: `--- a/filename
+++ b/filename
@@ -60,3 +60,4 @@ grape
...
...
...
+last line
\ No newline at end of file
`,
},
{
testName: "staging two whole hunks",
filename: "filename",
firstLineIndex: -100,
lastLineIndex: 100,
diffText: twoHunks,
expected: `--- a/filename
+++ b/filename
@@ -1,5 +1,5 @@
apple
-grape
+orange
...
...
...
@@ -8,6 +8,8 @@ grape
...
...
...
+pear
+lemon
...
...
...
`,
},
{
testName: "staging part of both hunks",
filename: "filename",
firstLineIndex: 7,
lastLineIndex: 15,
diffText: twoHunks,
expected: `--- a/filename
+++ b/filename
@@ -1,5 +1,6 @@
apple
+orange
grape
...
...
...
@@ -8,6 +9,7 @@ grape
...
...
...
+pear
...
...
...
`,
},
{
testName: "adding a new file",
filename: "newfile",
firstLineIndex: -100,
lastLineIndex: 100,
diffText: newFile,
expected: `--- a/newfile
+++ b/newfile
@@ -0,0 +1,3 @@
+apple
+orange
+grape
`,
},
{
testName: "adding part of a new file",
filename: "newfile",
firstLineIndex: 6,
lastLineIndex: 7,
diffText: newFile,
expected: `--- a/newfile
+++ b/newfile
@@ -0,0 +1,2 @@
+apple
+orange
`,
},
{
testName: "adding a new line to a previously empty file",
filename: "newfile",
firstLineIndex: -100,
lastLineIndex: 100,
diffText: addNewlineToPreviouslyEmptyFile,
expected: `--- a/newfile
+++ b/newfile
@@ -0,0 +1 @@
+new line
\ No newline at end of file
`,
},
{
testName: "adding a new line to a previously empty file, reversed",
filename: "newfile",
firstLineIndex: -100,
lastLineIndex: 100,
diffText: addNewlineToPreviouslyEmptyFile,
reverse: true,
expected: `--- a/newfile
+++ b/newfile
@@ -0,0 +1 @@
+new line
\ No newline at end of file
`,
},
{
testName: "adding part of a hunk",
filename: "filename",
firstLineIndex: 6,
lastLineIndex: 7,
reverse: false,
diffText: twoChangesInOneHunk,
expected: `--- a/filename
+++ b/filename
@@ -1,5 +1,5 @@
apple
-grape
+kiwi
orange
pear
lemon
`,
},
{
testName: "adding part of a hunk, reverse",
filename: "filename",
firstLineIndex: 6,
lastLineIndex: 7,
reverse: true,
diffText: twoChangesInOneHunk,
expected: `--- a/filename
+++ b/filename
@@ -1,5 +1,5 @@
apple
-grape
+kiwi
orange
banana
lemon
`,
},
{
testName: "renamed file, whole change selected, strips the rename so only the content change is applied",
firstLineIndex: 9,
lastLineIndex: 10,
stripRename: true,
diffText: renameWithModificationDiff,
expected: `diff --git a/newname b/newname
index dcd3485..1ba5540 100644
--- a/newname
+++ b/newname
@@ -1,5 +1,5 @@
apple
-orange
+grape
...
...
...
`,
},
{
testName: "renamed file, only removal selected, strips the rename",
firstLineIndex: 9,
lastLineIndex: 9,
stripRename: true,
diffText: renameWithModificationDiff,
expected: `diff --git a/newname b/newname
index dcd3485..1ba5540 100644
--- a/newname
+++ b/newname
@@ -1,5 +1,4 @@
apple
-orange
...
...
...
`,
},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
lineIndices := ExpandRange(s.firstLineIndex, s.lastLineIndex)
result := Parse(s.diffText).
Transform(TransformOpts{
Reverse: s.reverse,
FileNameOverride: s.filename,
StripRename: s.stripRename,
IncludedLineIndices: lineIndices,
}).
FormatPlain()
assert.Equal(t, s.expected, result)
})
}
}
func TestParseAndFormatPlain(t *testing.T) {
scenarios := []struct {
testName string
patchStr string
}{
{
testName: "simpleDiff",
patchStr: simpleDiff,
},
{
testName: "addNewlineToEndOfFile",
patchStr: addNewlineToEndOfFile,
},
{
testName: "removeNewlinefromEndOfFile",
patchStr: removeNewlinefromEndOfFile,
},
{
testName: "twoHunks",
patchStr: twoHunks,
},
{
testName: "twoChangesInOneHunk",
patchStr: twoChangesInOneHunk,
},
{
testName: "newFile",
patchStr: newFile,
},
{
testName: "deletedFile",
patchStr: deletedFile,
},
{
testName: "addNewlineToPreviouslyEmptyFile",
patchStr: addNewlineToPreviouslyEmptyFile,
},
{
testName: "exampleHunk",
patchStr: exampleHunk,
},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
// here we parse the patch, then format it, and ensure the result
// matches the original patch. Note that unified diffs allow omitting
// the new length in a hunk header if the value is 1, and currently we always
// omit the new length in such cases.
patch := Parse(s.patchStr)
result := formatPlain(patch)
assert.Equal(t, s.patchStr, result)
})
}
}
func TestLineNumberOfLine(t *testing.T) {
type scenario struct {
testName string
patchStr string
indexes []int
expecteds []int
}
scenarios := []scenario{
{
testName: "twoHunks",
patchStr: twoHunks,
// this is really more of a characteristic test than anything.
indexes: []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 1000},
expecteds: []int{1, 1, 1, 1, 1, 1, 2, 2, 3, 4, 5, 8, 8, 9, 10, 11, 12, 13, 14, 15, 15, 15, 15, 15, 15},
},
{
testName: "twoHunksWithMoreAdditionsThanRemovals",
patchStr: twoHunksWithMoreAdditionsThanRemovals,
indexes: []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 1000},
expecteds: []int{1, 1, 1, 1, 1, 1, 2, 2, 3, 4, 5, 6, 9, 9, 10, 11, 12, 13, 14, 15, 16, 16, 16, 16, 16, 16},
},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
for i, idx := range s.indexes {
patch := Parse(s.patchStr)
result := patch.LineNumberOfLine(idx)
assert.Equal(t, s.expecteds[i], result)
}
})
}
}
func TestOldLineNumberOfLine(t *testing.T) {
type scenario struct {
testName string
patchStr string
indexes []int
expecteds []int
}
scenarios := []scenario{
{
testName: "twoChangesInOneHunk",
patchStr: twoChangesInOneHunk,
indexes: []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 1000},
expecteds: []int{1, 1, 1, 1, 1, 1, 2, 3, 3, 4, 5, 5, 5},
},
{
testName: "consecutiveDeletions",
patchStr: consecutiveDeletions,
indexes: []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 1000},
expecteds: []int{1, 1, 1, 1, 1, 1, 2, 3, 4, 4},
},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
for i, idx := range s.indexes {
patch := Parse(s.patchStr)
result := patch.OldLineNumberOfLine(idx)
assert.Equal(t, s.expecteds[i], result)
}
})
}
}
func TestPatchLineForOldLineNumber(t *testing.T) {
type scenario struct {
testName string
patchStr string
lineNumbers []int
expecteds []int
}
scenarios := []scenario{
{
// The two consecutive deletions (grape, pear) sit at patch line
// indices 6 and 7; their old-file line numbers (2 and 3) keep them
// distinct even though their new-file line numbers are both 2.
testName: "consecutiveDeletions",
patchStr: consecutiveDeletions,
lineNumbers: []int{0, 1, 2, 3, 4, 5},
expecteds: []int{4, 4, 6, 7, 8, 8},
},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
for i, lineNumber := range s.lineNumbers {
patch := Parse(s.patchStr)
result := patch.PatchLineForOldLineNumber(lineNumber)
assert.Equal(t, s.expecteds[i], result)
}
})
}
}
func TestIsWellFormed(t *testing.T) {
assert.True(t, Parse(simpleDiff).IsWellFormed())
assert.True(t, Parse(twoHunks).IsWellFormed())
assert.True(t, Parse(consecutiveDeletions).IsWellFormed())
assert.True(t, Parse(newFile).IsWellFormed())
assert.True(t, Parse(deletedFile).IsWellFormed())
assert.True(t, Parse(addNewlineToEndOfFile).IsWellFormed())
// A diff whose body has been mangled so the +/- markers no longer start each
// line (as delta's line-number gutters do) reads as all-context, so the body
// lengths no longer match the header.
gutterMangled := `diff --git a/filename b/filename
index 9320895..6d79956 100644
--- a/filename
+++ b/filename
@@ -1,4 +1,2 @@
apple
grape
pear
lemon
`
assert.False(t, Parse(gutterMangled).IsWellFormed())
}
func TestGetNextStageableLineIndex(t *testing.T) {
type scenario struct {
testName string
patchStr string
indexes []int
expecteds []int
}
scenarios := []scenario{
{
testName: "twoHunks",
patchStr: twoHunks,
indexes: []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 1000},
expecteds: []int{6, 6, 6, 6, 6, 6, 6, 7, 15, 15, 15, 15, 15, 15, 15, 15, 16, 16, 16, 16, 16, 16, 16, 16, 16},
},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
for i, idx := range s.indexes {
patch := Parse(s.patchStr)
result := patch.GetNextChangeIdx(idx)
assert.Equal(t, s.expecteds[i], result)
}
})
}
}
func TestAdjustLineNumber(t *testing.T) {
type scenario struct {
oldLineNumbers []int
expectedResults []int
}
scenarios := []scenario{
{
oldLineNumbers: []int{1, 2, 3, 4, 5, 6, 7},
expectedResults: []int{1, 2, 2, 3, 4, 7, 8},
},
}
// The following diff was generated from old.txt:
// 1
// 2a
// 2b
// 3
// 4
// 7
// 8
// against new.txt:
// 1
// 2
// 3
// 4
// 5
// 6
// 7
// 8
// This test setup makes the test easy to understand, because the resulting
// adjusted line numbers are the same as the content of the lines in new.txt.
diff := `--- old.txt 2024-12-16 18:04:29
+++ new.txt 2024-12-16 18:04:27
@@ -2,2 +2 @@
-2a
-2b
+2
@@ -5,0 +5,2 @@
+5
+6
`
patch := Parse(diff)
for _, s := range scenarios {
t.Run("TestAdjustLineNumber", func(t *testing.T) {
for idx, oldLineNumber := range s.oldLineNumbers {
result := patch.AdjustLineNumber(oldLineNumber)
assert.Equal(t, s.expectedResults[idx], result)
}
})
}
}
func TestIsSingleHunkForWholeFile(t *testing.T) {
scenarios := []struct {
testName string
patchStr string
expectedResult bool
}{
{
testName: "simpleDiff",
patchStr: simpleDiff,
expectedResult: false,
},
{
testName: "addNewlineToEndOfFile",
patchStr: addNewlineToEndOfFile,
expectedResult: false,
},
{
testName: "removeNewlinefromEndOfFile",
patchStr: removeNewlinefromEndOfFile,
expectedResult: false,
},
{
testName: "twoHunks",
patchStr: twoHunks,
expectedResult: false,
},
{
testName: "twoChangesInOneHunk",
patchStr: twoChangesInOneHunk,
expectedResult: false,
},
{
testName: "newFile",
patchStr: newFile,
expectedResult: true,
},
{
testName: "deletedFile",
patchStr: deletedFile,
expectedResult: true,
},
{
testName: "addNewlineToPreviouslyEmptyFile",
patchStr: addNewlineToPreviouslyEmptyFile,
expectedResult: true,
},
{
testName: "exampleHunk",
patchStr: exampleHunk,
expectedResult: false,
},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
patch := Parse(s.patchStr)
assert.Equal(t, s.expectedResult, patch.IsSingleHunkForWholeFile())
})
}
}