Skip to content

Commit 590df91

Browse files
committed
fix: handle righthunksize == 0 scenario
1 parent c0b8201 commit 590df91

File tree

3 files changed

+204
-7
lines changed

3 files changed

+204
-7
lines changed

routers/web/repo/compare_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,47 @@ func TestAttachCommentsToLines(t *testing.T) {
3838
assert.Equal(t, int64(300), section.Lines[1].Comments[0].ID)
3939
assert.Equal(t, int64(301), section.Lines[1].Comments[1].ID)
4040
}
41+
42+
func TestAttachHiddenCommentIDs(t *testing.T) {
43+
section := &gitdiff.DiffSection{
44+
Lines: []*gitdiff.DiffLine{
45+
{
46+
Type: gitdiff.DiffLineSection,
47+
SectionInfo: &gitdiff.DiffLineSectionInfo{
48+
LastRightIdx: 10,
49+
RightIdx: 20,
50+
RightHunkSize: 5, // Normal expansion
51+
},
52+
},
53+
{
54+
Type: gitdiff.DiffLinePlain,
55+
},
56+
{
57+
Type: gitdiff.DiffLineSection,
58+
SectionInfo: &gitdiff.DiffLineSectionInfo{
59+
LastRightIdx: 30,
60+
RightIdx: 40,
61+
RightHunkSize: 0, // End-of-file expansion
62+
},
63+
},
64+
},
65+
}
66+
67+
lineComments := map[int64][]*issues_model.Comment{
68+
15: {{ID: 100}}, // in first section's hidden range
69+
35: {{ID: 200}}, // in second section's hidden range
70+
40: {{ID: 300}}, // at second section's RightIdx (end-of-file, should be included)
71+
50: {{ID: 400}}, // outside any range
72+
}
73+
74+
attachHiddenCommentIDs(section, lineComments)
75+
76+
// First section: normal expansion, RightIdx is exclusive
77+
assert.Equal(t, []int64{100}, section.Lines[0].SectionInfo.HiddenCommentIDs)
78+
79+
// Second line is not a section
80+
assert.Nil(t, section.Lines[1].SectionInfo)
81+
82+
// Third section: end-of-file expansion, RightIdx is inclusive
83+
assert.ElementsMatch(t, []int64{200, 300}, section.Lines[2].SectionInfo.HiddenCommentIDs)
84+
}

