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 <wxiaoguang@gmail.com>
2026-07-21 08:31:19 +00:00 · 2026-07-19 11:20:01 +08:00
parent e8befe0268
commit ae176cd649
7 changed files with 115 additions and 112 deletions
--- 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
-}
--- 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
--- 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
--- 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{}))
--- a/templates/repo/diff/box.tmpl
+++ b/templates/repo/diff/box.tmpl
@@ -72,12 +72,8 @@
 			<div id="diff-file-boxes" class="sixteen wide column">
 				{{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 @@
 								<div id="diff-rendered-{{$file.NameHash}}" class="file-body file-code {{if $.IsSplitStyle}}code-diff-split{{else}}code-diff-unified{{end}} tw-overflow-x-scroll">
 									<table class="chroma tw-w-full">
 										{{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}}
 									</table>
 								</div>
--- a/templates/repo/diff/csv_diff.tmpl
+++ b/templates/repo/diff/csv_diff.tmpl
@@ -1,6 +1,8 @@
+{{$root := ctx.RootData}}
+{{$file := $.file}}
 <tr>
 	<td>
-		{{$result := call .root.CreateCsvDiff .file .blobBase .blobHead}}
+		{{$result := call $root.CreateCsvDiff $file $file.LeftBlob $file.RightBlob}}
 		{{if $result.Error}}
 			<div class="ui center">{{$result.Error}}</div>
 		{{else if $result.Sections}}
--- 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}}
 <tr>
 	<td colspan="2">
 		<div class="image-diff"
-			data-path-before="{{.root.BeforeRawPath}}/{{PathEscapeSegments .file.OldName}}"
-			data-path-after="{{.root.RawPath}}/{{PathEscapeSegments .file.Name}}"
-			data-mime-before="{{.sniffedTypeBase.GetMimeType}}"
-			data-mime-after="{{.sniffedTypeHead.GetMimeType}}"
+			data-path-before="{{$root.BeforeRawPath}}/{{PathEscapeSegments $file.OldName}}"
+			data-path-after="{{$root.RawPath}}/{{PathEscapeSegments $file.Name}}"
+			data-mime-before="{{$file.LeftBlobMimeType}}"
+			data-mime-after="{{$file.RightBlobMimeType}}"
 		>
 			<overflow-menu class="ui secondary pointing tabular menu">
 				<div class="overflow-menu-items tw-justify-center" data-global-init="initTabSwitcher">
 					<a class="item active" data-tab="diff-side-by-side-{{.file.NameHash}}">{{ctx.Locale.Tr "repo.diff.image.side_by_side"}}</a>
-					{{if and .blobBase .blobHead}}
+					{{if and $hasLeft $hasRight}}
 					<a class="item" data-tab="diff-swipe-{{.file.NameHash}}">{{ctx.Locale.Tr "repo.diff.image.swipe"}}</a>
 					<a class="item" data-tab="diff-overlay-{{.file.NameHash}}">{{ctx.Locale.Tr "repo.diff.image.overlay"}}</a>
 					{{end}}
@@ -19,7 +23,7 @@
 			<div class="image-diff-tabs is-loading">
 				<div class="ui bottom attached tab image-diff-container active" data-tab="diff-side-by-side-{{.file.NameHash}}">
 					<div class="diff-side-by-side">
-						{{if .blobBase}}
+						{{if $hasLeft}}
 						<span class="side">
 							<p class="side-header">{{ctx.Locale.Tr "repo.diff.file_before"}}</p>
 							<span class="before-container"><img alt class="image-before"></span>
@@ -30,11 +34,11 @@
 									{{ctx.Locale.Tr "repo.diff.file_image_height"}}: <span class="text bounds-info-height"></span>
 									&nbsp;|&nbsp;
 								</span>
-								{{ctx.Locale.Tr "repo.diff.file_byte_size"}}: <span class="text">{{FileSize .blobBase.Size}}</span>
+								{{ctx.Locale.Tr "repo.diff.file_byte_size"}}: <span class="text">{{FileSize .file.LeftBlobSize}}</span>
 							</p>
 						</span>
 						{{end}}
-						{{if .blobHead}}
+						{{if $hasRight}}
 						<span class="side">
 							<p class="side-header">{{ctx.Locale.Tr "repo.diff.file_after"}}</p>
 							<span class="after-container"><img alt class="image-after"></span>
@@ -45,13 +49,13 @@
 									{{ctx.Locale.Tr "repo.diff.file_image_height"}}: <span class="text bounds-info-height"></span>
 									&nbsp;|&nbsp;
 								</span>
-								{{ctx.Locale.Tr "repo.diff.file_byte_size"}}: <span class="text">{{FileSize .blobHead.Size}}</span>
+								{{ctx.Locale.Tr "repo.diff.file_byte_size"}}: <span class="text">{{FileSize .file.RightBlobSize}}</span>
 							</p>
 						</span>
 						{{end}}
 					</div>
 				</div>
-				{{if and .blobBase .blobHead}}
+				{{if and $hasLeft $hasRight}}
 				<div class="ui bottom attached tab image-diff-container" data-tab="diff-swipe-{{.file.NameHash}}">
 					<div class="diff-swipe">
 						<div class="swipe-frame">