mirror of
https://github.com/go-gitea/gitea.git
synced 2025-10-27 17:06:28 +01:00
feat(diff): Enable commenting on expanded lines in PR diffs (#35662)
Fixes #32257 /claim #32257 Implemented commenting on unchanged lines in Pull Request diffs, lines are accessed by expanding the diff preview. Comments also appear in the "Files Changed" tab on the unchanged lines where they were placed. --------- Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Bryan Mutai
GitHub
@@ -640,3 +640,346 @@ func TestNoCrashes(t *testing.T) {
|
||||
ParsePatch(t.Context(), setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(testcase.gitdiff), "")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGeneratePatchForUnchangedLineFromReader(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
content string
|
||||
treePath string
|
||||
line int64
|
||||
contextLines int
|
||||
want string
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
name: "single line with context",
|
||||
content: "line1\nline2\nline3\nline4\nline5\n",
|
||||
treePath: "test.txt",
|
||||
line: 3,
|
||||
contextLines: 1,
|
||||
want: `diff --git a/test.txt b/test.txt
|
||||
--- a/test.txt
|
||||
+++ b/test.txt
|
||||
@@ -2,2 +2,2 @@
|
||||
line2
|
||||
line3
|
||||
`,
|
||||
},
|
||||
{
|
||||
name: "negative line number (left side)",
|
||||
content: "line1\nline2\nline3\nline4\nline5\n",
|
||||
treePath: "test.txt",
|
||||
line: -3,
|
||||
contextLines: 1,
|
||||
want: `diff --git a/test.txt b/test.txt
|
||||
--- a/test.txt
|
||||
+++ b/test.txt
|
||||
@@ -2,2 +2,2 @@
|
||||
line2
|
||||
line3
|
||||
`,
|
||||
},
|
||||
{
|
||||
name: "line near start of file",
|
||||
content: "line1\nline2\nline3\n",
|
||||
treePath: "test.txt",
|
||||
line: 2,
|
||||
contextLines: 5,
|
||||
want: `diff --git a/test.txt b/test.txt
|
||||
--- a/test.txt
|
||||
+++ b/test.txt
|
||||
@@ -1,2 +1,2 @@
|
||||
line1
|
||||
line2
|
||||
`,
|
||||
},
|
||||
{
|
||||
name: "first line with context",
|
||||
content: "line1\nline2\nline3\n",
|
||||
treePath: "test.txt",
|
||||
line: 1,
|
||||
contextLines: 3,
|
||||
want: `diff --git a/test.txt b/test.txt
|
||||
--- a/test.txt
|
||||
+++ b/test.txt
|
||||
@@ -1,1 +1,1 @@
|
||||
line1
|
||||
`,
|
||||
},
|
||||
{
|
||||
name: "zero context lines",
|
||||
content: "line1\nline2\nline3\n",
|
||||
treePath: "test.txt",
|
||||
line: 2,
|
||||
contextLines: 0,
|
||||
want: `diff --git a/test.txt b/test.txt
|
||||
--- a/test.txt
|
||||
+++ b/test.txt
|
||||
@@ -2,1 +2,1 @@
|
||||
line2
|
||||
`,
|
||||
},
|
||||
{
|
||||
name: "multi-line context",
|
||||
content: "package main\n\nfunc main() {\n fmt.Println(\"Hello\")\n}\n",
|
||||
treePath: "main.go",
|
||||
line: 4,
|
||||
contextLines: 2,
|
||||
want: `diff --git a/main.go b/main.go
|
||||
--- a/main.go
|
||||
+++ b/main.go
|
||||
@@ -2,3 +2,3 @@
|
||||
<SP>
|
||||
func main() {
|
||||
fmt.Println("Hello")
|
||||
`,
|
||||
},
|
||||
{
|
||||
name: "empty file",
|
||||
content: "",
|
||||
treePath: "empty.txt",
|
||||
line: 1,
|
||||
contextLines: 1,
|
||||
wantErr: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
reader := strings.NewReader(tt.content)
|
||||
got, err := generatePatchForUnchangedLineFromReader(reader, tt.treePath, tt.line, tt.contextLines)
|
||||
if tt.wantErr {
|
||||
assert.Error(t, err)
|
||||
} else {
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, strings.ReplaceAll(tt.want, "<SP>", " "), got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCalculateHiddenCommentIDsForLine(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
line *DiffLine
|
||||
lineComments map[int64][]*issues_model.Comment
|
||||
expected []int64
|
||||
}{
|
||||
{
|
||||
name: "comments in hidden range",
|
||||
line: &DiffLine{
|
||||
Type: DiffLineSection,
|
||||
SectionInfo: &DiffLineSectionInfo{
|
||||
LastRightIdx: 10,
|
||||
RightIdx: 20,
|
||||
},
|
||||
},
|
||||
lineComments: map[int64][]*issues_model.Comment{
|
||||
15: {{ID: 100}, {ID: 101}},
|
||||
12: {{ID: 102}},
|
||||
},
|
||||
expected: []int64{100, 101, 102},
|
||||
},
|
||||
{
|
||||
name: "comments outside hidden range",
|
||||
line: &DiffLine{
|
||||
Type: DiffLineSection,
|
||||
SectionInfo: &DiffLineSectionInfo{
|
||||
LastRightIdx: 10,
|
||||
RightIdx: 20,
|
||||
},
|
||||
},
|
||||
lineComments: map[int64][]*issues_model.Comment{
|
||||
5: {{ID: 100}},
|
||||
25: {{ID: 101}},
|
||||
},
|
||||
expected: nil,
|
||||
},
|
||||
{
|
||||
name: "negative line numbers (left side)",
|
||||
line: &DiffLine{
|
||||
Type: DiffLineSection,
|
||||
SectionInfo: &DiffLineSectionInfo{
|
||||
LastRightIdx: 10,
|
||||
RightIdx: 20,
|
||||
},
|
||||
},
|
||||
lineComments: map[int64][]*issues_model.Comment{
|
||||
-15: {{ID: 100}}, // Left-side comment, should NOT be counted
|
||||
15: {{ID: 101}}, // Right-side comment, should be counted
|
||||
},
|
||||
expected: []int64{101}, // Only right-side comment
|
||||
},
|
||||
{
|
||||
name: "boundary conditions - normal expansion (both boundaries exclusive)",
|
||||
line: &DiffLine{
|
||||
Type: DiffLineSection,
|
||||
SectionInfo: &DiffLineSectionInfo{
|
||||
LastRightIdx: 10,
|
||||
RightIdx: 20,
|
||||
RightHunkSize: 5, // Normal case: next section has content
|
||||
},
|
||||
},
|
||||
lineComments: map[int64][]*issues_model.Comment{
|
||||
10: {{ID: 100}}, // at LastRightIdx (visible line), should NOT be included
|
||||
20: {{ID: 101}}, // at RightIdx (visible line), should NOT be included
|
||||
11: {{ID: 102}}, // just inside range, should be included
|
||||
19: {{ID: 103}}, // just inside range, should be included
|
||||
},
|
||||
expected: []int64{102, 103},
|
||||
},
|
||||
{
|
||||
name: "boundary conditions - end of file expansion (RightIdx inclusive)",
|
||||
line: &DiffLine{
|
||||
Type: DiffLineSection,
|
||||
SectionInfo: &DiffLineSectionInfo{
|
||||
LastRightIdx: 54,
|
||||
RightIdx: 70,
|
||||
RightHunkSize: 0, // End of file: no more content after
|
||||
},
|
||||
},
|
||||
lineComments: map[int64][]*issues_model.Comment{
|
||||
54: {{ID: 54}}, // at LastRightIdx (visible line), should NOT be included
|
||||
70: {{ID: 70}}, // at RightIdx (last hidden line), SHOULD be included
|
||||
60: {{ID: 60}}, // inside range, should be included
|
||||
},
|
||||
expected: []int64{60, 70}, // Lines 60 and 70 are hidden
|
||||
},
|
||||
{
|
||||
name: "real-world scenario - start of file with hunk",
|
||||
line: &DiffLine{
|
||||
Type: DiffLineSection,
|
||||
SectionInfo: &DiffLineSectionInfo{
|
||||
LastRightIdx: 0, // No previous visible section
|
||||
RightIdx: 26, // Line 26 is first visible line of hunk
|
||||
RightHunkSize: 9, // Normal hunk with content
|
||||
},
|
||||
},
|
||||
lineComments: map[int64][]*issues_model.Comment{
|
||||
1: {{ID: 1}}, // Line 1 is hidden
|
||||
26: {{ID: 26}}, // Line 26 is visible (hunk start) - should NOT be hidden
|
||||
10: {{ID: 10}}, // Line 10 is hidden
|
||||
15: {{ID: 15}}, // Line 15 is hidden
|
||||
},
|
||||
expected: []int64{1, 10, 15}, // Lines 1, 10, 15 are hidden; line 26 is visible
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
FillHiddenCommentIDsForDiffLine(tt.line, tt.lineComments)
|
||||
assert.ElementsMatch(t, tt.expected, tt.line.SectionInfo.HiddenCommentIDs)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDiffLine_RenderBlobExcerptButtons(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
line *DiffLine
|
||||
fileNameHash string
|
||||
data *DiffBlobExcerptData
|
||||
expectContains []string
|
||||
expectNotContain []string
|
||||
}{
|
||||
{
|
||||
name: "expand up button with hidden comments",
|
||||
line: &DiffLine{
|
||||
Type: DiffLineSection,
|
||||
SectionInfo: &DiffLineSectionInfo{
|
||||
LastRightIdx: 0,
|
||||
RightIdx: 26,
|
||||
LeftIdx: 26,
|
||||
LastLeftIdx: 0,
|
||||
LeftHunkSize: 0,
|
||||
RightHunkSize: 0,
|
||||
HiddenCommentIDs: []int64{100},
|
||||
},
|
||||
},
|
||||
fileNameHash: "abc123",
|
||||
data: &DiffBlobExcerptData{
|
||||
BaseLink: "/repo/blob_excerpt",
|
||||
AfterCommitID: "commit123",
|
||||
DiffStyle: "unified",
|
||||
},
|
||||
expectContains: []string{
|
||||
"octicon-fold-up",
|
||||
"direction=up",
|
||||
"code-comment-more",
|
||||
"1 hidden comment(s)",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "expand up and down buttons with pull request",
|
||||
line: &DiffLine{
|
||||
Type: DiffLineSection,
|
||||
SectionInfo: &DiffLineSectionInfo{
|
||||
LastRightIdx: 10,
|
||||
RightIdx: 50,
|
||||
LeftIdx: 10,
|
||||
LastLeftIdx: 5,
|
||||
LeftHunkSize: 5,
|
||||
RightHunkSize: 5,
|
||||
HiddenCommentIDs: []int64{200, 201},
|
||||
},
|
||||
},
|
||||
fileNameHash: "def456",
|
||||
data: &DiffBlobExcerptData{
|
||||
BaseLink: "/repo/blob_excerpt",
|
||||
AfterCommitID: "commit456",
|
||||
DiffStyle: "split",
|
||||
PullIssueIndex: 42,
|
||||
},
|
||||
expectContains: []string{
|
||||
"octicon-fold-down",
|
||||
"octicon-fold-up",
|
||||
"direction=down",
|
||||
"direction=up",
|
||||
`data-hidden-comment-ids=",200,201,"`, // use leading and trailing commas to ensure exact match by CSS selector `attr*=",id,"`
|
||||
"pull_issue_index=42",
|
||||
"2 hidden comment(s)",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "no hidden comments",
|
||||
line: &DiffLine{
|
||||
Type: DiffLineSection,
|
||||
SectionInfo: &DiffLineSectionInfo{
|
||||
LastRightIdx: 10,
|
||||
RightIdx: 20,
|
||||
LeftIdx: 10,
|
||||
LastLeftIdx: 5,
|
||||
LeftHunkSize: 5,
|
||||
RightHunkSize: 5,
|
||||
HiddenCommentIDs: nil,
|
||||
},
|
||||
},
|
||||
fileNameHash: "ghi789",
|
||||
data: &DiffBlobExcerptData{
|
||||
BaseLink: "/repo/blob_excerpt",
|
||||
AfterCommitID: "commit789",
|
||||
},
|
||||
expectContains: []string{
|
||||
"code-expander-button",
|
||||
},
|
||||
expectNotContain: []string{
|
||||
"code-comment-more",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
result := tt.line.RenderBlobExcerptButtons(tt.fileNameHash, tt.data)
|
||||
resultStr := string(result)
|
||||
|
||||
for _, expected := range tt.expectContains {
|
||||
assert.Contains(t, resultStr, expected, "Expected to contain: %s", expected)
|
||||
}
|
||||
|
||||
for _, notExpected := range tt.expectNotContain {
|
||||
assert.NotContains(t, resultStr, notExpected, "Expected NOT to contain: %s", notExpected)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user