services/gitdiff/gitdiff.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,13 @@ func FillHiddenCommentIDsForDiffLine(line *DiffLine, lineComments map[int64][]*i
228228
absLineNum = -absLineNum
229229
}
230230
// Check if comments are in the hidden range
231-
if absLineNum > line.SectionInfo.LastRightIdx && absLineNum <= line.SectionInfo.RightIdx {
231+
// depending on 'end-of-file' expansion might be RightHunkSize = 0
232+
isEndOfFileExpansion := line.SectionInfo.RightHunkSize == 0
233+
inRange := absLineNum > line.SectionInfo.LastRightIdx &&
234+
(isEndOfFileExpansion && absLineNum <= line.SectionInfo.RightIdx ||
235+
!isEndOfFileExpansion && absLineNum < line.SectionInfo.RightIdx)
236+
237+
if inRange {
232238
for _, comment := range comments {
233239
hiddenCommentIDs = append(hiddenCommentIDs, comment.ID)
234240
}

services/gitdiff/gitdiff_test.go

Lines changed: 153 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -810,21 +810,57 @@ func TestCalculateHiddenCommentIDsForLine(t *testing.T) {
810810
expected: []int64{100},
811811
},
812812
{
813-
name: "boundary conditions - exclusive range",
813+
name: "boundary conditions - normal expansion (both boundaries exclusive)",
814814
line: &DiffLine{
815815
Type: DiffLineSection,
816816
SectionInfo: &DiffLineSectionInfo{
817-
LastRightIdx: 10,
818-
RightIdx: 20,
817+
LastRightIdx: 10,
818+
RightIdx: 20,
819+
RightHunkSize: 5, // Normal case: next section has content
819820
},
820821
},
821822
lineComments: map[int64][]*issues_model.Comment{
822-
10: {{ID: 100}}, // at LastRightIdx, should not be included
823-
20: {{ID: 101}}, // at RightIdx, should be included
823+
10: {{ID: 100}}, // at LastRightIdx (visible line), should NOT be included
824+
20: {{ID: 101}}, // at RightIdx (visible line), should NOT be included
824825
11: {{ID: 102}}, // just inside range, should be included
825826
19: {{ID: 103}}, // just inside range, should be included
826827
},
827-
expected: []int64{101, 102, 103},
828+
expected: []int64{102, 103},
829+
},
830+
{
831+
name: "boundary conditions - end of file expansion (RightIdx inclusive)",
832+
line: &DiffLine{
833+
Type: DiffLineSection,
834+
SectionInfo: &DiffLineSectionInfo{
835+
LastRightIdx: 54,
836+
RightIdx: 70,
837+
RightHunkSize: 0, // End of file: no more content after
838+
},
839+
},
840+
lineComments: map[int64][]*issues_model.Comment{
841+
54: {{ID: 54}}, // at LastRightIdx (visible line), should NOT be included
842+
70: {{ID: 70}}, // at RightIdx (last hidden line), SHOULD be included
843+
60: {{ID: 60}}, // inside range, should be included
844+
},
845+
expected: []int64{60, 70}, // Lines 60 and 70 are hidden
846+
},
847+
{
848+
name: "real-world scenario - start of file with hunk",
849+
line: &DiffLine{
850+
Type: DiffLineSection,
851+
SectionInfo: &DiffLineSectionInfo{
852+
LastRightIdx: 0, // No previous visible section
853+
RightIdx: 26, // Line 26 is first visible line of hunk
854+
RightHunkSize: 9, // Normal hunk with content
855+
},
856+
},
857+
lineComments: map[int64][]*issues_model.Comment{
858+
1: {{ID: 1}}, // Line 1 is hidden
859+
26: {{ID: 26}}, // Line 26 is visible (hunk start) - should NOT be hidden
860+
10: {{ID: 10}}, // Line 10 is hidden
861+
15: {{ID: 15}}, // Line 15 is hidden
862+
},
863+
expected: []int64{1, 10, 15}, // Lines 1, 10, 15 are hidden; line 26 is visible
828864
},
829865
}
830866

@@ -835,3 +871,114 @@ func TestCalculateHiddenCommentIDsForLine(t *testing.T) {
835871
})
836872
}
837873
}
874+
875+
func TestDiffLine_RenderBlobExcerptButtons(t *testing.T) {
876+
tests := []struct {
877+
name string
878+
line *DiffLine
879+
fileNameHash string
880+
data *DiffBlobExcerptData
881+
expectContains []string
882+
expectNotContain []string
883+
}{
884+
{
885+
name: "expand up button with hidden comments",
886+
line: &DiffLine{
887+
Type: DiffLineSection,
888+
SectionInfo: &DiffLineSectionInfo{
889+
LastRightIdx: 0,
890+
RightIdx: 26,
891+
LeftIdx: 26,
892+
LastLeftIdx: 0,
893+
LeftHunkSize: 0,
894+
RightHunkSize: 0,
895+
HiddenCommentIDs: []int64{100},
896+
},
897+
},
898+
fileNameHash: "abc123",
899+
data: &DiffBlobExcerptData{
900+
BaseLink: "/repo/blob_excerpt",
901+
AfterCommitID: "commit123",
902+
DiffStyle: "unified",
903+
},
904+
expectContains: []string{
905+
"octicon-fold-up",
906+
"direction=up",
907+
"code-comment-more",
908+
"1 hidden comment(s)",
909+
},
910+
},
911+
{
912+
name: "expand up and down buttons with pull request",
913+
line: &DiffLine{
914+
Type: DiffLineSection,
915+
SectionInfo: &DiffLineSectionInfo{
916+
LastRightIdx: 10,
917+
RightIdx: 50,
918+
LeftIdx: 10,
919+
LastLeftIdx: 5,
920+
LeftHunkSize: 5,
921+
RightHunkSize: 5,
922+
HiddenCommentIDs: []int64{200, 201},
923+
},
924+
},
925+
fileNameHash: "def456",
926+
data: &DiffBlobExcerptData{
927+
BaseLink: "/repo/blob_excerpt",
928+
AfterCommitID: "commit456",
929+
DiffStyle: "split",
930+
PullIssueIndex: 42,
931+
},
932+
expectContains: []string{
933+
"octicon-fold-down",
934+
"octicon-fold-up",
935+
"direction=down",
936+
"direction=up",
937+
"data-hidden-comment-ids=\"200,201\"",
938+
"pull_issue_index=42",
939+
"2 hidden comment(s)",
940+
},
941+
},
942+
{
943+
name: "no hidden comments",
944+
line: &DiffLine{
945+
Type: DiffLineSection,
946+
SectionInfo: &DiffLineSectionInfo{
947+
LastRightIdx: 10,
948+
RightIdx: 20,
949+
LeftIdx: 10,
950+
LastLeftIdx: 5,
951+
LeftHunkSize: 5,
952+
RightHunkSize: 5,
953+
HiddenCommentIDs: nil,
954+
},
955+
},
956+
fileNameHash: "ghi789",
957+
data: &DiffBlobExcerptData{
958+
BaseLink: "/repo/blob_excerpt",
959+
AfterCommitID: "commit789",
960+
},
961+
expectContains: []string{
962+
"code-expander-button",
963+
},
964+
expectNotContain: []string{
965+
"code-comment-more",
966+
},
967+
},
968+
}
969+
970+
for _, tt := range tests {
971+
t.Run(tt.name, func(t *testing.T) {
972+
result := tt.line.RenderBlobExcerptButtons(tt.fileNameHash, tt.data)
973+
resultStr := string(result)
974+
975+
for _, expected := range tt.expectContains {
976+
assert.Contains(t, resultStr, expected, "Expected to contain: %s", expected)
977+
}
978+
979+
for _, notExpected := range tt.expectNotContain {
980+
assert.NotContains(t, resultStr, notExpected, "Expected NOT to contain: %s", notExpected)
981+
}
982+
})
983+
}
984+
}

0 commit comments

Comments
 (0)