Alternative fix for HTML diff entity split

This commit both reverts PR go-gitea#13357 and uses the exiting implementation alredy used for spans to fix the same issue. That PR duplicates most of logic that is already present elsewhere and still was failing for some cases. This should be simpler as it uses the existing logic that already works for <span>s being split apart. Added both test cases as well.
mrsdizzie · Nov 4, 2020 · 23b0293 · 23b0293
1 parent b687707
commit 23b0293
Showing 1 changed file with 6 additions and 81 deletions.
diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go
@@ -182,6 +182,7 @@ var (
 	codeTagSuffix     = []byte(`</span>`)
 )
 var trailingSpanRegex = regexp.MustCompile(`<span\s*[[:alpha:]="]*?[>]?$`)
+var entityRegex = regexp.MustCompile(`&[#]*?[0-9a-z]*$`)
 
 // shouldWriteInline represents combinations where we manually write inline changes
 func shouldWriteInline(diff diffmatchpatch.Diff, lineType DiffLineType) bool {
@@ -213,6 +214,11 @@ func diffToHTML(fileName string, diffs []diffmatchpatch.Diff, lineType DiffLineT
 				match = diff.Text[m[0]:m[1]]
 				diff.Text = strings.TrimSuffix(diff.Text, match)
 			}
+			m = entityRegex.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>")
@@ -290,9 +296,6 @@ func init() {
 	diffMatchPatch.DiffEditCost = 100
 }
 
-var unterminatedEntityRE = regexp.MustCompile(`&[^ ;]*$`)
-var unstartedEntiyRE = regexp.MustCompile(`^[^ ;]*;`)
-
 // GetComputedInlineDiffFor computes inline diff for the given line.
 func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine) template.HTML {
 	if setting.Git.DisableDiffHighlight {
@@ -333,89 +336,11 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine) tem
 	diffRecord := diffMatchPatch.DiffMain(highlight.Code(diffSection.FileName, diff1[1:]), highlight.Code(diffSection.FileName, diff2[1:]), true)
 	diffRecord = diffMatchPatch.DiffCleanupEfficiency(diffRecord)
 
-	// Now we need to clean up the split entities
-	diffRecord = unsplitEntities(diffRecord)
 	diffRecord = diffMatchPatch.DiffCleanupEfficiency(diffRecord)
 
 	return diffToHTML(diffSection.FileName, diffRecord, diffLine.Type)
 }
 
-// unsplitEntities looks for broken up html entities. It relies on records being presimplified and the data being passed in being valid html
-func unsplitEntities(records []diffmatchpatch.Diff) []diffmatchpatch.Diff {
-	// Unsplitting entities is simple...
-	//
-	// Iterate through all be the last records because if we're the last record then there's nothing we can do
-	for i := 0; i+1 < len(records); i++ {
-		record := &records[i]
-
-		// Look for an unterminated entity at the end of the line
-		unterminated := unterminatedEntityRE.FindString(record.Text)
-		if len(unterminated) == 0 {
-			continue
-		}
-
-		switch record.Type {
-		case diffmatchpatch.DiffEqual:
-			// If we're an diff equal we want to give this unterminated entity to our next delete and insert
-			record.Text = record.Text[0 : len(record.Text)-len(unterminated)]
-			records[i+1].Text = unterminated + records[i+1].Text
-
-			nextType := records[i+1].Type
-
-			if nextType == diffmatchpatch.DiffEqual {
-				continue
-			}
-
-			// if the next in line is a delete then we will want the thing after that to be an insert and so on.
-			oneAfterType := diffmatchpatch.DiffInsert
-			if nextType == diffmatchpatch.DiffInsert {
-				oneAfterType = diffmatchpatch.DiffDelete
-			}
-
-			if i+2 < len(records) && records[i+2].Type == oneAfterType {
-				records[i+2].Text = unterminated + records[i+2].Text
-			} else {
-				records = append(records[:i+2], append([]diffmatchpatch.Diff{
-					{
-						Type: oneAfterType,
-						Text: unterminated,
-					}}, records[i+2:]...)...)
-			}
-		case diffmatchpatch.DiffDelete:
-			fallthrough
-		case diffmatchpatch.DiffInsert:
-			// if we're an insert or delete we want to claim the terminal bit of the entity from the next equal in line
-			targetType := diffmatchpatch.DiffInsert
-			if record.Type == diffmatchpatch.DiffInsert {
-				targetType = diffmatchpatch.DiffDelete
-			}
-			next := &records[i+1]
-			if next.Type == diffmatchpatch.DiffEqual {
-				// if the next is an equal we need to snaffle the entity end off the start and add an delete/insert
-				if terminal := unstartedEntiyRE.FindString(next.Text); len(terminal) > 0 {
-					record.Text += terminal
-					next.Text = next.Text[len(terminal):]
-					records = append(records[:i+2], append([]diffmatchpatch.Diff{
-						{
-							Type: targetType,
-							Text: unterminated,
-						}}, records[i+2:]...)...)
-				}
-			} else if next.Type == targetType {
-				// if the next is an insert we need to snaffle the entity end off the one after that and add it to both.
-				if i+2 < len(records) && records[i+2].Type == diffmatchpatch.DiffEqual {
-					if terminal := unstartedEntiyRE.FindString(records[i+2].Text); len(terminal) > 0 {
-						record.Text += terminal
-						next.Text += terminal
-						records[i+2].Text = records[i+2].Text[len(terminal):]
-					}
-				}
-			}
-		}
-	}
-	return records
-}
-
 // DiffFile represents a file diff.
 type DiffFile struct {
 	Name               string