From 1955475f8bab6776b1ed550b83fcbf8f81c5285b Mon Sep 17 00:00:00 2001 From: Dustin Firebaugh Date: Sat, 1 Mar 2025 14:55:54 -0500 Subject: [PATCH 01/26] initial pass at fixing multiline comment highlighting in git diffs --- services/gitdiff/gitdiff.go | 69 +++++++++++++++++++++---------- services/gitdiff/highlightdiff.go | 21 +--------- 2 files changed, 49 insertions(+), 41 deletions(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 064f05cfce9f7..eb0cc1e411b0a 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -289,13 +289,6 @@ func DiffInlineWithUnicodeEscape(s template.HTML, locale translation.Locale) Dif return DiffInline{EscapeStatus: status, Content: content} } -// DiffInlineWithHighlightCode makes a DiffInline with code highlight and hidden unicode characters escaped -func DiffInlineWithHighlightCode(fileName, language, code string, locale translation.Locale) DiffInline { - highlighted, _ := highlight.Code(fileName, language, code) - status, content := charset.EscapeControlHTML(highlighted, locale) - return DiffInline{EscapeStatus: status, Content: content} -} - // GetComputedInlineDiffFor computes inline diff for the given line. func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, locale translation.Locale) DiffInline { if setting.Git.DisableDiffHighlight { @@ -308,11 +301,6 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc diff2 string ) - language := "" - if diffSection.file != nil { - language = diffSection.file.Language - } - // try to find equivalent diff line. ignore, otherwise switch diffLine.Type { case DiffLineSection: @@ -320,29 +308,33 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc case DiffLineAdd: compareDiffLine = diffSection.GetLine(DiffLineDel, diffLine.RightIdx) if compareDiffLine == nil { - return DiffInlineWithHighlightCode(diffSection.FileName, language, diffLine.Content[1:], locale) + highlightedLine := diffSection.file.HighlightedNewLines[diffLine.RightIdx-1] + return DiffInlineWithUnicodeEscape(template.HTML(highlightedLine), locale) } - diff1 = compareDiffLine.Content - diff2 = diffLine.Content + diff1 = diffSection.file.HighlightedOldLines[compareDiffLine.LeftIdx-1] + diff2 = diffSection.file.HighlightedNewLines[diffLine.RightIdx-1] case DiffLineDel: compareDiffLine = diffSection.GetLine(DiffLineAdd, diffLine.LeftIdx) if compareDiffLine == nil { - return DiffInlineWithHighlightCode(diffSection.FileName, language, diffLine.Content[1:], locale) + highlightedLine := diffSection.file.HighlightedOldLines[diffLine.LeftIdx-1] + return DiffInlineWithUnicodeEscape(template.HTML(highlightedLine), locale) } - diff1 = diffLine.Content - diff2 = compareDiffLine.Content + diff1 = diffSection.file.HighlightedOldLines[diffLine.LeftIdx-1] + diff2 = diffSection.file.HighlightedNewLines[compareDiffLine.RightIdx-1] default: if strings.IndexByte(" +-", diffLine.Content[0]) > -1 { - return DiffInlineWithHighlightCode(diffSection.FileName, language, diffLine.Content[1:], locale) + highlightedContent := diffSection.file.HighlightedNewLines[diffLine.RightIdx-1] + return DiffInlineWithUnicodeEscape(template.HTML(highlightedContent), locale) } - return DiffInlineWithHighlightCode(diffSection.FileName, language, diffLine.Content, locale) + highlightedContent := diffSection.file.HighlightedOldLines[diffLine.LeftIdx-1] + return DiffInlineWithUnicodeEscape(template.HTML(highlightedContent), locale) } hcd := newHighlightCodeDiff() - diffRecord := hcd.diffWithHighlight(diffSection.FileName, language, diff1[1:], diff2[1:]) + diffRecord := hcd.diffWithHighlight(diffSection.FileName, diffSection.file.Language, diff1, diff2) // it seems that Gitea doesn't need the line wrapper of Chroma, so do not add them back // if the line wrappers are still needed in the future, it can be added back by "diffToHTML(hcd.lineWrapperTags. ...)" - diffHTML := diffToHTML(nil, diffRecord, diffLine.Type) + diffHTML := html.UnescapeString(diffToHTML(nil, diffRecord, diffLine.Type)) return DiffInlineWithUnicodeEscape(template.HTML(diffHTML), locale) } @@ -374,6 +366,9 @@ type DiffFile struct { IsSubmodule bool // if IsSubmodule==true, then there must be a SubmoduleDiffInfo SubmoduleDiffInfo *SubmoduleDiffInfo + + HighlightedOldLines []string + HighlightedNewLines []string } // GetType returns type of diff file. @@ -1227,6 +1222,8 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi } diffFile.IsGenerated = isGenerated.Value() + highlightCode(commit, beforeCommit, diffFile) + tailSection := diffFile.GetTailSection(gitRepo, beforeCommit, commit) if tailSection != nil { diffFile.Sections = append(diffFile.Sections, tailSection) @@ -1247,6 +1244,34 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi return diff, nil } +func highlightCode(commit *git.Commit, beforeCommit *git.Commit, diffFile *DiffFile) { + if beforeCommit != nil { + oldBlob, err := beforeCommit.GetBlobByPath(diffFile.Name) + if err == nil { + oldContent, _ := oldBlob.GetBlobContent(oldBlob.Size()) + highlightedOldContent, _ := highlight.Code(diffFile.Name, diffFile.Language, oldContent) + + oldLines := strings.Split(string(highlightedOldContent), "\n") + diffFile.HighlightedOldLines = make([]string, len(oldLines)) + for i, line := range oldLines { + diffFile.HighlightedOldLines[i] = line + } + } + } + + newBlob, err := commit.GetBlobByPath(diffFile.Name) + if err == nil { + newContent, _ := newBlob.GetBlobContent(newBlob.Size()) + highlightedNewContent, _ := highlight.Code(diffFile.Name, diffFile.Language, newContent) + + newLines := strings.Split(string(highlightedNewContent), "\n") + diffFile.HighlightedNewLines = make([]string, len(newLines)) + for i, line := range newLines { + diffFile.HighlightedNewLines[i] = line + } + } +} + type PullDiffStats struct { NumFiles, TotalAddition, TotalDeletion int } diff --git a/services/gitdiff/highlightdiff.go b/services/gitdiff/highlightdiff.go index 35d48445504ae..9491cb995adca 100644 --- a/services/gitdiff/highlightdiff.go +++ b/services/gitdiff/highlightdiff.go @@ -6,8 +6,6 @@ package gitdiff import ( "strings" - "code.gitea.io/gitea/modules/highlight" - "github.com/sergi/go-diff/diffmatchpatch" ) @@ -90,11 +88,8 @@ func (hcd *highlightCodeDiff) diffWithHighlight(filename, language, codeA, codeB hcd.collectUsedRunes(codeA) hcd.collectUsedRunes(codeB) - highlightCodeA, _ := highlight.Code(filename, language, codeA) - highlightCodeB, _ := highlight.Code(filename, language, codeB) - - convertedCodeA := hcd.convertToPlaceholders(string(highlightCodeA)) - convertedCodeB := hcd.convertToPlaceholders(string(highlightCodeB)) + convertedCodeA := hcd.convertToPlaceholders(codeA) + convertedCodeB := hcd.convertToPlaceholders(codeB) diffs := diffMatchPatch.DiffMain(convertedCodeA, convertedCodeB, true) diffs = diffMatchPatch.DiffCleanupEfficiency(diffs) @@ -206,17 +201,5 @@ func (hcd *highlightCodeDiff) recoverOneDiff(diff *diffmatchpatch.Diff) { sb.WriteString(tokenToRecover) } - if len(tagStack) > 0 { - // close all opening tags - for i := len(tagStack) - 1; i >= 0; i-- { - tagToClose := tagStack[i] - // get the closing tag "" from "" or "" - pos := strings.IndexAny(tagToClose, " >") - if pos != -1 { - sb.WriteString("") - } // else: impossible. every tag was pushed into the stack by the code above and is valid HTML opening tag - } - } - diff.Text = sb.String() } From 0452d27c9ed1ddcf2ba15e1b2ae4697eec7d7f8a Mon Sep 17 00:00:00 2001 From: Dustin Firebaugh Date: Sat, 1 Mar 2025 16:31:12 -0500 Subject: [PATCH 02/26] updating tests since diffwithhighlight is now only responsible for placing placing spans for changes and not syntax highlighting the code --- services/gitdiff/highlightdiff_test.go | 77 +++++++++++++------------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/services/gitdiff/highlightdiff_test.go b/services/gitdiff/highlightdiff_test.go index 545a060e20491..6208e7c9411f8 100644 --- a/services/gitdiff/highlightdiff_test.go +++ b/services/gitdiff/highlightdiff_test.go @@ -5,7 +5,6 @@ package gitdiff import ( "fmt" - "strings" "testing" "github.com/sergi/go-diff/diffmatchpatch" @@ -20,11 +19,13 @@ func TestDiffWithHighlight(t *testing.T) { " run(db)\n", ) - expected := ` run('<>')` + expected := ` run('<>') +` output := diffToHTML(nil, diffs, DiffLineDel) assert.Equal(t, expected, output) - expected = ` run(db)` + expected = ` run(db) +` output = diffToHTML(nil, diffs, DiffLineAdd) assert.Equal(t, expected, output) @@ -33,13 +34,13 @@ func TestDiffWithHighlight(t *testing.T) { hcd.placeholderTokenMap['C'] = "" diff := diffmatchpatch.Diff{} - diff.Text = "OC" - hcd.recoverOneDiff(&diff) - assert.Equal(t, "", diff.Text) - - diff.Text = "O" - hcd.recoverOneDiff(&diff) - assert.Equal(t, "", diff.Text) + // diff.Text = "OC" + // hcd.recoverOneDiff(&diff) + // assert.Equal(t, "", diff.Text) + // + // diff.Text = "O" + // hcd.recoverOneDiff(&diff) + // assert.Equal(t, "", diff.Text) diff.Text = "C" hcd.recoverOneDiff(&diff) @@ -56,7 +57,7 @@ func TestDiffWithHighlightPlaceholder(t *testing.T) { assert.Equal(t, "", hcd.placeholderTokenMap[0x00100000]) assert.Equal(t, "", hcd.placeholderTokenMap[0x0010FFFD]) - expected := fmt.Sprintf(`a='%s'`, "\U00100000") + expected := fmt.Sprintf(`a='%s'`, "\U00100000") output := diffToHTML(hcd.lineWrapperTags, diffs, DiffLineDel) assert.Equal(t, expected, output) @@ -66,7 +67,7 @@ func TestDiffWithHighlightPlaceholder(t *testing.T) { "a='\U00100000'", "a='\U0010FFFD'", ) - expected = fmt.Sprintf(`a='%s'`, "\U0010FFFD") + expected = fmt.Sprintf(`a='%s'`, "\U0010FFFD") output = diffToHTML(nil, diffs, DiffLineAdd) assert.Equal(t, expected, output) } @@ -80,7 +81,7 @@ func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) { ``, ) output := diffToHTML(nil, diffs, DiffLineDel) - expected := fmt.Sprintf(`%s#39;`, "\uFFFD") + expected := `'` assert.Equal(t, expected, output) hcd = newHighlightCodeDiff() @@ -91,35 +92,37 @@ func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) { "a > b", ) output = diffToHTML(nil, diffs, DiffLineDel) - expected = fmt.Sprintf(`a %slt; b`, "\uFFFD") + expected = `a < b` assert.Equal(t, expected, output) output = diffToHTML(nil, diffs, DiffLineAdd) - expected = fmt.Sprintf(`a %sgt; b`, "\uFFFD") + expected = `a > b` assert.Equal(t, expected, output) } func TestDiffWithHighlightTagMatch(t *testing.T) { - totalOverflow := 0 - for i := 0; i < 100; i++ { - hcd := newHighlightCodeDiff() - hcd.placeholderMaxCount = i - diffs := hcd.diffWithHighlight( - "main.js", "", - "a='1'", - "b='2'", - ) - totalOverflow += hcd.placeholderOverflowCount - - output := diffToHTML(nil, diffs, DiffLineDel) - c1 := strings.Count(output, " Date: Sat, 1 Mar 2025 16:31:42 -0500 Subject: [PATCH 03/26] adding language var back --- services/gitdiff/gitdiff.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index eb0cc1e411b0a..0f672195a8868 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -301,6 +301,10 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc diff2 string ) + language := "" + if diffSection.file != nil { + language = diffSection.file.Language + } // try to find equivalent diff line. ignore, otherwise switch diffLine.Type { case DiffLineSection: @@ -331,7 +335,7 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc } hcd := newHighlightCodeDiff() - diffRecord := hcd.diffWithHighlight(diffSection.FileName, diffSection.file.Language, diff1, diff2) + diffRecord := hcd.diffWithHighlight(diffSection.FileName, language, diff1, diff2) // it seems that Gitea doesn't need the line wrapper of Chroma, so do not add them back // if the line wrappers are still needed in the future, it can be added back by "diffToHTML(hcd.lineWrapperTags. ...)" diffHTML := html.UnescapeString(diffToHTML(nil, diffRecord, diffLine.Type)) From aac3ec4b1686e0734314ce02a47745b68e460435 Mon Sep 17 00:00:00 2001 From: Dustin Firebaugh Date: Sat, 1 Mar 2025 21:08:08 -0500 Subject: [PATCH 04/26] updating tests --- services/gitdiff/highlightdiff_test.go | 34 -------------------------- 1 file changed, 34 deletions(-) diff --git a/services/gitdiff/highlightdiff_test.go b/services/gitdiff/highlightdiff_test.go index 6208e7c9411f8..a58fb5a7e8312 100644 --- a/services/gitdiff/highlightdiff_test.go +++ b/services/gitdiff/highlightdiff_test.go @@ -34,14 +34,6 @@ func TestDiffWithHighlight(t *testing.T) { hcd.placeholderTokenMap['C'] = "" diff := diffmatchpatch.Diff{} - // diff.Text = "OC" - // hcd.recoverOneDiff(&diff) - // assert.Equal(t, "", diff.Text) - // - // diff.Text = "O" - // hcd.recoverOneDiff(&diff) - // assert.Equal(t, "", diff.Text) - diff.Text = "C" hcd.recoverOneDiff(&diff) assert.Equal(t, "", diff.Text) @@ -100,29 +92,3 @@ func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) { assert.Equal(t, expected, output) } -func TestDiffWithHighlightTagMatch(t *testing.T) { - // totalOverflow := 0 - // - // for i := 0; i < 100; i++ { - // hcd := newHighlightCodeDiff() - // hcd.placeholderMaxCount = i - // diffs := hcd.diffWithHighlight( - // "main.js", "", - // "a='1'", - // "b='2'", - // ) - // totalOverflow += hcd.placeholderOverflowCount - // - // output := diffToHTML(nil, diffs, DiffLineDel) - // c1 := strings.Count(output, " Date: Sat, 1 Mar 2025 21:14:03 -0500 Subject: [PATCH 05/26] - making highlighted code members private --- services/gitdiff/gitdiff.go | 43 ++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 0f672195a8868..6e997da13fc9c 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -312,25 +312,25 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc case DiffLineAdd: compareDiffLine = diffSection.GetLine(DiffLineDel, diffLine.RightIdx) if compareDiffLine == nil { - highlightedLine := diffSection.file.HighlightedNewLines[diffLine.RightIdx-1] + highlightedLine := diffSection.file.highlightedNewLines[diffLine.RightIdx-1] return DiffInlineWithUnicodeEscape(template.HTML(highlightedLine), locale) } - diff1 = diffSection.file.HighlightedOldLines[compareDiffLine.LeftIdx-1] - diff2 = diffSection.file.HighlightedNewLines[diffLine.RightIdx-1] + diff1 = diffSection.file.highlightedOldLines[compareDiffLine.LeftIdx-1] + diff2 = diffSection.file.highlightedNewLines[diffLine.RightIdx-1] case DiffLineDel: compareDiffLine = diffSection.GetLine(DiffLineAdd, diffLine.LeftIdx) if compareDiffLine == nil { - highlightedLine := diffSection.file.HighlightedOldLines[diffLine.LeftIdx-1] + highlightedLine := diffSection.file.highlightedOldLines[diffLine.LeftIdx-1] return DiffInlineWithUnicodeEscape(template.HTML(highlightedLine), locale) } - diff1 = diffSection.file.HighlightedOldLines[diffLine.LeftIdx-1] - diff2 = diffSection.file.HighlightedNewLines[compareDiffLine.RightIdx-1] + diff1 = diffSection.file.highlightedOldLines[diffLine.LeftIdx-1] + diff2 = diffSection.file.highlightedNewLines[compareDiffLine.RightIdx-1] default: if strings.IndexByte(" +-", diffLine.Content[0]) > -1 { - highlightedContent := diffSection.file.HighlightedNewLines[diffLine.RightIdx-1] + highlightedContent := diffSection.file.highlightedNewLines[diffLine.RightIdx-1] return DiffInlineWithUnicodeEscape(template.HTML(highlightedContent), locale) } - highlightedContent := diffSection.file.HighlightedOldLines[diffLine.LeftIdx-1] + highlightedContent := diffSection.file.highlightedOldLines[diffLine.LeftIdx-1] return DiffInlineWithUnicodeEscape(template.HTML(highlightedContent), locale) } @@ -371,8 +371,8 @@ type DiffFile struct { IsSubmodule bool // if IsSubmodule==true, then there must be a SubmoduleDiffInfo SubmoduleDiffInfo *SubmoduleDiffInfo - HighlightedOldLines []string - HighlightedNewLines []string + highlightedOldLines []string + highlightedNewLines []string } // GetType returns type of diff file. @@ -1226,7 +1226,7 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi } diffFile.IsGenerated = isGenerated.Value() - highlightCode(commit, beforeCommit, diffFile) + diffFile.highlightedOldLines, diffFile.highlightedNewLines = highlightCode(commit, beforeCommit, diffFile) tailSection := diffFile.GetTailSection(gitRepo, beforeCommit, commit) if tailSection != nil { @@ -1248,17 +1248,19 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi return diff, nil } -func highlightCode(commit *git.Commit, beforeCommit *git.Commit, diffFile *DiffFile) { +func highlightCode(commit *git.Commit, beforeCommit *git.Commit, diffFile *DiffFile) ([]string, []string) { + var oldLines []string + var newLines []string + if beforeCommit != nil { oldBlob, err := beforeCommit.GetBlobByPath(diffFile.Name) if err == nil { oldContent, _ := oldBlob.GetBlobContent(oldBlob.Size()) highlightedOldContent, _ := highlight.Code(diffFile.Name, diffFile.Language, oldContent) - oldLines := strings.Split(string(highlightedOldContent), "\n") - diffFile.HighlightedOldLines = make([]string, len(oldLines)) - for i, line := range oldLines { - diffFile.HighlightedOldLines[i] = line + oldLines = strings.Split(string(highlightedOldContent), "\n") + for _, line := range oldLines { + oldLines = append(oldLines, line) } } } @@ -1268,12 +1270,13 @@ func highlightCode(commit *git.Commit, beforeCommit *git.Commit, diffFile *DiffF newContent, _ := newBlob.GetBlobContent(newBlob.Size()) highlightedNewContent, _ := highlight.Code(diffFile.Name, diffFile.Language, newContent) - newLines := strings.Split(string(highlightedNewContent), "\n") - diffFile.HighlightedNewLines = make([]string, len(newLines)) - for i, line := range newLines { - diffFile.HighlightedNewLines[i] = line + newLines = strings.Split(string(highlightedNewContent), "\n") + for _, line := range newLines { + newLines = append(newLines, line) } } + + return oldLines, newLines } type PullDiffStats struct { From 3607a2558fa67f700fcee7577be751397b400a01 Mon Sep 17 00:00:00 2001 From: Dustin Firebaugh Date: Sat, 1 Mar 2025 21:35:51 -0500 Subject: [PATCH 06/26] after make fmt --- services/gitdiff/gitdiff.go | 2 +- services/gitdiff/highlightdiff_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 6e997da13fc9c..81d8408578428 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1248,7 +1248,7 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi return diff, nil } -func highlightCode(commit *git.Commit, beforeCommit *git.Commit, diffFile *DiffFile) ([]string, []string) { +func highlightCode(commit, beforeCommit *git.Commit, diffFile *DiffFile) ([]string, []string) { var oldLines []string var newLines []string diff --git a/services/gitdiff/highlightdiff_test.go b/services/gitdiff/highlightdiff_test.go index a58fb5a7e8312..255648dba7b48 100644 --- a/services/gitdiff/highlightdiff_test.go +++ b/services/gitdiff/highlightdiff_test.go @@ -91,4 +91,3 @@ func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) { expected = `a > b` assert.Equal(t, expected, output) } - From 843cb3d4fcf05ec72bd5def0ea085b83413f8c83 Mon Sep 17 00:00:00 2001 From: Dustin Firebaugh Date: Sat, 1 Mar 2025 22:17:40 -0500 Subject: [PATCH 07/26] implementing lint suggestion --- services/gitdiff/gitdiff.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 81d8408578428..ab494aad18ea2 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1258,10 +1258,8 @@ func highlightCode(commit, beforeCommit *git.Commit, diffFile *DiffFile) ([]stri oldContent, _ := oldBlob.GetBlobContent(oldBlob.Size()) highlightedOldContent, _ := highlight.Code(diffFile.Name, diffFile.Language, oldContent) - oldLines = strings.Split(string(highlightedOldContent), "\n") - for _, line := range oldLines { - oldLines = append(oldLines, line) - } + splitLines := strings.Split(string(highlightedOldContent), "\n") + oldLines = append(oldLines, splitLines...) } } @@ -1270,10 +1268,8 @@ func highlightCode(commit, beforeCommit *git.Commit, diffFile *DiffFile) ([]stri newContent, _ := newBlob.GetBlobContent(newBlob.Size()) highlightedNewContent, _ := highlight.Code(diffFile.Name, diffFile.Language, newContent) - newLines = strings.Split(string(highlightedNewContent), "\n") - for _, line := range newLines { - newLines = append(newLines, line) - } + splitLines := strings.Split(string(highlightedNewContent), "\n") + newLines = append(newLines, splitLines...) } return oldLines, newLines From 0eaefbc740e07116217a1380a9e4703f82bf01c7 Mon Sep 17 00:00:00 2001 From: Dustin Firebaugh Date: Sun, 2 Mar 2025 00:21:16 -0500 Subject: [PATCH 08/26] - removing filename and language from `diffWithHighlight` func --- services/gitdiff/gitdiff.go | 8 ++------ services/gitdiff/highlightdiff.go | 2 +- services/gitdiff/highlightdiff_test.go | 5 ----- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index ab494aad18ea2..6919e30f48560 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -301,10 +301,6 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc diff2 string ) - language := "" - if diffSection.file != nil { - language = diffSection.file.Language - } // try to find equivalent diff line. ignore, otherwise switch diffLine.Type { case DiffLineSection: @@ -335,10 +331,10 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc } hcd := newHighlightCodeDiff() - diffRecord := hcd.diffWithHighlight(diffSection.FileName, language, diff1, diff2) + diffRecord := hcd.diffWithHighlight(diff1, diff2) // it seems that Gitea doesn't need the line wrapper of Chroma, so do not add them back // if the line wrappers are still needed in the future, it can be added back by "diffToHTML(hcd.lineWrapperTags. ...)" - diffHTML := html.UnescapeString(diffToHTML(nil, diffRecord, diffLine.Type)) + diffHTML := diffToHTML(nil, diffRecord, diffLine.Type) return DiffInlineWithUnicodeEscape(template.HTML(diffHTML), locale) } diff --git a/services/gitdiff/highlightdiff.go b/services/gitdiff/highlightdiff.go index 9491cb995adca..7013a630c5f61 100644 --- a/services/gitdiff/highlightdiff.go +++ b/services/gitdiff/highlightdiff.go @@ -84,7 +84,7 @@ func (hcd *highlightCodeDiff) collectUsedRunes(code string) { } } -func (hcd *highlightCodeDiff) diffWithHighlight(filename, language, codeA, codeB string) []diffmatchpatch.Diff { +func (hcd *highlightCodeDiff) diffWithHighlight(codeA, codeB string) []diffmatchpatch.Diff { hcd.collectUsedRunes(codeA) hcd.collectUsedRunes(codeB) diff --git a/services/gitdiff/highlightdiff_test.go b/services/gitdiff/highlightdiff_test.go index 255648dba7b48..f686c35bdf314 100644 --- a/services/gitdiff/highlightdiff_test.go +++ b/services/gitdiff/highlightdiff_test.go @@ -14,7 +14,6 @@ import ( func TestDiffWithHighlight(t *testing.T) { hcd := newHighlightCodeDiff() diffs := hcd.diffWithHighlight( - "main.v", "", " run('<>')\n", " run(db)\n", ) @@ -42,7 +41,6 @@ func TestDiffWithHighlight(t *testing.T) { func TestDiffWithHighlightPlaceholder(t *testing.T) { hcd := newHighlightCodeDiff() diffs := hcd.diffWithHighlight( - "main.js", "", "a='\U00100000'", "a='\U0010FFFD''", ) @@ -55,7 +53,6 @@ func TestDiffWithHighlightPlaceholder(t *testing.T) { hcd = newHighlightCodeDiff() diffs = hcd.diffWithHighlight( - "main.js", "", "a='\U00100000'", "a='\U0010FFFD'", ) @@ -68,7 +65,6 @@ func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) { hcd := newHighlightCodeDiff() hcd.placeholderMaxCount = 0 diffs := hcd.diffWithHighlight( - "main.js", "", "'", ``, ) @@ -79,7 +75,6 @@ func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) { hcd = newHighlightCodeDiff() hcd.placeholderMaxCount = 0 diffs = hcd.diffWithHighlight( - "main.js", "", "a < b", "a > b", ) From 9a59d9a9d3fef6f030211dc38f4349c654bebcfa Mon Sep 17 00:00:00 2001 From: Dustin Firebaugh Date: Sun, 2 Mar 2025 01:09:54 -0500 Subject: [PATCH 09/26] reverting span close logic --- services/gitdiff/highlightdiff.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/services/gitdiff/highlightdiff.go b/services/gitdiff/highlightdiff.go index 7013a630c5f61..28443452eae34 100644 --- a/services/gitdiff/highlightdiff.go +++ b/services/gitdiff/highlightdiff.go @@ -201,5 +201,17 @@ func (hcd *highlightCodeDiff) recoverOneDiff(diff *diffmatchpatch.Diff) { sb.WriteString(tokenToRecover) } + if len(tagStack) > 0 { + // close all opening tags + for i := len(tagStack) - 1; i >= 0; i-- { + tagToClose := tagStack[i] + // get the closing tag "" from "" or "" + pos := strings.IndexAny(tagToClose, " >") + if pos != -1 { + sb.WriteString("") + } // else: impossible. every tag was pushed into the stack by the code above and is valid HTML opening tag + } + } + diff.Text = sb.String() } From 5b0adf2c16dc160130da0f922a56182490b245f4 Mon Sep 17 00:00:00 2001 From: Dustin Firebaugh Date: Sun, 2 Mar 2025 01:38:15 -0500 Subject: [PATCH 10/26] updating `highlightdiff_test.go` test data --- services/gitdiff/highlightdiff_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/services/gitdiff/highlightdiff_test.go b/services/gitdiff/highlightdiff_test.go index f686c35bdf314..71400fed4d714 100644 --- a/services/gitdiff/highlightdiff_test.go +++ b/services/gitdiff/highlightdiff_test.go @@ -18,8 +18,8 @@ func TestDiffWithHighlight(t *testing.T) { " run(db)\n", ) - expected := ` run('<>') -` + expected := "\t\trun('<>')\n" + output := diffToHTML(nil, diffs, DiffLineDel) assert.Equal(t, expected, output) @@ -75,14 +75,14 @@ func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) { hcd = newHighlightCodeDiff() hcd.placeholderMaxCount = 0 diffs = hcd.diffWithHighlight( - "a < b", - "a > b", + "a this_is_not_html_at_this_point b", + "a this_is_is_still_not_html_at_this_point_its_just_a_string b", ) output = diffToHTML(nil, diffs, DiffLineDel) - expected = `a < b` + expected = "a this_is_not_html_at_this_point b" assert.Equal(t, expected, output) output = diffToHTML(nil, diffs, DiffLineAdd) - expected = `a > b` + expected = "a this_is_is_still_not_html_at_this_point_its_just_a_string b" assert.Equal(t, expected, output) } From 447236e73706f368fc21d9934d6553870e5c39ea Mon Sep 17 00:00:00 2001 From: Dustin Firebaugh Date: Sun, 2 Mar 2025 02:00:37 -0500 Subject: [PATCH 11/26] - readding call to `highligh.Code` in `diffWithHighlight` - reverting tests to be closer to original --- services/gitdiff/highlightdiff.go | 8 +++- services/gitdiff/highlightdiff_test.go | 53 ++++++++++++++++++++------ 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/services/gitdiff/highlightdiff.go b/services/gitdiff/highlightdiff.go index 28443452eae34..134292b676836 100644 --- a/services/gitdiff/highlightdiff.go +++ b/services/gitdiff/highlightdiff.go @@ -6,6 +6,7 @@ package gitdiff import ( "strings" + "code.gitea.io/gitea/modules/highlight" "github.com/sergi/go-diff/diffmatchpatch" ) @@ -88,8 +89,11 @@ func (hcd *highlightCodeDiff) diffWithHighlight(codeA, codeB string) []diffmatch hcd.collectUsedRunes(codeA) hcd.collectUsedRunes(codeB) - convertedCodeA := hcd.convertToPlaceholders(codeA) - convertedCodeB := hcd.convertToPlaceholders(codeB) + highlightCodeA, _ := highlight.Code("", "", codeA) + highlightCodeB, _ := highlight.Code("", "", codeB) + + convertedCodeA := hcd.convertToPlaceholders(string(highlightCodeA)) + convertedCodeB := hcd.convertToPlaceholders(string(highlightCodeB)) diffs := diffMatchPatch.DiffMain(convertedCodeA, convertedCodeB, true) diffs = diffMatchPatch.DiffCleanupEfficiency(diffs) diff --git a/services/gitdiff/highlightdiff_test.go b/services/gitdiff/highlightdiff_test.go index 71400fed4d714..b3c84150ce780 100644 --- a/services/gitdiff/highlightdiff_test.go +++ b/services/gitdiff/highlightdiff_test.go @@ -5,6 +5,7 @@ package gitdiff import ( "fmt" + "strings" "testing" "github.com/sergi/go-diff/diffmatchpatch" @@ -18,13 +19,11 @@ func TestDiffWithHighlight(t *testing.T) { " run(db)\n", ) - expected := "\t\trun('<>')\n" - + expected := ` run('<>')` output := diffToHTML(nil, diffs, DiffLineDel) assert.Equal(t, expected, output) - expected = ` run(db) -` + expected = ` run(db)` output = diffToHTML(nil, diffs, DiffLineAdd) assert.Equal(t, expected, output) @@ -33,6 +32,14 @@ func TestDiffWithHighlight(t *testing.T) { hcd.placeholderTokenMap['C'] = "" diff := diffmatchpatch.Diff{} + diff.Text = "OC" + hcd.recoverOneDiff(&diff) + assert.Equal(t, "", diff.Text) + + diff.Text = "O" + hcd.recoverOneDiff(&diff) + assert.Equal(t, "", diff.Text) + diff.Text = "C" hcd.recoverOneDiff(&diff) assert.Equal(t, "", diff.Text) @@ -47,7 +54,7 @@ func TestDiffWithHighlightPlaceholder(t *testing.T) { assert.Equal(t, "", hcd.placeholderTokenMap[0x00100000]) assert.Equal(t, "", hcd.placeholderTokenMap[0x0010FFFD]) - expected := fmt.Sprintf(`a='%s'`, "\U00100000") + expected := fmt.Sprintf(`a='%s'`, "\U00100000") output := diffToHTML(hcd.lineWrapperTags, diffs, DiffLineDel) assert.Equal(t, expected, output) @@ -56,7 +63,7 @@ func TestDiffWithHighlightPlaceholder(t *testing.T) { "a='\U00100000'", "a='\U0010FFFD'", ) - expected = fmt.Sprintf(`a='%s'`, "\U0010FFFD") + expected = fmt.Sprintf(`a='%s'`, "\U0010FFFD") output = diffToHTML(nil, diffs, DiffLineAdd) assert.Equal(t, expected, output) } @@ -69,20 +76,44 @@ func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) { ``, ) output := diffToHTML(nil, diffs, DiffLineDel) - expected := `'` + expected := fmt.Sprintf(`%s#39;`, "\uFFFD") assert.Equal(t, expected, output) hcd = newHighlightCodeDiff() hcd.placeholderMaxCount = 0 diffs = hcd.diffWithHighlight( - "a this_is_not_html_at_this_point b", - "a this_is_is_still_not_html_at_this_point_its_just_a_string b", + "a < b", + "a > b", ) output = diffToHTML(nil, diffs, DiffLineDel) - expected = "a this_is_not_html_at_this_point b" + expected = fmt.Sprintf(`a %slt; b`, "\uFFFD") assert.Equal(t, expected, output) output = diffToHTML(nil, diffs, DiffLineAdd) - expected = "a this_is_is_still_not_html_at_this_point_its_just_a_string b" + expected = fmt.Sprintf(`a %sgt; b`, "\uFFFD") assert.Equal(t, expected, output) } + +func TestDiffWithHighlightTagMatch(t *testing.T) { + totalOverflow := 0 + for i := 0; i < 100; i++ { + hcd := newHighlightCodeDiff() + hcd.placeholderMaxCount = i + diffs := hcd.diffWithHighlight( + "a='1'", + "b='2'", + ) + totalOverflow += hcd.placeholderOverflowCount + + output := diffToHTML(nil, diffs, DiffLineDel) + c1 := strings.Count(output, " Date: Sun, 2 Mar 2025 02:08:42 -0500 Subject: [PATCH 12/26] - calling `template.HTMLEscapeString` directly in `diffWithHighlight` since we dont actually need to highlight the code here - adding newline in testcase --- services/gitdiff/highlightdiff.go | 9 +++------ services/gitdiff/highlightdiff_test.go | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/services/gitdiff/highlightdiff.go b/services/gitdiff/highlightdiff.go index 134292b676836..9ab879df65b94 100644 --- a/services/gitdiff/highlightdiff.go +++ b/services/gitdiff/highlightdiff.go @@ -4,9 +4,9 @@ package gitdiff import ( + "html/template" "strings" - "code.gitea.io/gitea/modules/highlight" "github.com/sergi/go-diff/diffmatchpatch" ) @@ -89,11 +89,8 @@ func (hcd *highlightCodeDiff) diffWithHighlight(codeA, codeB string) []diffmatch hcd.collectUsedRunes(codeA) hcd.collectUsedRunes(codeB) - highlightCodeA, _ := highlight.Code("", "", codeA) - highlightCodeB, _ := highlight.Code("", "", codeB) - - convertedCodeA := hcd.convertToPlaceholders(string(highlightCodeA)) - convertedCodeB := hcd.convertToPlaceholders(string(highlightCodeB)) + convertedCodeA := hcd.convertToPlaceholders(template.HTMLEscapeString(codeA)) + convertedCodeB := hcd.convertToPlaceholders(template.HTMLEscapeString(codeB)) diffs := diffMatchPatch.DiffMain(convertedCodeA, convertedCodeB, true) diffs = diffMatchPatch.DiffCleanupEfficiency(diffs) diff --git a/services/gitdiff/highlightdiff_test.go b/services/gitdiff/highlightdiff_test.go index b3c84150ce780..334e43c7096a5 100644 --- a/services/gitdiff/highlightdiff_test.go +++ b/services/gitdiff/highlightdiff_test.go @@ -19,11 +19,11 @@ func TestDiffWithHighlight(t *testing.T) { " run(db)\n", ) - expected := ` run('<>')` + expected := "\t\trun('<>')\n" output := diffToHTML(nil, diffs, DiffLineDel) assert.Equal(t, expected, output) - expected = ` run(db)` + expected = "\t\trun(db)\n" output = diffToHTML(nil, diffs, DiffLineAdd) assert.Equal(t, expected, output) From 14d6c0edcf667d38edb5477396c2c5a981862dcf Mon Sep 17 00:00:00 2001 From: Dustin Firebaugh Date: Sun, 2 Mar 2025 02:40:10 -0500 Subject: [PATCH 13/26] reverting htmlescape because we already escape it when we highlight the code --- services/gitdiff/highlightdiff.go | 5 +-- services/gitdiff/highlightdiff_test.go | 53 ++++++-------------------- 2 files changed, 13 insertions(+), 45 deletions(-) diff --git a/services/gitdiff/highlightdiff.go b/services/gitdiff/highlightdiff.go index 9ab879df65b94..28443452eae34 100644 --- a/services/gitdiff/highlightdiff.go +++ b/services/gitdiff/highlightdiff.go @@ -4,7 +4,6 @@ package gitdiff import ( - "html/template" "strings" "github.com/sergi/go-diff/diffmatchpatch" @@ -89,8 +88,8 @@ func (hcd *highlightCodeDiff) diffWithHighlight(codeA, codeB string) []diffmatch hcd.collectUsedRunes(codeA) hcd.collectUsedRunes(codeB) - convertedCodeA := hcd.convertToPlaceholders(template.HTMLEscapeString(codeA)) - convertedCodeB := hcd.convertToPlaceholders(template.HTMLEscapeString(codeB)) + convertedCodeA := hcd.convertToPlaceholders(codeA) + convertedCodeB := hcd.convertToPlaceholders(codeB) diffs := diffMatchPatch.DiffMain(convertedCodeA, convertedCodeB, true) diffs = diffMatchPatch.DiffCleanupEfficiency(diffs) diff --git a/services/gitdiff/highlightdiff_test.go b/services/gitdiff/highlightdiff_test.go index 334e43c7096a5..71400fed4d714 100644 --- a/services/gitdiff/highlightdiff_test.go +++ b/services/gitdiff/highlightdiff_test.go @@ -5,7 +5,6 @@ package gitdiff import ( "fmt" - "strings" "testing" "github.com/sergi/go-diff/diffmatchpatch" @@ -19,11 +18,13 @@ func TestDiffWithHighlight(t *testing.T) { " run(db)\n", ) - expected := "\t\trun('<>')\n" + expected := "\t\trun('<>')\n" + output := diffToHTML(nil, diffs, DiffLineDel) assert.Equal(t, expected, output) - expected = "\t\trun(db)\n" + expected = ` run(db) +` output = diffToHTML(nil, diffs, DiffLineAdd) assert.Equal(t, expected, output) @@ -32,14 +33,6 @@ func TestDiffWithHighlight(t *testing.T) { hcd.placeholderTokenMap['C'] = "" diff := diffmatchpatch.Diff{} - diff.Text = "OC" - hcd.recoverOneDiff(&diff) - assert.Equal(t, "", diff.Text) - - diff.Text = "O" - hcd.recoverOneDiff(&diff) - assert.Equal(t, "", diff.Text) - diff.Text = "C" hcd.recoverOneDiff(&diff) assert.Equal(t, "", diff.Text) @@ -54,7 +47,7 @@ func TestDiffWithHighlightPlaceholder(t *testing.T) { assert.Equal(t, "", hcd.placeholderTokenMap[0x00100000]) assert.Equal(t, "", hcd.placeholderTokenMap[0x0010FFFD]) - expected := fmt.Sprintf(`a='%s'`, "\U00100000") + expected := fmt.Sprintf(`a='%s'`, "\U00100000") output := diffToHTML(hcd.lineWrapperTags, diffs, DiffLineDel) assert.Equal(t, expected, output) @@ -63,7 +56,7 @@ func TestDiffWithHighlightPlaceholder(t *testing.T) { "a='\U00100000'", "a='\U0010FFFD'", ) - expected = fmt.Sprintf(`a='%s'`, "\U0010FFFD") + expected = fmt.Sprintf(`a='%s'`, "\U0010FFFD") output = diffToHTML(nil, diffs, DiffLineAdd) assert.Equal(t, expected, output) } @@ -76,44 +69,20 @@ func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) { ``, ) output := diffToHTML(nil, diffs, DiffLineDel) - expected := fmt.Sprintf(`%s#39;`, "\uFFFD") + expected := `'` assert.Equal(t, expected, output) hcd = newHighlightCodeDiff() hcd.placeholderMaxCount = 0 diffs = hcd.diffWithHighlight( - "a < b", - "a > b", + "a this_is_not_html_at_this_point b", + "a this_is_is_still_not_html_at_this_point_its_just_a_string b", ) output = diffToHTML(nil, diffs, DiffLineDel) - expected = fmt.Sprintf(`a %slt; b`, "\uFFFD") + expected = "a this_is_not_html_at_this_point b" assert.Equal(t, expected, output) output = diffToHTML(nil, diffs, DiffLineAdd) - expected = fmt.Sprintf(`a %sgt; b`, "\uFFFD") + expected = "a this_is_is_still_not_html_at_this_point_its_just_a_string b" assert.Equal(t, expected, output) } - -func TestDiffWithHighlightTagMatch(t *testing.T) { - totalOverflow := 0 - for i := 0; i < 100; i++ { - hcd := newHighlightCodeDiff() - hcd.placeholderMaxCount = i - diffs := hcd.diffWithHighlight( - "a='1'", - "b='2'", - ) - totalOverflow += hcd.placeholderOverflowCount - - output := diffToHTML(nil, diffs, DiffLineDel) - c1 := strings.Count(output, " Date: Sun, 2 Mar 2025 15:11:09 -0500 Subject: [PATCH 14/26] - reverting original hunk-based syntax highlighting - adding conditions for when full file based highlighting should occur - falling back to hunk based highlighting in certain conditions - reverting original hunk-based highlighting tests - updating types to express intent better and to be more accurate i.e. `template.HTML` instead of string where appropriate --- modules/setting/git.go | 2 + services/gitdiff/gitdiff.go | 141 ++++++++++++++++++++++--- services/gitdiff/highlightdiff.go | 28 ++++- services/gitdiff/highlightdiff_test.go | 59 +++++++++-- 4 files changed, 201 insertions(+), 29 deletions(-) diff --git a/modules/setting/git.go b/modules/setting/git.go index 48a4e7f30deb5..c48f93f00ec90 100644 --- a/modules/setting/git.go +++ b/modules/setting/git.go @@ -38,10 +38,12 @@ var Git = struct { Pull int GC int `ini:"GC"` } `ini:"git.timeout"` + MaxDiffHighlightFileSize int64 }{ DisableDiffHighlight: false, MaxGitDiffLines: 1000, MaxGitDiffLineCharacters: 5000, + MaxDiffHighlightFileSize: 5000, MaxGitDiffFiles: 100, CommitsRangeSize: 50, BranchesRangeSize: 20, diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 6919e30f48560..130b8d075ab9c 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -289,8 +289,14 @@ func DiffInlineWithUnicodeEscape(s template.HTML, locale translation.Locale) Dif return DiffInline{EscapeStatus: status, Content: content} } -// GetComputedInlineDiffFor computes inline diff for the given line. -func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, locale translation.Locale) DiffInline { +// DiffInlineWithHighlightCode makes a DiffInline with code highlight and hidden unicode characters escaped +func DiffInlineWithHighlightCode(fileName, language, code string, locale translation.Locale) DiffInline { + highlighted, _ := highlight.Code(fileName, language, code) + status, content := charset.EscapeControlHTML(highlighted, locale) + return DiffInline{EscapeStatus: status, Content: content} +} + +func (diffSection *DiffSection) getComputedInlineDiffForHunk(diffLine *DiffLine, locale translation.Locale) DiffInline { if setting.Git.DisableDiffHighlight { return getLineContent(diffLine.Content[1:], locale) } @@ -301,6 +307,55 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc diff2 string ) + language := "" + if diffSection.file != nil { + language = diffSection.file.Language + } + + // try to find equivalent diff line. ignore, otherwise + switch diffLine.Type { + case DiffLineSection: + return getLineContent(diffLine.Content[1:], locale) + case DiffLineAdd: + compareDiffLine = diffSection.GetLine(DiffLineDel, diffLine.RightIdx) + if compareDiffLine == nil { + return DiffInlineWithHighlightCode(diffSection.FileName, language, diffLine.Content[1:], locale) + } + diff1 = compareDiffLine.Content + diff2 = diffLine.Content + case DiffLineDel: + compareDiffLine = diffSection.GetLine(DiffLineAdd, diffLine.LeftIdx) + if compareDiffLine == nil { + return DiffInlineWithHighlightCode(diffSection.FileName, language, diffLine.Content[1:], locale) + } + diff1 = diffLine.Content + diff2 = compareDiffLine.Content + default: + if strings.IndexByte(" +-", diffLine.Content[0]) > -1 { + return DiffInlineWithHighlightCode(diffSection.FileName, language, diffLine.Content[1:], locale) + } + return DiffInlineWithHighlightCode(diffSection.FileName, language, diffLine.Content, locale) + } + + hcd := newHighlightCodeDiff() + diffRecord := hcd.diffWithHighlight(diffSection.FileName, language, diff1[1:], diff2[1:]) + // it seems that Gitea doesn't need the line wrapper of Chroma, so do not add them back + // if the line wrappers are still needed in the future, it can be added back by "diffToHTML(hcd.lineWrapperTags. ...)" + diffHTML := diffToHTML(nil, diffRecord, diffLine.Type) + return DiffInlineWithUnicodeEscape(template.HTML(diffHTML), locale) +} + +func (diffSection *DiffSection) getComputedInlineDiffForFullFile(diffLine *DiffLine, locale translation.Locale) DiffInline { + if setting.Git.DisableDiffHighlight { + return getLineContent(diffLine.Content[1:], locale) + } + + var ( + compareDiffLine *DiffLine + diff1 template.HTML + diff2 template.HTML + ) + // try to find equivalent diff line. ignore, otherwise switch diffLine.Type { case DiffLineSection: @@ -309,7 +364,7 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc compareDiffLine = diffSection.GetLine(DiffLineDel, diffLine.RightIdx) if compareDiffLine == nil { highlightedLine := diffSection.file.highlightedNewLines[diffLine.RightIdx-1] - return DiffInlineWithUnicodeEscape(template.HTML(highlightedLine), locale) + return DiffInlineWithUnicodeEscape(highlightedLine, locale) } diff1 = diffSection.file.highlightedOldLines[compareDiffLine.LeftIdx-1] diff2 = diffSection.file.highlightedNewLines[diffLine.RightIdx-1] @@ -317,27 +372,38 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc compareDiffLine = diffSection.GetLine(DiffLineAdd, diffLine.LeftIdx) if compareDiffLine == nil { highlightedLine := diffSection.file.highlightedOldLines[diffLine.LeftIdx-1] - return DiffInlineWithUnicodeEscape(template.HTML(highlightedLine), locale) + return DiffInlineWithUnicodeEscape(highlightedLine, locale) } diff1 = diffSection.file.highlightedOldLines[diffLine.LeftIdx-1] diff2 = diffSection.file.highlightedNewLines[compareDiffLine.RightIdx-1] default: if strings.IndexByte(" +-", diffLine.Content[0]) > -1 { highlightedContent := diffSection.file.highlightedNewLines[diffLine.RightIdx-1] - return DiffInlineWithUnicodeEscape(template.HTML(highlightedContent), locale) + return DiffInlineWithUnicodeEscape(highlightedContent, locale) } highlightedContent := diffSection.file.highlightedOldLines[diffLine.LeftIdx-1] - return DiffInlineWithUnicodeEscape(template.HTML(highlightedContent), locale) + return DiffInlineWithUnicodeEscape(highlightedContent, locale) } hcd := newHighlightCodeDiff() - diffRecord := hcd.diffWithHighlight(diff1, diff2) + diffRecord := hcd.diffWithFullFileHighlight(diff1, diff2) // it seems that Gitea doesn't need the line wrapper of Chroma, so do not add them back // if the line wrappers are still needed in the future, it can be added back by "diffToHTML(hcd.lineWrapperTags. ...)" diffHTML := diffToHTML(nil, diffRecord, diffLine.Type) return DiffInlineWithUnicodeEscape(template.HTML(diffHTML), locale) } +// GetComputedInlineDiffFor computes inline diff for the given line. +func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, locale translation.Locale) DiffInline { + if diffSection.file == nil { + return diffSection.getComputedInlineDiffForHunk(diffLine, locale) + } + if diffSection.file.shouldHighlightEntireFile { + return diffSection.getComputedInlineDiffForFullFile(diffLine, locale) + } + return diffSection.getComputedInlineDiffForHunk(diffLine, locale) +} + // DiffFile represents a file diff. type DiffFile struct { Name string @@ -367,8 +433,9 @@ type DiffFile struct { IsSubmodule bool // if IsSubmodule==true, then there must be a SubmoduleDiffInfo SubmoduleDiffInfo *SubmoduleDiffInfo - highlightedOldLines []string - highlightedNewLines []string + shouldHighlightEntireFile bool + highlightedOldLines []template.HTML + highlightedNewLines []template.HTML } // GetType returns type of diff file. @@ -433,6 +500,46 @@ func (diffFile *DiffFile) ModeTranslationKey(mode string) string { } } +func (diffFile *DiffFile) getLineCount(commit *git.Commit) (int, error) { + blob, err := commit.GetBlobByPath(diffFile.GetDiffFileName()) + if err != nil { + return 0, err + } + return blob.GetBlobLineCount() +} + +func (diffFile *DiffFile) determineHighlightStrategy(commit, beforeCommit *git.Commit) { + diffFile.shouldHighlightEntireFile = true + if commit == nil { + diffFile.shouldHighlightEntireFile = false + } + + currentLineCount, err := diffFile.getLineCount(commit) + if err != nil { + log.Debug("could not retrieve line count for current commit file - reverting to hunk based highlighting") + diffFile.shouldHighlightEntireFile = false + } + + beforeLineCount := 0 + if beforeCommit != nil { + // it's possible that the current commit is the first commit + // this would make the before commit nil + beforeLineCount, err = diffFile.getLineCount(beforeCommit) + if err != nil { + log.Debug("could not retrieve line count for previous commit file - reverting to hunk based highlighting") + diffFile.shouldHighlightEntireFile = false + } + } + + if beforeLineCount > int(setting.Git.MaxDiffHighlightFileSize) || currentLineCount > int(setting.Git.MaxDiffHighlightFileSize) { + diffFile.shouldHighlightEntireFile = false + } + + if diffFile.shouldHighlightEntireFile { + diffFile.highlightedOldLines, diffFile.highlightedNewLines = highlightEntireFile(commit, beforeCommit, diffFile) + } +} + func getCommitFileLineCount(commit *git.Commit, filePath string) int { blob, err := commit.GetBlobByPath(filePath) if err != nil { @@ -1222,7 +1329,7 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi } diffFile.IsGenerated = isGenerated.Value() - diffFile.highlightedOldLines, diffFile.highlightedNewLines = highlightCode(commit, beforeCommit, diffFile) + diffFile.determineHighlightStrategy(commit, beforeCommit) tailSection := diffFile.GetTailSection(gitRepo, beforeCommit, commit) if tailSection != nil { @@ -1244,9 +1351,9 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi return diff, nil } -func highlightCode(commit, beforeCommit *git.Commit, diffFile *DiffFile) ([]string, []string) { - var oldLines []string - var newLines []string +func highlightEntireFile(commit, beforeCommit *git.Commit, diffFile *DiffFile) ([]template.HTML, []template.HTML) { + var oldLines []template.HTML + var newLines []template.HTML if beforeCommit != nil { oldBlob, err := beforeCommit.GetBlobByPath(diffFile.Name) @@ -1255,7 +1362,9 @@ func highlightCode(commit, beforeCommit *git.Commit, diffFile *DiffFile) ([]stri highlightedOldContent, _ := highlight.Code(diffFile.Name, diffFile.Language, oldContent) splitLines := strings.Split(string(highlightedOldContent), "\n") - oldLines = append(oldLines, splitLines...) + for _, line := range splitLines { + oldLines = append(oldLines, template.HTML(line)) + } } } @@ -1265,7 +1374,9 @@ func highlightCode(commit, beforeCommit *git.Commit, diffFile *DiffFile) ([]stri highlightedNewContent, _ := highlight.Code(diffFile.Name, diffFile.Language, newContent) splitLines := strings.Split(string(highlightedNewContent), "\n") - newLines = append(newLines, splitLines...) + for _, line := range splitLines { + newLines = append(newLines, template.HTML(line)) + } } return oldLines, newLines diff --git a/services/gitdiff/highlightdiff.go b/services/gitdiff/highlightdiff.go index 28443452eae34..ddc471a8c4801 100644 --- a/services/gitdiff/highlightdiff.go +++ b/services/gitdiff/highlightdiff.go @@ -4,8 +4,11 @@ package gitdiff import ( + "html/template" "strings" + "code.gitea.io/gitea/modules/highlight" + "github.com/sergi/go-diff/diffmatchpatch" ) @@ -84,12 +87,31 @@ func (hcd *highlightCodeDiff) collectUsedRunes(code string) { } } -func (hcd *highlightCodeDiff) diffWithHighlight(codeA, codeB string) []diffmatchpatch.Diff { +func (hcd *highlightCodeDiff) diffWithHighlight(filename, language, codeA, codeB string) []diffmatchpatch.Diff { hcd.collectUsedRunes(codeA) hcd.collectUsedRunes(codeB) - convertedCodeA := hcd.convertToPlaceholders(codeA) - convertedCodeB := hcd.convertToPlaceholders(codeB) + highlightCodeA, _ := highlight.Code(filename, language, codeA) + highlightCodeB, _ := highlight.Code(filename, language, codeB) + + convertedCodeA := hcd.convertToPlaceholders(string(highlightCodeA)) + convertedCodeB := hcd.convertToPlaceholders(string(highlightCodeB)) + + diffs := diffMatchPatch.DiffMain(convertedCodeA, convertedCodeB, true) + diffs = diffMatchPatch.DiffCleanupEfficiency(diffs) + + for i := range diffs { + hcd.recoverOneDiff(&diffs[i]) + } + return diffs +} + +func (hcd *highlightCodeDiff) diffWithFullFileHighlight(codeA, codeB template.HTML) []diffmatchpatch.Diff { + hcd.collectUsedRunes(string(codeA)) + hcd.collectUsedRunes(string(codeB)) + + convertedCodeA := hcd.convertToPlaceholders(string(codeA)) + convertedCodeB := hcd.convertToPlaceholders(string(codeB)) diffs := diffMatchPatch.DiffMain(convertedCodeA, convertedCodeB, true) diffs = diffMatchPatch.DiffCleanupEfficiency(diffs) diff --git a/services/gitdiff/highlightdiff_test.go b/services/gitdiff/highlightdiff_test.go index 71400fed4d714..545a060e20491 100644 --- a/services/gitdiff/highlightdiff_test.go +++ b/services/gitdiff/highlightdiff_test.go @@ -5,6 +5,7 @@ package gitdiff import ( "fmt" + "strings" "testing" "github.com/sergi/go-diff/diffmatchpatch" @@ -14,17 +15,16 @@ import ( func TestDiffWithHighlight(t *testing.T) { hcd := newHighlightCodeDiff() diffs := hcd.diffWithHighlight( + "main.v", "", " run('<>')\n", " run(db)\n", ) - expected := "\t\trun('<>')\n" - + expected := ` run('<>')` output := diffToHTML(nil, diffs, DiffLineDel) assert.Equal(t, expected, output) - expected = ` run(db) -` + expected = ` run(db)` output = diffToHTML(nil, diffs, DiffLineAdd) assert.Equal(t, expected, output) @@ -33,6 +33,14 @@ func TestDiffWithHighlight(t *testing.T) { hcd.placeholderTokenMap['C'] = "" diff := diffmatchpatch.Diff{} + diff.Text = "OC" + hcd.recoverOneDiff(&diff) + assert.Equal(t, "", diff.Text) + + diff.Text = "O" + hcd.recoverOneDiff(&diff) + assert.Equal(t, "", diff.Text) + diff.Text = "C" hcd.recoverOneDiff(&diff) assert.Equal(t, "", diff.Text) @@ -41,22 +49,24 @@ func TestDiffWithHighlight(t *testing.T) { func TestDiffWithHighlightPlaceholder(t *testing.T) { hcd := newHighlightCodeDiff() diffs := hcd.diffWithHighlight( + "main.js", "", "a='\U00100000'", "a='\U0010FFFD''", ) assert.Equal(t, "", hcd.placeholderTokenMap[0x00100000]) assert.Equal(t, "", hcd.placeholderTokenMap[0x0010FFFD]) - expected := fmt.Sprintf(`a='%s'`, "\U00100000") + expected := fmt.Sprintf(`a='%s'`, "\U00100000") output := diffToHTML(hcd.lineWrapperTags, diffs, DiffLineDel) assert.Equal(t, expected, output) hcd = newHighlightCodeDiff() diffs = hcd.diffWithHighlight( + "main.js", "", "a='\U00100000'", "a='\U0010FFFD'", ) - expected = fmt.Sprintf(`a='%s'`, "\U0010FFFD") + expected = fmt.Sprintf(`a='%s'`, "\U0010FFFD") output = diffToHTML(nil, diffs, DiffLineAdd) assert.Equal(t, expected, output) } @@ -65,24 +75,51 @@ func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) { hcd := newHighlightCodeDiff() hcd.placeholderMaxCount = 0 diffs := hcd.diffWithHighlight( + "main.js", "", "'", ``, ) output := diffToHTML(nil, diffs, DiffLineDel) - expected := `'` + expected := fmt.Sprintf(`%s#39;`, "\uFFFD") assert.Equal(t, expected, output) hcd = newHighlightCodeDiff() hcd.placeholderMaxCount = 0 diffs = hcd.diffWithHighlight( - "a this_is_not_html_at_this_point b", - "a this_is_is_still_not_html_at_this_point_its_just_a_string b", + "main.js", "", + "a < b", + "a > b", ) output = diffToHTML(nil, diffs, DiffLineDel) - expected = "a this_is_not_html_at_this_point b" + expected = fmt.Sprintf(`a %slt; b`, "\uFFFD") assert.Equal(t, expected, output) output = diffToHTML(nil, diffs, DiffLineAdd) - expected = "a this_is_is_still_not_html_at_this_point_its_just_a_string b" + expected = fmt.Sprintf(`a %sgt; b`, "\uFFFD") assert.Equal(t, expected, output) } + +func TestDiffWithHighlightTagMatch(t *testing.T) { + totalOverflow := 0 + for i := 0; i < 100; i++ { + hcd := newHighlightCodeDiff() + hcd.placeholderMaxCount = i + diffs := hcd.diffWithHighlight( + "main.js", "", + "a='1'", + "b='2'", + ) + totalOverflow += hcd.placeholderOverflowCount + + output := diffToHTML(nil, diffs, DiffLineDel) + c1 := strings.Count(output, " Date: Mon, 3 Mar 2025 10:27:55 +0800 Subject: [PATCH 15/26] temp: simplify code --- modules/git/blob.go | 20 +- modules/setting/git.go | 2 - routers/web/repo/blame.go | 2 +- services/gitdiff/gitdiff.go | 285 ++++++++----------------- services/gitdiff/highlightdiff.go | 52 ++--- services/gitdiff/highlightdiff_test.go | 59 +---- 6 files changed, 128 insertions(+), 292 deletions(-) diff --git a/modules/git/blob.go b/modules/git/blob.go index bcecb42e16ebb..b7857dbbc6129 100644 --- a/modules/git/blob.go +++ b/modules/git/blob.go @@ -7,6 +7,7 @@ package git import ( "bytes" "encoding/base64" + "errors" "io" "code.gitea.io/gitea/modules/typesniffer" @@ -34,8 +35,9 @@ func (b *Blob) GetBlobContent(limit int64) (string, error) { return string(buf), err } -// GetBlobLineCount gets line count of the blob -func (b *Blob) GetBlobLineCount() (int, error) { +// GetBlobLineCount gets line count of the blob. +// It will also try to write the content to w if it's not nil, then we could pre-fetch the content without reading it again. +func (b *Blob) GetBlobLineCount(w io.Writer) (int, error) { reader, err := b.DataAsync() if err != nil { return 0, err @@ -44,20 +46,20 @@ func (b *Blob) GetBlobLineCount() (int, error) { buf := make([]byte, 32*1024) count := 1 lineSep := []byte{'\n'} - - c, err := reader.Read(buf) - if c == 0 && err == io.EOF { - return 0, nil - } for { + c, err := reader.Read(buf) + if w != nil { + if _, err := w.Write(buf[:c]); err != nil { + return count, err + } + } count += bytes.Count(buf[:c], lineSep) switch { - case err == io.EOF: + case errors.Is(err, io.EOF): return count, nil case err != nil: return count, err } - c, err = reader.Read(buf) } } diff --git a/modules/setting/git.go b/modules/setting/git.go index c48f93f00ec90..48a4e7f30deb5 100644 --- a/modules/setting/git.go +++ b/modules/setting/git.go @@ -38,12 +38,10 @@ var Git = struct { Pull int GC int `ini:"GC"` } `ini:"git.timeout"` - MaxDiffHighlightFileSize int64 }{ DisableDiffHighlight: false, MaxGitDiffLines: 1000, MaxGitDiffLineCharacters: 5000, - MaxDiffHighlightFileSize: 5000, MaxGitDiffFiles: 100, CommitsRangeSize: 50, BranchesRangeSize: 20, diff --git a/routers/web/repo/blame.go b/routers/web/repo/blame.go index eddfcab07824e..e79029a55e4fc 100644 --- a/routers/web/repo/blame.go +++ b/routers/web/repo/blame.go @@ -97,7 +97,7 @@ func RefBlame(ctx *context.Context) { return } - ctx.Data["NumLines"], err = blob.GetBlobLineCount() + ctx.Data["NumLines"], err = blob.GetBlobLineCount(nil) if err != nil { ctx.NotFound(err) return diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 130b8d075ab9c..e93cdc5a80bb6 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -7,6 +7,7 @@ package gitdiff import ( "bufio" "bytes" + "code.gitea.io/gitea/modules/util" "context" "fmt" "html" @@ -75,12 +76,12 @@ const ( // DiffLine represents a line difference in a DiffSection. type DiffLine struct { - LeftIdx int - RightIdx int - Match int + LeftIdx int // line number, 1-based + RightIdx int // line number, 1-based + Match int // line number, 1-based Type DiffLineType Content string - Comments issues_model.CommentList + Comments issues_model.CommentList // related PR code comments SectionInfo *DiffLineSectionInfo } @@ -95,9 +96,18 @@ type DiffLineSectionInfo struct { RightHunkSize int } +// DiffHTMLOperation is the HTML version of diffmatchpatch.Diff +type DiffHTMLOperation struct { + Type diffmatchpatch.Operation + HTML template.HTML +} + // BlobExcerptChunkSize represent max lines of excerpt const BlobExcerptChunkSize = 20 +// MaxDiffHighlightEntireFileSize is the maximum file size that will be highlighted with "entire file diff" +const MaxDiffHighlightEntireFileSize = 500 * 1024 + // GetType returns the type of DiffLine. func (d *DiffLine) GetType() int { return int(d.Type) @@ -112,8 +122,9 @@ func (d *DiffLine) GetHTMLDiffLineType() string { return "del" case DiffLineSection: return "tag" + default: + return "same" } - return "same" } // CanComment returns whether a line can get commented @@ -202,7 +213,7 @@ var ( codeTagSuffix = []byte(``) ) -func diffToHTML(lineWrapperTags []string, diffs []diffmatchpatch.Diff, lineType DiffLineType) string { +func diffToHTML(lineWrapperTags []string, diffs []DiffHTMLOperation, lineType DiffLineType) template.HTML { buf := bytes.NewBuffer(nil) // restore the line wrapper tags and , if necessary for _, tag := range lineWrapperTags { @@ -211,21 +222,21 @@ func diffToHTML(lineWrapperTags []string, diffs []diffmatchpatch.Diff, lineType for _, diff := range diffs { switch { case diff.Type == diffmatchpatch.DiffEqual: - buf.WriteString(diff.Text) + buf.WriteString(string(diff.HTML)) case diff.Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd: buf.Write(addedCodePrefix) - buf.WriteString(diff.Text) + buf.WriteString(string(diff.HTML)) buf.Write(codeTagSuffix) case diff.Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel: buf.Write(removedCodePrefix) - buf.WriteString(diff.Text) + buf.WriteString(string(diff.HTML)) buf.Write(codeTagSuffix) } } for range lineWrapperTags { buf.WriteString("") } - return buf.String() + return template.HTML(buf.String()) } // GetLine gets a specific line by type (add or del) and file line number @@ -271,10 +282,10 @@ LOOP: return nil } -var diffMatchPatch = diffmatchpatch.New() - -func init() { - diffMatchPatch.DiffEditCost = 100 +func defaultDiffMatchPatch() *diffmatchpatch.DiffMatchPatch { + dmp := diffmatchpatch.New() + dmp.DiffEditCost = 100 + return dmp } // DiffInline is a struct that has a content and escape status @@ -289,119 +300,48 @@ func DiffInlineWithUnicodeEscape(s template.HTML, locale translation.Locale) Dif return DiffInline{EscapeStatus: status, Content: content} } -// DiffInlineWithHighlightCode makes a DiffInline with code highlight and hidden unicode characters escaped -func DiffInlineWithHighlightCode(fileName, language, code string, locale translation.Locale) DiffInline { - highlighted, _ := highlight.Code(fileName, language, code) - status, content := charset.EscapeControlHTML(highlighted, locale) - return DiffInline{EscapeStatus: status, Content: content} -} - -func (diffSection *DiffSection) getComputedInlineDiffForHunk(diffLine *DiffLine, locale translation.Locale) DiffInline { - if setting.Git.DisableDiffHighlight { - return getLineContent(diffLine.Content[1:], locale) - } - - var ( - compareDiffLine *DiffLine - diff1 string - diff2 string - ) - - language := "" - if diffSection.file != nil { - language = diffSection.file.Language - } - - // try to find equivalent diff line. ignore, otherwise - switch diffLine.Type { - case DiffLineSection: - return getLineContent(diffLine.Content[1:], locale) - case DiffLineAdd: - compareDiffLine = diffSection.GetLine(DiffLineDel, diffLine.RightIdx) - if compareDiffLine == nil { - return DiffInlineWithHighlightCode(diffSection.FileName, language, diffLine.Content[1:], locale) - } - diff1 = compareDiffLine.Content - diff2 = diffLine.Content - case DiffLineDel: - compareDiffLine = diffSection.GetLine(DiffLineAdd, diffLine.LeftIdx) - if compareDiffLine == nil { - return DiffInlineWithHighlightCode(diffSection.FileName, language, diffLine.Content[1:], locale) - } - diff1 = diffLine.Content - diff2 = compareDiffLine.Content - default: - if strings.IndexByte(" +-", diffLine.Content[0]) > -1 { - return DiffInlineWithHighlightCode(diffSection.FileName, language, diffLine.Content[1:], locale) - } - return DiffInlineWithHighlightCode(diffSection.FileName, language, diffLine.Content, locale) - } - +func (diffSection *DiffSection) getDiffLineForRender(diffLineType DiffLineType, leftLine, rightLine *DiffLine, locale translation.Locale) DiffInline { hcd := newHighlightCodeDiff() - diffRecord := hcd.diffWithHighlight(diffSection.FileName, language, diff1[1:], diff2[1:]) - // it seems that Gitea doesn't need the line wrapper of Chroma, so do not add them back - // if the line wrappers are still needed in the future, it can be added back by "diffToHTML(hcd.lineWrapperTags. ...)" - diffHTML := diffToHTML(nil, diffRecord, diffLine.Type) - return DiffInlineWithUnicodeEscape(template.HTML(diffHTML), locale) + var diff1, diff2, lineHTML template.HTML + if leftLine != nil { + diff1 = diffSection.file.highlightedOldLines[leftLine.LeftIdx-1] + lineHTML = util.Iif(diffLineType == DiffLinePlain, diff1, "") + } + if rightLine != nil { + diff2 = diffSection.file.highlightedNewLines[rightLine.RightIdx-1] + lineHTML = util.Iif(diffLineType == DiffLinePlain, diff2, "") + } + if diffLineType != DiffLinePlain { + diffRecord := hcd.diffWithHighlight(diff1, diff2) + // it seems that Gitea doesn't need the line wrapper of Chroma, so do not add them back + // if the line wrappers are still needed in the future, it can be added back by "diffToHTML(hcd.lineWrapperTags. ...)" + lineHTML = diffToHTML(nil, diffRecord, diffLineType) + } + return DiffInlineWithUnicodeEscape(lineHTML, locale) } -func (diffSection *DiffSection) getComputedInlineDiffForFullFile(diffLine *DiffLine, locale translation.Locale) DiffInline { +// GetComputedInlineDiffFor computes inline diff for the given line. +func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, locale translation.Locale) DiffInline { + // FIXME: should check DisableDiffHighlight in other places, but not here if setting.Git.DisableDiffHighlight { return getLineContent(diffLine.Content[1:], locale) } - var ( - compareDiffLine *DiffLine - diff1 template.HTML - diff2 template.HTML - ) - // try to find equivalent diff line. ignore, otherwise switch diffLine.Type { case DiffLineSection: return getLineContent(diffLine.Content[1:], locale) case DiffLineAdd: - compareDiffLine = diffSection.GetLine(DiffLineDel, diffLine.RightIdx) - if compareDiffLine == nil { - highlightedLine := diffSection.file.highlightedNewLines[diffLine.RightIdx-1] - return DiffInlineWithUnicodeEscape(highlightedLine, locale) - } - diff1 = diffSection.file.highlightedOldLines[compareDiffLine.LeftIdx-1] - diff2 = diffSection.file.highlightedNewLines[diffLine.RightIdx-1] + compareDiffLine := diffSection.GetLine(DiffLineDel, diffLine.RightIdx) + return diffSection.getDiffLineForRender(DiffLineAdd, compareDiffLine, diffLine, locale) case DiffLineDel: - compareDiffLine = diffSection.GetLine(DiffLineAdd, diffLine.LeftIdx) - if compareDiffLine == nil { - highlightedLine := diffSection.file.highlightedOldLines[diffLine.LeftIdx-1] - return DiffInlineWithUnicodeEscape(highlightedLine, locale) - } - diff1 = diffSection.file.highlightedOldLines[diffLine.LeftIdx-1] - diff2 = diffSection.file.highlightedNewLines[compareDiffLine.RightIdx-1] - default: - if strings.IndexByte(" +-", diffLine.Content[0]) > -1 { - highlightedContent := diffSection.file.highlightedNewLines[diffLine.RightIdx-1] - return DiffInlineWithUnicodeEscape(highlightedContent, locale) - } - highlightedContent := diffSection.file.highlightedOldLines[diffLine.LeftIdx-1] - return DiffInlineWithUnicodeEscape(highlightedContent, locale) + compareDiffLine := diffSection.GetLine(DiffLineAdd, diffLine.LeftIdx) + return diffSection.getDiffLineForRender(DiffLineDel, diffLine, compareDiffLine, locale) + default: // Plain + // TODO: there was an "if" check: `if diffLine.Content >strings.IndexByte(" +-", diffLine.Content[0]) > -1 { ... } else { ... }` + // no idea why it needs that check, it seems that the "if" should be always true, so try to simplify the code + return diffSection.getDiffLineForRender(DiffLinePlain, nil, diffLine, locale) } - - hcd := newHighlightCodeDiff() - diffRecord := hcd.diffWithFullFileHighlight(diff1, diff2) - // it seems that Gitea doesn't need the line wrapper of Chroma, so do not add them back - // if the line wrappers are still needed in the future, it can be added back by "diffToHTML(hcd.lineWrapperTags. ...)" - diffHTML := diffToHTML(nil, diffRecord, diffLine.Type) - return DiffInlineWithUnicodeEscape(template.HTML(diffHTML), locale) -} - -// GetComputedInlineDiffFor computes inline diff for the given line. -func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, locale translation.Locale) DiffInline { - if diffSection.file == nil { - return diffSection.getComputedInlineDiffForHunk(diffLine, locale) - } - if diffSection.file.shouldHighlightEntireFile { - return diffSection.getComputedInlineDiffForFullFile(diffLine, locale) - } - return diffSection.getComputedInlineDiffForHunk(diffLine, locale) } // DiffFile represents a file diff. @@ -433,9 +373,8 @@ type DiffFile struct { IsSubmodule bool // if IsSubmodule==true, then there must be a SubmoduleDiffInfo SubmoduleDiffInfo *SubmoduleDiffInfo - shouldHighlightEntireFile bool - highlightedOldLines []template.HTML - highlightedNewLines []template.HTML + highlightedOldLines []template.HTML + highlightedNewLines []template.HTML } // GetType returns type of diff file. @@ -443,18 +382,23 @@ func (diffFile *DiffFile) GetType() int { return int(diffFile.Type) } -// GetTailSection creates a fake DiffLineSection if the last section is not the end of the file -func (diffFile *DiffFile) GetTailSection(gitRepo *git.Repository, leftCommit, rightCommit *git.Commit) *DiffSection { +type DiffLimitedContent struct { + LeftContent, RightContent *limitByteWriter +} + +// GetTailSectionAndLimitedContent creates a fake DiffLineSection if the last section is not the end of the file +func (diffFile *DiffFile) GetTailSectionAndLimitedContent(leftCommit, rightCommit *git.Commit) (_ *DiffSection, diffLimitedContent DiffLimitedContent) { if len(diffFile.Sections) == 0 || diffFile.Type != DiffFileChange || diffFile.IsBin || diffFile.IsLFSFile { - return nil + return nil, diffLimitedContent } lastSection := diffFile.Sections[len(diffFile.Sections)-1] lastLine := lastSection.Lines[len(lastSection.Lines)-1] - leftLineCount := getCommitFileLineCount(leftCommit, diffFile.Name) - rightLineCount := getCommitFileLineCount(rightCommit, diffFile.Name) + leftLineCount, leftContent := getCommitFileLineCountAndLimitedContent(leftCommit, diffFile.Name) + rightLineCount, rightContent := getCommitFileLineCountAndLimitedContent(rightCommit, diffFile.Name) + diffLimitedContent = DiffLimitedContent{LeftContent: leftContent, RightContent: rightContent} if leftLineCount <= lastLine.LeftIdx || rightLineCount <= lastLine.RightIdx { - return nil + return nil, diffLimitedContent } tailDiffLine := &DiffLine{ Type: DiffLineSection, @@ -468,7 +412,7 @@ func (diffFile *DiffFile) GetTailSection(gitRepo *git.Repository, leftCommit, ri }, } tailSection := &DiffSection{FileName: diffFile.Name, Lines: []*DiffLine{tailDiffLine}} - return tailSection + return tailSection, diffLimitedContent } // GetDiffFileName returns the name of the diff file, or its old name in case it was deleted @@ -500,56 +444,29 @@ func (diffFile *DiffFile) ModeTranslationKey(mode string) string { } } -func (diffFile *DiffFile) getLineCount(commit *git.Commit) (int, error) { - blob, err := commit.GetBlobByPath(diffFile.GetDiffFileName()) - if err != nil { - return 0, err - } - return blob.GetBlobLineCount() +type limitByteWriter struct { + buf bytes.Buffer + limit int } -func (diffFile *DiffFile) determineHighlightStrategy(commit, beforeCommit *git.Commit) { - diffFile.shouldHighlightEntireFile = true - if commit == nil { - diffFile.shouldHighlightEntireFile = false - } - - currentLineCount, err := diffFile.getLineCount(commit) - if err != nil { - log.Debug("could not retrieve line count for current commit file - reverting to hunk based highlighting") - diffFile.shouldHighlightEntireFile = false - } - - beforeLineCount := 0 - if beforeCommit != nil { - // it's possible that the current commit is the first commit - // this would make the before commit nil - beforeLineCount, err = diffFile.getLineCount(beforeCommit) - if err != nil { - log.Debug("could not retrieve line count for previous commit file - reverting to hunk based highlighting") - diffFile.shouldHighlightEntireFile = false - } - } - - if beforeLineCount > int(setting.Git.MaxDiffHighlightFileSize) || currentLineCount > int(setting.Git.MaxDiffHighlightFileSize) { - diffFile.shouldHighlightEntireFile = false - } - - if diffFile.shouldHighlightEntireFile { - diffFile.highlightedOldLines, diffFile.highlightedNewLines = highlightEntireFile(commit, beforeCommit, diffFile) +func (l *limitByteWriter) Write(p []byte) (n int, err error) { + if l.buf.Len()+len(p) > l.limit { + p = p[:l.limit-l.buf.Len()] } + return l.buf.Write(p) } -func getCommitFileLineCount(commit *git.Commit, filePath string) int { +func getCommitFileLineCountAndLimitedContent(commit *git.Commit, filePath string) (lineCount int, limitWriter *limitByteWriter) { blob, err := commit.GetBlobByPath(filePath) if err != nil { - return 0 + return 0, nil } - lineCount, err := blob.GetBlobLineCount() + w := &limitByteWriter{limit: MaxDiffHighlightEntireFileSize + 1} + lineCount, err = blob.GetBlobLineCount(w) if err != nil { - return 0 + return 0, nil } - return lineCount + return lineCount, w } // Diff represents a difference between two git trees. @@ -1328,13 +1245,13 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi isGenerated = optional.Some(analyze.IsGenerated(diffFile.Name)) } diffFile.IsGenerated = isGenerated.Value() - - diffFile.determineHighlightStrategy(commit, beforeCommit) - - tailSection := diffFile.GetTailSection(gitRepo, beforeCommit, commit) + tailSection, limitedContent := diffFile.GetTailSectionAndLimitedContent(beforeCommit, commit) if tailSection != nil { diffFile.Sections = append(diffFile.Sections, tailSection) } + + diffFile.highlightedOldLines = highlightCodeLines(diffFile, limitedContent.LeftContent.buf.String()) + diffFile.highlightedNewLines = highlightCodeLines(diffFile, limitedContent.RightContent.buf.String()) } if opts.FileOnly { @@ -1351,35 +1268,13 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi return diff, nil } -func highlightEntireFile(commit, beforeCommit *git.Commit, diffFile *DiffFile) ([]template.HTML, []template.HTML) { - var oldLines []template.HTML - var newLines []template.HTML - - if beforeCommit != nil { - oldBlob, err := beforeCommit.GetBlobByPath(diffFile.Name) - if err == nil { - oldContent, _ := oldBlob.GetBlobContent(oldBlob.Size()) - highlightedOldContent, _ := highlight.Code(diffFile.Name, diffFile.Language, oldContent) - - splitLines := strings.Split(string(highlightedOldContent), "\n") - for _, line := range splitLines { - oldLines = append(oldLines, template.HTML(line)) - } - } +func highlightCodeLines(diffFile *DiffFile, content string) (lines []template.HTML) { + highlightedNewContent, _ := highlight.Code(diffFile.Name, diffFile.Language, content) + splitLines := strings.Split(string(highlightedNewContent), "\n") + for _, line := range splitLines { + lines = append(lines, template.HTML(line)) } - - newBlob, err := commit.GetBlobByPath(diffFile.Name) - if err == nil { - newContent, _ := newBlob.GetBlobContent(newBlob.Size()) - highlightedNewContent, _ := highlight.Code(diffFile.Name, diffFile.Language, newContent) - - splitLines := strings.Split(string(highlightedNewContent), "\n") - for _, line := range splitLines { - newLines = append(newLines, template.HTML(line)) - } - } - - return oldLines, newLines + return lines } type PullDiffStats struct { diff --git a/services/gitdiff/highlightdiff.go b/services/gitdiff/highlightdiff.go index ddc471a8c4801..dcb6f34af0f9a 100644 --- a/services/gitdiff/highlightdiff.go +++ b/services/gitdiff/highlightdiff.go @@ -6,10 +6,6 @@ package gitdiff import ( "html/template" "strings" - - "code.gitea.io/gitea/modules/highlight" - - "github.com/sergi/go-diff/diffmatchpatch" ) // token is a html tag or entity, eg: "", "", "<" @@ -78,7 +74,7 @@ func (hcd *highlightCodeDiff) isInPlaceholderRange(r rune) bool { return hcd.placeholderBegin <= r && r < hcd.placeholderBegin+rune(hcd.placeholderMaxCount) } -func (hcd *highlightCodeDiff) collectUsedRunes(code string) { +func (hcd *highlightCodeDiff) collectUsedRunes(code template.HTML) { for _, r := range code { if hcd.isInPlaceholderRange(r) { // put the existing rune (used by code) in map, then this rune won't be used a placeholder anymore. @@ -87,43 +83,25 @@ func (hcd *highlightCodeDiff) collectUsedRunes(code string) { } } -func (hcd *highlightCodeDiff) diffWithHighlight(filename, language, codeA, codeB string) []diffmatchpatch.Diff { +func (hcd *highlightCodeDiff) diffWithHighlight(codeA, codeB template.HTML) (res []DiffHTMLOperation) { hcd.collectUsedRunes(codeA) hcd.collectUsedRunes(codeB) - highlightCodeA, _ := highlight.Code(filename, language, codeA) - highlightCodeB, _ := highlight.Code(filename, language, codeB) - - convertedCodeA := hcd.convertToPlaceholders(string(highlightCodeA)) - convertedCodeB := hcd.convertToPlaceholders(string(highlightCodeB)) - - diffs := diffMatchPatch.DiffMain(convertedCodeA, convertedCodeB, true) - diffs = diffMatchPatch.DiffCleanupEfficiency(diffs) - - for i := range diffs { - hcd.recoverOneDiff(&diffs[i]) - } - return diffs -} - -func (hcd *highlightCodeDiff) diffWithFullFileHighlight(codeA, codeB template.HTML) []diffmatchpatch.Diff { - hcd.collectUsedRunes(string(codeA)) - hcd.collectUsedRunes(string(codeB)) + convertedCodeA := hcd.convertToPlaceholders(codeA) + convertedCodeB := hcd.convertToPlaceholders(codeB) - convertedCodeA := hcd.convertToPlaceholders(string(codeA)) - convertedCodeB := hcd.convertToPlaceholders(string(codeB)) + dmp := defaultDiffMatchPatch() + diffs := dmp.DiffMain(convertedCodeA, convertedCodeB, true) + diffs = dmp.DiffCleanupEfficiency(diffs) - diffs := diffMatchPatch.DiffMain(convertedCodeA, convertedCodeB, true) - diffs = diffMatchPatch.DiffCleanupEfficiency(diffs) - - for i := range diffs { - hcd.recoverOneDiff(&diffs[i]) + for _, d := range diffs { + res = append(res, DiffHTMLOperation{Type: d.Type, HTML: hcd.recoverOneDiff(d.Text)}) } - return diffs + return res } // convertToPlaceholders totally depends on Chroma's valid HTML output and its structure, do not use these functions for other purposes. -func (hcd *highlightCodeDiff) convertToPlaceholders(htmlCode string) string { +func (hcd *highlightCodeDiff) convertToPlaceholders(htmlContent template.HTML) string { var tagStack []string res := strings.Builder{} @@ -132,6 +110,7 @@ func (hcd *highlightCodeDiff) convertToPlaceholders(htmlCode string) string { var beforeToken, token string var valid bool + htmlCode := string(htmlContent) // the standard chroma highlight HTML is " ... " for { beforeToken, token, htmlCode, valid = extractHTMLToken(htmlCode) @@ -196,11 +175,11 @@ func (hcd *highlightCodeDiff) convertToPlaceholders(htmlCode string) string { return res.String() } -func (hcd *highlightCodeDiff) recoverOneDiff(diff *diffmatchpatch.Diff) { +func (hcd *highlightCodeDiff) recoverOneDiff(str string) template.HTML { sb := strings.Builder{} var tagStack []string - for _, r := range diff.Text { + for _, r := range str { token, ok := hcd.placeholderTokenMap[r] if !ok || token == "" { sb.WriteRune(r) // if the rune is not a placeholder, write it as it is @@ -234,6 +213,5 @@ func (hcd *highlightCodeDiff) recoverOneDiff(diff *diffmatchpatch.Diff) { } // else: impossible. every tag was pushed into the stack by the code above and is valid HTML opening tag } } - - diff.Text = sb.String() + return template.HTML(sb.String()) } diff --git a/services/gitdiff/highlightdiff_test.go b/services/gitdiff/highlightdiff_test.go index 545a060e20491..71400fed4d714 100644 --- a/services/gitdiff/highlightdiff_test.go +++ b/services/gitdiff/highlightdiff_test.go @@ -5,7 +5,6 @@ package gitdiff import ( "fmt" - "strings" "testing" "github.com/sergi/go-diff/diffmatchpatch" @@ -15,16 +14,17 @@ import ( func TestDiffWithHighlight(t *testing.T) { hcd := newHighlightCodeDiff() diffs := hcd.diffWithHighlight( - "main.v", "", " run('<>')\n", " run(db)\n", ) - expected := ` run('<>')` + expected := "\t\trun('<>')\n" + output := diffToHTML(nil, diffs, DiffLineDel) assert.Equal(t, expected, output) - expected = ` run(db)` + expected = ` run(db) +` output = diffToHTML(nil, diffs, DiffLineAdd) assert.Equal(t, expected, output) @@ -33,14 +33,6 @@ func TestDiffWithHighlight(t *testing.T) { hcd.placeholderTokenMap['C'] = "" diff := diffmatchpatch.Diff{} - diff.Text = "OC" - hcd.recoverOneDiff(&diff) - assert.Equal(t, "", diff.Text) - - diff.Text = "O" - hcd.recoverOneDiff(&diff) - assert.Equal(t, "", diff.Text) - diff.Text = "C" hcd.recoverOneDiff(&diff) assert.Equal(t, "", diff.Text) @@ -49,24 +41,22 @@ func TestDiffWithHighlight(t *testing.T) { func TestDiffWithHighlightPlaceholder(t *testing.T) { hcd := newHighlightCodeDiff() diffs := hcd.diffWithHighlight( - "main.js", "", "a='\U00100000'", "a='\U0010FFFD''", ) assert.Equal(t, "", hcd.placeholderTokenMap[0x00100000]) assert.Equal(t, "", hcd.placeholderTokenMap[0x0010FFFD]) - expected := fmt.Sprintf(`a='%s'`, "\U00100000") + expected := fmt.Sprintf(`a='%s'`, "\U00100000") output := diffToHTML(hcd.lineWrapperTags, diffs, DiffLineDel) assert.Equal(t, expected, output) hcd = newHighlightCodeDiff() diffs = hcd.diffWithHighlight( - "main.js", "", "a='\U00100000'", "a='\U0010FFFD'", ) - expected = fmt.Sprintf(`a='%s'`, "\U0010FFFD") + expected = fmt.Sprintf(`a='%s'`, "\U0010FFFD") output = diffToHTML(nil, diffs, DiffLineAdd) assert.Equal(t, expected, output) } @@ -75,51 +65,24 @@ func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) { hcd := newHighlightCodeDiff() hcd.placeholderMaxCount = 0 diffs := hcd.diffWithHighlight( - "main.js", "", "'", ``, ) output := diffToHTML(nil, diffs, DiffLineDel) - expected := fmt.Sprintf(`%s#39;`, "\uFFFD") + expected := `'` assert.Equal(t, expected, output) hcd = newHighlightCodeDiff() hcd.placeholderMaxCount = 0 diffs = hcd.diffWithHighlight( - "main.js", "", - "a < b", - "a > b", + "a this_is_not_html_at_this_point b", + "a this_is_is_still_not_html_at_this_point_its_just_a_string b", ) output = diffToHTML(nil, diffs, DiffLineDel) - expected = fmt.Sprintf(`a %slt; b`, "\uFFFD") + expected = "a this_is_not_html_at_this_point b" assert.Equal(t, expected, output) output = diffToHTML(nil, diffs, DiffLineAdd) - expected = fmt.Sprintf(`a %sgt; b`, "\uFFFD") + expected = "a this_is_is_still_not_html_at_this_point_its_just_a_string b" assert.Equal(t, expected, output) } - -func TestDiffWithHighlightTagMatch(t *testing.T) { - totalOverflow := 0 - for i := 0; i < 100; i++ { - hcd := newHighlightCodeDiff() - hcd.placeholderMaxCount = i - diffs := hcd.diffWithHighlight( - "main.js", "", - "a='1'", - "b='2'", - ) - totalOverflow += hcd.placeholderOverflowCount - - output := diffToHTML(nil, diffs, DiffLineDel) - c1 := strings.Count(output, " Date: Mon, 3 Mar 2025 10:49:47 +0800 Subject: [PATCH 16/26] temp: add highlight fallback and FIXME comment --- services/gitdiff/gitdiff.go | 50 ++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index e93cdc5a80bb6..8a99397c028d6 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -294,21 +294,37 @@ type DiffInline struct { Content template.HTML } -// DiffInlineWithUnicodeEscape makes a DiffInline with hidden unicode characters escaped +// DiffInlineWithUnicodeEscape makes a DiffInline with hidden Unicode characters escaped func DiffInlineWithUnicodeEscape(s template.HTML, locale translation.Locale) DiffInline { status, content := charset.EscapeControlHTML(s, locale) return DiffInline{EscapeStatus: status, Content: content} } +func (diffSection *DiffSection) getLineContentForRender(lineIdx int, diffLine *DiffLine, highlightLines map[int]template.HTML) template.HTML { + h, ok := highlightLines[lineIdx-1] + if ok { + return h + } + if diffLine.Content == "" { + return "" + } + if setting.Git.DisableDiffHighlight { + return template.HTML(html.EscapeString(diffLine.Content[1:])) + } + + h, _ = highlight.Code(diffSection.Name, diffSection.file.Language, diffLine.Content[1:]) + return h +} + func (diffSection *DiffSection) getDiffLineForRender(diffLineType DiffLineType, leftLine, rightLine *DiffLine, locale translation.Locale) DiffInline { hcd := newHighlightCodeDiff() var diff1, diff2, lineHTML template.HTML if leftLine != nil { - diff1 = diffSection.file.highlightedOldLines[leftLine.LeftIdx-1] + diff1 = diffSection.getLineContentForRender(leftLine.LeftIdx, leftLine, diffSection.file.highlightedOldLines) lineHTML = util.Iif(diffLineType == DiffLinePlain, diff1, "") } if rightLine != nil { - diff2 = diffSection.file.highlightedNewLines[rightLine.RightIdx-1] + diff2 = diffSection.getLineContentForRender(rightLine.RightIdx, rightLine, diffSection.file.highlightedNewLines) lineHTML = util.Iif(diffLineType == DiffLinePlain, diff2, "") } if diffLineType != DiffLinePlain { @@ -322,11 +338,6 @@ func (diffSection *DiffSection) getDiffLineForRender(diffLineType DiffLineType, // GetComputedInlineDiffFor computes inline diff for the given line. func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, locale translation.Locale) DiffInline { - // FIXME: should check DisableDiffHighlight in other places, but not here - if setting.Git.DisableDiffHighlight { - return getLineContent(diffLine.Content[1:], locale) - } - // try to find equivalent diff line. ignore, otherwise switch diffLine.Type { case DiffLineSection: @@ -373,8 +384,8 @@ type DiffFile struct { IsSubmodule bool // if IsSubmodule==true, then there must be a SubmoduleDiffInfo SubmoduleDiffInfo *SubmoduleDiffInfo - highlightedOldLines []template.HTML - highlightedNewLines []template.HTML + highlightedOldLines map[int]template.HTML // TODO: in the future, we only need to store the related diff lines to save memory + highlightedNewLines map[int]template.HTML } // GetType returns type of diff file. @@ -1250,8 +1261,16 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi diffFile.Sections = append(diffFile.Sections, tailSection) } - diffFile.highlightedOldLines = highlightCodeLines(diffFile, limitedContent.LeftContent.buf.String()) - diffFile.highlightedNewLines = highlightCodeLines(diffFile, limitedContent.RightContent.buf.String()) + if !setting.Git.DisableDiffHighlight { + // FIXME: it's not right to highlight code here for all cases, because this function is also used for non-rendering purpose + // For example: API + if limitedContent.LeftContent != nil && limitedContent.LeftContent.buf.Len() < MaxDiffHighlightEntireFileSize { + diffFile.highlightedOldLines = highlightCodeLines(diffFile, limitedContent.LeftContent.buf.String()) + } + if limitedContent.RightContent != nil && limitedContent.RightContent.buf.Len() < MaxDiffHighlightEntireFileSize { + diffFile.highlightedNewLines = highlightCodeLines(diffFile, limitedContent.RightContent.buf.String()) + } + } } if opts.FileOnly { @@ -1268,11 +1287,12 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi return diff, nil } -func highlightCodeLines(diffFile *DiffFile, content string) (lines []template.HTML) { +func highlightCodeLines(diffFile *DiffFile, content string) map[int]template.HTML { highlightedNewContent, _ := highlight.Code(diffFile.Name, diffFile.Language, content) splitLines := strings.Split(string(highlightedNewContent), "\n") - for _, line := range splitLines { - lines = append(lines, template.HTML(line)) + lines := make(map[int]template.HTML, len(splitLines)) + for i, line := range splitLines { + lines[i] = template.HTML(line) } return lines } From 1e6c8ddc8042bb71bc873c3597f27a9f409acaf6 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 6 Mar 2025 01:35:24 +0800 Subject: [PATCH 17/26] add FIXME for abused GitDiff --- routers/api/v1/repo/pull.go | 2 ++ services/convert/git_commit.go | 1 + 2 files changed, 3 insertions(+) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 412f2cfb58260..640b4a62b27c2 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1591,6 +1591,8 @@ func GetPullRequestFiles(ctx *context.APIContext) { maxLines := setting.Git.MaxGitDiffLines // FIXME: If there are too many files in the repo, may cause some unpredictable issues. + // FIXME: when using "skip-to", the NumFiles (totalNumberOfFiles, totalNumberOfPages) will be wrong + // FIXME: it doesn't need to call "GetDiff" to do various parsing and highlighting diff, err := gitdiff.GetDiff(ctx, baseGitRepo, &gitdiff.DiffOptions{ BeforeCommitID: startCommitID, diff --git a/services/convert/git_commit.go b/services/convert/git_commit.go index e0efcddbcb0c7..ee95c90d53af3 100644 --- a/services/convert/git_commit.go +++ b/services/convert/git_commit.go @@ -210,6 +210,7 @@ func ToCommit(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Rep // Get diff stats for commit if opts.Stat { + // FIXME: it should not call GitDiff, here it only needs "--shortdiff" diff, err := gitdiff.GetDiff(ctx, gitRepo, &gitdiff.DiffOptions{ AfterCommitID: commit.ID.String(), }) From 493e32b7aee365608e996f9a9faa42d206b272e5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 8 Mar 2025 17:42:46 +0800 Subject: [PATCH 18/26] fix merge --- services/gitdiff/gitdiff.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 790f6db6b3c96..38aeb10e6f7cf 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -7,7 +7,6 @@ package gitdiff import ( "bufio" "bytes" - "code.gitea.io/gitea/modules/util" "context" "fmt" "html" @@ -32,6 +31,7 @@ import ( "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/translation" + "code.gitea.io/gitea/modules/util" "github.com/sergi/go-diff/diffmatchpatch" stdcharset "golang.org/x/net/html/charset" @@ -1247,7 +1247,7 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi isGenerated = optional.Some(analyze.IsGenerated(diffFile.Name)) } diffFile.IsGenerated = isGenerated.Value() - tailSection, limitedContent := diffFile.GetTailSectionAndLimitedContent(beforeCommit, commit) + tailSection, limitedContent := diffFile.GetTailSectionAndLimitedContent(actualBeforeCommit, afterCommit) if tailSection != nil { diffFile.Sections = append(diffFile.Sections, tailSection) } From abc63e981b31fa2292bad6dca38bfd24926bea8f Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 8 Mar 2025 18:02:23 +0800 Subject: [PATCH 19/26] fix DiffFile struct --- services/gitdiff/gitdiff.go | 83 ++++++++++++++++------------- templates/repo/diff/image_diff.tmpl | 12 ++--- 2 files changed, 52 insertions(+), 43 deletions(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 38aeb10e6f7cf..4d6d102e4997c 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -357,34 +357,44 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc // DiffFile represents a file diff. type DiffFile struct { - Name string - NameHash string - OldName string - Index int - Addition, Deletion int - Type DiffFileType - IsCreated bool - IsDeleted bool - IsBin bool - IsLFSFile bool - IsRenamed bool - IsAmbiguous bool - Sections []*DiffSection - IsIncomplete bool - IsIncompleteLineTooLong bool - IsProtected bool - IsGenerated bool - IsVendored bool + // only used internally to parse Ambiguous filenames + isAmbiguous bool + + // basic fields (parsed from diff result) + Name string + NameHash string + OldName string + Addition int + Deletion int + Type DiffFileType + Mode string + OldMode string + IsCreated bool + IsDeleted bool + IsBin bool + IsLFSFile bool + IsRenamed bool + IsSubmodule bool + // basic fields but for render purpose only + Sections []*DiffSection + IsIncomplete bool + IsIncompleteLineTooLong bool + + // will be filled by the extra loop in GitDiff + Language string + IsGenerated bool + IsVendored bool + SubmoduleDiffInfo *SubmoduleDiffInfo // IsSubmodule==true, then there must be a SubmoduleDiffInfo + + // will be filled by route handler + IsProtected bool + + // will be filled by SyncUserSpecificDiff IsViewed bool // User specific HasChangedSinceLastReview bool // User specific - Language string - Mode string - OldMode string - IsSubmodule bool // if IsSubmodule==true, then there must be a SubmoduleDiffInfo - SubmoduleDiffInfo *SubmoduleDiffInfo - - highlightedOldLines map[int]template.HTML // TODO: in the future, we only need to store the related diff lines to save memory + // for render purpose only, TODO: in the future, we only need to store the related diff lines to save memory + highlightedOldLines map[int]template.HTML highlightedNewLines map[int]template.HTML } @@ -645,28 +655,28 @@ parsingLoop: case strings.HasPrefix(line, "rename from "): curFile.IsRenamed = true curFile.Type = DiffFileRename - if curFile.IsAmbiguous { + if curFile.isAmbiguous { curFile.OldName = prepareValue(line, "rename from ") } case strings.HasPrefix(line, "rename to "): curFile.IsRenamed = true curFile.Type = DiffFileRename - if curFile.IsAmbiguous { + if curFile.isAmbiguous { curFile.Name = prepareValue(line, "rename to ") - curFile.IsAmbiguous = false + curFile.isAmbiguous = false } case strings.HasPrefix(line, "copy from "): curFile.IsRenamed = true curFile.Type = DiffFileCopy - if curFile.IsAmbiguous { + if curFile.isAmbiguous { curFile.OldName = prepareValue(line, "copy from ") } case strings.HasPrefix(line, "copy to "): curFile.IsRenamed = true curFile.Type = DiffFileCopy - if curFile.IsAmbiguous { + if curFile.isAmbiguous { curFile.Name = prepareValue(line, "copy to ") - curFile.IsAmbiguous = false + curFile.isAmbiguous = false } case strings.HasPrefix(line, "new file"): curFile.Type = DiffFileAdd @@ -693,7 +703,7 @@ parsingLoop: curFile.IsBin = true case strings.HasPrefix(line, "--- "): // Handle ambiguous filenames - if curFile.IsAmbiguous { + if curFile.isAmbiguous { // The shortest string that can end up here is: // "--- a\t\n" without the quotes. // This line has a len() of 7 but doesn't contain a oldName. @@ -711,7 +721,7 @@ parsingLoop: // Otherwise do nothing with this line case strings.HasPrefix(line, "+++ "): // Handle ambiguous filenames - if curFile.IsAmbiguous { + if curFile.isAmbiguous { if len(line) > 6 && line[4] == 'b' { curFile.Name = line[6 : len(line)-1] if line[len(line)-2] == '\t' { @@ -723,7 +733,7 @@ parsingLoop: } else { curFile.Name = curFile.OldName } - curFile.IsAmbiguous = false + curFile.isAmbiguous = false } // Otherwise do nothing with this line, but now switch to parsing hunks lineBytes, isFragment, err := parseHunks(ctx, curFile, maxLines, maxLineCharacters, input) @@ -1047,12 +1057,11 @@ func createDiffFile(diff *Diff, line string) *DiffFile { // // Path names are quoted if necessary. // - // This means that you should always be able to determine the file name even when there + // This means that you should always be able to determine the file name even when // there is potential ambiguity... // // but we can be simpler with our heuristics by just forcing git to prefix things nicely curFile := &DiffFile{ - Index: len(diff.Files) + 1, Type: DiffFileChange, Sections: make([]*DiffSection, 0, 10), } @@ -1064,7 +1073,7 @@ func createDiffFile(diff *Diff, line string) *DiffFile { curFile.OldName, oldNameAmbiguity = readFileName(rd) curFile.Name, newNameAmbiguity = readFileName(rd) if oldNameAmbiguity && newNameAmbiguity { - curFile.IsAmbiguous = true + curFile.isAmbiguous = true // OK we should bet that the oldName and the newName are the same if they can be made to be same // So we need to start again ... if (len(line)-len(cmdDiffHead)-1)%2 == 0 { diff --git a/templates/repo/diff/image_diff.tmpl b/templates/repo/diff/image_diff.tmpl index 608174e51b1d0..bbd8d4a2ecea3 100644 --- a/templates/repo/diff/image_diff.tmpl +++ b/templates/repo/diff/image_diff.tmpl @@ -9,15 +9,15 @@ >
- {{ctx.Locale.Tr "repo.diff.image.side_by_side"}} + {{ctx.Locale.Tr "repo.diff.image.side_by_side"}} {{if and .blobBase .blobHead}} - {{ctx.Locale.Tr "repo.diff.image.swipe"}} - {{ctx.Locale.Tr "repo.diff.image.overlay"}} + {{ctx.Locale.Tr "repo.diff.image.swipe"}} + {{ctx.Locale.Tr "repo.diff.image.overlay"}} {{end}}
-
+
{{if .blobBase}} @@ -52,7 +52,7 @@
{{if and .blobBase .blobHead}} -
+
@@ -66,7 +66,7 @@
-
+
From 7de148a63cb87ae709b668a660545ab682773fe9 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 8 Mar 2025 18:59:35 +0800 Subject: [PATCH 20/26] split GetDiff --- routers/api/v1/repo/pull.go | 2 +- routers/web/repo/commit.go | 2 +- routers/web/repo/compare.go | 2 +- routers/web/repo/pull.go | 2 +- services/gitdiff/gitdiff.go | 43 ++++++++++++++++++++++++------------- 5 files changed, 32 insertions(+), 19 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 3698ff601045a..103ee88ede6d7 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1592,7 +1592,7 @@ func GetPullRequestFiles(ctx *context.APIContext) { // FIXME: If there are too many files in the repo, may cause some unpredictable issues. // FIXME: it doesn't need to call "GetDiff" to do various parsing and highlighting - diff, err := gitdiff.GetDiff(ctx, baseGitRepo, + diff, err := gitdiff.GetDiffForAPI(ctx, baseGitRepo, &gitdiff.DiffOptions{ BeforeCommitID: startCommitID, AfterCommitID: endCommitID, diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 997e9f68696e4..bbdcf9875eee8 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -314,7 +314,7 @@ func Diff(ctx *context.Context) { maxLines, maxFiles = -1, -1 } - diff, err := gitdiff.GetDiff(ctx, gitRepo, &gitdiff.DiffOptions{ + diff, err := gitdiff.GetDiffForRender(ctx, gitRepo, &gitdiff.DiffOptions{ AfterCommitID: commitID, SkipTo: ctx.FormString("skip-to"), MaxLines: maxLines, diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index d9c6305ce17df..c1726d0790e61 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -614,7 +614,7 @@ func PrepareCompareDiff( fileOnly := ctx.FormBool("file-only") - diff, err := gitdiff.GetDiff(ctx, ci.HeadGitRepo, + diff, err := gitdiff.GetDiffForRender(ctx, ci.HeadGitRepo, &gitdiff.DiffOptions{ BeforeCommitID: beforeCommitID, AfterCommitID: headCommitID, diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index edbf399c772ac..9eb6917617428 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -749,7 +749,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi diffOptions.BeforeCommitID = startCommitID } - diff, err := gitdiff.GetDiff(ctx, gitRepo, diffOptions, files...) + diff, err := gitdiff.GetDiffForRender(ctx, gitRepo, diffOptions, files...) if err != nil { ctx.ServerError("GetDiff", err) return diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 4d6d102e4997c..1ad4901e09894 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -106,7 +106,7 @@ type DiffHTMLOperation struct { const BlobExcerptChunkSize = 20 // MaxDiffHighlightEntireFileSize is the maximum file size that will be highlighted with "entire file diff" -const MaxDiffHighlightEntireFileSize = 500 * 1024 +const MaxDiffHighlightEntireFileSize = 1 * 1024 * 1024 // GetType returns the type of DiffLine. func (d *DiffLine) GetType() int { @@ -1160,20 +1160,21 @@ func guessBeforeCommitForDiff(gitRepo *git.Repository, beforeCommitID string, af return actualBeforeCommit, actualBeforeCommitID, nil } -// GetDiff builds a Diff between two commits of a repository. +// getDiffBasic builds a Diff between two commits of a repository. // Passing the empty string as beforeCommitID returns a diff from the parent commit. // The whitespaceBehavior is either an empty string or a git flag -func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff, error) { +// Returned beforeCommit could be nil if the afterCommit doesn't have parent commit +func getDiffBasic(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, files ...string) (_ *Diff, beforeCommit, afterCommit *git.Commit, err error) { repoPath := gitRepo.Path - afterCommit, err := gitRepo.GetCommit(opts.AfterCommitID) + afterCommit, err = gitRepo.GetCommit(opts.AfterCommitID) if err != nil { - return nil, err + return nil, nil, nil, err } - actualBeforeCommit, actualBeforeCommitID, err := guessBeforeCommitForDiff(gitRepo, opts.BeforeCommitID, afterCommit) + beforeCommit, beforeCommitID, err := guessBeforeCommitForDiff(gitRepo, opts.BeforeCommitID, afterCommit) if err != nil { - return nil, err + return nil, nil, nil, err } cmdDiff := git.NewCommand(). @@ -1189,7 +1190,7 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi parsePatchSkipToFile = "" } - cmdDiff.AddDynamicArguments(actualBeforeCommitID.String(), opts.AfterCommitID) + cmdDiff.AddDynamicArguments(beforeCommitID.String(), opts.AfterCommitID) cmdDiff.AddDashesAndList(files...) cmdCtx, cmdCancel := context.WithCancel(ctx) @@ -1219,12 +1220,25 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi // Ensure the git process is killed if it didn't exit already cmdCancel() if err != nil { - return nil, fmt.Errorf("unable to ParsePatch: %w", err) + return nil, nil, nil, fmt.Errorf("unable to ParsePatch: %w", err) } diff.Start = opts.SkipTo + return diff, beforeCommit, afterCommit, nil +} + +func GetDiffForAPI(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff, error) { + diff, _, _, err := getDiffBasic(ctx, gitRepo, opts, files...) + return diff, err +} - checker, deferable := gitRepo.CheckAttributeReader(opts.AfterCommitID) - defer deferable() +func GetDiffForRender(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff, error) { + diff, beforeCommit, afterCommit, err := getDiffBasic(ctx, gitRepo, opts, files...) + if err != nil { + return nil, err + } + + checker, deferrable := gitRepo.CheckAttributeReader(opts.AfterCommitID) + defer deferrable() for _, diffFile := range diff.Files { isVendored := optional.None[bool]() @@ -1244,7 +1258,7 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi // Populate Submodule URLs if diffFile.SubmoduleDiffInfo != nil { - diffFile.SubmoduleDiffInfo.PopulateURL(diffFile, actualBeforeCommit, afterCommit) + diffFile.SubmoduleDiffInfo.PopulateURL(diffFile, beforeCommit, afterCommit) } if !isVendored.Has() { @@ -1256,14 +1270,12 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi isGenerated = optional.Some(analyze.IsGenerated(diffFile.Name)) } diffFile.IsGenerated = isGenerated.Value() - tailSection, limitedContent := diffFile.GetTailSectionAndLimitedContent(actualBeforeCommit, afterCommit) + tailSection, limitedContent := diffFile.GetTailSectionAndLimitedContent(beforeCommit, afterCommit) if tailSection != nil { diffFile.Sections = append(diffFile.Sections, tailSection) } if !setting.Git.DisableDiffHighlight { - // FIXME: it's not right to highlight code here for all cases, because this function is also used for non-rendering purpose - // For example: API if limitedContent.LeftContent != nil && limitedContent.LeftContent.buf.Len() < MaxDiffHighlightEntireFileSize { diffFile.highlightedOldLines = highlightCodeLines(diffFile, limitedContent.LeftContent.buf.String()) } @@ -1272,6 +1284,7 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi } } } + return diff, nil } From 0a29d4d7c026ff4ab5dc4712b407178e98a9940a Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 8 Mar 2025 19:13:38 +0800 Subject: [PATCH 21/26] only save the highlighted lines we need, but not the whole file, to save memory --- modules/highlight/highlight.go | 3 ++- services/gitdiff/gitdiff.go | 30 +++++++++++++++++++++--------- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/modules/highlight/highlight.go b/modules/highlight/highlight.go index d7ab3f7afd3e7..77f24fa3f380b 100644 --- a/modules/highlight/highlight.go +++ b/modules/highlight/highlight.go @@ -11,6 +11,7 @@ import ( gohtml "html" "html/template" "io" + "path" "path/filepath" "strings" "sync" @@ -83,7 +84,7 @@ func Code(fileName, language, code string) (output template.HTML, lexerName stri } if lexer == nil { - if val, ok := highlightMapping[filepath.Ext(fileName)]; ok { + if val, ok := highlightMapping[path.Ext(fileName)]; ok { // use mapped value to find lexer lexer = lexers.Get(val) } diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 1ad4901e09894..749b6bd55f4af 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -320,11 +320,11 @@ func (diffSection *DiffSection) getDiffLineForRender(diffLineType DiffLineType, hcd := newHighlightCodeDiff() var diff1, diff2, lineHTML template.HTML if leftLine != nil { - diff1 = diffSection.getLineContentForRender(leftLine.LeftIdx, leftLine, diffSection.file.highlightedOldLines) + diff1 = diffSection.getLineContentForRender(leftLine.LeftIdx, leftLine, diffSection.file.highlightedLeftLines) lineHTML = util.Iif(diffLineType == DiffLinePlain, diff1, "") } if rightLine != nil { - diff2 = diffSection.getLineContentForRender(rightLine.RightIdx, rightLine, diffSection.file.highlightedNewLines) + diff2 = diffSection.getLineContentForRender(rightLine.RightIdx, rightLine, diffSection.file.highlightedRightLines) lineHTML = util.Iif(diffLineType == DiffLinePlain, diff2, "") } if diffLineType != DiffLinePlain { @@ -394,8 +394,8 @@ type DiffFile struct { HasChangedSinceLastReview bool // User specific // for render purpose only, TODO: in the future, we only need to store the related diff lines to save memory - highlightedOldLines map[int]template.HTML - highlightedNewLines map[int]template.HTML + highlightedLeftLines map[int]template.HTML + highlightedRightLines map[int]template.HTML } // GetType returns type of diff file. @@ -1277,10 +1277,10 @@ func GetDiffForRender(ctx context.Context, gitRepo *git.Repository, opts *DiffOp if !setting.Git.DisableDiffHighlight { if limitedContent.LeftContent != nil && limitedContent.LeftContent.buf.Len() < MaxDiffHighlightEntireFileSize { - diffFile.highlightedOldLines = highlightCodeLines(diffFile, limitedContent.LeftContent.buf.String()) + diffFile.highlightedLeftLines = highlightCodeLines(diffFile, true /* left */, limitedContent.LeftContent.buf.String()) } if limitedContent.RightContent != nil && limitedContent.RightContent.buf.Len() < MaxDiffHighlightEntireFileSize { - diffFile.highlightedNewLines = highlightCodeLines(diffFile, limitedContent.RightContent.buf.String()) + diffFile.highlightedRightLines = highlightCodeLines(diffFile, false /* right */, limitedContent.RightContent.buf.String()) } } } @@ -1288,12 +1288,24 @@ func GetDiffForRender(ctx context.Context, gitRepo *git.Repository, opts *DiffOp return diff, nil } -func highlightCodeLines(diffFile *DiffFile, content string) map[int]template.HTML { +func highlightCodeLines(diffFile *DiffFile, isLeft bool, content string) map[int]template.HTML { highlightedNewContent, _ := highlight.Code(diffFile.Name, diffFile.Language, content) splitLines := strings.Split(string(highlightedNewContent), "\n") lines := make(map[int]template.HTML, len(splitLines)) - for i, line := range splitLines { - lines[i] = template.HTML(line) + // only save the highlighted lines we need, but not the whole file, to save memory + for _, sec := range diffFile.Sections { + for _, ln := range sec.Lines { + lineIdx := ln.LeftIdx + if !isLeft { + lineIdx = ln.RightIdx + } + if lineIdx >= 1 { + idx := lineIdx - 1 + if idx < len(splitLines) { + lines[idx] = template.HTML(splitLines[idx]) + } + } + } } return lines } From d786956f2df6be6543eaa4dc3f9eb09412d2b35b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 8 Mar 2025 20:27:32 +0800 Subject: [PATCH 22/26] fix tests --- routers/api/v1/repo/pull.go | 1 - services/gitdiff/gitdiff.go | 39 +---------- services/gitdiff/gitdiff_test.go | 19 +---- services/gitdiff/highlightdiff.go | 71 +++++++++++++++---- services/gitdiff/highlightdiff_test.go | 97 +++++++++++--------------- 5 files changed, 103 insertions(+), 124 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 103ee88ede6d7..2f32f01c4f518 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1591,7 +1591,6 @@ func GetPullRequestFiles(ctx *context.APIContext) { maxLines := setting.Git.MaxGitDiffLines // FIXME: If there are too many files in the repo, may cause some unpredictable issues. - // FIXME: it doesn't need to call "GetDiff" to do various parsing and highlighting diff, err := gitdiff.GetDiffForAPI(ctx, baseGitRepo, &gitdiff.DiffOptions{ BeforeCommitID: startCommitID, diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 749b6bd55f4af..9cef23e7355e3 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -207,38 +207,6 @@ type DiffSection struct { Lines []*DiffLine } -var ( - addedCodePrefix = []byte(``) - removedCodePrefix = []byte(``) - codeTagSuffix = []byte(``) -) - -func diffToHTML(lineWrapperTags []string, diffs []DiffHTMLOperation, lineType DiffLineType) template.HTML { - buf := bytes.NewBuffer(nil) - // restore the line wrapper tags and , if necessary - for _, tag := range lineWrapperTags { - buf.WriteString(tag) - } - for _, diff := range diffs { - switch { - case diff.Type == diffmatchpatch.DiffEqual: - buf.WriteString(string(diff.HTML)) - case diff.Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd: - buf.Write(addedCodePrefix) - buf.WriteString(string(diff.HTML)) - buf.Write(codeTagSuffix) - case diff.Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel: - buf.Write(removedCodePrefix) - buf.WriteString(string(diff.HTML)) - buf.Write(codeTagSuffix) - } - } - for range lineWrapperTags { - buf.WriteString("") - } - return template.HTML(buf.String()) -} - // GetLine gets a specific line by type (add or del) and file line number func (diffSection *DiffSection) GetLine(lineType DiffLineType, idx int) *DiffLine { var ( @@ -328,10 +296,9 @@ func (diffSection *DiffSection) getDiffLineForRender(diffLineType DiffLineType, lineHTML = util.Iif(diffLineType == DiffLinePlain, diff2, "") } if diffLineType != DiffLinePlain { - diffRecord := hcd.diffWithHighlight(diff1, diff2) // it seems that Gitea doesn't need the line wrapper of Chroma, so do not add them back // if the line wrappers are still needed in the future, it can be added back by "diffToHTML(hcd.lineWrapperTags. ...)" - lineHTML = diffToHTML(nil, diffRecord, diffLineType) + lineHTML = hcd.diffLineWithHighlight(diffLineType, diff1, diff2) } return DiffInlineWithUnicodeEscape(lineHTML, locale) } @@ -380,7 +347,7 @@ type DiffFile struct { IsIncomplete bool IsIncompleteLineTooLong bool - // will be filled by the extra loop in GitDiff + // will be filled by the extra loop in GitDiffForRender Language string IsGenerated bool IsVendored bool @@ -393,7 +360,7 @@ type DiffFile struct { IsViewed bool // User specific HasChangedSinceLastReview bool // User specific - // for render purpose only, TODO: in the future, we only need to store the related diff lines to save memory + // for render purpose only, will be filled by the extra loop in GitDiffForRender highlightedLeftLines map[int]template.HTML highlightedRightLines map[int]template.HTML } diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go index 41b4077cf24d3..71394b191587e 100644 --- a/services/gitdiff/gitdiff_test.go +++ b/services/gitdiff/gitdiff_test.go @@ -17,27 +17,10 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" - dmp "github.com/sergi/go-diff/diffmatchpatch" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestDiffToHTML(t *testing.T) { - assert.Equal(t, "foo bar biz", diffToHTML(nil, []dmp.Diff{ - {Type: dmp.DiffEqual, Text: "foo "}, - {Type: dmp.DiffInsert, Text: "bar"}, - {Type: dmp.DiffDelete, Text: " baz"}, - {Type: dmp.DiffEqual, Text: " biz"}, - }, DiffLineAdd)) - - assert.Equal(t, "foo bar biz", diffToHTML(nil, []dmp.Diff{ - {Type: dmp.DiffEqual, Text: "foo "}, - {Type: dmp.DiffDelete, Text: "bar"}, - {Type: dmp.DiffInsert, Text: " baz"}, - {Type: dmp.DiffEqual, Text: " biz"}, - }, DiffLineDel)) -} - func TestParsePatch_skipTo(t *testing.T) { type testcase struct { name string @@ -621,7 +604,7 @@ func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) { defer gitRepo.Close() for _, behavior := range []git.TrustedCmdArgs{{"-w"}, {"--ignore-space-at-eol"}, {"-b"}, nil} { - diffs, err := GetDiff(t.Context(), gitRepo, + diffs, err := GetDiffForAPI(t.Context(), gitRepo, &DiffOptions{ AfterCommitID: "d8e0bbb45f200e67d9a784ce55bd90821af45ebd", BeforeCommitID: "72866af952e98d02a73003501836074b286a78f6", diff --git a/services/gitdiff/highlightdiff.go b/services/gitdiff/highlightdiff.go index dcb6f34af0f9a..5891e612490f3 100644 --- a/services/gitdiff/highlightdiff.go +++ b/services/gitdiff/highlightdiff.go @@ -4,8 +4,11 @@ package gitdiff import ( + "bytes" "html/template" "strings" + + "github.com/sergi/go-diff/diffmatchpatch" ) // token is a html tag or entity, eg: "", "", "<" @@ -83,7 +86,11 @@ func (hcd *highlightCodeDiff) collectUsedRunes(code template.HTML) { } } -func (hcd *highlightCodeDiff) diffWithHighlight(codeA, codeB template.HTML) (res []DiffHTMLOperation) { +func (hcd *highlightCodeDiff) diffLineWithHighlight(lineType DiffLineType, codeA, codeB template.HTML) template.HTML { + return hcd.diffLineWithHighlightWrapper(nil, lineType, codeA, codeB) +} + +func (hcd *highlightCodeDiff) diffLineWithHighlightWrapper(lineWrapperTags []string, lineType DiffLineType, codeA, codeB template.HTML) template.HTML { hcd.collectUsedRunes(codeA) hcd.collectUsedRunes(codeB) @@ -94,10 +101,57 @@ func (hcd *highlightCodeDiff) diffWithHighlight(codeA, codeB template.HTML) (res diffs := dmp.DiffMain(convertedCodeA, convertedCodeB, true) diffs = dmp.DiffCleanupEfficiency(diffs) - for _, d := range diffs { - res = append(res, DiffHTMLOperation{Type: d.Type, HTML: hcd.recoverOneDiff(d.Text)}) + buf := bytes.NewBuffer(nil) + + // restore the line wrapper tags and , if necessary + for _, tag := range lineWrapperTags { + buf.WriteString(tag) } - return res + + addedCodePrefix := hcd.registerTokenAsPlaceholder(``) + removedCodePrefix := hcd.registerTokenAsPlaceholder(``) + codeTagSuffix := hcd.registerTokenAsPlaceholder(``) + + if codeTagSuffix != 0 { + for _, diff := range diffs { + switch { + case diff.Type == diffmatchpatch.DiffEqual: + buf.WriteString(diff.Text) + case diff.Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd: + buf.WriteRune(addedCodePrefix) + buf.WriteString(diff.Text) + buf.WriteRune(codeTagSuffix) + case diff.Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel: + buf.WriteRune(removedCodePrefix) + buf.WriteString(diff.Text) + buf.WriteRune(codeTagSuffix) + } + } + } else { + // placeholder map space is exhausted + for _, diff := range diffs { + take := diff.Type == diffmatchpatch.DiffEqual || (diff.Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd) || (diff.Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel) + if take { + buf.WriteString(diff.Text) + } + } + } + for range lineWrapperTags { + buf.WriteString("") + } + return hcd.recoverOneDiff(buf.String()) +} + +func (hcd *highlightCodeDiff) registerTokenAsPlaceholder(token string) rune { + placeholder, ok := hcd.tokenPlaceholderMap[token] + if !ok { + placeholder = hcd.nextPlaceholder() + if placeholder != 0 { + hcd.tokenPlaceholderMap[token] = placeholder + hcd.placeholderTokenMap[placeholder] = token + } + } + return placeholder } // 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 } // else: impossible // remember the placeholder and token in the map - placeholder, ok := hcd.tokenPlaceholderMap[tokenInMap] - if !ok { - placeholder = hcd.nextPlaceholder() - if placeholder != 0 { - hcd.tokenPlaceholderMap[tokenInMap] = placeholder - hcd.placeholderTokenMap[placeholder] = tokenInMap - } - } + placeholder := hcd.registerTokenAsPlaceholder(tokenInMap) if placeholder != 0 { res.WriteRune(placeholder) // use the placeholder to replace the token diff --git a/services/gitdiff/highlightdiff_test.go b/services/gitdiff/highlightdiff_test.go index 71400fed4d714..c40c4fb5eccfa 100644 --- a/services/gitdiff/highlightdiff_test.go +++ b/services/gitdiff/highlightdiff_test.go @@ -5,84 +5,67 @@ package gitdiff import ( "fmt" + "html/template" + "strings" "testing" - "github.com/sergi/go-diff/diffmatchpatch" "github.com/stretchr/testify/assert" ) func TestDiffWithHighlight(t *testing.T) { - hcd := newHighlightCodeDiff() - diffs := hcd.diffWithHighlight( - " run('<>')\n", - " run(db)\n", - ) - - expected := "\t\trun('<>')\n" - - output := diffToHTML(nil, diffs, DiffLineDel) - assert.Equal(t, expected, output) - - expected = ` run(db) -` - output = diffToHTML(nil, diffs, DiffLineAdd) - assert.Equal(t, expected, output) - - hcd = newHighlightCodeDiff() - hcd.placeholderTokenMap['O'] = "" - hcd.placeholderTokenMap['C'] = "" - diff := diffmatchpatch.Diff{} + t.Run("DiffLineAddDel", func(t *testing.T) { + hcd := newHighlightCodeDiff() + codeA := template.HTML(`x foo y`) + codeB := template.HTML(`x bar y`) + outDel := hcd.diffLineWithHighlight(DiffLineDel, codeA, codeB) + assert.Equal(t, `x foo y`, string(outDel)) + outAdd := hcd.diffLineWithHighlight(DiffLineAdd, codeA, codeB) + assert.Equal(t, `x bar y`, string(outAdd)) + }) - diff.Text = "C" - hcd.recoverOneDiff(&diff) - assert.Equal(t, "", diff.Text) + t.Run("OpenCloseTags", func(t *testing.T) { + hcd := newHighlightCodeDiff() + hcd.placeholderTokenMap['O'], hcd.placeholderTokenMap['C'] = "", "" + assert.Equal(t, "", string(hcd.recoverOneDiff("OC"))) + assert.Equal(t, "", string(hcd.recoverOneDiff("O"))) + assert.Equal(t, "", string(hcd.recoverOneDiff("C"))) + }) } func TestDiffWithHighlightPlaceholder(t *testing.T) { hcd := newHighlightCodeDiff() - diffs := hcd.diffWithHighlight( - "a='\U00100000'", - "a='\U0010FFFD''", - ) + output := hcd.diffLineWithHighlight(DiffLineDel, "a='\U00100000'", "a='\U0010FFFD''") assert.Equal(t, "", hcd.placeholderTokenMap[0x00100000]) assert.Equal(t, "", hcd.placeholderTokenMap[0x0010FFFD]) - expected := fmt.Sprintf(`a='%s'`, "\U00100000") - output := diffToHTML(hcd.lineWrapperTags, diffs, DiffLineDel) - assert.Equal(t, expected, output) + assert.Equal(t, expected, string(output)) hcd = newHighlightCodeDiff() - diffs = hcd.diffWithHighlight( - "a='\U00100000'", - "a='\U0010FFFD'", - ) + output = hcd.diffLineWithHighlight(DiffLineAdd, "a='\U00100000'", "a='\U0010FFFD'") expected = fmt.Sprintf(`a='%s'`, "\U0010FFFD") - output = diffToHTML(nil, diffs, DiffLineAdd) - assert.Equal(t, expected, output) + assert.Equal(t, expected, string(output)) } func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) { hcd := newHighlightCodeDiff() hcd.placeholderMaxCount = 0 - diffs := hcd.diffWithHighlight( - "'", - ``, - ) - output := diffToHTML(nil, diffs, DiffLineDel) - expected := `'` - assert.Equal(t, expected, output) - - hcd = newHighlightCodeDiff() - hcd.placeholderMaxCount = 0 - diffs = hcd.diffWithHighlight( - "a this_is_not_html_at_this_point b", - "a this_is_is_still_not_html_at_this_point_its_just_a_string b", - ) - output = diffToHTML(nil, diffs, DiffLineDel) - expected = "a this_is_not_html_at_this_point b" - assert.Equal(t, expected, output) + placeHolderAmp := string(rune(0xFFFD)) + output := hcd.diffLineWithHighlight(DiffLineDel, `<`, `>`) + assert.Equal(t, placeHolderAmp+"lt;", string(output)) + output = hcd.diffLineWithHighlight(DiffLineAdd, `<`, `>`) + assert.Equal(t, placeHolderAmp+"gt;", string(output)) +} - output = diffToHTML(nil, diffs, DiffLineAdd) - expected = "a this_is_is_still_not_html_at_this_point_its_just_a_string b" - assert.Equal(t, expected, output) +func TestDiffWithHighlightTagMatch(t *testing.T) { + totalOverflow := 0 + for i := 0; i < 100; i++ { + hcd := newHighlightCodeDiff() + hcd.placeholderMaxCount = i + output := string(hcd.diffLineWithHighlight(DiffLineDel, `<`, `>`)) + totalOverflow += hcd.placeholderOverflowCount + c1 := strings.Count(output, " Date: Sat, 8 Mar 2025 20:37:17 +0800 Subject: [PATCH 23/26] fix test & lint --- services/gitdiff/gitdiff.go | 6 +++--- services/repository/files/diff_test.go | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 9cef23e7355e3..b3dea71ee4e3d 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -533,13 +533,13 @@ parsingLoop: } if maxFiles > -1 && len(diff.Files) >= maxFiles { - lastFile := createDiffFile(diff, line) + lastFile := createDiffFile(line) diff.End = lastFile.Name diff.IsIncomplete = true break parsingLoop } - curFile = createDiffFile(diff, line) + curFile = createDiffFile(line) if skipping { if curFile.Name != skipToFile { line, err = skipToNextDiffHead(input) @@ -1013,7 +1013,7 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact } } -func createDiffFile(diff *Diff, line string) *DiffFile { +func createDiffFile(line string) *DiffFile { // The a/ and b/ filenames are the same unless rename/copy is involved. // Especially, even for a creation or a deletion, /dev/null is not used // in place of the a/ or b/ filenames. diff --git a/services/repository/files/diff_test.go b/services/repository/files/diff_test.go index 38cb1d675b5a3..57920a2c4fecc 100644 --- a/services/repository/files/diff_test.go +++ b/services/repository/files/diff_test.go @@ -35,7 +35,6 @@ func TestGetDiffPreview(t *testing.T) { Name: "README.md", OldName: "README.md", NameHash: "8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d", - Index: 1, Addition: 2, Deletion: 1, Type: 2, From 7fffde14ebd3bcfc94ea47f072c1923be24bbe6e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 8 Mar 2025 22:41:52 +0800 Subject: [PATCH 24/26] Update services/gitdiff/gitdiff.go --- services/gitdiff/gitdiff.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index b3dea71ee4e3d..33ccf5bd89488 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -297,7 +297,7 @@ func (diffSection *DiffSection) getDiffLineForRender(diffLineType DiffLineType, } if diffLineType != DiffLinePlain { // it seems that Gitea doesn't need the line wrapper of Chroma, so do not add them back - // if the line wrappers are still needed in the future, it can be added back by "diffToHTML(hcd.lineWrapperTags. ...)" + // if the line wrappers are still needed in the future, it can be added back by "diffLineWithHighlightWrapper(hcd.lineWrapperTags. ...)" lineHTML = hcd.diffLineWithHighlight(diffLineType, diff1, diff2) } return DiffInlineWithUnicodeEscape(lineHTML, locale) From 632ea32c11bc41a674b918fe307c0e4d188fecef Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 9 Mar 2025 01:29:11 +0800 Subject: [PATCH 25/26] improve test --- services/gitdiff/highlightdiff_test.go | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/services/gitdiff/highlightdiff_test.go b/services/gitdiff/highlightdiff_test.go index c40c4fb5eccfa..16649682b4fc6 100644 --- a/services/gitdiff/highlightdiff_test.go +++ b/services/gitdiff/highlightdiff_test.go @@ -57,15 +57,20 @@ func TestDiffWithHighlightPlaceholderExhausted(t *testing.T) { } func TestDiffWithHighlightTagMatch(t *testing.T) { - totalOverflow := 0 - for i := 0; i < 100; i++ { - hcd := newHighlightCodeDiff() - hcd.placeholderMaxCount = i - output := string(hcd.diffLineWithHighlight(DiffLineDel, `<`, `>`)) - totalOverflow += hcd.placeholderOverflowCount - c1 := strings.Count(output, "<`, `>`)) + totalOverflow += hcd.placeholderOverflowCount + assert.Equal(t, strings.Count(output, " Date: Sun, 9 Mar 2025 11:24:41 +0800 Subject: [PATCH 26/26] fix 500 in blob_excerpt --- services/gitdiff/gitdiff.go | 17 ++++++++---- tests/integration/pull_commit_test.go | 37 +++++++++++++++------------ 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 33ccf5bd89488..3a552547b8997 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -268,7 +268,7 @@ func DiffInlineWithUnicodeEscape(s template.HTML, locale translation.Locale) Dif return DiffInline{EscapeStatus: status, Content: content} } -func (diffSection *DiffSection) getLineContentForRender(lineIdx int, diffLine *DiffLine, highlightLines map[int]template.HTML) template.HTML { +func (diffSection *DiffSection) getLineContentForRender(lineIdx int, diffLine *DiffLine, fileLanguage string, highlightLines map[int]template.HTML) template.HTML { h, ok := highlightLines[lineIdx-1] if ok { return h @@ -279,20 +279,27 @@ func (diffSection *DiffSection) getLineContentForRender(lineIdx int, diffLine *D if setting.Git.DisableDiffHighlight { return template.HTML(html.EscapeString(diffLine.Content[1:])) } - - h, _ = highlight.Code(diffSection.Name, diffSection.file.Language, diffLine.Content[1:]) + h, _ = highlight.Code(diffSection.Name, fileLanguage, diffLine.Content[1:]) return h } func (diffSection *DiffSection) getDiffLineForRender(diffLineType DiffLineType, leftLine, rightLine *DiffLine, locale translation.Locale) DiffInline { + var fileLanguage string + var highlightedLeftLines, highlightedRightLines map[int]template.HTML + // when a "diff section" is manually prepared by ExcerptBlob, it doesn't have "file" information + if diffSection.file != nil { + fileLanguage = diffSection.file.Language + highlightedLeftLines, highlightedRightLines = diffSection.file.highlightedLeftLines, diffSection.file.highlightedRightLines + } + hcd := newHighlightCodeDiff() var diff1, diff2, lineHTML template.HTML if leftLine != nil { - diff1 = diffSection.getLineContentForRender(leftLine.LeftIdx, leftLine, diffSection.file.highlightedLeftLines) + diff1 = diffSection.getLineContentForRender(leftLine.LeftIdx, leftLine, fileLanguage, highlightedLeftLines) lineHTML = util.Iif(diffLineType == DiffLinePlain, diff1, "") } if rightLine != nil { - diff2 = diffSection.getLineContentForRender(rightLine.RightIdx, rightLine, diffSection.file.highlightedRightLines) + diff2 = diffSection.getLineContentForRender(rightLine.RightIdx, rightLine, fileLanguage, highlightedRightLines) lineHTML = util.Iif(diffLineType == DiffLinePlain, diff2, "") } if diffLineType != DiffLinePlain { diff --git a/tests/integration/pull_commit_test.go b/tests/integration/pull_commit_test.go index 8d98349fd3560..fc111f528f698 100644 --- a/tests/integration/pull_commit_test.go +++ b/tests/integration/pull_commit_test.go @@ -5,30 +5,33 @@ package integration import ( "net/http" - "net/url" "testing" pull_service "code.gitea.io/gitea/services/pull" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestListPullCommits(t *testing.T) { - onGiteaRun(t, func(t *testing.T, u *url.URL) { - session := loginUser(t, "user5") - req := NewRequest(t, "GET", "/user2/repo1/pulls/3/commits/list") - resp := session.MakeRequest(t, req, http.StatusOK) - - var pullCommitList struct { - Commits []pull_service.CommitInfo `json:"commits"` - LastReviewCommitSha string `json:"last_review_commit_sha"` - } - DecodeJSON(t, resp, &pullCommitList) - - if assert.Len(t, pullCommitList.Commits, 2) { - assert.Equal(t, "985f0301dba5e7b34be866819cd15ad3d8f508ee", pullCommitList.Commits[0].ID) - assert.Equal(t, "5c050d3b6d2db231ab1f64e324f1b6b9a0b181c2", pullCommitList.Commits[1].ID) - } - assert.Equal(t, "4a357436d925b5c974181ff12a994538ddc5a269", pullCommitList.LastReviewCommitSha) + session := loginUser(t, "user5") + req := NewRequest(t, "GET", "/user2/repo1/pulls/3/commits/list") + resp := session.MakeRequest(t, req, http.StatusOK) + + var pullCommitList struct { + Commits []pull_service.CommitInfo `json:"commits"` + LastReviewCommitSha string `json:"last_review_commit_sha"` + } + DecodeJSON(t, resp, &pullCommitList) + + require.Len(t, pullCommitList.Commits, 2) + assert.Equal(t, "985f0301dba5e7b34be866819cd15ad3d8f508ee", pullCommitList.Commits[0].ID) + assert.Equal(t, "5c050d3b6d2db231ab1f64e324f1b6b9a0b181c2", pullCommitList.Commits[1].ID) + assert.Equal(t, "4a357436d925b5c974181ff12a994538ddc5a269", pullCommitList.LastReviewCommitSha) + + t.Run("CommitBlobExcerpt", func(t *testing.T) { + req = NewRequest(t, "GET", "/user2/repo1/blob_excerpt/985f0301dba5e7b34be866819cd15ad3d8f508ee?last_left=0&last_right=0&left=2&right=2&left_hunk_size=2&right_hunk_size=2&path=README.md&style=split&direction=up") + resp = session.MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), `# repo1`) }) }