repos / git-pr

a self-hosted git collaboration server
git clone https://github.com/picosh/git-pr.git

commit
faecaa7
parent
674b407
author
Eric Bower
date
2025-12-13 12:06:09 -0500 EST
chore: more granular diffs
9 files changed,  +245, -48
A fixtures/context_lines_v1.patch
+24, -0
 1@@ -0,0 +1,24 @@
 2+From aaaa1111111111111111111111111111aaaaaaaa Mon Sep 17 00:00:00 2001
 3+From: Test User <test@example.com>
 4+Date: Tue, 23 Jul 2024 10:07:57 -0400
 5+Subject: [PATCH] fix: update config value
 6+
 7+---
 8+ config.txt | 2 +-
 9+ 1 file changed, 1 insertion(+), 1 deletion(-)
10+
11+diff --git a/config.txt b/config.txt
12+index 1234567..abcdefg 100644
13+--- a/config.txt
14++++ b/config.txt
15+@@ -5,7 +5,7 @@
16+ line 5
17+ line 6
18+ line 7
19+-old_value=100
20++new_value=200
21+ line 9
22+ line 10
23+ line 11
24+-- 
25+2.45.2
A fixtures/context_lines_v2.patch
+24, -0
 1@@ -0,0 +1,24 @@
 2+From bbbb2222222222222222222222222222bbbbbbbb Mon Sep 17 00:00:00 2001
 3+From: Test User <test@example.com>
 4+Date: Tue, 23 Jul 2024 10:07:57 -0400
 5+Subject: [PATCH] fix: update config value
 6+
 7+---
 8+ config.txt | 2 +-
 9+ 1 file changed, 1 insertion(+), 1 deletion(-)
10+
11+diff --git a/config.txt b/config.txt
12+index 2345678..bcdefgh 100644
13+--- a/config.txt
14++++ b/config.txt
15+@@ -15,7 +15,7 @@
16+ line 15
17+ line 16
18+ line 17
19+-old_value=100
20++new_value=200
21+ line 19
22+ line 20
23+ line 21
24+-- 
25+2.45.2
A fixtures/hunk_header_v1.patch
+24, -0
 1@@ -0,0 +1,24 @@
 2+From cccc3333333333333333333333333333cccccccc Mon Sep 17 00:00:00 2001
 3+From: Test User <test@example.com>
 4+Date: Tue, 23 Jul 2024 10:07:57 -0400
 5+Subject: [PATCH] fix: change timeout value
 6+
 7+---
 8+ server.go | 2 +-
 9+ 1 file changed, 1 insertion(+), 1 deletion(-)
10+
11+diff --git a/server.go b/server.go
12+index 1111111..2222222 100644
13+--- a/server.go
14++++ b/server.go
15+@@ -10,7 +10,7 @@ func init() {
16+ 	log.Println("starting")
17+ 	db.Connect()
18+ 	cache.Init()
19+-	timeout := 30
20++	timeout := 60
21+ 	server.Start()
22+ 	log.Println("ready")
23+ }
24+-- 
25+2.45.2
A fixtures/hunk_header_v2.patch
+24, -0
 1@@ -0,0 +1,24 @@
 2+From dddd4444444444444444444444444444dddddddd Mon Sep 17 00:00:00 2001
 3+From: Test User <test@example.com>
 4+Date: Tue, 23 Jul 2024 10:07:57 -0400
 5+Subject: [PATCH] fix: change timeout value
 6+
 7+---
 8+ server.go | 2 +-
 9+ 1 file changed, 1 insertion(+), 1 deletion(-)
