diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 3aacc830f..164e1c0ca 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -181,64 +181,60 @@ var ( removedCodePrefix = []byte(`<span class="removed-code">`) codeTagSuffix = []byte(`</span>`) ) -var addSpanRegex = regexp.MustCompile(`<span\s*[a-z="]*$`) +var trailingSpanRegex = regexp.MustCompile(`<span\s*[[:alpha:]="]*?[>]?$`) + +// shouldWriteInline represents combinations where we manually write inline changes +func shouldWriteInline(diff diffmatchpatch.Diff, lineType DiffLineType) bool { + if true && + diff.Type == diffmatchpatch.DiffEqual || + diff.Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd || + diff.Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel { + return true + } + return false +} func diffToHTML(fileName string, diffs []diffmatchpatch.Diff, lineType DiffLineType) template.HTML { buf := bytes.NewBuffer(nil) - var addSpan string - for i := range diffs { + match := "" + + for _, diff := range diffs { + if shouldWriteInline(diff, lineType) { + if len(match) > 0 { + diff.Text = match + diff.Text + match = "" + } + // Chroma HTML syntax highlighting is done before diffing individual lines in order to maintain consistency. + // Since inline changes might split in the middle of a chroma span tag, make we manually put it back together + // before writing so we don't try insert added/removed code spans in the middle of an existing chroma span + // and create broken HTML. + m := trailingSpanRegex.FindStringSubmatchIndex(diff.Text) + if m != nil { + match = diff.Text[m[0]:m[1]] + diff.Text = strings.TrimSuffix(diff.Text, match) + } + // Print an existing closing span first before opening added/remove-code span so it doesn't unintentionally close it + if strings.HasPrefix(diff.Text, "</span>") { + buf.WriteString("</span>") + diff.Text = strings.TrimPrefix(diff.Text, "</span>") + } + // If we weren't able to fix it then this should avoid broken HTML by not inserting more spans below + // The previous/next diff section will contain the rest of the tag that is missing here + if strings.Count(diff.Text, "<") != strings.Count(diff.Text, ">") { + buf.WriteString(diff.Text) + continue + } + } 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, either partially or fully. If found, just move the openining span code forward into the next section - // see TestDiffToHTML for examples - if len(addSpan) > 0 { - diffs[i].Text = addSpan + diffs[i].Text - addSpan = "" - } - m := addSpanRegex.FindStringSubmatchIndex(diffs[i].Text) - if m != nil { - addSpan = diffs[i].Text[m[0]:m[1]] - buf.WriteString(strings.TrimSuffix(diffs[i].Text, addSpan)) - } else { - addSpan = "" - buf.WriteString(getLineContent(diffs[i].Text)) - } - case diffs[i].Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd: - if len(addSpan) > 0 { - diffs[i].Text = addSpan + diffs[i].Text - addSpan = "" - } - // 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>") - } - m := addSpanRegex.FindStringSubmatchIndex(diffs[i].Text) - if m != nil { - addSpan = diffs[i].Text[m[0]:m[1]] - diffs[i].Text = strings.TrimSuffix(diffs[i].Text, addSpan) - } + case diff.Type == diffmatchpatch.DiffEqual: + buf.WriteString(diff.Text) + case diff.Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd: buf.Write(addedCodePrefix) - buf.WriteString(diffs[i].Text) + buf.WriteString(diff.Text) buf.Write(codeTagSuffix) - case diffs[i].Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel: - if len(addSpan) > 0 { - diffs[i].Text = addSpan + diffs[i].Text - addSpan = "" - } - if strings.HasPrefix(diffs[i].Text, "</span>") { - buf.WriteString("</span>") - diffs[i].Text = strings.TrimPrefix(diffs[i].Text, "</span>") - } - m := addSpanRegex.FindStringSubmatchIndex(diffs[i].Text) - if m != nil { - addSpan = diffs[i].Text[m[0]:m[1]] - diffs[i].Text = strings.TrimSuffix(diffs[i].Text, addSpan) - } + case diff.Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel: buf.Write(removedCodePrefix) - buf.WriteString(diffs[i].Text) + buf.WriteString(diff.Text) buf.Write(codeTagSuffix) } } diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go index bb7cb4031..e7eeca700 100644 --- a/services/gitdiff/gitdiff_test.go +++ b/services/gitdiff/gitdiff_test.go @@ -50,7 +50,7 @@ func TestDiffToHTML(t *testing.T) { {Type: dmp.DiffEqual, Text: "</span> <span class=\"p\">{</span>"}, }, DiffLineAdd)) - 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\">"## [%s](%s/%s/%s/%s?q=&type=all&state=closed&milestone=%d) - %s"</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=\"nx\"><span class=\"removed-code\">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\">"2006-01-02"</span><span class=\"p\">)</span></span><span class=\"p\">)</span>", diffToHTML("", []dmp.Diff{ + 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\">"## [%s](%s/%s/%s/%s?q=&type=all&state=closed&milestone=%d) - %s"</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\">"2006-01-02"</span><span class=\"p\">)</span></span><span class=\"p\">)</span>", diffToHTML("", []dmp.Diff{ {Type: dmp.DiffEqual, Text: "<span class=\"nx\">tagURL</span> <span class=\"o\">:=</span> <span class=\"n"}, {Type: dmp.DiffDelete, Text: "x\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Sprintf</span><span class=\"p\">(</span><span class=\"s\">"## [%s](%s/%s/%s/%s?q=&type=all&state=closed&milestone=%d) - %s"</span><span class=\"p\">,</span> <span class=\"nx\">ge</span><span class=\"p\">.</span><span class=\"nx\">Milestone\""}, {Type: dmp.DiffInsert, Text: "f\">getGiteaTagURL</span><span class=\"p\">(</span><span class=\"nx\">client"}, @@ -60,7 +60,7 @@ func TestDiffToHTML(t *testing.T) { {Type: dmp.DiffEqual, Text: "</span><span class=\"p\">)</span>"}, }, DiffLineDel)) - 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=\"nx\"><span class=\"removed-code\">language</span></span><span class=\"removed-code\"><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{ + 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><span class=\"removed-code\"><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{ {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\">"}, {Type: dmp.DiffDelete, Text: "language</span><span "}, {Type: dmp.DiffEqual, Text: "c"},