From ae176cd6493480f27b138fb77701d6ac251ebe2c Mon Sep 17 00:00:00 2001 From: YumeMichi Date: Sun, 19 Jul 2026 11:20:01 +0800 Subject: [PATCH] refactor: clean up fragile diff render templates, use backend typed structs (#38517) Blob.Size requires a context after the repository context removal (5b078f72aa). Actually the template code should just render, it should not depend on the fragile dynamic calls to backend functions. --------- Co-authored-by: wxiaoguang --- modules/git/blob.go | 23 ++---- routers/web/repo/blame.go | 2 +- routers/web/repo/compare.go | 42 ---------- services/gitdiff/gitdiff.go | 116 ++++++++++++++++++++-------- templates/repo/diff/box.tmpl | 14 ++-- templates/repo/diff/csv_diff.tmpl | 4 +- templates/repo/diff/image_diff.tmpl | 26 ++++--- 7 files changed, 115 insertions(+), 112 deletions(-) diff --git a/modules/git/blob.go b/modules/git/blob.go index 2f3e56529fe..0565353ef7f 100644 --- a/modules/git/blob.go +++ b/modules/git/blob.go @@ -12,7 +12,6 @@ import ( "io" "strings" - "gitea.dev/modules/typesniffer" "gitea.dev/modules/util" ) @@ -44,28 +43,29 @@ func (b *Blob) GetBlobContent(ctx context.Context, limit int64) (string, 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(ctx context.Context, w io.Writer) (int, error) { +func (b *Blob) GetBlobLineCount(ctx context.Context, w io.Writer) (size int64, count int, _ error) { reader, err := b.DataAsync(ctx) if err != nil { - return 0, err + return 0, 0, err } defer reader.Close() buf := make([]byte, 32*1024) - count := 1 + size, count = 0, 1 lineSep := []byte{'\n'} for { c, err := reader.Read(buf) + size += int64(c) if w != nil { if _, err := w.Write(buf[:c]); err != nil { - return count, err + return size, count, err } } count += bytes.Count(buf[:c], lineSep) switch { case errors.Is(err, io.EOF): - return count, nil + return size, count, nil case err != nil: - return count, err + return size, count, err } } } @@ -102,12 +102,3 @@ loop: _ = encoder.Close() return base64buf.String(), nil } - -// GuessContentType guesses the content type of the blob. -func (b *Blob) GuessContentType(ctx context.Context) (typesniffer.SniffedType, error) { - buf, err := b.GetBlobBytes(ctx, typesniffer.SniffContentSize) - if err != nil { - return typesniffer.SniffedType{}, err - } - return typesniffer.DetectContentType(buf), nil -} diff --git a/routers/web/repo/blame.go b/routers/web/repo/blame.go index 713767134f0..9b656ad0c16 100644 --- a/routers/web/repo/blame.go +++ b/routers/web/repo/blame.go @@ -73,7 +73,7 @@ func RefBlame(ctx *context.Context) { return } - ctx.Data["NumLines"], err = blob.GetBlobLineCount(ctx, nil) + _, ctx.Data["NumLines"], err = blob.GetBlobLineCount(ctx, nil) if err != nil { ctx.NotFound(err) return diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 3cf42d47c12..5ee4aa7a744 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -10,7 +10,6 @@ import ( "io" "net/http" "net/url" - "path/filepath" "sort" "strings" "unicode" @@ -35,7 +34,6 @@ import ( "gitea.dev/modules/setting" api "gitea.dev/modules/structs" "gitea.dev/modules/templates" - "gitea.dev/modules/typesniffer" "gitea.dev/modules/util" "gitea.dev/routers/common" "gitea.dev/services/context" @@ -56,35 +54,7 @@ func setCompareContext(ctx *context.Context, before, head *git.Commit, headOwner ctx.Data["BeforeCommit"] = before ctx.Data["HeadCommit"] = head - ctx.Data["GetBlobByPathForCommit"] = func(commit *git.Commit, path string) *git.Blob { - if commit == nil { - return nil - } - - blob, err := commit.GetBlobByPath(ctx, ctx.Repo.GitRepo, path) - if err != nil { - return nil - } - return blob - } - - ctx.Data["GetSniffedTypeForBlob"] = func(blob *git.Blob) typesniffer.SniffedType { - st := typesniffer.SniffedType{} - - if blob == nil { - return st - } - - st, err := blob.GuessContentType(ctx) - if err != nil { - log.Error("GuessContentType failed: %v", err) - return st - } - return st - } - setPathsCompareContext(ctx, before, head, headOwner, headName) - setImageCompareContext(ctx) setCsvCompareContext(ctx) } @@ -108,20 +78,8 @@ func setPathsCompareContext(ctx *context.Context, base, head *git.Commit, headOw } } -// setImageCompareContext sets context data that is required by image compare template -func setImageCompareContext(ctx *context.Context) { - ctx.Data["IsSniffedTypeAnImage"] = func(st typesniffer.SniffedType) bool { - return st.IsImage() && (setting.UI.SVG.Enabled || !st.IsSvgImage()) - } -} - // setCsvCompareContext sets context data that is required by the CSV compare template func setCsvCompareContext(ctx *context.Context) { - ctx.Data["IsCsvFile"] = func(diffFile *gitdiff.DiffFile) bool { - extension := strings.ToLower(filepath.Ext(diffFile.Name)) - return extension == ".csv" || extension == ".tsv" - } - type CsvDiffResult struct { Sections []*gitdiff.TableDiffSection Error string diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 466c623f6a4..1effa754037 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -38,6 +38,7 @@ import ( "gitea.dev/modules/setting" "gitea.dev/modules/svg" "gitea.dev/modules/translation" + "gitea.dev/modules/typesniffer" "gitea.dev/modules/util" "github.com/alecthomas/chroma/v2" @@ -465,6 +466,14 @@ type DiffFile struct { highlightRender diffVarMutable[chroma.Lexer] // cache render (atm: lexer) for current file, only detect once for line-by-line mode highlightedLeftLines diffVarMutable[map[int]template.HTML] highlightedRightLines diffVarMutable[map[int]template.HTML] + + // image diff and csv diff need some of the following fields + LeftBlob, RightBlob *git.Blob + LeftBlobSize, RightBlobSize int64 + LeftBlobMimeType, RightBlobMimeType string + + IsBlobTypeImage bool + IsBlobTypeCsv bool } // GetType returns type of diff file. @@ -472,31 +481,64 @@ func (diffFile *DiffFile) GetType() int { return int(diffFile.Type) } -type DiffLimitedContent struct { - LeftContent, RightContent *limitByteWriter +type DiffRenderDetail struct { + needTailSection bool + leftLineCount, rightLineCount int + leftContent, rightContent *limitByteWriter } -// GetTailSectionAndLimitedContent creates a fake DiffLineSection if the last section is not the end of the file -func (diffFile *DiffFile) GetTailSectionAndLimitedContent(ctx context.Context, gitRepo *git.Repository, leftCommit, rightCommit *git.Commit) (_ *DiffSection, diffLimitedContent DiffLimitedContent) { - var leftLineCount, rightLineCount int - diffLimitedContent = DiffLimitedContent{} - if diffFile.IsBin || diffFile.IsLFSFile { - return nil, diffLimitedContent +func (diffFile *DiffFile) prepareDiffRenderDetail(ctx context.Context, gitRepo *git.Repository, leftCommit, rightCommit *git.Commit) (ret DiffRenderDetail) { + if diffFile.IsLFSFile { + return ret } + + // pre-fetch the blob info & content + // * for "bin" type: need the pre-fetched buffer to detect content type (e.g.: help to render image diff) + // * for "text" type: need to read up to "highlight limit size" to do full-file-highlighting + contentLimit := util.Iif(diffFile.IsBin, typesniffer.SniffContentSize, MaxFullFileHighlightSizeLimit) + var leftLineCount, rightLineCount int + var leftBlobType, rightBlobType typesniffer.SniffedType if (diffFile.Type == DiffFileDel || diffFile.Type == DiffFileChange) && leftCommit != nil { - leftLineCount, diffLimitedContent.LeftContent = getCommitFileLineCountAndLimitedContent(ctx, gitRepo, leftCommit, diffFile.OldName) + c := getCommitFileBlobAndLimitedContent(ctx, gitRepo, leftCommit, diffFile.OldName, contentLimit) + diffFile.LeftBlob, diffFile.LeftBlobSize, leftLineCount, ret.leftContent = c.gitBlob, c.blobSize, c.lineCount, c.limitedContent + leftBlobType = typesniffer.DetectContentType(ret.leftContent.buf.Bytes()) } if (diffFile.Type == DiffFileAdd || diffFile.Type == DiffFileChange) && rightCommit != nil { - rightLineCount, diffLimitedContent.RightContent = getCommitFileLineCountAndLimitedContent(ctx, gitRepo, rightCommit, diffFile.OldName) + c := getCommitFileBlobAndLimitedContent(ctx, gitRepo, rightCommit, diffFile.OldName, contentLimit) + diffFile.RightBlob, diffFile.RightBlobSize, rightLineCount, ret.rightContent = c.gitBlob, c.blobSize, c.lineCount, c.limitedContent + rightBlobType = typesniffer.DetectContentType(ret.rightContent.buf.Bytes()) } - if len(diffFile.Sections) == 0 || diffFile.Type != DiffFileChange { - return nil, diffLimitedContent + + isFileTypeImage := func(st typesniffer.SniffedType) bool { + return st.IsImage() && (setting.UI.SVG.Enabled || !st.IsSvgImage()) } + isFileTypeCsv := func(name string) bool { + extension := strings.ToLower(path.Ext(name)) + return extension == ".csv" || extension == ".tsv" + } + diffFile.LeftBlobMimeType, diffFile.RightBlobMimeType = leftBlobType.GetMimeType(), rightBlobType.GetMimeType() + diffFile.IsBlobTypeImage = isFileTypeImage(leftBlobType) || isFileTypeImage(rightBlobType) + diffFile.IsBlobTypeCsv = isFileTypeCsv(diffFile.Name) + + if diffFile.IsBin || len(diffFile.Sections) == 0 || diffFile.Type != DiffFileChange { + return ret + } + + // check whether the text file diff needs a tail section lastSection := diffFile.Sections[len(diffFile.Sections)-1] lastLine := lastSection.Lines[len(lastSection.Lines)-1] if leftLineCount <= lastLine.LeftIdx || rightLineCount <= lastLine.RightIdx { - return nil, diffLimitedContent + return ret } + ret.needTailSection = true + return ret +} + +func (diffFile *DiffFile) addTailSection(detail DiffRenderDetail) { + // if the last diff section still doesn't reach to the end of file, we need to add a "tail section" to + // make users can expand the remaining unchanged lines to see the full file content. + lastSection := diffFile.Sections[len(diffFile.Sections)-1] + lastLine := lastSection.Lines[len(lastSection.Lines)-1] tailDiffLine := &DiffLine{ Type: DiffLineSection, Content: " ", @@ -505,12 +547,12 @@ func (diffFile *DiffFile) GetTailSectionAndLimitedContent(ctx context.Context, g Path: diffFile.Name, LastLeftIdx: lastLine.LeftIdx, LastRightIdx: lastLine.RightIdx, - LeftIdx: leftLineCount, - RightIdx: rightLineCount, + LeftIdx: detail.leftLineCount, + RightIdx: detail.rightLineCount, }, } tailSection := &DiffSection{FileName: diffFile.Name, Lines: []*DiffLine{tailDiffLine}} - return tailSection, diffLimitedContent + diffFile.Sections = append(diffFile.Sections, tailSection) } // GetDiffFileName returns the name of the diff file, or its old name in case it was deleted @@ -577,17 +619,24 @@ func (l *limitByteWriter) Write(p []byte) (n int, err error) { return l.buf.Write(p) } -func getCommitFileLineCountAndLimitedContent(ctx context.Context, gitRepo *git.Repository, commit *git.Commit, filePath string) (lineCount int, limitWriter *limitByteWriter) { - blob, err := commit.GetBlobByPath(ctx, gitRepo, filePath) +func getCommitFileBlobAndLimitedContent(ctx context.Context, gitRepo *git.Repository, commit *git.Commit, filePath string, limit int) (ret struct { + gitBlob *git.Blob + blobSize int64 + lineCount int + limitedContent *limitByteWriter +}, +) { + var err error + ret.limitedContent = &limitByteWriter{limit: limit} + ret.gitBlob, err = commit.GetBlobByPath(ctx, gitRepo, filePath) if err != nil { - return 0, nil + return ret } - w := &limitByteWriter{limit: MaxFullFileHighlightSizeLimit + 1} - lineCount, err = blob.GetBlobLineCount(ctx, w) + ret.blobSize, ret.lineCount, err = ret.gitBlob.GetBlobLineCount(ctx, ret.limitedContent) if err != nil { - return 0, nil + return ret } - return lineCount, w + return ret } // Diff represents a difference between two git trees. @@ -1374,19 +1423,22 @@ func GetDiffForRender(ctx context.Context, repoLink string, gitRepo *git.Reposit isGenerated = optional.Some(analyze.IsGenerated(diffFile.Name)) } diffFile.IsGenerated = isGenerated.Value() - tailSection, limitedContent := diffFile.GetTailSectionAndLimitedContent(ctx, gitRepo, beforeCommit, afterCommit) - if tailSection != nil { - diffFile.Sections = append(diffFile.Sections, tailSection) + + // prepare more details for the rendering, e.g.: blob (file) size, type, limited content to highlight, etc + renderDetail := diffFile.prepareDiffRenderDetail(ctx, gitRepo, beforeCommit, afterCommit) + if renderDetail.needTailSection { + diffFile.addTailSection(renderDetail) } - shouldFullFileHighlight := attrDiff.Value() == "" // only do highlight if no custom diff command + // only do highlight for text files which have no custom diff command + shouldFullFileHighlight := !diffFile.IsBin && !diffFile.IsLFSFile && attrDiff.Value() == "" shouldFullFileHighlight = shouldFullFileHighlight && time.Since(startTime) < MaxFullFileHighlightTimeLimit if shouldFullFileHighlight { - if limitedContent.LeftContent != nil { - diffFile.highlightedLeftLines.value = highlightCodeLinesForDiffFile(diffFile, true /* left */, limitedContent.LeftContent.buf.Bytes()) + if renderDetail.leftContent != nil { + diffFile.highlightedLeftLines.value = highlightCodeLinesForDiffFile(diffFile, true /* left */, renderDetail.leftContent.buf.Bytes()) } - if limitedContent.RightContent != nil { - diffFile.highlightedRightLines.value = highlightCodeLinesForDiffFile(diffFile, false /* right */, limitedContent.RightContent.buf.Bytes()) + if renderDetail.rightContent != nil { + diffFile.highlightedRightLines.value = highlightCodeLinesForDiffFile(diffFile, false /* right */, renderDetail.rightContent.buf.Bytes()) } } } @@ -1404,7 +1456,7 @@ func highlightCodeLinesForDiffFile(diffFile *DiffFile, isLeft bool, rawContent [ } func highlightCodeLines(name, lang string, sections []*DiffSection, isLeft bool, rawContent []byte) map[int]template.HTML { - if setting.Git.DisableDiffHighlight || len(rawContent) > MaxFullFileHighlightSizeLimit { + if setting.Git.DisableDiffHighlight || len(rawContent) >= MaxFullFileHighlightSizeLimit { return nil } content := util.UnsafeBytesToString(charset.ToUTF8(rawContent, charset.ConvertOpts{})) diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 463575b84d3..7f3090561cf 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -72,12 +72,8 @@
{{range $i, $file := .Diff.Files}} {{/*notice: the index of Diff.Files should not be used for element ID, because the index will be restarted from 0 when doing load-more for PRs with a lot of files*/}} - {{$blobBase := call $.GetBlobByPathForCommit $.BeforeCommit $file.OldName}} - {{$blobHead := call $.GetBlobByPathForCommit $.HeadCommit $file.Name}} - {{$sniffedTypeBase := call $.GetSniffedTypeForBlob $blobBase}} - {{$sniffedTypeHead := call $.GetSniffedTypeForBlob $blobHead}} - {{$isImage:= or (call $.IsSniffedTypeAnImage $sniffedTypeBase) (call $.IsSniffedTypeAnImage $sniffedTypeHead)}} - {{$isCsv := (call $.IsCsvFile $file)}} + {{$isImage:= $file.IsBlobTypeImage}} + {{$isCsv := $file.IsBlobTypeCsv}} {{$showFileViewToggle := or $isImage (and (not $file.IsIncomplete) $isCsv)}} {{$isExpandable := or (gt $file.Addition 0) (gt $file.Deletion 0) $file.IsBin}} {{$isReviewFile := and $.IsSigned $.PageIsPullFiles (not $.Repository.IsArchived) $.IsShowingAllCommits}} @@ -198,9 +194,9 @@
{{if $isImage}} - {{template "repo/diff/image_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead "sniffedTypeBase" $sniffedTypeBase "sniffedTypeHead" $sniffedTypeHead}} - {{else}} - {{template "repo/diff/csv_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead "sniffedTypeBase" $sniffedTypeBase "sniffedTypeHead" $sniffedTypeHead}} + {{template "repo/diff/image_diff" dict "file" $file}} + {{else if $isCsv}} + {{template "repo/diff/csv_diff" dict "file" $file}} {{end}}
diff --git a/templates/repo/diff/csv_diff.tmpl b/templates/repo/diff/csv_diff.tmpl index 0f46da306e7..1e1e568a58a 100644 --- a/templates/repo/diff/csv_diff.tmpl +++ b/templates/repo/diff/csv_diff.tmpl @@ -1,6 +1,8 @@ +{{$root := ctx.RootData}} +{{$file := $.file}} - {{$result := call .root.CreateCsvDiff .file .blobBase .blobHead}} + {{$result := call $root.CreateCsvDiff $file $file.LeftBlob $file.RightBlob}} {{if $result.Error}}
{{$result.Error}}
{{else if $result.Sections}} diff --git a/templates/repo/diff/image_diff.tmpl b/templates/repo/diff/image_diff.tmpl index 5a9dbc90674..affaeeb3e3d 100644 --- a/templates/repo/diff/image_diff.tmpl +++ b/templates/repo/diff/image_diff.tmpl @@ -1,16 +1,20 @@ -{{if or .blobBase .blobHead}} +{{$root := ctx.RootData}} +{{$file := $.file}} +{{$hasLeft := Iif $file.LeftBlobMimeType true false}} +{{$hasRight := Iif $file.RightBlobMimeType true false}} +{{if or $hasLeft $hasRight}}
{{ctx.Locale.Tr "repo.diff.image.side_by_side"}} - {{if and .blobBase .blobHead}} + {{if and $hasLeft $hasRight}} {{ctx.Locale.Tr "repo.diff.image.swipe"}} {{ctx.Locale.Tr "repo.diff.image.overlay"}} {{end}} @@ -19,7 +23,7 @@
- {{if .blobBase}} + {{if $hasLeft}}

{{ctx.Locale.Tr "repo.diff.file_before"}}

@@ -30,11 +34,11 @@ {{ctx.Locale.Tr "repo.diff.file_image_height"}}:  |  - {{ctx.Locale.Tr "repo.diff.file_byte_size"}}: {{FileSize .blobBase.Size}} + {{ctx.Locale.Tr "repo.diff.file_byte_size"}}: {{FileSize .file.LeftBlobSize}}

{{end}} - {{if .blobHead}} + {{if $hasRight}}

{{ctx.Locale.Tr "repo.diff.file_after"}}

@@ -45,13 +49,13 @@ {{ctx.Locale.Tr "repo.diff.file_image_height"}}:  |  - {{ctx.Locale.Tr "repo.diff.file_byte_size"}}: {{FileSize .blobHead.Size}} + {{ctx.Locale.Tr "repo.diff.file_byte_size"}}: {{FileSize .file.RightBlobSize}}

{{end}}
- {{if and .blobBase .blobHead}} + {{if and $hasLeft $hasRight}}