Skip to content

Commit

Permalink
Fix Syntax highlight for token change in added/deleted code (#12238)
Browse files Browse the repository at this point in the history
* Fix Syntax highlight for token change in added/deleted code

For diffs we first syntax highlight the before and after line then use a 3rd party diff library to find the difference in them and create a substring based on that, which we then highlight a 2nd time to show the specific difference within a line that has changed. In a specific case if the diffrence also changes the chroma class it will split in the middle of the attr and cause broken HTML:

```
<span class="nx">oldtext<span>
<span class="k">var newtext<span>
```

Will then split on

```
<span class="
```

Where the difference starts, and produce something broken like:

```
<span class="<span class="removed-code">nx"oldtext</span></span

```

Fix that by detecting when it happens and putting the HTML back together properly before highlighting the added/deleted code.

Fixes #12235

* fix lint

* apply fix to all diff sections. Also handle case where insert/remove starts with a closing span

* Add a test for this new code

* remove comment

Co-authored-by: Lauris BH <lauris@nix.lv>
  • Loading branch information
mrsdizzie and lafriks authored Jul 16, 2020
1 parent 6ea2701 commit 9542b73
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 3 deletions.
43 changes: 40 additions & 3 deletions services/gitdiff/gitdiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,19 +183,56 @@ var (

func diffToHTML(fileName string, diffs []diffmatchpatch.Diff, lineType DiffLineType) template.HTML {
buf := bytes.NewBuffer(nil)

var addSpan bool
for i := range diffs {
switch {
case diffs[i].Type == diffmatchpatch.DiffEqual:
// Looking for the case where our 3rd party diff library previously detected a string difference
// in the middle of a span class because we highlight them first. This happens when added/deleted code
// also changes the chroma class name. If found, just move the openining span code forward into the next section
if addSpan {
diffs[i].Text = "<span class=\"" + diffs[i].Text
}
if strings.HasSuffix(diffs[i].Text, "<span class=\"") {
addSpan = true
buf.WriteString(strings.TrimSuffix(diffs[i].Text, "<span class=\""))
} else {
addSpan = false
buf.WriteString(getLineContent(diffs[i].Text))
}
case diffs[i].Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd:
if addSpan {
addSpan = false
diffs[i].Text = "<span class=\"" + diffs[i].Text
}
// Print existing closing span first before opening added-code span so it doesn't unintentionally close it
if strings.HasPrefix(diffs[i].Text, "</span>") {
buf.WriteString("</span>")
diffs[i].Text = strings.TrimPrefix(diffs[i].Text, "</span>")
}
if strings.HasSuffix(diffs[i].Text, "<span class=\"") {
addSpan = true
diffs[i].Text = strings.TrimSuffix(diffs[i].Text, "<span class=\"")
}
buf.Write(addedCodePrefix)
buf.WriteString(getLineContent(diffs[i].Text))
buf.Write(codeTagSuffix)
case diffs[i].Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel:
if addSpan {
addSpan = false
diffs[i].Text = "<span class=\"" + diffs[i].Text
}
if strings.HasPrefix(diffs[i].Text, "</span>") {
buf.WriteString("</span>")
diffs[i].Text = strings.TrimPrefix(diffs[i].Text, "</span>")
}
if strings.HasSuffix(diffs[i].Text, "<span class=\"") {
addSpan = true
diffs[i].Text = strings.TrimSuffix(diffs[i].Text, "<span class=\"")
}
buf.Write(removedCodePrefix)
buf.WriteString(getLineContent(diffs[i].Text))
buf.Write(codeTagSuffix)
case diffs[i].Type == diffmatchpatch.DiffEqual:
buf.WriteString(getLineContent(diffs[i].Text))
}
}
return template.HTML(buf.Bytes())
Expand Down
8 changes: 8 additions & 0 deletions services/gitdiff/gitdiff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ func TestDiffToHTML(t *testing.T) {
{Type: dmp.DiffInsert, Text: " baz"},
{Type: dmp.DiffEqual, Text: " biz"},
}, DiffLineDel))

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{
{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=\""},
{Type: dmp.DiffInsert, Text: "p\">(</span><span class=\""},
{Type: dmp.DiffEqual, Text: "nx\">lexer</span> <span class=\"o\">!=</span> <span class=\"kc\">nil"},
{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\">)"},
{Type: dmp.DiffEqual, Text: "</span> <span class=\"p\">{</span>"},
}, DiffLineAdd))
}

func TestParsePatch(t *testing.T) {
Expand Down

0 comments on commit 9542b73

Please sign in to comment.