10+
11+diff --git a/server.go b/server.go
12+index 3333333..4444444 100644
13+--- a/server.go
14++++ b/server.go
15+@@ -25,7 +25,7 @@ func init() {
16+ 	log.Println("starting")
17+ 	db.Connect()
18+ 	cache.Init()
19+-	timeout := 30
20++	timeout := 60
21+ 	server.Start()
22+ 	log.Println("ready")
23+ }
24+-- 
25+2.45.2
M range_diff.go
+70, -33
  1@@ -47,12 +47,14 @@ func output(a []*PatchRange, b []*PatchRange) []*RangeDiffOutput {
  2 	for i, patchA := range a {
  3 		if patchA.Matching == -1 {
  4 			hdr := NewRangeDiffHeader(patchA, nil, i+1, -1)
  5+			files := outputRemovedPatch(patchA)
  6 			outputs = append(
  7 				outputs,
  8 				&RangeDiffOutput{
  9 					Header: hdr,
 10 					Type:   "rm",
 11 					Order:  i + 1,
 12+					Files:  files,
 13 				},
 14 			)
 15 		}
 16@@ -61,12 +63,14 @@ func output(a []*PatchRange, b []*PatchRange) []*RangeDiffOutput {
 17 	for j, patchB := range b {
 18 		if patchB.Matching == -1 {
 19 			hdr := NewRangeDiffHeader(nil, patchB, -1, j+1)
 20+			files := outputAddedPatch(patchB)
 21 			outputs = append(
 22 				outputs,
 23 				&RangeDiffOutput{
 24 					Header: hdr,
 25 					Type:   "add",
 26 					Order:  j + 1,
 27+					Files:  files,
 28 				},
 29 			)
 30 			continue
 31@@ -153,6 +157,33 @@ func DoDiff(src, dst string) []RangeDiffDiff {
 32 	return toRangeDiffDiff(diffs)
 33 }
 34 
 35+// extractChangedLines extracts only added and deleted lines from a file's fragments,
 36+// ignoring context lines. This is used for comparing patches where context lines
 37+// may differ due to rebasing but the actual changes are the same.
 38+func extractChangedLines(file *gitdiff.File) string {
 39+	var result strings.Builder
 40+	for _, frag := range file.TextFragments {
 41+		for _, line := range frag.Lines {
 42+			if line.Op == gitdiff.OpAdd || line.Op == gitdiff.OpDelete {
 43+				result.WriteString(line.String())
 44+			}
 45+		}
 46+	}
 47+	return result.String()
 48+}
 49+
 50+// extractAllLines extracts all lines (including context) from a file's fragments.
 51+// This is used for displaying the full diff with context.
 52+func extractAllLines(file *gitdiff.File) string {
 53+	var result strings.Builder
 54+	for _, frag := range file.TextFragments {
 55+		for _, line := range frag.Lines {
 56+			result.WriteString(line.String())
 57+		}
 58+	}
 59+	return result.String()
 60+}
 61+
 62 type RangeDiffFile struct {
 63 	OldFile *gitdiff.File
 64 	NewFile *gitdiff.File
 65@@ -171,29 +202,17 @@ func outputDiff(patchA, patchB *PatchRange) []*RangeDiffFile {
 66 				if fileA.NewName == "" {
 67 					continue
 68 				}
 69-				strA := ""
 70-				for _, frag := range fileA.TextFragments {
 71-					for _, line := range frag.Lines {
 72-						strA += line.String()
 73-					}
 74-				}
 75-				strB := ""
 76-				for _, frag := range fileB.TextFragments {
 77-					for _, line := range frag.Lines {
 78-						strB += line.String()
 79-					}
 80-				}
 81-				curDiff := DoDiff(strA, strB)
 82-				hasDiff := false
 83-				for _, dd := range curDiff {
 84-					if dd.OuterType != "equal" {
 85-						hasDiff = true
 86-						break
 87-					}
 88-				}
 89-				if !hasDiff {
 90+				// Compare only +/- lines to determine if there's a meaningful diff
 91+				changedA := extractChangedLines(fileA)
 92+				changedB := extractChangedLines(fileB)
 93+				if changedA == changedB {
 94+					// No difference in actual changes, skip this file
 95 					continue
 96 				}
 97+				// Use full lines (with context) for display
 98+				strA := extractAllLines(fileA)
 99+				strB := extractAllLines(fileB)
100+				curDiff := DoDiff(strA, strB)
101 				fp := &RangeDiffFile{
102 					OldFile: fileA,
103 					NewFile: fileB,
104@@ -205,12 +224,7 @@ func outputDiff(patchA, patchB *PatchRange) []*RangeDiffFile {
105 
106 		// find files in patchA but not in patchB
107 		if !found {
108-			strA := ""
109-			for _, frag := range fileA.TextFragments {
110-				for _, line := range frag.Lines {
111-					strA += line.String()
112-				}
113-			}
114+			strA := extractAllLines(fileA)
115 			fp := &RangeDiffFile{
116 				OldFile: fileA,
117 				NewFile: nil,
118@@ -231,12 +245,7 @@ func outputDiff(patchA, patchB *PatchRange) []*RangeDiffFile {
119 		}
120 
121 		if !found {
122-			strB := ""
123-			for _, frag := range fileB.TextFragments {
124-				for _, line := range frag.Lines {
125-					strB += line.String()
126-				}
127-			}
128+			strB := extractAllLines(fileB)
129 			fp := &RangeDiffFile{
130 				OldFile: nil,
131 				NewFile: fileB,
132@@ -249,6 +258,34 @@ func outputDiff(patchA, patchB *PatchRange) []*RangeDiffFile {
133 	return diffs
134 }
135 
136+func outputAddedPatch(patch *PatchRange) []*RangeDiffFile {
137+	diffs := []*RangeDiffFile{}
138+	for _, file := range patch.Files {
139+		strB := extractAllLines(file)
140+		fp := &RangeDiffFile{
141+			OldFile: nil,
142+			NewFile: file,
143+			Diff:    DoDiff("", strB),
144+		}
145+		diffs = append(diffs, fp)
146+	}
147+	return diffs
148+}
149+
150+func outputRemovedPatch(patch *PatchRange) []*RangeDiffFile {
151+	diffs := []*RangeDiffFile{}
152+	for _, file := range patch.Files {
153+		strA := extractAllLines(file)
154+		fp := &RangeDiffFile{
155+			OldFile: file,
156+			NewFile: nil,
157+			Diff:    DoDiff(strA, ""),
158+		}
159+		diffs = append(diffs, fp)
160+	}
161+	return diffs
162+}
163+
164 // RangeDiffHeader is a header combining old and new change pairs.
165 type RangeDiffHeader struct {
166 	OldIdx       int
A range_diff_issues.txt
+12, -0
 1@@ -0,0 +1,12 @@
 2+When git format-patch is run at different times or from different base commits, even for identical code changes, the patches can differ in:
 3+
 4+Element
 5+Why it changes
 6+Hunk headers (@@ -10,7 +10,7 @@)
 7+Line numbers shift if upstream changed
 8+Context lines
 9+Different surrounding code after rebase
10+Hunk boundaries
11+Git may split/merge hunks differently
12+Function context (@@ ... @@ func name)
13+Different function nearby
M range_diff_test.go
+47, -11
 1@@ -74,11 +74,14 @@ func TestRangeDiffTrivialReordering(t *testing.T) {
 2 */
 3 func TestRangeDiffRemovedCommit(t *testing.T) {
 4 	actual := cmp("a_b_reorder.patch", "a_c_rm_commit.patch")
 5-	expected := `1:  33c682a < -:  ------- chore: add torch and create random tensor
 6-2:  22dde12 = 1:  7dbb94c docs: readme
 7-`
 8-	if expected != actual {
 9-		t.Fatal(fail(expected, actual))
10+	if !strings.Contains(actual, "1:  33c682a < -:  ------- chore: add torch and create random tensor") {
11+		t.Fatal("expected removed commit header not found")
12+	}
13+	if !strings.Contains(actual, "2:  22dde12 = 1:  7dbb94c docs: readme") {
14+		t.Fatal("expected equal commit header not found")
15+	}
16+	if !strings.Contains(actual, "requirements.txt") {
17+		t.Fatal("expected file diff for removed commit")
18 	}
19 }
20 
21@@ -92,12 +95,17 @@ func TestRangeDiffRemovedCommit(t *testing.T) {
22 */
23 func TestRangeDiffAddedCommit(t *testing.T) {
24 	actual := cmp("a_b_reorder.patch", "a_c_added_commit.patch")
25-	expected := `1:  33c682a = 1:  33c682a chore: add torch and create random tensor
26-2:  22dde12 = 2:  22dde12 docs: readme
27--:  ------- > 3:  b248060 chore: make tensor 6x6
28-`
29-	if expected != actual {
30-		t.Fatal(fail(expected, actual))
31+	if !strings.Contains(actual, "1:  33c682a = 1:  33c682a chore: add torch and create random tensor") {
32+		t.Fatal("expected first equal commit header not found")
33+	}
34+	if !strings.Contains(actual, "2:  22dde12 = 2:  22dde12 docs: readme") {
35+		t.Fatal("expected second equal commit header not found")
36+	}
37+	if !strings.Contains(actual, "-:  ------- > 3:  b248060 chore: make tensor 6x6") {
38+		t.Fatal("expected added commit header not found")
39+	}
40+	if !strings.Contains(actual, "train.py") {
41+		t.Fatal("expected file diff for added commit")
42 	}
43 }
44 
45@@ -412,3 +420,31 @@ func TestRangeDiffMultipleFilesInCommit(t *testing.T) {
46 		t.Fatal("expected LICENSE.md in diff output")
47 	}
48 }
49+
50+func TestRangeDiffIgnoresContextLines(t *testing.T) {
51+	actual := cmp("context_lines_v1.patch", "context_lines_v2.patch")
52+
53+	if !strings.Contains(actual, "=") {
54+		t.Fatal("expected equal marker (=) since +/- lines are identical")
55+	}
56+	if strings.Contains(actual, "!") {
57+		t.Fatal("should not show diff marker (!) when only context lines differ")
58+	}
59+	if strings.Contains(actual, "old_value") || strings.Contains(actual, "new_value") {
60+		t.Fatal("should not have file diff output when changes are equal")
61+	}
62+}
63+
64+func TestRangeDiffNormalizesHunkHeaders(t *testing.T) {
65+	actual := cmp("hunk_header_v1.patch", "hunk_header_v2.patch")
66+
67+	if !strings.Contains(actual, "=") {
68+		t.Fatal("expected equal marker (=) since changes are identical despite different hunk headers")
69+	}
70+	if strings.Contains(actual, "!") {
71+		t.Fatal("should not show diff marker (!) when only hunk header line numbers differ")
72+	}
73+	if strings.Contains(actual, "@@ server.go") {
74+		t.Fatal("should not have file diff output when changes are equal")
75+	}
76+}
M tmpl/components/range-diff.html
+4, -2
 1@@ -82,8 +82,10 @@
 2                     <div class="flex-1" style="width: 48%;">
 3                       <h3 class="text-md">new</h3>
 4                       <div>
 5-                        {{if .NewFile.OldName}}old:<code>{{.NewFile.OldName}}</code>{{end}}
 6-                        {{if .NewFile.NewName}}new:<code>{{.NewFile.NewName}}</code>{{end}}
 7+                        {{if .NewFile}}
 8+                          {{if .NewFile.OldName}}old:<code>{{.NewFile.OldName}}</code>{{end}}
 9+                          {{if .NewFile.NewName}}new:<code>{{.NewFile.NewName}}</code>{{end}}
10+                        {{end}}
11                       </div>
12                       <pre class="m-0">{{- range .Diff -}}
13                         {{- if eq .OuterType "insert" -}}
M util.go
+16, -2
 1@@ -172,6 +172,8 @@ func ParsePatchset(patchset io.Reader) ([]*Patch, error) {
 2 // changes related to a patch.
 3 // We cannot rely on patch.CommitSha because it includes the commit date
 4 // that will change when a user fetches and applies the patch locally.
 5+// We only include +/- lines (not context) so that rebased patches with
 6+// different context lines but identical changes are considered equal.
 7 func calcContentSha(diffFiles []*gitdiff.File, header *gitdiff.PatchHeader) string {
 8 	authorName := ""
 9 	authorEmail := ""
10@@ -203,13 +205,25 @@ func calcContentSha(diffFiles []*gitdiff.File, header *gitdiff.PatchHeader) stri
11 			continue
12 		}
13 
14+		// Include file names and mode changes, but not OID prefixes since those
15+		// change when context lines shift (e.g., after rebase)
16 		dff := fmt.Sprintf(
17-			"%s->%s %s..%s %s->%s\n",
18+			"%s->%s %s->%s\n",
19 			diff.OldName, diff.NewName,
20-			diff.OldOIDPrefix, diff.NewOIDPrefix,
21 			diff.OldMode.String(), diff.NewMode.String(),
22 		)
23 		content += dff
24+
25+		// Include only added and deleted lines, not context lines.
26+		// This ensures patches with identical changes but different context
27+		// (due to rebasing) are considered equal.
28+		for _, frag := range diff.TextFragments {
29+			for _, line := range frag.Lines {
30+				if line.Op == gitdiff.OpAdd || line.Op == gitdiff.OpDelete {
31+					content += line.String()
32+				}
33+			}
34+		}
35 	}
36 	sha := sha256.Sum256([]byte(content))
37 	shaStr := hex.EncodeToString(sha[:])