0
0
mirror of https://github.com/go-gitea/gitea.git synced 2026-02-12 21:56:39 +01:00

Use full-file highlighting for diff sections (#36561)

* Fix #35252
* Fix #35999
* Improve diff rendering, don't add unnecessary "added"/"removed" tags for a full-line change
* Also fix a "space trimming" bug in #36539 and add tests
* Use chroma "SQL" lexer instead of "MySQL" to workaround a bug (35999)
This commit is contained in:
wxiaoguang 2026-02-10 11:29:28 +08:00 committed by GitHub
parent 269bc1b112
commit 8cc8150922
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 391 additions and 192 deletions

View File

@ -98,6 +98,10 @@ func getChromaLexerByLanguage(fileName, lang string) chroma.Lexer {
lang = "C++"
}
}
if lang == "" && util.AsciiEqualFold(ext, ".sql") {
// there is a bug when using MySQL lexer: "--\nSELECT", the second line will be rendered as comment incorrectly
lang = "SQL"
}
// lexers.Get is slow if the language name can't be matched directly: it does extra "Match" call to iterate all lexers
return lexers.Get(lang)
}

View File

@ -108,6 +108,12 @@ c=2
),
lexerName: "Python",
},
{
name: "test.sql",
code: "--\nSELECT",
want: []template.HTML{"<span class=\"c1\">--\n</span>", `<span class="k">SELECT</span>`},
lexerName: "SQL",
},
}
for _, tt := range tests {

View File

@ -4,7 +4,6 @@
package repo
import (
"bufio"
gocontext "context"
"encoding/csv"
"errors"
@ -744,13 +743,16 @@ func attachHiddenCommentIDs(section *gitdiff.DiffSection, lineComments map[int64
// ExcerptBlob render blob excerpt contents
func ExcerptBlob(ctx *context.Context) {
commitID := ctx.PathParam("sha")
lastLeft := ctx.FormInt("last_left")
lastRight := ctx.FormInt("last_right")
idxLeft := ctx.FormInt("left")
idxRight := ctx.FormInt("right")
leftHunkSize := ctx.FormInt("left_hunk_size")
rightHunkSize := ctx.FormInt("right_hunk_size")
direction := ctx.FormString("direction")
opts := gitdiff.BlobExcerptOptions{
LastLeft: ctx.FormInt("last_left"),
LastRight: ctx.FormInt("last_right"),
LeftIndex: ctx.FormInt("left"),
RightIndex: ctx.FormInt("right"),
LeftHunkSize: ctx.FormInt("left_hunk_size"),
RightHunkSize: ctx.FormInt("right_hunk_size"),
Direction: ctx.FormString("direction"),
Language: ctx.FormString("filelang"),
}
filePath := ctx.FormString("path")
gitRepo := ctx.Repo.GitRepo
@ -770,61 +772,27 @@ func ExcerptBlob(ctx *context.Context) {
diffBlobExcerptData.BaseLink = ctx.Repo.RepoLink + "/wiki/blob_excerpt"
}
chunkSize := gitdiff.BlobExcerptChunkSize
commit, err := gitRepo.GetCommit(commitID)
if err != nil {
ctx.HTTPError(http.StatusInternalServerError, "GetCommit")
ctx.ServerError("GetCommit", err)
return
}
section := &gitdiff.DiffSection{
FileName: filePath,
}
if direction == "up" && (idxLeft-lastLeft) > chunkSize {
idxLeft -= chunkSize
idxRight -= chunkSize
leftHunkSize += chunkSize
rightHunkSize += chunkSize
section.Lines, err = getExcerptLines(commit, filePath, idxLeft-1, idxRight-1, chunkSize)
} else if direction == "down" && (idxLeft-lastLeft) > chunkSize {
section.Lines, err = getExcerptLines(commit, filePath, lastLeft, lastRight, chunkSize)
lastLeft += chunkSize
lastRight += chunkSize
} else {
offset := -1
if direction == "down" {
offset = 0
}
section.Lines, err = getExcerptLines(commit, filePath, lastLeft, lastRight, idxRight-lastRight+offset)
leftHunkSize = 0
rightHunkSize = 0
idxLeft = lastLeft
idxRight = lastRight
}
blob, err := commit.Tree.GetBlobByPath(filePath)
if err != nil {
ctx.HTTPError(http.StatusInternalServerError, "getExcerptLines")
ctx.ServerError("GetBlobByPath", err)
return
}
newLineSection := &gitdiff.DiffLine{
Type: gitdiff.DiffLineSection,
SectionInfo: &gitdiff.DiffLineSectionInfo{
Path: filePath,
LastLeftIdx: lastLeft,
LastRightIdx: lastRight,
LeftIdx: idxLeft,
RightIdx: idxRight,
LeftHunkSize: leftHunkSize,
RightHunkSize: rightHunkSize,
},
reader, err := blob.DataAsync()
if err != nil {
ctx.ServerError("DataAsync", err)
return
}
if newLineSection.GetExpandDirection() != "" {
newLineSection.Content = fmt.Sprintf("@@ -%d,%d +%d,%d @@\n", idxLeft, leftHunkSize, idxRight, rightHunkSize)
switch direction {
case "up":
section.Lines = append([]*gitdiff.DiffLine{newLineSection}, section.Lines...)
case "down":
section.Lines = append(section.Lines, newLineSection)
}
defer reader.Close()
section, err := gitdiff.BuildBlobExcerptDiffSection(filePath, reader, opts)
if err != nil {
ctx.ServerError("BuildBlobExcerptDiffSection", err)
return
}
diffBlobExcerptData.PullIssueIndex = ctx.FormInt64("pull_issue_index")
@ -865,37 +833,3 @@ func ExcerptBlob(ctx *context.Context) {
ctx.HTML(http.StatusOK, tplBlobExcerpt)
}
func getExcerptLines(commit *git.Commit, filePath string, idxLeft, idxRight, chunkSize int) ([]*gitdiff.DiffLine, error) {
blob, err := commit.Tree.GetBlobByPath(filePath)
if err != nil {
return nil, err
}
reader, err := blob.DataAsync()
if err != nil {
return nil, err
}
defer reader.Close()
scanner := bufio.NewScanner(reader)
var diffLines []*gitdiff.DiffLine
for line := 0; line < idxRight+chunkSize; line++ {
if ok := scanner.Scan(); !ok {
break
}
if line < idxRight {
continue
}
lineText := scanner.Text()
diffLine := &gitdiff.DiffLine{
LeftIdx: idxLeft + (line - idxRight) + 1,
RightIdx: line + 1,
Type: gitdiff.DiffLinePlain,
Content: " " + lineText,
}
diffLines = append(diffLines, diffLine)
}
if err = scanner.Err(); err != nil {
return nil, fmt.Errorf("getExcerptLines scan: %w", err)
}
return diffLines, nil
}

View File

@ -6,12 +6,13 @@ package repo
import (
"net/http"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/services/context"
files_service "code.gitea.io/gitea/services/repository/files"
)
func DiffPreviewPost(ctx *context.Context) {
content := ctx.FormString("content")
newContent := ctx.FormString("content")
treePath := files_service.CleanGitTreePath(ctx.Repo.TreePath)
if treePath == "" {
ctx.HTTPError(http.StatusBadRequest, "file name to diff is invalid")
@ -27,7 +28,12 @@ func DiffPreviewPost(ctx *context.Context) {
return
}
diff, err := files_service.GetDiffPreview(ctx, ctx.Repo.Repository, ctx.Repo.BranchName, treePath, content)
oldContent, err := entry.Blob().GetBlobContent(setting.UI.MaxDisplayFileSize)
if err != nil {
ctx.ServerError("GetBlobContent", err)
return
}
diff, err := files_service.GetDiffPreview(ctx, ctx.Repo.Repository, ctx.Repo.BranchName, treePath, oldContent, newContent)
if err != nil {
ctx.ServerError("GetDiffPreview", err)
return

View File

@ -81,6 +81,8 @@ type DiffLine struct {
// DiffLineSectionInfo represents diff line section meta data
type DiffLineSectionInfo struct {
language *diffVarMutable[string]
Path string
// These line "idx" are 1-based line numbers
@ -165,16 +167,19 @@ func (d *DiffLine) GetLineTypeMarker() string {
}
func (d *DiffLine) getBlobExcerptQuery() string {
query := fmt.Sprintf(
language := ""
if d.SectionInfo.language != nil { // for normal cases, it can't be nil, this check is only for some tests
language = d.SectionInfo.language.value
}
return fmt.Sprintf(
"last_left=%d&last_right=%d&"+
"left=%d&right=%d&"+
"left_hunk_size=%d&right_hunk_size=%d&"+
"path=%s",
"path=%s&filelang=%s",
d.SectionInfo.LastLeftIdx, d.SectionInfo.LastRightIdx,
d.SectionInfo.LeftIdx, d.SectionInfo.RightIdx,
d.SectionInfo.LeftHunkSize, d.SectionInfo.RightHunkSize,
url.QueryEscape(d.SectionInfo.Path))
return query
url.QueryEscape(d.SectionInfo.Path), url.QueryEscape(language))
}
func (d *DiffLine) GetExpandDirection() string {
@ -266,11 +271,12 @@ func FillHiddenCommentIDsForDiffLine(line *DiffLine, lineComments map[int64][]*i
line.SectionInfo.HiddenCommentIDs = hiddenCommentIDs
}
func getDiffLineSectionInfo(treePath, line string, lastLeftIdx, lastRightIdx int) *DiffLineSectionInfo {
func newDiffLineSectionInfo(curFile *DiffFile, line string, lastLeftIdx, lastRightIdx int) *DiffLineSectionInfo {
leftLine, leftHunk, rightLine, rightHunk := git.ParseDiffHunkString(line)
return &DiffLineSectionInfo{
Path: treePath,
Path: curFile.Name,
language: &curFile.language,
LastLeftIdx: lastLeftIdx,
LastRightIdx: lastRightIdx,
LeftIdx: leftLine,
@ -290,7 +296,10 @@ func getLineContent(content string, locale translation.Locale) DiffInline {
// DiffSection represents a section of a DiffFile.
type DiffSection struct {
file *DiffFile
language *diffVarMutable[string]
highlightedLeftLines *diffVarMutable[map[int]template.HTML]
highlightedRightLines *diffVarMutable[map[int]template.HTML]
FileName string
Lines []*DiffLine
}
@ -339,9 +348,9 @@ func (diffSection *DiffSection) getDiffLineForRender(diffLineType DiffLineType,
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
if diffSection.language != nil {
fileLanguage = diffSection.language.value
highlightedLeftLines, highlightedRightLines = diffSection.highlightedLeftLines.value, diffSection.highlightedRightLines.value
}
var lineHTML template.HTML
@ -392,6 +401,11 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc
}
}
// diffVarMutable is a wrapper to make a variable mutable to be shared across structs
type diffVarMutable[T any] struct {
value T
}
// DiffFile represents a file diff.
type DiffFile struct {
// only used internally to parse Ambiguous filenames
@ -418,7 +432,6 @@ type DiffFile struct {
IsIncompleteLineTooLong bool
// will be filled by the extra loop in GitDiffForRender
Language string
IsGenerated bool
IsVendored bool
SubmoduleDiffInfo *SubmoduleDiffInfo // IsSubmodule==true, then there must be a SubmoduleDiffInfo
@ -430,9 +443,10 @@ type DiffFile struct {
IsViewed bool // User specific
HasChangedSinceLastReview bool // User specific
// for render purpose only, will be filled by the extra loop in GitDiffForRender
highlightedLeftLines map[int]template.HTML
highlightedRightLines map[int]template.HTML
// for render purpose only, will be filled by the extra loop in GitDiffForRender, the maps of lines are 0-based
language diffVarMutable[string]
highlightedLeftLines diffVarMutable[map[int]template.HTML]
highlightedRightLines diffVarMutable[map[int]template.HTML]
}
// GetType returns type of diff file.
@ -469,6 +483,7 @@ func (diffFile *DiffFile) GetTailSectionAndLimitedContent(leftCommit, rightCommi
Type: DiffLineSection,
Content: " ",
SectionInfo: &DiffLineSectionInfo{
language: &diffFile.language,
Path: diffFile.Name,
LastLeftIdx: lastLine.LeftIdx,
LastRightIdx: lastLine.RightIdx,
@ -907,6 +922,14 @@ func skipToNextDiffHead(input *bufio.Reader) (line string, err error) {
return line, err
}
func newDiffSectionForDiffFile(curFile *DiffFile) *DiffSection {
return &DiffSection{
language: &curFile.language,
highlightedLeftLines: &curFile.highlightedLeftLines,
highlightedRightLines: &curFile.highlightedRightLines,
}
}
func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio.Reader) (lineBytes []byte, isFragment bool, err error) {
sb := strings.Builder{}
@ -964,12 +987,12 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
line := sb.String()
// Create a new section to represent this hunk
curSection = &DiffSection{file: curFile}
curSection = newDiffSectionForDiffFile(curFile)
lastLeftIdx = -1
curFile.Sections = append(curFile.Sections, curSection)
// FIXME: the "-1" can't be right, these "line idx" are all 1-based, maybe there are other bugs that covers this bug.
lineSectionInfo := getDiffLineSectionInfo(curFile.Name, line, leftLine-1, rightLine-1)
lineSectionInfo := newDiffLineSectionInfo(curFile, line, leftLine-1, rightLine-1)
diffLine := &DiffLine{
Type: DiffLineSection,
Content: line,
@ -1004,7 +1027,7 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
rightLine++
if curSection == nil {
// Create a new section to represent this hunk
curSection = &DiffSection{file: curFile}
curSection = newDiffSectionForDiffFile(curFile)
curFile.Sections = append(curFile.Sections, curSection)
lastLeftIdx = -1
}
@ -1037,7 +1060,7 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
}
if curSection == nil {
// Create a new section to represent this hunk
curSection = &DiffSection{file: curFile}
curSection = newDiffSectionForDiffFile(curFile)
curFile.Sections = append(curFile.Sections, curSection)
lastLeftIdx = -1
}
@ -1064,7 +1087,7 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
lastLeftIdx = -1
if curSection == nil {
// Create a new section to represent this hunk
curSection = &DiffSection{file: curFile}
curSection = newDiffSectionForDiffFile(curFile)
curFile.Sections = append(curFile.Sections, curSection)
}
curSection.Lines = append(curSection.Lines, diffLine)
@ -1309,7 +1332,7 @@ func GetDiffForRender(ctx context.Context, repoLink string, gitRepo *git.Reposit
isVendored, isGenerated = attrs.GetVendored(), attrs.GetGenerated()
language := attrs.GetLanguage()
if language.Has() {
diffFile.Language = language.Value()
diffFile.language.value = language.Value()
}
attrDiff = attrs.Get(attribute.Diff).ToString()
}
@ -1335,11 +1358,11 @@ func GetDiffForRender(ctx context.Context, repoLink string, gitRepo *git.Reposit
shouldFullFileHighlight := !setting.Git.DisableDiffHighlight && attrDiff.Value() == ""
if shouldFullFileHighlight {
if limitedContent.LeftContent != nil && limitedContent.LeftContent.buf.Len() < MaxDiffHighlightEntireFileSize {
diffFile.highlightedLeftLines = highlightCodeLines(diffFile, true /* left */, limitedContent.LeftContent.buf.Bytes())
if limitedContent.LeftContent != nil {
diffFile.highlightedLeftLines.value = highlightCodeLinesForDiffFile(diffFile, true /* left */, limitedContent.LeftContent.buf.Bytes())
}
if limitedContent.RightContent != nil && limitedContent.RightContent.buf.Len() < MaxDiffHighlightEntireFileSize {
diffFile.highlightedRightLines = highlightCodeLines(diffFile, false /* right */, limitedContent.RightContent.buf.Bytes())
if limitedContent.RightContent != nil {
diffFile.highlightedRightLines.value = highlightCodeLinesForDiffFile(diffFile, false /* right */, limitedContent.RightContent.buf.Bytes())
}
}
}
@ -1347,13 +1370,26 @@ func GetDiffForRender(ctx context.Context, repoLink string, gitRepo *git.Reposit
return diff, nil
}
func highlightCodeLines(diffFile *DiffFile, isLeft bool, rawContent []byte) map[int]template.HTML {
func FillDiffFileHighlightLinesByContent(diffFile *DiffFile, left, right []byte) {
diffFile.highlightedLeftLines.value = highlightCodeLinesForDiffFile(diffFile, true /* left */, left)
diffFile.highlightedRightLines.value = highlightCodeLinesForDiffFile(diffFile, false /* right */, right)
}
func highlightCodeLinesForDiffFile(diffFile *DiffFile, isLeft bool, rawContent []byte) map[int]template.HTML {
return highlightCodeLines(diffFile.Name, diffFile.language.value, diffFile.Sections, isLeft, rawContent)
}
func highlightCodeLines(name, lang string, sections []*DiffSection, isLeft bool, rawContent []byte) map[int]template.HTML {
if setting.Git.DisableDiffHighlight || len(rawContent) > MaxDiffHighlightEntireFileSize {
return nil
}
content := util.UnsafeBytesToString(charset.ToUTF8(rawContent, charset.ConvertOpts{}))
highlightedNewContent, _ := highlight.RenderCodeFast(diffFile.Name, diffFile.Language, content)
highlightedNewContent, _ := highlight.RenderCodeFast(name, lang, content)
unsafeLines := highlight.UnsafeSplitHighlightedLines(highlightedNewContent)
lines := make(map[int]template.HTML, len(unsafeLines))
// only save the highlighted lines we need, but not the whole file, to save memory
for _, sec := range diffFile.Sections {
for _, sec := range sections {
for _, ln := range sec.Lines {
lineIdx := ln.LeftIdx
if !isLeft {

View File

@ -0,0 +1,121 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package gitdiff
import (
"bufio"
"bytes"
"fmt"
"html/template"
"io"
"code.gitea.io/gitea/modules/setting"
)
type BlobExcerptOptions struct {
LastLeft int
LastRight int
LeftIndex int
RightIndex int
LeftHunkSize int
RightHunkSize int
Direction string
Language string
}
func fillExcerptLines(section *DiffSection, filePath string, reader io.Reader, lang string, idxLeft, idxRight, chunkSize int) error {
buf := &bytes.Buffer{}
scanner := bufio.NewScanner(reader)
var diffLines []*DiffLine
for line := 0; line < idxRight+chunkSize; line++ {
if ok := scanner.Scan(); !ok {
break
}
lineText := scanner.Text()
if buf.Len()+len(lineText) < int(setting.UI.MaxDisplayFileSize) {
buf.WriteString(lineText)
buf.WriteByte('\n')
}
if line < idxRight {
continue
}
diffLine := &DiffLine{
LeftIdx: idxLeft + (line - idxRight) + 1,
RightIdx: line + 1,
Type: DiffLinePlain,
Content: " " + lineText,
}
diffLines = append(diffLines, diffLine)
}
if err := scanner.Err(); err != nil {
return fmt.Errorf("fillExcerptLines scan: %w", err)
}
section.Lines = diffLines
// DiffLinePlain always uses right lines
section.highlightedRightLines.value = highlightCodeLines(filePath, lang, []*DiffSection{section}, false /* right */, buf.Bytes())
return nil
}
func BuildBlobExcerptDiffSection(filePath string, reader io.Reader, opts BlobExcerptOptions) (*DiffSection, error) {
lastLeft, lastRight, idxLeft, idxRight := opts.LastLeft, opts.LastRight, opts.LeftIndex, opts.RightIndex
leftHunkSize, rightHunkSize, direction := opts.LeftHunkSize, opts.RightHunkSize, opts.Direction
language := opts.Language
chunkSize := BlobExcerptChunkSize
section := &DiffSection{
language: &diffVarMutable[string]{value: language},
highlightedLeftLines: &diffVarMutable[map[int]template.HTML]{},
highlightedRightLines: &diffVarMutable[map[int]template.HTML]{},
FileName: filePath,
}
var err error
if direction == "up" && (idxLeft-lastLeft) > chunkSize {
idxLeft -= chunkSize
idxRight -= chunkSize
leftHunkSize += chunkSize
rightHunkSize += chunkSize
err = fillExcerptLines(section, filePath, reader, language, idxLeft-1, idxRight-1, chunkSize)
} else if direction == "down" && (idxLeft-lastLeft) > chunkSize {
err = fillExcerptLines(section, filePath, reader, language, lastLeft, lastRight, chunkSize)
lastLeft += chunkSize
lastRight += chunkSize
} else {
offset := -1
if direction == "down" {
offset = 0
}
err = fillExcerptLines(section, filePath, reader, language, lastLeft, lastRight, idxRight-lastRight+offset)
leftHunkSize = 0
rightHunkSize = 0
idxLeft = lastLeft
idxRight = lastRight
}
if err != nil {
return nil, err
}
newLineSection := &DiffLine{
Type: DiffLineSection,
SectionInfo: &DiffLineSectionInfo{
language: &diffVarMutable[string]{value: opts.Language},
Path: filePath,
LastLeftIdx: lastLeft,
LastRightIdx: lastRight,
LeftIdx: idxLeft,
RightIdx: idxRight,
LeftHunkSize: leftHunkSize,
RightHunkSize: rightHunkSize,
},
}
if newLineSection.GetExpandDirection() != "" {
newLineSection.Content = fmt.Sprintf("@@ -%d,%d +%d,%d @@\n", idxLeft, leftHunkSize, idxRight, rightHunkSize)
switch direction {
case "up":
section.Lines = append([]*DiffLine{newLineSection}, section.Lines...)
case "down":
section.Lines = append(section.Lines, newLineSection)
}
}
return section, nil
}

View File

@ -0,0 +1,39 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package gitdiff
import (
"bytes"
"strconv"
"testing"
"code.gitea.io/gitea/modules/translation"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestBuildBlobExcerptDiffSection(t *testing.T) {
data := &bytes.Buffer{}
for i := range 100 {
data.WriteString("a = " + strconv.Itoa(i+1) + "\n")
}
locale := translation.MockLocale{}
lineMiddle := 50
diffSection, err := BuildBlobExcerptDiffSection("a.py", bytes.NewReader(data.Bytes()), BlobExcerptOptions{
LeftIndex: lineMiddle,
RightIndex: lineMiddle,
LeftHunkSize: 10,
RightHunkSize: 10,
Direction: "up",
})
require.NoError(t, err)
assert.Len(t, diffSection.highlightedRightLines.value, BlobExcerptChunkSize)
assert.NotEmpty(t, diffSection.highlightedRightLines.value[lineMiddle-BlobExcerptChunkSize-1])
assert.NotEmpty(t, diffSection.highlightedRightLines.value[lineMiddle-2]) // 0-based
diffInline := diffSection.GetComputedInlineDiffFor(diffSection.Lines[1], locale)
assert.Equal(t, `<span class="n">a</span> <span class="o">=</span> <span class="mi">30</span>`+"\n", string(diffInline.Content))
}

View File

@ -1111,22 +1111,20 @@ func TestDiffLine_GetExpandDirection(t *testing.T) {
func TestHighlightCodeLines(t *testing.T) {
t.Run("CharsetDetecting", func(t *testing.T) {
diffFile := &DiffFile{
Name: "a.c",
Language: "c",
Name: "a.c",
Sections: []*DiffSection{
{
Lines: []*DiffLine{{LeftIdx: 1}},
},
},
}
ret := highlightCodeLines(diffFile, true, []byte("// abc\xcc def\xcd")) // ISO-8859-1 bytes
ret := highlightCodeLinesForDiffFile(diffFile, true, []byte("// abc\xcc def\xcd")) // ISO-8859-1 bytes
assert.Equal(t, "<span class=\"c1\">// abcÌ defÍ\n</span>", string(ret[0]))
})
t.Run("LeftLines", func(t *testing.T) {
diffFile := &DiffFile{
Name: "a.c",
Language: "c",
Name: "a.c",
Sections: []*DiffSection{
{
Lines: []*DiffLine{
@ -1138,7 +1136,7 @@ func TestHighlightCodeLines(t *testing.T) {
},
}
const nl = "\n"
ret := highlightCodeLines(diffFile, true, []byte("a\nb\n"))
ret := highlightCodeLinesForDiffFile(diffFile, true, []byte("a\nb\n"))
assert.Equal(t, map[int]template.HTML{
0: `<span class="n">a</span>` + nl,
1: `<span class="n">b</span>`,

View File

@ -23,7 +23,7 @@ func extractDiffTokenRemainingFullTag(s string) (token, after string, valid bool
// keep in mind: even if we'd like to relax this check,
// we should never ignore "&" because it is for HTML entity and can't be safely used in the diff algorithm,
// because diff between "&lt;" and "&gt;" will generate broken result.
isSymbolChar := 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' || c == '_' || c == '-'
isSymbolChar := 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' || c == '_' || c == '-' || c == '.'
if !isSymbolChar {
return "", s, false
}
@ -40,7 +40,7 @@ func extractDiffTokenRemainingFullTag(s string) (token, after string, valid bool
// Returned token:
// * full tag with content: "<<span>content</span>>", it is used to optimize diff results to highlight the whole changed symbol
// * opening/close tag: "<span ...>" or "</span>"
// * opening/closing tag: "<span ...>" or "</span>"
// * HTML entity: "&lt;"
func extractDiffToken(s string) (before, token, after string, valid bool) {
for pos1 := 0; pos1 < len(s); pos1++ {
@ -123,6 +123,25 @@ func (hcd *highlightCodeDiff) collectUsedRunes(code template.HTML) {
}
}
func (hcd *highlightCodeDiff) diffEqualPartIsSpaceOnly(s string) bool {
for _, r := range s {
if r >= hcd.placeholderBegin {
recovered := hcd.placeholderTokenMap[r]
if strings.HasPrefix(recovered, "<<") {
return false // a full tag with content, it can't be space-only
} else if strings.HasPrefix(recovered, "<") {
continue // a single opening/closing tag, skip the tag and continue to check the content
}
return false // otherwise, it must be an HTML entity, it can't be space-only
}
isSpace := r == ' ' || r == '\t' || r == '\n' || r == '\r'
if !isSpace {
return false
}
}
return true
}
func (hcd *highlightCodeDiff) diffLineWithHighlight(lineType DiffLineType, codeA, codeB template.HTML) template.HTML {
hcd.collectUsedRunes(codeA)
hcd.collectUsedRunes(codeB)
@ -142,7 +161,21 @@ func (hcd *highlightCodeDiff) diffLineWithHighlight(lineType DiffLineType, codeA
removedCodePrefix := hcd.registerTokenAsPlaceholder(`<span class="removed-code">`)
removedCodeSuffix := hcd.registerTokenAsPlaceholder(`</span><!-- removed-code -->`)
if removedCodeSuffix != 0 {
equalPartSpaceOnly := true
for _, diff := range diffs {
if diff.Type != diffmatchpatch.DiffEqual {
continue
}
if equalPartSpaceOnly = hcd.diffEqualPartIsSpaceOnly(diff.Text); !equalPartSpaceOnly {
break
}
}
// only add "added"/"removed" tags when needed:
// * non-space contents appear in the DiffEqual parts (not a full-line add/del)
// * placeholder map still works (not exhausted, can get removedCodeSuffix)
addDiffTags := !equalPartSpaceOnly && removedCodeSuffix != 0
if addDiffTags {
for _, diff := range diffs {
switch {
case diff.Type == diffmatchpatch.DiffEqual:
@ -158,7 +191,7 @@ func (hcd *highlightCodeDiff) diffLineWithHighlight(lineType DiffLineType, codeA
}
}
} else {
// placeholder map space is exhausted
// the caller will still add added/removed backgrounds for the whole line
for _, diff := range diffs {
take := diff.Type == diffmatchpatch.DiffEqual || (diff.Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd) || (diff.Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel)
if take {
@ -186,14 +219,7 @@ func (hcd *highlightCodeDiff) convertToPlaceholders(htmlContent template.HTML) s
var tagStack []string
res := strings.Builder{}
htmlCode := strings.TrimSpace(string(htmlContent))
// the standard chroma highlight HTML is `<span class="line [hl]"><span class="cl"> ... </span></span>`
// the line wrapper tags should be removed before diff
if strings.HasPrefix(htmlCode, `<span class="line`) || strings.HasPrefix(htmlCode, `<span class="cl"`) {
htmlCode = strings.TrimSuffix(htmlCode, "</span>")
}
htmlCode := string(htmlContent)
var beforeToken, token string
var valid bool
for {
@ -204,10 +230,16 @@ func (hcd *highlightCodeDiff) convertToPlaceholders(htmlContent template.HTML) s
// write the content before the token into result string, and consume the token in the string
res.WriteString(beforeToken)
// the standard chroma highlight HTML is `<span class="line [hl]"><span class="cl"> ... </span></span>`
// the line wrapper tags should be removed before diff
if strings.HasPrefix(token, `<span class="line`) || strings.HasPrefix(token, `<span class="cl"`) {
continue
}
var tokenInMap string
if strings.HasPrefix(token, "</") { // for closing tag
if len(tagStack) == 0 {
break // invalid diff result, no opening tag but see closing tag
continue // no opening tag but see closing tag, skip it
}
// make sure the closing tag in map is related to the open tag, to make the diff algorithm can match the opening/closing tags
// the closing tag will be recorded in the map by key "</span><!-- <span the-opening> -->" for "<span the-opening>"

View File

@ -14,25 +14,57 @@ import (
"github.com/stretchr/testify/assert"
)
func TestDiffWithHighlight(t *testing.T) {
t.Run("DiffLineAddDel", func(t *testing.T) {
func BenchmarkHighlightDiff(b *testing.B) {
for b.Loop() {
// still fast enough: BenchmarkHighlightDiff-12 1000000 1027 ns/op
// TODO: the real bottleneck is that "diffLineWithHighlight" is called twice when rendering "added" and "removed" lines by the caller
// Ideally the caller should cache the diff result, and then use the diff result to render "added" and "removed" lines separately
hcd := newHighlightCodeDiff()
codeA := template.HTML(`x <span class="k">foo</span> y`)
codeB := template.HTML(`x <span class="k">bar</span> y`)
outDel := hcd.diffLineWithHighlight(DiffLineDel, codeA, codeB)
assert.Equal(t, `x <span class="removed-code"><span class="k">foo</span></span> y`, string(outDel))
outAdd := hcd.diffLineWithHighlight(DiffLineAdd, codeA, codeB)
assert.Equal(t, `x <span class="added-code"><span class="k">bar</span></span> y`, string(outAdd))
hcd.diffLineWithHighlight(DiffLineDel, codeA, codeB)
}
}
func TestDiffWithHighlight(t *testing.T) {
t.Run("DiffLineAddDel", func(t *testing.T) {
t.Run("WithDiffTags", func(t *testing.T) {
hcd := newHighlightCodeDiff()
codeA := template.HTML(`x <span class="k">foo</span> y`)
codeB := template.HTML(`x <span class="k">bar</span> y`)
outDel := hcd.diffLineWithHighlight(DiffLineDel, codeA, codeB)
assert.Equal(t, `x <span class="removed-code"><span class="k">foo</span></span> y`, string(outDel))
outAdd := hcd.diffLineWithHighlight(DiffLineAdd, codeA, codeB)
assert.Equal(t, `x <span class="added-code"><span class="k">bar</span></span> y`, string(outAdd))
})
t.Run("NoRedundantTags", func(t *testing.T) {
// the equal parts only contain spaces, in this case, don't use "added/removed" tags
// because the diff lines already have a background color to indicate the change
hcd := newHighlightCodeDiff()
codeA := template.HTML("<span> </span> \t<span>foo</span> ")
codeB := template.HTML(" <span>bar</span> \n")
outDel := hcd.diffLineWithHighlight(DiffLineDel, codeA, codeB)
assert.Equal(t, string(codeA), string(outDel))
outAdd := hcd.diffLineWithHighlight(DiffLineAdd, codeA, codeB)
assert.Equal(t, string(codeB), string(outAdd))
})
})
t.Run("CleanUp", func(t *testing.T) {
hcd := newHighlightCodeDiff()
codeA := template.HTML(`<span class="cm">this is a comment</span>`)
codeB := template.HTML(`<span class="cm">this is updated comment</span>`)
codeA := template.HTML(` <span class="cm">this is a comment</span>`)
codeB := template.HTML(` <span class="cm">this is updated comment</span>`)
outDel := hcd.diffLineWithHighlight(DiffLineDel, codeA, codeB)
assert.Equal(t, `<span class="cm">this is <span class="removed-code">a</span> comment</span>`, string(outDel))
assert.Equal(t, ` <span class="cm">this is <span class="removed-code">a</span> comment</span>`, string(outDel))
outAdd := hcd.diffLineWithHighlight(DiffLineAdd, codeA, codeB)
assert.Equal(t, `<span class="cm">this is <span class="added-code">updated</span> comment</span>`, string(outAdd))
assert.Equal(t, ` <span class="cm">this is <span class="added-code">updated</span> comment</span>`, string(outAdd))
codeA = `<span class="line"><span>line1</span></span>` + "\n" + `<span class="cl"><span>line2</span></span>`
codeB = `<span class="cl"><span>line1</span></span>` + "\n" + `<span class="line"><span>line!</span></span>`
outDel = hcd.diffLineWithHighlight(DiffLineDel, codeA, codeB)
assert.Equal(t, `<span>line1</span>`+"\n"+`<span class="removed-code"><span>line2</span></span>`, string(outDel))
outAdd = hcd.diffLineWithHighlight(DiffLineAdd, codeA, codeB)
assert.Equal(t, `<span>line1</span>`+"\n"+`<span class="added-code"><span>line!</span></span>`, string(outAdd))
})
t.Run("OpenCloseTags", func(t *testing.T) {

View File

@ -12,7 +12,7 @@ import (
)
// GetDiffPreview produces and returns diff result of a file which is not yet committed.
func GetDiffPreview(ctx context.Context, repo *repo_model.Repository, branch, treePath, content string) (*gitdiff.Diff, error) {
func GetDiffPreview(ctx context.Context, repo *repo_model.Repository, branch, treePath, oldContent, newContent string) (*gitdiff.Diff, error) {
if branch == "" {
branch = repo.DefaultBranch
}
@ -29,7 +29,7 @@ func GetDiffPreview(ctx context.Context, repo *repo_model.Repository, branch, tr
}
// Add the object to the database
objectHash, err := t.HashObjectAndWrite(ctx, strings.NewReader(content))
objectHash, err := t.HashObjectAndWrite(ctx, strings.NewReader(newContent))
if err != nil {
return nil, err
}
@ -38,5 +38,5 @@ func GetDiffPreview(ctx context.Context, repo *repo_model.Repository, branch, tr
if err := t.AddObjectToIndex(ctx, "100644", objectHash, treePath); err != nil {
return nil, err
}
return t.DiffIndex(ctx)
return t.DiffIndex(ctx, oldContent, newContent)
}

View File

@ -27,8 +27,30 @@ func TestGetDiffPreview(t *testing.T) {
branch := ctx.Repo.Repository.DefaultBranch
treePath := "README.md"
oldContent := "# repo1\n\nDescription for repo1"
content := "# repo1\n\nDescription for repo1\nthis is a new line"
t.Run("Errors", func(t *testing.T) {
t.Run("empty repo", func(t *testing.T) {
diff, err := GetDiffPreview(ctx, &repo_model.Repository{}, branch, treePath, oldContent, content)
assert.Nil(t, diff)
assert.EqualError(t, err, "repository does not exist [id: 0, uid: 0, owner_name: , name: ]")
})
t.Run("bad branch", func(t *testing.T) {
badBranch := "bad_branch"
diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, badBranch, treePath, oldContent, content)
assert.Nil(t, diff)
assert.EqualError(t, err, "branch does not exist [name: "+badBranch+"]")
})
t.Run("empty treePath", func(t *testing.T) {
diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, branch, "", oldContent, content)
assert.Nil(t, diff)
assert.EqualError(t, err, "path is invalid [path: ]")
})
})
expectedDiff := &gitdiff.Diff{
Files: []*gitdiff.DiffFile{
{
@ -112,56 +134,22 @@ func TestGetDiffPreview(t *testing.T) {
}
t.Run("with given branch", func(t *testing.T) {
diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, branch, treePath, content)
diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, branch, treePath, oldContent, content)
assert.NoError(t, err)
expectedBs, err := json.Marshal(expectedDiff)
assert.NoError(t, err)
bs, err := json.Marshal(diff)
assert.NoError(t, err)
assert.Equal(t, string(expectedBs), string(bs))
assert.JSONEq(t, string(expectedBs), string(bs))
})
t.Run("empty branch, same results", func(t *testing.T) {
diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, "", treePath, content)
diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, "", treePath, oldContent, content)
assert.NoError(t, err)
expectedBs, err := json.Marshal(expectedDiff)
assert.NoError(t, err)
bs, err := json.Marshal(diff)
assert.NoError(t, err)
assert.Equal(t, expectedBs, bs)
})
}
func TestGetDiffPreviewErrors(t *testing.T) {
unittest.PrepareTestEnv(t)
ctx, _ := contexttest.MockContext(t, "user2/repo1")
ctx.SetPathParam("id", "1")
contexttest.LoadRepo(t, ctx, 1)
contexttest.LoadRepoCommit(t, ctx)
contexttest.LoadUser(t, ctx, 2)
contexttest.LoadGitRepo(t, ctx)
defer ctx.Repo.GitRepo.Close()
branch := ctx.Repo.Repository.DefaultBranch
treePath := "README.md"
content := "# repo1\n\nDescription for repo1\nthis is a new line"
t.Run("empty repo", func(t *testing.T) {
diff, err := GetDiffPreview(ctx, &repo_model.Repository{}, branch, treePath, content)
assert.Nil(t, diff)
assert.EqualError(t, err, "repository does not exist [id: 0, uid: 0, owner_name: , name: ]")
})
t.Run("bad branch", func(t *testing.T) {
badBranch := "bad_branch"
diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, badBranch, treePath, content)
assert.Nil(t, diff)
assert.EqualError(t, err, "branch does not exist [name: "+badBranch+"]")
})
t.Run("empty treePath", func(t *testing.T) {
diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, branch, "", content)
assert.Nil(t, diff)
assert.EqualError(t, err, "path is invalid [path: ]")
assert.JSONEq(t, string(expectedBs), string(bs))
})
}

View File

@ -361,7 +361,7 @@ func (t *TemporaryUploadRepository) Push(ctx context.Context, doer *user_model.U
}
// DiffIndex returns a Diff of the current index to the head
func (t *TemporaryUploadRepository) DiffIndex(ctx context.Context) (*gitdiff.Diff, error) {
func (t *TemporaryUploadRepository) DiffIndex(ctx context.Context, oldContent, newContent string) (*gitdiff.Diff, error) {
var diff *gitdiff.Diff
cmd := gitcmd.NewCommand("diff-index", "--src-prefix=\\a/", "--dst-prefix=\\b/", "--cached", "-p", "HEAD")
stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe()
@ -383,6 +383,9 @@ func (t *TemporaryUploadRepository) DiffIndex(ctx context.Context) (*gitdiff.Dif
return nil, fmt.Errorf("unable to run diff-index pipeline in temporary repo: %w", err)
}
if len(diff.Files) > 0 {
gitdiff.FillDiffFileHighlightLinesByContent(diff.Files[0], util.UnsafeStringToBytes(oldContent), util.UnsafeStringToBytes(newContent))
}
return diff, nil
}

View File

@ -148,10 +148,10 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra
func testEditorDiffPreview(t *testing.T) {
session := loginUser(t, "user2")
req := NewRequestWithValues(t, "POST", "/user2/repo1/_preview/master/README.md", map[string]string{
"content": "Hello, World (Edited)\n",
"content": "# repo1 (Edited)",
})
resp := session.MakeRequest(t, req, http.StatusOK)
assert.Contains(t, resp.Body.String(), `<span class="added-code">Hello, World (Edited)</span>`)
assert.Contains(t, resp.Body.String(), `<span class="added-code"> (Edited)</span>`)
}
func testEditorPatchFile(t *testing.T) {

View File

@ -36,6 +36,6 @@ func TestListPullCommits(t *testing.T) {
defer tests.PrintCurrentTest(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(), `<td class="lines-code lines-code-new"><code class="code-inner"># repo1</code>`)
assert.Contains(t, resp.Body.String(), `<td class="lines-code lines-code-new"><code class="code-inner"><span class="gh"># repo1`+"\n"+`</span></code>`)
})
}