Skip to content

Commit d786956

Browse files
wxiaoguangwxiaoguang
committed
fix tests
1 parent 0a29d4d commit d786956

File tree

5 files changed

+103
-124
lines changed

5 files changed

+103
-124
lines changed

‎routers/api/v1/repo/pull.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1591,7 +1591,6 @@ func GetPullRequestFiles(ctx *context.APIContext) {
15911591
maxLines := setting.Git.MaxGitDiffLines
15921592

15931593
// FIXME: If there are too many files in the repo, may cause some unpredictable issues.
1594-
// FIXME: it doesn't need to call "GetDiff" to do various parsing and highlighting
15951594
diff, err := gitdiff.GetDiffForAPI(ctx, baseGitRepo,
15961595
&gitdiff.DiffOptions{
15971596
BeforeCommitID: startCommitID,

‎services/gitdiff/gitdiff.go‎

Lines changed: 3 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -207,38 +207,6 @@ type DiffSection struct {
207207
Lines []*DiffLine
208208
}
209209

210-
var (
211-
addedCodePrefix = []byte(`<span class="added-code">`)
212-
removedCodePrefix = []byte(`<span class="removed-code">`)
213-
codeTagSuffix = []byte(`</span>`)
214-
)
215-
216-
func diffToHTML(lineWrapperTags []string, diffs []DiffHTMLOperation, lineType DiffLineType) template.HTML {
217-
buf := bytes.NewBuffer(nil)
218-
// restore the line wrapper tags <span class="line"> and <span class="cl">, if necessary
219-
for _, tag := range lineWrapperTags {
220-
buf.WriteString(tag)
221-
}
222-
for _, diff := range diffs {
223-
switch {
224-
case diff.Type == diffmatchpatch.DiffEqual:
225-
buf.WriteString(string(diff.HTML))
226-
case diff.Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd:
227-
buf.Write(addedCodePrefix)
228-
buf.WriteString(string(diff.HTML))
229-
buf.Write(codeTagSuffix)
230-
case diff.Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel:
231-
buf.Write(removedCodePrefix)
232-
buf.WriteString(string(diff.HTML))
233-
buf.Write(codeTagSuffix)
234-
}
235-
}
236-
for range lineWrapperTags {
237-
buf.WriteString("</span>")
238-
}
239-
return template.HTML(buf.String())
240-
}
241-
242210
// GetLine gets a specific line by type (add or del) and file line number
243211
func (diffSection *DiffSection) GetLine(lineType DiffLineType, idx int) *DiffLine {
244212
var (
@@ -328,10 +296,9 @@ func (diffSection *DiffSection) getDiffLineForRender(diffLineType DiffLineType,
328296
lineHTML = util.Iif(diffLineType == DiffLinePlain, diff2, "")
329297
}
330298
if diffLineType != DiffLinePlain {
331-
diffRecord := hcd.diffWithHighlight(diff1, diff2)
332299
// it seems that Gitea doesn't need the line wrapper of Chroma, so do not add them back
333300
// if the line wrappers are still needed in the future, it can be added back by "diffToHTML(hcd.lineWrapperTags. ...)"
334-
lineHTML = diffToHTML(nil, diffRecord, diffLineType)
301+
lineHTML = hcd.diffLineWithHighlight(diffLineType, diff1, diff2)
335302
}
336303
return DiffInlineWithUnicodeEscape(lineHTML, locale)
337304
}
@@ -380,7 +347,7 @@ type DiffFile struct {
380347
IsIncomplete bool
381348
IsIncompleteLineTooLong bool
382349

383-
// will be filled by the extra loop in GitDiff
350+
// will be filled by the extra loop in GitDiffForRender
384351
Language string
385352
IsGenerated bool
386353
IsVendored bool
@@ -393,7 +360,7 @@ type DiffFile struct {
393360
IsViewed bool // User specific
394361
HasChangedSinceLastReview bool // User specific
395362

396-
// for render purpose only, TODO: in the future, we only need to store the related diff lines to save memory
363+
// for render purpose only, will be filled by the extra loop in GitDiffForRender
397364
highlightedLeftLines map[int]template.HTML
398365
highlightedRightLines map[int]template.HTML
399366
}

‎services/gitdiff/gitdiff_test.go‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,27 +17,10 @@ import (
1717
"code.gitea.io/gitea/modules/json"
1818
"code.gitea.io/gitea/modules/setting"
1919

20-
dmp "github.com/sergi/go-diff/diffmatchpatch"
2120
"github.com/stretchr/testify/assert"
2221
"github.com/stretchr/testify/require"
2322
)
2423

25-
func TestDiffToHTML(t *testing.T) {
26-
assert.Equal(t, "foo <span class=\"added-code\">bar</span> biz", diffToHTML(nil, []dmp.Diff{
27-
{Type: dmp.DiffEqual, Text: "foo "},
28-
{Type: dmp.DiffInsert, Text: "bar"},
29-
{Type: dmp.DiffDelete, Text: " baz"},
30-
{Type: dmp.DiffEqual, Text: " biz"},
31-
}, DiffLineAdd))
32-
33-
assert.Equal(t, "foo <span class=\"removed-code\">bar</span> biz", diffToHTML(nil, []dmp.Diff{
34-
{Type: dmp.DiffEqual, Text: "foo "},
35-
{Type: dmp.DiffDelete, Text: "bar"},
36-
{Type: dmp.DiffInsert, Text: " baz"},
37-
{Type: dmp.DiffEqual, Text: " biz"},
38-
}, DiffLineDel))
39-
}
40-
4124
func TestParsePatch_skipTo(t *testing.T) {
4225
type testcase struct {
4326
name string
@@ -621,7 +604,7 @@ func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) {
621604

622605
defer gitRepo.Close()
623606
for _, behavior := range []git.TrustedCmdArgs{{"-w"}, {"--ignore-space-at-eol"}, {"-b"}, nil} {
624-
diffs, err := GetDiff(t.Context(), gitRepo,
607+
diffs, err := GetDiffForAPI(t.Context(), gitRepo,
625608
&DiffOptions{
626609
AfterCommitID: "d8e0bbb45f200e67d9a784ce55bd90821af45ebd",
627610
BeforeCommitID: "72866af952e98d02a73003501836074b286a78f6",

‎services/gitdiff/highlightdiff.go‎

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@
44
package gitdiff
55

66
import (
7+
"bytes"
78
"html/template"
89
"strings"
10+
11+
"github.com/sergi/go-diff/diffmatchpatch"
912
)
1013

1114
// token is a html tag or entity, eg: "<span ...>", "</span>", "&lt;"
@@ -83,7 +86,11 @@ func (hcd *highlightCodeDiff) collectUsedRunes(code template.HTML) {
8386
}
8487
}
8588

86-
func (hcd *highlightCodeDiff) diffWithHighlight(codeA, codeB template.HTML) (res []DiffHTMLOperation) {
89+
func (hcd *highlightCodeDiff) diffLineWithHighlight(lineType DiffLineType, codeA, codeB template.HTML) template.HTML {
90+
return hcd.diffLineWithHighlightWrapper(nil, lineType, codeA, codeB)
91+
}
92+
93+
func (hcd *highlightCodeDiff) diffLineWithHighlightWrapper(lineWrapperTags []string, lineType DiffLineType, codeA, codeB template.HTML) template.HTML {
8794
hcd.collectUsedRunes(codeA)
8895
hcd.collectUsedRunes(codeB)
8996

@@ -94,10 +101,57 @@ func (hcd *highlightCodeDiff) diffWithHighlight(codeA, codeB template.HTML) (res
94101
diffs := dmp.DiffMain(convertedCodeA, convertedCodeB, true)
95102
diffs = dmp.DiffCleanupEfficiency(diffs)
96103

97-
for _, d := range diffs {
98-
res = append(res, DiffHTMLOperation{Type: d.Type, HTML: hcd.recoverOneDiff(d.Text)})
104+
buf := bytes.NewBuffer(nil)
105+
106+
// restore the line wrapper tags <span class="line"> and <span class="cl">, if necessary
107+
for _, tag := range lineWrapperTags {
108+
buf.WriteString(tag)
99109
}
100-
return res
110+
111+
addedCodePrefix := hcd.registerTokenAsPlaceholder(`<span class="added-code">`)
112+
removedCodePrefix := hcd.registerTokenAsPlaceholder(`<span class="removed-code">`)
113+
codeTagSuffix := hcd.registerTokenAsPlaceholder(`</span>`)
114+
115+
if codeTagSuffix != 0 {
116+
for _, diff := range diffs {
117+
switch {
118+
case diff.Type == diffmatchpatch.DiffEqual:
119+
buf.WriteString(diff.Text)
120+
case diff.Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd:
121+
buf.WriteRune(addedCodePrefix)
122+
buf.WriteString(diff.Text)
123+
buf.WriteRune(codeTagSuffix)
124+
case diff.Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel:
125+
buf.WriteRune(removedCodePrefix)
126+
buf.WriteString(diff.Text)
127+
buf.WriteRune(codeTagSuffix)
128+
}
129+
}
130+
} else {
131+
// placeholder map space is exhausted
132+
for _, diff := range diffs {
133+
take := diff.Type == diffmatchpatch.DiffEqual || (diff.Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd) || (diff.Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel)
134+
if take {
135+
buf.WriteString(diff.Text)
136+
}
137+
}
138+
}
139+
for range lineWrapperTags {
140+
buf.WriteString("</span>")
141+
}
142+
return hcd.recoverOneDiff(buf.String())
143+
}
144+
145+
func (hcd *highlightCodeDiff) registerTokenAsPlaceholder(token string) rune {
146+
placeholder, ok := hcd.tokenPlaceholderMap[token]
147+
if !ok {
148+
placeholder = hcd.nextPlaceholder()
149+
if placeholder != 0 {
150+
hcd.tokenPlaceholderMap[token] = placeholder
151+
hcd.placeholderTokenMap[placeholder] = token
152+
}
153+
}
154+
return placeholder
101155
}
102156

103157
// convertToPlaceholders totally depends on Chroma's valid HTML output and its structure, do not use these functions for other purposes.
@@ -147,14 +201,7 @@ func (hcd *highlightCodeDiff) convertToPlaceholders(htmlContent template.HTML) s
147201
} // else: impossible
148202

149203
// remember the placeholder and token in the map
150-
placeholder, ok := hcd.tokenPlaceholderMap[tokenInMap]
151-
if !ok {
152-
placeholder = hcd.nextPlaceholder()
153-
if placeholder != 0 {
154-
hcd.tokenPlaceholderMap[tokenInMap] = placeholder
155-
hcd.placeholderTokenMap[placeholder] = tokenInMap
156-
}
157-
}
204+
placeholder := hcd.registerTokenAsPlaceholder(tokenInMap)
158205

159206
if placeholder != 0 {
160207
res.WriteRune(placeholder) // use the placeholder to replace the token

‎services/gitdiff/highlightdiff_test.go‎

Lines changed: 40 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -5,84 +5,67 @@ package gitdiff
55

66
import (
77
"fmt"
8+
"html/template"
9+
"strings"
810
"testing"
911

10-
"github.com/sergi/go-diff/diffmatchpatch"
1112
"github.com/stretchr/testify/assert"
1213
)
1314

1415
func TestDiffWithHighlight(t *testing.T) {
15-
hcd := newHighlightCodeDiff()
16-
diffs := hcd.diffWithHighlight(
17-
" run('<>')\n",
18-
" run(db)\n",
19-
)
20-
21-
expected := "\t\trun(<span class=\"removed-code\">'<>'</></span>)\n"
22-
23-
output := diffToHTML(nil, diffs, DiffLineDel)
24-
assert.Equal(t, expected, output)
25-
26-
expected = ` run(<span class="added-code">db</span>)
27-
`
28-
output = diffToHTML(nil, diffs, DiffLineAdd)
29-
assert.Equal(t, expected, output)
30-
31-
hcd = newHighlightCodeDiff()
32-
hcd.placeholderTokenMap['O'] = "<span>"
33-
hcd.placeholderTokenMap['C'] = "</span>"
34-
diff := diffmatchpatch.Diff{}
16+
t.Run("DiffLineAddDel", func(t *testing.T) {
17+
hcd := newHighlightCodeDiff()
18+
codeA := template.HTML(`x <span class="k">foo</span> y`)
19+
codeB := template.HTML(`x <span class="k">bar</span> y`)
20+
outDel := hcd.diffLineWithHighlight(DiffLineDel, codeA, codeB)
21+
assert.Equal(t, `x <span class="k"><span class="removed-code">foo</span></span> y`, string(outDel))
22+
outAdd := hcd.diffLineWithHighlight(DiffLineAdd, codeA, codeB)
23+
assert.Equal(t, `x <span class="k"><span class="added-code">bar</span></span> y`, string(outAdd))
24+
})
3525

36-
diff.Text = "C"
37-
hcd.recoverOneDiff(&diff)
38-
assert.Equal(t, "", diff.Text)
26+
t.Run("OpenCloseTags", func(t *testing.T) {
27+
hcd := newHighlightCodeDiff()
28+
hcd.placeholderTokenMap['O'], hcd.placeholderTokenMap['C'] = "<span>", "</span>"
29+
assert.Equal(t, "<span></span>", string(hcd.recoverOneDiff("OC")))
30+
assert.Equal(t, "<span></span>", string(hcd.recoverOneDiff("O")))
31+
assert.Equal(t, "", string(hcd.recoverOneDiff("C")))
32+
})
3933
}
4034

4135
func TestDiffWithHighlightPlaceholder(t *testing.T) {
4236
hcd := newHighlightCodeDiff()
43-
diffs := hcd.diffWithHighlight(
44-
"a='\U00100000'",
45-
"a='\U0010FFFD''",
46-
)
37+
output := hcd.diffLineWithHighlight(DiffLineDel, "a='\U00100000'", "a='\U0010FFFD''")
4738
assert.Equal(t, "", hcd.placeholderTokenMap[0x00100000])
4839
assert.Equal(t, "", hcd.placeholderTokenMap[0x0010FFFD])
49-
5040
expected := fmt.Sprintf(`a='<span class="removed-code">%s</span>'`, "\U00100000")
51-
output := diffToHTML(hcd.lineWrapperTags, diffs, DiffLineDel)
52-
assert.Equal(t, expected, output)
41+
assert.Equal(t, expected, string(output))
5342

5443
hcd = newHighlightCodeDiff()
55-
diffs = hcd.diffWithHighlight(
56-
"a='\U00100000'",
57-
"a='\U0010FFFD'",
58-
)
44+
output = hcd.diffLineWithHighlight(DiffLineAdd, "a='\U00100000'", "a='\U0010FFFD'")
5945
expected = fmt.Sprintf(`a='<span class="added-code">%s</span>'`, "\U0010FFFD")
60-
output = diffToHTML(nil, diffs, DiffLineAdd)
61-
assert.Equal(t, expected, output)
46+
assert.Equal(t, expected, string(output))
6247
}
6348

6449
func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) {
6550
hcd := newHighlightCodeDiff()
6651
hcd.placeholderMaxCount = 0
67-
diffs := hcd.diffWithHighlight(
68-
"'",
69-
``,
70-
)
71-
output := diffToHTML(nil, diffs, DiffLineDel)
72-
expected := `<span class="removed-code">'</span>`
73-
assert.Equal(t, expected, output)
74-
75-
hcd = newHighlightCodeDiff()
76-
hcd.placeholderMaxCount = 0
77-
diffs = hcd.diffWithHighlight(
78-
"a this_is_not_html_at_this_point b",
79-
"a this_is_is_still_not_html_at_this_point_its_just_a_string b",
80-
)
81-
output = diffToHTML(nil, diffs, DiffLineDel)
82-
expected = "a this_is_not_html_at_this_point b"
83-
assert.Equal(t, expected, output)
52+
placeHolderAmp := string(rune(0xFFFD))
53+
output := hcd.diffLineWithHighlight(DiffLineDel, `<span class="k">&lt;</span>`, `<span class="k">&gt;</span>`)
54+
assert.Equal(t, placeHolderAmp+"lt;", string(output))
55+
output = hcd.diffLineWithHighlight(DiffLineAdd, `<span class="k">&lt;</span>`, `<span class="k">&gt;</span>`)
56+
assert.Equal(t, placeHolderAmp+"gt;", string(output))
57+
}
8458

85-
output = diffToHTML(nil, diffs, DiffLineAdd)
86-
expected = "a this_is_<span class=\"added-code\">is_still_</span>not_html_at_this_point<span class=\"added-code\">_its_just_a_string</span> b"
87-
assert.Equal(t, expected, output)
59+
func TestDiffWithHighlightTagMatch(t *testing.T) {
60+
totalOverflow := 0
61+
for i := 0; i < 100; i++ {
62+
hcd := newHighlightCodeDiff()
63+
hcd.placeholderMaxCount = i
64+
output := string(hcd.diffLineWithHighlight(DiffLineDel, `<span class="k">&lt;</span>`, `<span class="k">&gt;</span>`))
65+
totalOverflow += hcd.placeholderOverflowCount
66+
c1 := strings.Count(output, "<span")
67+
c2 := strings.Count(output, "</span")
68+
assert.Equal(t, c1, c2)
69+
}
70+
assert.NotZero(t, totalOverflow)
8871
}

0 commit comments

Comments
 (0)