History

Improve code diff highlight, fix incorrect rendered diff result (#19958)

Use Unicode placeholders to replace HTML tags and HTML entities first, then do diff, then recover the HTML tags and HTML entities. Now the code diff with highlight has stable behavior, and won't emit broken tags.

wxiaoguang committed 4 years ago

Commit 3310dd11 parent 14178c55 files · +379 −378Browse files

Showing the changes to services/gitdiff/gitdiff_test.go (1 of 5 files). Show all

Unified · Split

services/gitdiff/gitdiff_test.go+4−84
@@ -7,7 +7,6 @@ package gitdiff · 6 unmodified lines
77
88 import (
99 "fmt"
10− "html/template"
1110 "strconv"
1211 "strings"
1312 "testing"
@@ -17,93 +16,27 @@ import ( · 3 unmodified lines
1716 "code.gitea.io/gitea/models/unittest"
1817 user_model "code.gitea.io/gitea/models/user"
1918 "code.gitea.io/gitea/modules/git"
20− "code.gitea.io/gitea/modules/highlight"
2119 "code.gitea.io/gitea/modules/json"
2220 "code.gitea.io/gitea/modules/setting"
2321
2422 dmp "github.com/sergi/go-diff/diffmatchpatch"
2523 "github.com/stretchr/testify/assert"
26− "gopkg.in/ini.v1"
2724 )
2825
29−func assertEqual(t *testing.T, s1 string, s2 template.HTML) {
30− if s1 != string(s2) {
31− t.Errorf("Did not receive expected results:\nExpected: %s\nActual: %s", s1, s2)
32− }
33−}
34−
3526 func TestDiffToHTML(t *testing.T) {
36− setting.Cfg = ini.Empty()
37− assertEqual(t, "foo <span class=\"added-code\">bar</span> biz", diffToHTML("", []dmp.Diff{
27+ assert.Equal(t, "foo <span class=\"added-code\">bar</span> biz", diffToHTML(nil, []dmp.Diff{
3828 {Type: dmp.DiffEqual, Text: "foo "},
3929 {Type: dmp.DiffInsert, Text: "bar"},
4030 {Type: dmp.DiffDelete, Text: " baz"},
4131 {Type: dmp.DiffEqual, Text: " biz"},
42− }, DiffLineAdd).Content)
32+ }, DiffLineAdd))
4333
44− assertEqual(t, "foo <span class=\"removed-code\">bar</span> biz", diffToHTML("", []dmp.Diff{
34+ assert.Equal(t, "foo <span class=\"removed-code\">bar</span> biz", diffToHTML(nil, []dmp.Diff{
4535 {Type: dmp.DiffEqual, Text: "foo "},
4636 {Type: dmp.DiffDelete, Text: "bar"},
4737 {Type: dmp.DiffInsert, Text: " baz"},
4838 {Type: dmp.DiffEqual, Text: " biz"},
49− }, DiffLineDel).Content)
50−
51− assertEqual(t, "<span class=\"k\">if</span> <span class=\"p\">!</span><span class=\"nx\">nohl</span> <span class=\"o\">&amp;&amp;</span> <span class=\"added-code\"><span class=\"p\">(</span></span><span class=\"nx\">lexer</span> <span class=\"o\">!=</span> <span class=\"kc\">nil</span><span class=\"added-code\"> <span class=\"o\">||</span> <span class=\"nx\">r</span><span class=\"p\">.</span><span class=\"nx\">GuessLanguage</span><span class=\"p\">)</span></span> <span class=\"p\">{</span>", diffToHTML("", []dmp.Diff{
52− {Type: dmp.DiffEqual, Text: "<span class=\"k\">if</span> <span class=\"p\">!</span><span class=\"nx\">nohl</span> <span class=\"o\">&amp;&amp;</span> <span class=\""},
53− {Type: dmp.DiffInsert, Text: "p\">(</span><span class=\""},
54− {Type: dmp.DiffEqual, Text: "nx\">lexer</span> <span class=\"o\">!=</span> <span class=\"kc\">nil"},
55− {Type: dmp.DiffInsert, Text: "</span> <span class=\"o\">||</span> <span class=\"nx\">r</span><span class=\"p\">.</span><span class=\"nx\">GuessLanguage</span><span class=\"p\">)"},
56− {Type: dmp.DiffEqual, Text: "</span> <span class=\"p\">{</span>"},
57− }, DiffLineAdd).Content)
58−
59− assertEqual(t, "<span class=\"nx\">tagURL</span> <span class=\"o\">:=</span> <span class=\"removed-code\"><span class=\"nx\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Sprintf</span><span class=\"p\">(</span><span class=\"s\">&#34;## [%s](%s/%s/%s/%s?q=&amp;type=all&amp;state=closed&amp;milestone=%d) - %s&#34;</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Milestone\"</span></span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">BaseURL</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Owner</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Repo</span><span class=\"p\">,</span> <span class=\"removed-code\"><span class=\"nx\">from</span><span class=\"p\">,</span> <span class=\"nx\">milestoneID</span><span class=\"p\">,</span> <span class=\"nx\">time</span><span class=\"p\">.</span><span class=\"nf\">Now</span><span class=\"p\">(</span><span class=\"p\">)</span><span class=\"p\">.</span><span class=\"nf\">Format</span><span class=\"p\">(</span><span class=\"s\">&#34;2006-01-02&#34;</span><span class=\"p\">)</span></span><span class=\"p\">)</span>", diffToHTML("", []dmp.Diff{
60− {Type: dmp.DiffEqual, Text: "<span class=\"nx\">tagURL</span> <span class=\"o\">:=</span> <span class=\"n"},
61− {Type: dmp.DiffDelete, Text: "x\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Sprintf</span><span class=\"p\">(</span><span class=\"s\">&#34;## [%s](%s/%s/%s/%s?q=&amp;type=all&amp;state=closed&amp;milestone=%d) - %s&#34;</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Milestone\""},
62− {Type: dmp.DiffInsert, Text: "f\">getGiteaTagURL</span><span class=\"p\">(</span><span class=\"nx\">client"},
63− {Type: dmp.DiffEqual, Text: "</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">BaseURL</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Owner</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Repo</span><span class=\"p\">,</span> <span class=\"nx\">"},
64− {Type: dmp.DiffDelete, Text: "from</span><span class=\"p\">,</span> <span class=\"nx\">milestoneID</span><span class=\"p\">,</span> <span class=\"nx\">time</span><span class=\"p\">.</span><span class=\"nf\">Now</span><span class=\"p\">(</span><span class=\"p\">)</span><span class=\"p\">.</span><span class=\"nf\">Format</span><span class=\"p\">(</span><span class=\"s\">&#34;2006-01-02&#34;</span><span class=\"p\">)"},
65− {Type: dmp.DiffInsert, Text: "ge</span><span class=\"p\">.</span><span class=\"nx\">Milestone</span><span class=\"p\">,</span> <span class=\"nx\">from</span><span class=\"p\">,</span> <span class=\"nx\">milestoneID"},
66− {Type: dmp.DiffEqual, Text: "</span><span class=\"p\">)</span>"},
67− }, DiffLineDel).Content)
68−
69− assertEqual(t, "<span class=\"nx\">r</span><span class=\"p\">.</span><span class=\"nf\">WrapperRenderer</span><span class=\"p\">(</span><span class=\"nx\">w</span><span class=\"p\">,</span> <span class=\"removed-code\"><span class=\"nx\">language</span><span class=\"p\">,</span> <span class=\"kc\">true</span><span class=\"p\">,</span> <span class=\"nx\">attrs</span></span><span class=\"p\">,</span> <span class=\"kc\">false</span><span class=\"p\">)</span>", diffToHTML("", []dmp.Diff{
70− {Type: dmp.DiffEqual, Text: "<span class=\"nx\">r</span><span class=\"p\">.</span><span class=\"nf\">WrapperRenderer</span><span class=\"p\">(</span><span class=\"nx\">w</span><span class=\"p\">,</span> <span class=\"nx\">"},
71− {Type: dmp.DiffDelete, Text: "language</span><span "},
72− {Type: dmp.DiffEqual, Text: "c"},
73− {Type: dmp.DiffDelete, Text: "lass=\"p\">,</span> <span class=\"kc\">true</span><span class=\"p\">,</span> <span class=\"nx\">attrs"},
74− {Type: dmp.DiffEqual, Text: "</span><span class=\"p\">,</span> <span class=\"kc\">false</span><span class=\"p\">)</span>"},
75− }, DiffLineDel).Content)
76−
77− assertEqual(t, "<span class=\"added-code\">language</span><span class=\"p\">,</span> <span class=\"kc\">true</span><span class=\"p\">,</span> <span class=\"nx\">attrs</span></span><span class=\"p\">,</span> <span class=\"kc\">false</span><span class=\"p\">)</span>", diffToHTML("", []dmp.Diff{
78− {Type: dmp.DiffInsert, Text: "language</span><span "},
79− {Type: dmp.DiffEqual, Text: "c"},
80− {Type: dmp.DiffInsert, Text: "lass=\"p\">,</span> <span class=\"kc\">true</span><span class=\"p\">,</span> <span class=\"nx\">attrs"},
81− {Type: dmp.DiffEqual, Text: "</span><span class=\"p\">,</span> <span class=\"kc\">false</span><span class=\"p\">)</span>"},
82− }, DiffLineAdd).Content)
83−
84− assertEqual(t, "<span class=\"k\">print</span><span class=\"added-code\"><span class=\"p\">(</span></span><span class=\"sa\"></span><span class=\"s2\">&#34;</span><span class=\"s2\">// </span><span class=\"s2\">&#34;</span><span class=\"p\">,</span> <span class=\"n\">sys</span><span class=\"o\">.</span><span class=\"n\">argv</span><span class=\"added-code\"><span class=\"p\">)</span></span>", diffToHTML("", []dmp.Diff{
85− {Type: dmp.DiffEqual, Text: "<span class=\"k\">print</span>"},
86− {Type: dmp.DiffInsert, Text: "<span"},
87− {Type: dmp.DiffEqual, Text: " "},
88− {Type: dmp.DiffInsert, Text: "class=\"p\">(</span>"},
89− {Type: dmp.DiffEqual, Text: "<span class=\"sa\"></span><span class=\"s2\">&#34;</span><span class=\"s2\">// </span><span class=\"s2\">&#34;</span><span class=\"p\">,</span> <span class=\"n\">sys</span><span class=\"o\">.</span><span class=\"n\">argv</span>"},
90− {Type: dmp.DiffInsert, Text: "<span class=\"p\">)</span>"},
91− }, DiffLineAdd).Content)
92−
93− assertEqual(t, "sh <span class=\"added-code\">&#39;useradd -u $(stat -c &#34;%u&#34; .gitignore) jenkins&#39;</span>", diffToHTML("", []dmp.Diff{
94− {Type: dmp.DiffEqual, Text: "sh &#3"},
95− {Type: dmp.DiffDelete, Text: "4;useradd -u 111 jenkins&#34"},
96− {Type: dmp.DiffInsert, Text: "9;useradd -u $(stat -c &#34;%u&#34; .gitignore) jenkins&#39"},
97− {Type: dmp.DiffEqual, Text: ";"},
98− }, DiffLineAdd).Content)
99−
100− assertEqual(t, "<span class=\"x\"> &lt;h<span class=\"added-code\">4 class=&#34;release-list-title df ac&#34;</span>&gt;</span>", diffToHTML("", []dmp.Diff{
101− {Type: dmp.DiffEqual, Text: "<span class=\"x\"> &lt;h"},
102− {Type: dmp.DiffInsert, Text: "4 class=&#"},
103− {Type: dmp.DiffEqual, Text: "3"},
104− {Type: dmp.DiffInsert, Text: "4;release-list-title df ac&#34;"},
105− {Type: dmp.DiffEqual, Text: "&gt;</span>"},
106− }, DiffLineAdd).Content)
39+ }, DiffLineDel))
10740 }
10841
10942 func TestParsePatch_skipTo(t *testing.T) {
@@ -592,7 +525,6 @@ index 0000000..6bb8f39 · 482 unmodified lines
592525 if err != nil {
593526 t.Errorf("ParsePatch failed: %s", err)
594527 }
595− println(result)
596528
597529 diff2 := `diff --git "a/A \\ B" "b/A \\ B"
598530 --- "a/A \\ B"
@@ -712,18 +644,6 @@ func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) { · 113 unmodified lines
712644 }
713645 }
714646
715−func TestDiffToHTML_14231(t *testing.T) {
716− setting.Cfg = ini.Empty()
717− diffRecord := diffMatchPatch.DiffMain(highlight.Code("main.v", "", " run()\n"), highlight.Code("main.v", "", " run(db)\n"), true)
718− diffRecord = diffMatchPatch.DiffCleanupEfficiency(diffRecord)
719−
720− expected := `<span class="line"><span class="cl"> <span class="n">run</span><span class="added-code"><span class="o">(</span><span class="n">db</span></span><span class="o">)</span>
721−</span></span>`
722− output := diffToHTML("main.v", diffRecord, DiffLineAdd)
723−
724− assertEqual(t, expected, output.Content)
725−}
726−
727647 func TestNoCrashes(t *testing.T) {
728648 type testcase struct {
729649 gitdiff string