enhance: truncate but show long lines in diffs (#39279)

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Sergio Benitez
2026-09-12 11:56:25 +00:00
committed by GitHub
co-authored by wxiaoguang
parent da37b7916b
commit 1e13badb39
13 changed files with 430 additions and 222 deletions
+12 -5
View File
@@ -8,6 +8,7 @@ import (
"io" "io"
"strings" "strings"
"gitea.dev/modules/htmlutil"
"gitea.dev/modules/setting" "gitea.dev/modules/setting"
"gitea.dev/modules/translation" "gitea.dev/modules/translation"
) )
@@ -27,13 +28,19 @@ func EscapeOptionsForView() EscapeOptions {
} }
} }
func EscapeControlHTMLTo(html template.HTML, locale translation.Locale, w htmlutil.HTMLWriter, opts ...EscapeOptions) *EscapeStatus {
if !setting.UI.AmbiguousUnicodeDetection {
w.WriteHTML(html)
return &EscapeStatus{}
}
escaped, _ := EscapeControlReader(strings.NewReader(string(html)), w.OriginWriter(), locale, opts...)
return escaped
}
// EscapeControlHTML escapes the Unicode control sequences in a provided html document // EscapeControlHTML escapes the Unicode control sequences in a provided html document
func EscapeControlHTML(html template.HTML, locale translation.Locale, opts ...EscapeOptions) (escaped *EscapeStatus, output template.HTML) { func EscapeControlHTML(html template.HTML, locale translation.Locale, opts ...EscapeOptions) (escaped *EscapeStatus, output template.HTML) {
if !setting.UI.AmbiguousUnicodeDetection { sb, w := htmlutil.NewHTMLStringWriter()
return &EscapeStatus{}, html escaped = EscapeControlHTMLTo(html, locale, w, opts...)
}
sb := &strings.Builder{}
escaped, _ = EscapeControlReader(strings.NewReader(string(html)), sb, locale, opts...) // err has been handled in EscapeControlReader
return escaped, template.HTML(sb.String()) return escaped, template.HTML(sb.String())
} }
+9 -1
View File
@@ -140,6 +140,14 @@ func isHeader(lof string, inHunk bool) bool {
return strings.HasPrefix(lof, cmdDiffHead) || (!inHunk && (strings.HasPrefix(lof, "---") || strings.HasPrefix(lof, "+++"))) return strings.HasPrefix(lof, cmdDiffHead) || (!inHunk && (strings.HasPrefix(lof, "---") || strings.HasPrefix(lof, "+++")))
} }
func NewGitDiffScanner(r io.Reader) *bufio.Scanner {
// TODO: GIT-DIFF-PARSE-LONG-LINE: ideally it shouldn't use bufio.Scanner which has a limit.
// It will cause errors if a line is very long.
scanner := bufio.NewScanner(r)
scanner.Buffer(nil, max(512*1024, int(setting.UI.MaxDisplayFileSize/16)))
return scanner
}
// CutDiffAroundLine cuts a diff of a file in way that only the given line + numberOfLine above it will be shown // CutDiffAroundLine cuts a diff of a file in way that only the given line + numberOfLine above it will be shown
// it also recalculates hunks and adds the appropriate headers to the new diff. // it also recalculates hunks and adds the appropriate headers to the new diff.
// Warning: Only one-file diffs are allowed. // Warning: Only one-file diffs are allowed.
@@ -149,7 +157,7 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi
return "", nil return "", nil
} }
scanner := bufio.NewScanner(originalDiff) scanner := NewGitDiffScanner(originalDiff)
hunk := make([]string, 0) hunk := make([]string, 0)
// begin is the start of the hunk containing searched line // begin is the start of the hunk containing searched line
+7 -2
View File
@@ -93,7 +93,7 @@ type HTMLWriter interface {
OriginWriter() io.Writer OriginWriter() io.Writer
WriteString(s string) HTMLWriter WriteString(s string) HTMLWriter
WriteHTML(s template.HTML) HTMLWriter WriteHTML(s template.HTML) HTMLWriter
WriteFormat(fmt template.HTML, args ...any) HTMLWriter WriteFormatf(fmt template.HTML, args ...any) HTMLWriter
Err() error Err() error
} }
@@ -120,7 +120,7 @@ func (h *htmlWriter) WriteHTML(s template.HTML) HTMLWriter {
return h return h
} }
func (h *htmlWriter) WriteFormat(fmt template.HTML, args ...any) HTMLWriter { func (h *htmlWriter) WriteFormatf(fmt template.HTML, args ...any) HTMLWriter {
if _, err := HTMLPrintf(h.w, fmt, args...); err != nil { if _, err := HTMLPrintf(h.w, fmt, args...); err != nil {
h.errs = append(h.errs, err) h.errs = append(h.errs, err)
} }
@@ -135,6 +135,11 @@ func NewHTMLWriter(w io.Writer) HTMLWriter {
return &htmlWriter{w: w} return &htmlWriter{w: w}
} }
func NewHTMLStringWriter() (*strings.Builder, HTMLWriter) {
sb := &strings.Builder{}
return sb, &htmlWriter{w: sb}
}
type HTMLBuilder struct { type HTMLBuilder struct {
sb strings.Builder sb strings.Builder
} }
+1 -1
View File
@@ -34,7 +34,7 @@ func TestHTMLBuilder(t *testing.T) {
func TestHTMLWriter(t *testing.T) { func TestHTMLWriter(t *testing.T) {
sb := new(strings.Builder) sb := new(strings.Builder)
w := NewHTMLWriter(sb) w := NewHTMLWriter(sb)
w.WriteString("<").WriteHTML("<hr>").WriteFormat("<span>%s%s</span>", ">", EscapeString(">")) w.WriteString("<").WriteHTML("<hr>").WriteFormatf("<span>%s%s</span>", ">", EscapeString(">"))
assert.Equal(t, "&lt;<hr><span>&gt;&gt;</span>", sb.String()) assert.Equal(t, "&lt;<hr><span>&gt;&gt;</span>", sb.String())
assert.NoError(t, w.Err()) assert.NoError(t, w.Err())
} }
+13 -13
View File
@@ -39,18 +39,18 @@ type mimeHandler struct {
} }
func renderCellCodeOutputTextPlain(w htmlutil.HTMLWriter, text string) error { func renderCellCodeOutputTextPlain(w htmlutil.HTMLWriter, text string) error {
w.WriteFormat(`<div class="cell-output-text"><pre>%s</pre></div>`, text) w.WriteFormatf(`<div class="cell-output-text"><pre>%s</pre></div>`, text)
return w.Err() return w.Err()
} }
func renderCellCodeOutputUnsupported(w htmlutil.HTMLWriter, message string) error { func renderCellCodeOutputUnsupported(w htmlutil.HTMLWriter, message string) error {
w.WriteFormat(`<div class="cell-output-unsupported">%s</div>`, message) w.WriteFormatf(`<div class="cell-output-unsupported">%s</div>`, message)
return w.Err() return w.Err()
} }
var dataMimeHandlers = sync.OnceValue(func() []mimeHandler { var dataMimeHandlers = sync.OnceValue(func() []mimeHandler {
renderImage := func(w htmlutil.HTMLWriter, subtype, payload string) error { renderImage := func(w htmlutil.HTMLWriter, subtype, payload string) error {
w.WriteFormat(`<div class="cell-output-image"><img src="data:image/%s;base64,%s"></div>`, subtype, payload) w.WriteFormatf(`<div class="cell-output-image"><img src="data:image/%s;base64,%s"></div>`, subtype, payload)
return w.Err() return w.Err()
} }
renderUnsupportedOutput := func(message string) func(htmlutil.HTMLWriter, string) error { renderUnsupportedOutput := func(message string) func(htmlutil.HTMLWriter, string) error {
@@ -75,11 +75,11 @@ var dataMimeHandlers = sync.OnceValue(func() []mimeHandler {
// To future developers: don't allow custom CSS classes or attributes, // To future developers: don't allow custom CSS classes or attributes,
// because ".link-action" or "data-fetch-xxx" can send POST requests and lead to XSS. // because ".link-action" or "data-fetch-xxx" can send POST requests and lead to XSS.
// If you'd really like to support more, do remember to correctly sanitize the values. // If you'd really like to support more, do remember to correctly sanitize the values.
w.WriteFormat(`<div class="cell-output-html">%s</div>`, markup.Sanitize(d)) w.WriteFormatf(`<div class="cell-output-html">%s</div>`, markup.Sanitize(d))
return w.Err() return w.Err()
}}, }},
{"text/latex", func(w htmlutil.HTMLWriter, d string) error { {"text/latex", func(w htmlutil.HTMLWriter, d string) error {
w.WriteFormat(`<div class="cell-output-latex"><pre><code class="language-math display">%s</code></pre></div>`, trimMathDelimiters(d)) w.WriteFormatf(`<div class="cell-output-latex"><pre><code class="language-math display">%s</code></pre></div>`, trimMathDelimiters(d))
return w.Err() return w.Err()
}}, }},
{"text/plain", renderCellCodeOutputTextPlain}, {"text/plain", renderCellCodeOutputTextPlain},
@@ -142,14 +142,14 @@ func (renderer) Render(ctx *markup.RenderContext, input io.Reader, outputWriter
// the size is (should be) checked and/or limited by the caller to avoid OOM // the size is (should be) checked and/or limited by the caller to avoid OOM
var notebook Notebook var notebook Notebook
if err := json.NewDecoder(input).Decode(&notebook); err != nil { if err := json.NewDecoder(input).Decode(&notebook); err != nil {
htmlWriter.WriteFormat(`<div class="ui error message">Failed to parse notebook JSON: %v</div>`, err) htmlWriter.WriteFormatf(`<div class="ui error message">Failed to parse notebook JSON: %v</div>`, err)
return htmlWriter.Err() return htmlWriter.Err()
} }
// Check nbformat version // Check nbformat version
if notebook.Nbformat < 4 { if notebook.Nbformat < 4 {
msg := htmlutil.HTMLFormat("This notebook uses an older format (nbformat %d). Only nbformat 4+ is supported for rendering. Please upgrade the notebook in Jupyter or view the raw JSON.", notebook.Nbformat) msg := htmlutil.HTMLFormat("This notebook uses an older format (nbformat %d). Only nbformat 4+ is supported for rendering. Please upgrade the notebook in Jupyter or view the raw JSON.", notebook.Nbformat)
htmlWriter.WriteFormat(`<div class="file-not-rendered-prompt">%s</div>`, msg) htmlWriter.WriteFormatf(`<div class="file-not-rendered-prompt">%s</div>`, msg)
return htmlWriter.Err() return htmlWriter.Err()
} }
@@ -205,7 +205,7 @@ func renderCellCode(output htmlutil.HTMLWriter, cell Cell, language string) erro
output.WriteHTML(`<div class="cell-line">`) output.WriteHTML(`<div class="cell-line">`)
{ {
if executionCount != nil { if executionCount != nil {
output.WriteFormat(`<div class="cell-left cell-prompt">In [%d]:</div>`, *executionCount) output.WriteFormatf(`<div class="cell-left cell-prompt">In [%d]:</div>`, *executionCount)
} else { } else {
output.WriteHTML(`<div class="cell-left cell-prompt">In [ ]:</div>`) output.WriteHTML(`<div class="cell-left cell-prompt">In [ ]:</div>`)
} }
@@ -213,7 +213,7 @@ func renderCellCode(output htmlutil.HTMLWriter, cell Cell, language string) erro
// Highlight code // Highlight code
preAttrs, codeAttrs := highlight.CodeBlockAttributes(language) preAttrs, codeAttrs := highlight.CodeBlockAttributes(language)
lexer := highlight.DetectChromaLexerByFileName("", language) lexer := highlight.DetectChromaLexerByFileName("", language)
output.WriteFormat(`<div class="cell-right cell-input"><pre %s><code %s>`, preAttrs, codeAttrs) output.WriteFormatf(`<div class="cell-right cell-input"><pre %s><code %s>`, preAttrs, codeAttrs)
output.WriteHTML(highlight.RenderCodeByLexer(lexer, source)) output.WriteHTML(highlight.RenderCodeByLexer(lexer, source))
output.WriteHTML("</code></pre></div>") output.WriteHTML("</code></pre></div>")
} }
@@ -232,7 +232,7 @@ func renderCellCode(output htmlutil.HTMLWriter, cell Cell, language string) erro
output.WriteHTML(`<div class="cell-line">`) output.WriteHTML(`<div class="cell-line">`)
{ {
if hasExecutionResult && executionCount != nil { if hasExecutionResult && executionCount != nil {
output.WriteFormat(`<div class="cell-left cell-prompt">Out [%d]:</div>`, *executionCount) output.WriteFormatf(`<div class="cell-left cell-prompt">Out [%d]:</div>`, *executionCount)
} else { } else {
output.WriteHTML(`<div class="cell-left cell-prompt"></div>`) output.WriteHTML(`<div class="cell-left cell-prompt"></div>`)
} }
@@ -250,7 +250,7 @@ func renderCellCode(output htmlutil.HTMLWriter, cell Cell, language string) erro
} }
func renderCellPrompt(output htmlutil.HTMLWriter, left, right template.HTML) { func renderCellPrompt(output htmlutil.HTMLWriter, left, right template.HTML) {
output.WriteFormat(` output.WriteFormatf(`
<div class="notebook-cell"> <div class="notebook-cell">
<div class="cell-line"> <div class="cell-line">
<div class="cell-left cell-prompt">%s</div> <div class="cell-left cell-prompt">%s</div>
@@ -335,7 +335,7 @@ func renderCellCodeOutput(output htmlutil.HTMLWriter, out Output) {
// Stream output // Stream output
if out.OutputType == "stream" && out.Text != nil { if out.OutputType == "stream" && out.Text != nil {
streamName := util.Iif(out.Name == "stderr", "stderr", "stdout") streamName := util.Iif(out.Name == "stderr", "stderr", "stdout")
output.WriteFormat(`<pre class="cell-output-stream stream-%s">%s</pre>`, streamName, joinSource(out.Text)) output.WriteFormatf(`<pre class="cell-output-stream stream-%s">%s</pre>`, streamName, joinSource(out.Text))
return return
} }
@@ -352,7 +352,7 @@ func renderCellCodeOutput(output htmlutil.HTMLWriter, out Output) {
if traceback == "" && out.Ename != "" { if traceback == "" && out.Ename != "" {
traceback = fmt.Sprintf("%s: %s", out.Ename, out.Evalue) traceback = fmt.Sprintf("%s: %s", out.Ename, out.Evalue)
} }
output.WriteFormat(`<pre class="cell-output-error">%s</pre>`, traceback) output.WriteFormatf(`<pre class="cell-output-error">%s</pre>`, traceback)
return return
} }
+1 -1
View File
@@ -2608,8 +2608,8 @@
"repo.diff.file_image_width": "Width", "repo.diff.file_image_width": "Width",
"repo.diff.file_image_height": "Height", "repo.diff.file_image_height": "Height",
"repo.diff.file_byte_size": "Size", "repo.diff.file_byte_size": "Size",
"repo.diff.line_truncated": "Line truncated",
"repo.diff.file_suppressed": "File diff suppressed because it is too large.", "repo.diff.file_suppressed": "File diff suppressed because it is too large.",
"repo.diff.file_suppressed_line_too_long": "File diff suppressed because one or more lines are too long.",
"repo.diff.too_many_files": "Loaded %[1]d of %[2]d files, more files were not shown because too many files have changed in this diff.", "repo.diff.too_many_files": "Loaded %[1]d of %[2]d files, more files were not shown because too many files have changed in this diff.",
"repo.diff.show_more": "Show more", "repo.diff.show_more": "Show more",
"repo.diff.load": "Load diff", "repo.diff.load": "Load diff",
+157 -162
View File
@@ -9,15 +9,16 @@ import (
"bytes" "bytes"
"context" "context"
"fmt" "fmt"
"html"
"html/template" "html/template"
"io" "io"
"net/url" "net/url"
"path" "path"
"slices"
"sort" "sort"
"strconv" "strconv"
"strings" "strings"
"time" "time"
"unicode/utf8"
"gitea.dev/models/db" "gitea.dev/models/db"
git_model "gitea.dev/models/git" git_model "gitea.dev/models/git"
@@ -79,8 +80,9 @@ type DiffLine struct {
Content string Content string
Comments issues_model.CommentList // related PR code comments Comments issues_model.CommentList // related PR code comments
SectionInfo *DiffLineSectionInfo SectionInfo *DiffLineSectionInfo
IsTruncated bool
cachedDiffInline *DiffInline cachedDiffInline *DiffInlineComputed
} }
// DiffLineSectionInfo represents diff line section metadata // DiffLineSectionInfo represents diff line section metadata
@@ -327,28 +329,32 @@ func defaultDiffMatchPatch() *diffmatchpatch.DiffMatchPatch {
return dmp return dmp
} }
// DiffInline is a struct that has a content and escape status // DiffInlineComputed is the final computed diff line for template rendering
type DiffInline struct { type DiffInlineComputed struct {
EscapeStatus *charset.EscapeStatus EscapeStatus *charset.EscapeStatus
Content template.HTML Content template.HTML
IsTruncated bool
} }
// diffInlineWithUnicodeEscape makes a DiffInline with hidden Unicode characters escaped // computeDiffInline makes a DiffInline with computed content, e.g.: Unicode escaping, truncation hint, etc
func diffInlineWithUnicodeEscape(s template.HTML, locale translation.Locale) DiffInline { func computeDiffInline(s template.HTML, isTruncated bool, locale translation.Locale) DiffInlineComputed {
status, content := charset.EscapeControlHTML(s, locale) sb, w := htmlutil.NewHTMLStringWriter()
return DiffInline{EscapeStatus: status, Content: content} status := charset.EscapeControlHTMLTo(s, locale, w)
if isTruncated {
w.WriteFormatf(`<span class="ui label diff-line-truncated">%s</span>`, locale.Tr("repo.diff.line_truncated"))
}
return DiffInlineComputed{EscapeStatus: status, IsTruncated: isTruncated, Content: template.HTML(sb.String())}
} }
func (diffSection *DiffSection) getLineContentForRender(lineIdx int, diffLine *DiffLine, fileLanguage string, 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 h, ok := highlightLines[lineIdx-1]; ok && !diffLine.IsTruncated {
if ok {
return h return h
} }
if diffLine.Content == "" { if diffLine.Content == "" {
return "" return ""
} }
if setting.Git.DisableDiffHighlight { if setting.Git.DisableDiffHighlight {
return template.HTML(html.EscapeString(diffLine.Content[1:])) return htmlutil.EscapeString(diffLine.Content[1:])
} }
if diffSection.highlightLexer.value == nil { if diffSection.highlightLexer.value == nil {
diffSection.highlightLexer.value = highlight.DetectChromaLexerByFileName(diffSection.FileName, fileLanguage) diffSection.highlightLexer.value = highlight.DetectChromaLexerByFileName(diffSection.FileName, fileLanguage)
@@ -356,7 +362,7 @@ func (diffSection *DiffSection) getLineContentForRender(lineIdx int, diffLine *D
return highlight.RenderCodeByLexer(diffSection.highlightLexer.value, diffLine.Content[1:]) return highlight.RenderCodeByLexer(diffSection.highlightLexer.value, diffLine.Content[1:])
} }
func (diffSection *DiffSection) getDiffLineForRender(diffLineType DiffLineType, leftLine, rightLine *DiffLine, locale translation.Locale) DiffInline { func (diffSection *DiffSection) getDiffLineForRender(diffLineType DiffLineType, leftLine, rightLine *DiffLine, locale translation.Locale) DiffInlineComputed {
sideIdx := util.Iif(diffLineType == DiffLineDel, 0, 1) // del=left, add=right sideIdx := util.Iif(diffLineType == DiffLineDel, 0, 1) // del=left, add=right
lines := [2]*DiffLine{leftLine, rightLine} lines := [2]*DiffLine{leftLine, rightLine}
@@ -376,36 +382,39 @@ func (diffSection *DiffSection) getDiffLineForRender(diffLineType DiffLineType,
// left and right are the same, no need to do line-level diff, can just pick any side // left and right are the same, no need to do line-level diff, can just pick any side
// caller always uses the "right side" for this type // caller always uses the "right side" for this type
lineHTML := diffSection.getLineContentForRender(rightLine.RightIdx, rightLine, fileLanguage, highlightedRightLines) lineHTML := diffSection.getLineContentForRender(rightLine.RightIdx, rightLine, fileLanguage, highlightedRightLines)
return diffInlineWithUnicodeEscape(lineHTML, locale) return computeDiffInline(lineHTML, rightLine.IsTruncated, locale)
} }
var diffs [2]template.HTML var diffs [2]template.HTML
var truncations [2]bool
if leftLine != nil { if leftLine != nil {
diffs[0] = diffSection.getLineContentForRender(leftLine.LeftIdx, leftLine, fileLanguage, highlightedLeftLines) diffs[0] = diffSection.getLineContentForRender(leftLine.LeftIdx, leftLine, fileLanguage, highlightedLeftLines)
truncations[0] = leftLine.IsTruncated
} }
if rightLine != nil { if rightLine != nil {
diffs[1] = diffSection.getLineContentForRender(rightLine.RightIdx, rightLine, fileLanguage, highlightedRightLines) diffs[1] = diffSection.getLineContentForRender(rightLine.RightIdx, rightLine, fileLanguage, highlightedRightLines)
truncations[1] = rightLine.IsTruncated
} }
if leftLine != nil && rightLine != nil { if leftLine != nil && rightLine != nil && !truncations[0] && !truncations[1] {
// if only some parts of a line are changed, highlight these changed parts as "deleted/added". // if only some parts of a line are changed, highlight these changed parts as "deleted/added".
// "diff" the left&right sides together, then cache the diff result for another side, // "diff" the left&right sides together, then cache the diff result for another side,
// because when viewing the diff page, both "deleted" and "added" lines will to be rendered eventually, // because when viewing the diff page, both "deleted" and "added" lines will to be rendered eventually,
// so here only diff them once, then next render can just use the cached result, no need to "diff" again. // so here only diff them once, then next render can just use the cached result, no need to "diff" again.
hcd := newHighlightCodeDiff() hcd := newHighlightCodeDiff()
lineHTMLDel, lineHTMLAdd := hcd.diffLineWithHighlight(diffs[0], diffs[1]) lineHTMLDel, lineHTMLAdd := hcd.diffLineWithHighlight(diffs[0], diffs[1])
leftLine.cachedDiffInline = new(diffInlineWithUnicodeEscape(lineHTMLDel, locale)) leftLine.cachedDiffInline = new(computeDiffInline(lineHTMLDel, truncations[0], locale))
rightLine.cachedDiffInline = new(diffInlineWithUnicodeEscape(lineHTMLAdd, locale)) rightLine.cachedDiffInline = new(computeDiffInline(lineHTMLAdd, truncations[1], locale))
return *lines[sideIdx].cachedDiffInline return *lines[sideIdx].cachedDiffInline
} }
// if left is empty or right is empty (a line is fully deleted or added), then we do not need to diff anymore. // if left is empty or right is empty (a line is fully deleted or added), or either side is truncated (too long),
// the tmpl code already adds background colors for these cases. // then we do not need to diff anymore, the tmpl code already adds background colors for these cases.
return diffInlineWithUnicodeEscape(diffs[sideIdx], locale) return computeDiffInline(diffs[sideIdx], truncations[sideIdx], locale)
} }
// GetComputedInlineDiffFor computes inline diff for the given line. // GetComputedInlineDiffFor computes inline diff for the given line.
func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, locale translation.Locale) DiffInline { func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, locale translation.Locale) DiffInlineComputed {
defer func() { defer func() {
if err := recover(); err != nil { if err := recover(); err != nil {
// the logic is too complex in this function, help to catch any panic because Golang template doesn't print the stack // the logic is too complex in this function, help to catch any panic because Golang template doesn't print the stack
@@ -416,7 +425,7 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc
switch diffLine.Type { switch diffLine.Type {
case DiffLineSection: case DiffLineSection:
// section content is a diff hunk header, it isn't code diff, its trailing context might come from the file content, might not // section content is a diff hunk header, it isn't code diff, its trailing context might come from the file content, might not
return diffInlineWithUnicodeEscape(htmlutil.EscapeString(diffLine.Content), locale) return computeDiffInline(htmlutil.EscapeString(diffLine.Content), diffLine.IsTruncated, locale)
case DiffLineAdd: case DiffLineAdd:
compareDiffLine := diffSection.GetLine(diffLine.Match) compareDiffLine := diffSection.GetLine(diffLine.Match)
return diffSection.getDiffLineForRender(DiffLineAdd, compareDiffLine, diffLine, locale) return diffSection.getDiffLineForRender(DiffLineAdd, compareDiffLine, diffLine, locale)
@@ -456,9 +465,9 @@ type DiffFile struct {
IsRenamed bool IsRenamed bool
IsSubmodule bool IsSubmodule bool
// basic fields but for render purpose only // basic fields but for render purpose only
Sections []*DiffSection Sections []*DiffSection
IsIncomplete bool IsIncomplete bool // file is too large
IsIncompleteLineTooLong bool HasTruncatedLines bool // some lines are too long
// will be filled by the extra loop in GitDiffForRender // will be filled by the extra loop in GitDiffForRender
IsGenerated bool IsGenerated bool
@@ -492,6 +501,10 @@ func (diffFile *DiffFile) GetType() int {
return int(diffFile.Type) return int(diffFile.Type)
} }
func (diffFile *DiffFile) CanShowFileViewToggle() bool {
return diffFile.IsBlobTypeImage || (diffFile.IsBlobTypeCsv && !diffFile.IsIncomplete && !diffFile.HasTruncatedLines)
}
type DiffRenderDetail struct { type DiffRenderDetail struct {
needTailSection bool needTailSection bool
leftLineCount, rightLineCount int leftLineCount, rightLineCount int
@@ -685,62 +698,64 @@ func (diff *Diff) LoadComments(ctx context.Context, issue *issues_model.Issue, c
const cmdDiffHead = "diff --git " const cmdDiffHead = "diff --git "
// ParsePatch builds a Diff object from a io.Reader and some parameters. // to correctly parse a diff line, the input buffer size should be large enough to read a full line for git diff headers
func ParsePatch(ctx context.Context, maxLines, maxLineCharacters, maxFiles int, reader io.Reader, skipToFile string) (*Diff, error) { var defaultDiffLineBufferSize = 8 * 1024
log.Debug("ParsePatch(%d, %d, %d, ..., %s)", maxLines, maxLineCharacters, maxFiles, skipToFile)
var curFile *DiffFile
skipping := skipToFile != "" // ParsePatch builds a Diff object by parsing git diff output
func ParsePatch(ctx context.Context, maxLines, maxLineCharacters, maxFiles int, reader io.Reader, skipToFile string) (_ *Diff, retErr error) {
log.Debug("ParsePatch(%d, %d, %d, ..., %s)", maxLines, maxLineCharacters, maxFiles, skipToFile)
diff := &Diff{Files: make([]*DiffFile, 0)} diff := &Diff{Files: make([]*DiffFile, 0)}
readerSize := max(maxLineCharacters, defaultDiffLineBufferSize)
sb := strings.Builder{}
// OK let's set a reasonable buffer size.
// This should be at least the size of maxLineCharacters or 4096 whichever is larger.
readerSize := max(maxLineCharacters, 4096)
input := bufio.NewReaderSize(reader, readerSize) input := bufio.NewReaderSize(reader, readerSize)
line, err := input.ReadString('\n') line, err := input.ReadString('\n')
if err != nil { if err != nil {
if err == io.EOF { return diff, util.Iif(err == io.EOF, nil, err)
return diff, nil
}
return diff, err
} }
prepareValue := func(s, p string) string { skipping := skipToFile != ""
for {
nextLine, err := diff.parseOneDiffFile(ctx, maxLines, maxLineCharacters, maxFiles, &skipping, input, skipToFile, line)
if nextLine == "" || err == io.EOF {
break
} else if err != nil {
return diff, err
}
line = nextLine
}
diff.postProcessFiles()
return diff, nil
}
func (diff *Diff) parseOneDiffFile(ctx context.Context, maxLines, maxLineCharacters, maxFiles int, skipping *bool, input *bufio.Reader, skipToFile, startLine string) (nextLine string, err error) {
line := startLine
extractGitDiffHead := func(s, p string) string {
return strings.TrimSpace(strings.TrimPrefix(s, p)) return strings.TrimSpace(strings.TrimPrefix(s, p))
} }
parsingLoop: {
for {
// 1. A patch file always begins with `diff --git ` + `a/path b/path` (possibly quoted) // 1. A patch file always begins with `diff --git ` + `a/path b/path` (possibly quoted)
// if it does not we have bad input! // if it does not we have bad input!
if !strings.HasPrefix(line, cmdDiffHead) { if !strings.HasPrefix(line, cmdDiffHead) {
return diff, fmt.Errorf("invalid first file line: %s", line) return "", fmt.Errorf("invalid first file line: %s", line)
} }
if maxFiles > -1 && len(diff.Files) >= maxFiles { if maxFiles > -1 && len(diff.Files) >= maxFiles {
lastFile := createDiffFile(line) lastFile := createDiffFile(line)
diff.End = lastFile.Name diff.End = lastFile.Name
diff.IsIncomplete = true diff.IsIncomplete = true
break parsingLoop return "", nil
} }
curFile = createDiffFile(line) curFile := createDiffFile(line)
if skipping { if *skipping {
if curFile.Name != skipToFile { if curFile.Name != skipToFile {
line, err = skipToNextDiffHead(input) return skipToNextDiffHead(input)
if err != nil {
if err == io.EOF {
return diff, nil
}
return diff, err
}
continue
} }
skipping = false *skipping = false
} }
diff.Files = append(diff.Files, curFile) diff.Files = append(diff.Files, curFile)
@@ -783,27 +798,23 @@ parsingLoop:
// Binary files a/<path> and b/<path> differ // Binary files a/<path> and b/<path> differ
// //
// but one of a/<path> and b/<path> could be /dev/null. // but one of a/<path> and b/<path> could be /dev/null.
curFileLoop:
for { for {
line, err = input.ReadString('\n') line, err = input.ReadString('\n')
if err != nil { if err != nil {
if err != io.EOF { return line, err
return diff, err
}
break parsingLoop
} }
switch { switch {
case strings.HasPrefix(line, cmdDiffHead): case strings.HasPrefix(line, cmdDiffHead):
break curFileLoop return line, nil
case strings.HasPrefix(line, "old mode ") || case strings.HasPrefix(line, "old mode ") ||
strings.HasPrefix(line, "new mode "): strings.HasPrefix(line, "new mode "):
if strings.HasPrefix(line, "old mode ") { if strings.HasPrefix(line, "old mode ") {
curFile.OldEntryMode = prepareValue(line, "old mode ") curFile.OldEntryMode = extractGitDiffHead(line, "old mode ")
} }
if strings.HasPrefix(line, "new mode ") { if strings.HasPrefix(line, "new mode ") {
curFile.EntryMode = prepareValue(line, "new mode ") curFile.EntryMode = extractGitDiffHead(line, "new mode ")
} }
if strings.HasSuffix(line, " 160000\n") { if strings.HasSuffix(line, " 160000\n") {
curFile.IsSubmodule, curFile.SubmoduleDiffInfo = true, &SubmoduleDiffInfo{} curFile.IsSubmodule, curFile.SubmoduleDiffInfo = true, &SubmoduleDiffInfo{}
@@ -812,33 +823,33 @@ parsingLoop:
curFile.IsRenamed = true curFile.IsRenamed = true
curFile.Type = DiffFileRename curFile.Type = DiffFileRename
if curFile.isAmbiguous { if curFile.isAmbiguous {
curFile.OldName = prepareValue(line, "rename from ") curFile.OldName = extractGitDiffHead(line, "rename from ")
} }
case strings.HasPrefix(line, "rename to "): case strings.HasPrefix(line, "rename to "):
curFile.IsRenamed = true curFile.IsRenamed = true
curFile.Type = DiffFileRename curFile.Type = DiffFileRename
if curFile.isAmbiguous { if curFile.isAmbiguous {
curFile.Name = prepareValue(line, "rename to ") curFile.Name = extractGitDiffHead(line, "rename to ")
curFile.isAmbiguous = false curFile.isAmbiguous = false
} }
case strings.HasPrefix(line, "copy from "): case strings.HasPrefix(line, "copy from "):
curFile.IsRenamed = true curFile.IsRenamed = true
curFile.Type = DiffFileCopy curFile.Type = DiffFileCopy
if curFile.isAmbiguous { if curFile.isAmbiguous {
curFile.OldName = prepareValue(line, "copy from ") curFile.OldName = extractGitDiffHead(line, "copy from ")
} }
case strings.HasPrefix(line, "copy to "): case strings.HasPrefix(line, "copy to "):
curFile.IsRenamed = true curFile.IsRenamed = true
curFile.Type = DiffFileCopy curFile.Type = DiffFileCopy
if curFile.isAmbiguous { if curFile.isAmbiguous {
curFile.Name = prepareValue(line, "copy to ") curFile.Name = extractGitDiffHead(line, "copy to ")
curFile.isAmbiguous = false curFile.isAmbiguous = false
} }
case strings.HasPrefix(line, "new file"): case strings.HasPrefix(line, "new file"):
curFile.Type = DiffFileAdd curFile.Type = DiffFileAdd
curFile.IsCreated = true curFile.IsCreated = true
if strings.HasPrefix(line, "new file mode ") { if strings.HasPrefix(line, "new file mode ") {
curFile.EntryMode = prepareValue(line, "new file mode ") curFile.EntryMode = extractGitDiffHead(line, "new file mode ")
} }
if strings.HasSuffix(line, " 160000\n") { if strings.HasSuffix(line, " 160000\n") {
curFile.IsSubmodule, curFile.SubmoduleDiffInfo = true, &SubmoduleDiffInfo{} curFile.IsSubmodule, curFile.SubmoduleDiffInfo = true, &SubmoduleDiffInfo{}
@@ -892,31 +903,16 @@ parsingLoop:
curFile.isAmbiguous = false curFile.isAmbiguous = false
} }
// Otherwise do nothing with this line, but now switch to parsing hunks // Otherwise do nothing with this line, but now switch to parsing hunks
lineBytes, isFragment, err := parseHunks(ctx, curFile, maxLines, maxLineCharacters, input) nextLine, err := parseHunks(ctx, curFile, maxLines, maxLineCharacters, input)
if err != nil { return string(nextLine), err
if err != io.EOF { default:
return diff, err // ignore other extended header lines
}
break parsingLoop
}
sb.Reset()
_, _ = sb.Write(lineBytes)
for isFragment {
lineBytes, isFragment, err = input.ReadLine()
if err != nil {
// Now by the definition of ReadLine this cannot be io.EOF
return diff, fmt.Errorf("unable to ReadLine: %w", err)
}
_, _ = sb.Write(lineBytes)
}
line = sb.String()
sb.Reset()
break curFileLoop
} }
} }
} }
}
func (diff *Diff) postProcessFiles() {
// TODO: There are numerous issues with this: // TODO: There are numerous issues with this:
// - we might want to consider detecting encoding while parsing but... // - we might want to consider detecting encoding while parsing but...
// - we're likely to fail to get the correct encoding here anyway as we won't have enough information // - we're likely to fail to get the correct encoding here anyway as we won't have enough information
@@ -964,38 +960,18 @@ parsingLoop:
} }
} }
} }
return diff, nil
} }
func skipToNextDiffHead(input *bufio.Reader) (line string, err error) { func skipToNextDiffHead(input *bufio.Reader) (line string, err error) {
// need to skip until the next cmdDiffHead
var isFragment, wasFragment bool
var lineBytes []byte
for { for {
lineBytes, isFragment, err = input.ReadLine() lineBytes, _, err := readGitDiffLineWithDiscard(input)
if err != nil { if err != nil {
return "", err return "", err
} }
if wasFragment {
wasFragment = isFragment
continue
}
if bytes.HasPrefix(lineBytes, []byte(cmdDiffHead)) { if bytes.HasPrefix(lineBytes, []byte(cmdDiffHead)) {
break return string(lineBytes), nil
} }
wasFragment = isFragment
} }
line = string(lineBytes)
if isFragment {
var tail string
tail, err = input.ReadString('\n')
if err != nil {
return "", err
}
line += tail
}
return line, err
} }
func newDiffSectionForDiffFile(curFile *DiffFile) *DiffSection { func newDiffSectionForDiffFile(curFile *DiffFile) *DiffSection {
@@ -1007,9 +983,59 @@ func newDiffSectionForDiffFile(curFile *DiffFile) *DiffSection {
} }
} }
func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio.Reader) (lineBytes []byte, isFragment bool, err error) { func readGitDiffLineWithDiscard(r *bufio.Reader) (_ []byte, truncated bool, _ error) {
sb := strings.Builder{} // HINT: GIT-DIFF-PARSE-LONG-LINE: it can't use Scanner which has a default limit and will cause errors if a line is very long
line, isPrefix, err := r.ReadLine()
if !isPrefix {
return line, false, err
}
if bytes.HasPrefix(line, []byte(cmdDiffHead)) {
// "diff head" is special, even if it is very long, we still want to fully read it
line = slices.Clone(line)
lineRemaining, err := r.ReadBytes('\n')
lineRemaining = bytes.TrimRight(lineRemaining, "\r\n")
return append(line, lineRemaining...), false, err
}
// discard remaining bytes, only return the prefix
line = slices.Clone(line)
for isPrefix && err == nil {
_, isPrefix, err = r.ReadLine()
}
return line, true, err
}
// tryFixTruncatedString tries to fix the truncated diff line by removing the last corrupted rune
func tryFixTruncatedString(s string) string {
b := util.UnsafeStringToBytes(s)
var idx int
for idx = 0; idx < len(s); {
r, l := utf8.DecodeRune(b[idx:])
if r == utf8.RuneError && l == 1 {
break
}
idx += l
}
// for valid utf8 diff line, remove the last truncated rune
remainingLen := len(s) - idx
if idx > 0 && remainingLen < utf8.UTFMax {
return s[:idx]
}
// for non-utf8 diff line, try to find an ASCII char at the ending, remove the potentially truncated chars after that
for i := 1; i <= utf8.UTFMax; i++ {
idx = len(s) - i
if 0 < idx && idx+1 < len(s) && s[idx] < 127 {
return s[:idx+1]
}
}
// otherwise, just return the input as is
return s
}
func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio.Reader) (nextLine []byte, err error) {
var curSection *DiffSection var curSection *DiffSection
curFileLFSPrefix := false curFileLFSPrefix := false
@@ -1022,27 +1048,12 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
} }
for { for {
for isFragment { lineBytes, truncated, err := readGitDiffLineWithDiscard(input)
curFile.IsIncomplete = true
curFile.IsIncompleteLineTooLong = true
_, isFragment, err = input.ReadLine()
if err != nil {
// Now by the definition of ReadLine this cannot be io.EOF
return nil, false, fmt.Errorf("unable to ReadLine: %w", err)
}
}
sb.Reset()
lineBytes, isFragment, err = input.ReadLine()
if err != nil { if err != nil {
if err == io.EOF { return lineBytes, err
return lineBytes, isFragment, err
}
err = fmt.Errorf("unable to ReadLine: %w", err)
return nil, false, err
} }
if lineBytes[0] == 'd' { if bytes.HasPrefix(lineBytes, []byte(cmdDiffHead)) {
// End of hunks return lineBytes, err
return lineBytes, isFragment, err
} }
switch lineBytes[0] { switch lineBytes[0] {
@@ -1051,19 +1062,7 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
curFile.IsIncomplete = true curFile.IsIncomplete = true
continue continue
} }
line := string(lineBytes)
_, _ = sb.Write(lineBytes)
for isFragment {
// This is very odd indeed - we're in a section header and the line is too long
// This really shouldn't happen...
lineBytes, isFragment, err = input.ReadLine()
if err != nil {
// Now by the definition of ReadLine this cannot be io.EOF
return nil, false, fmt.Errorf("unable to ReadLine: %w", err)
}
_, _ = sb.Write(lineBytes)
}
line := sb.String()
// Create a new section to represent this hunk // Create a new section to represent this hunk
curSection = newDiffSectionForDiffFile(curFile) curSection = newDiffSectionForDiffFile(curFile)
@@ -1086,7 +1085,7 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
case '\\': case '\\':
// This is used only to indicate that the current file does not have a terminal newline // This is used only to indicate that the current file does not have a terminal newline
if !bytes.Equal(lineBytes, []byte("\\ No newline at end of file")) { if !bytes.Equal(lineBytes, []byte("\\ No newline at end of file")) {
return nil, false, fmt.Errorf("unexpected line in hunk: %s", string(lineBytes)) return nil, fmt.Errorf("unexpected line in hunk: %s", string(lineBytes))
} }
// Technically this should be the end the file! // Technically this should be the end the file!
// FIXME: we should be putting a marker at the end of the file if there is no terminal new line // FIXME: we should be putting a marker at the end of the file if there is no terminal new line
@@ -1170,27 +1169,23 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
curSection.Lines = append(curSection.Lines, diffLine) curSection.Lines = append(curSection.Lines, diffLine)
default: default:
// This is unexpected // This is unexpected
return nil, false, fmt.Errorf("unexpected line in hunk: %s", string(lineBytes)) return nil, fmt.Errorf("unexpected line in hunk: %s", string(lineBytes))
} }
curLine := curSection.Lines[len(curSection.Lines)-1]
curLine.IsTruncated = truncated
line := string(lineBytes) line := string(lineBytes)
if isFragment { curFile.HasTruncatedLines = curFile.HasTruncatedLines || truncated
curFile.IsIncomplete = true
curFile.IsIncompleteLineTooLong = true
for isFragment {
lineBytes, isFragment, err = input.ReadLine()
if err != nil {
// Now by the definition of ReadLine this cannot be io.EOF
return lineBytes, isFragment, fmt.Errorf("unable to ReadLine: %w", err)
}
}
}
if len(line) > maxLineCharacters { if len(line) > maxLineCharacters {
curFile.IsIncomplete = true
curFile.IsIncompleteLineTooLong = true
line = line[:maxLineCharacters] line = line[:maxLineCharacters]
curLine.IsTruncated = true
}
curLine.Content = line
if curLine.IsTruncated {
curFile.HasTruncatedLines = true
curLine.Content = tryFixTruncatedString(curLine.Content)
} }
curSection.Lines[len(curSection.Lines)-1].Content = line
// handle LFS // handle LFS
if line[1:] == lfs.MetaFileIdentifier { if line[1:] == lfs.MetaFileIdentifier {
@@ -1489,7 +1484,7 @@ func highlightCodeLines(name, lang string, sections []*DiffSection, isLeft bool,
} }
if lineIdx >= 1 { if lineIdx >= 1 {
idx := lineIdx - 1 idx := lineIdx - 1
if idx < len(unsafeLines) { if idx < len(unsafeLines) && !ln.IsTruncated {
lines[idx] = template.HTML(util.UnsafeBytesToString(unsafeLines[idx])) lines[idx] = template.HTML(util.UnsafeBytesToString(unsafeLines[idx]))
} }
} }
+2 -2
View File
@@ -4,12 +4,12 @@
package gitdiff package gitdiff
import ( import (
"bufio"
"bytes" "bytes"
"fmt" "fmt"
"html/template" "html/template"
"io" "io"
"gitea.dev/modules/git"
"gitea.dev/modules/setting" "gitea.dev/modules/setting"
"github.com/alecthomas/chroma/v2" "github.com/alecthomas/chroma/v2"
@@ -29,7 +29,7 @@ type BlobExcerptOptions struct {
func (diffSection *DiffSection) fillExcerptLines(reader io.Reader, leftStart, rightStart, chunkSize int) error { func (diffSection *DiffSection) fillExcerptLines(reader io.Reader, leftStart, rightStart, chunkSize int) error {
buf := &bytes.Buffer{} buf := &bytes.Buffer{}
scanner := bufio.NewScanner(reader) scanner := git.NewGitDiffScanner(reader)
var diffLines []*DiffLine var diffLines []*DiffLine
for rightLineIdx := 1; rightLineIdx < rightStart+chunkSize; rightLineIdx++ { for rightLineIdx := 1; rightLineIdx < rightStart+chunkSize; rightLineIdx++ {
if ok := scanner.Scan(); !ok { if ok := scanner.Scan(); !ok {
+48
View File
@@ -0,0 +1,48 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package gitdiff
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestTryFixTruncatedString(t *testing.T) {
assert.Equal(t, "", tryFixTruncatedString(""))
t.Run("UTF8", func(t *testing.T) {
s := "🌞a🌛"
assert.Equal(t, s[:1], tryFixTruncatedString(s[:1]))
assert.Equal(t, s[:2], tryFixTruncatedString(s[:2]))
assert.Equal(t, s[:3], tryFixTruncatedString(s[:3]))
assert.Equal(t, "🌞", tryFixTruncatedString(s[:4]))
assert.Equal(t, "🌞a", tryFixTruncatedString(s[:5]))
assert.Equal(t, "🌞a", tryFixTruncatedString(s[:6]))
assert.Equal(t, "🌞a", tryFixTruncatedString(s[:7]))
assert.Equal(t, "🌞a", tryFixTruncatedString(s[:8]))
assert.Equal(t, "🌞a🌛", tryFixTruncatedString(s[:9]))
s = "a🌞🌛"
assert.Equal(t, "a", tryFixTruncatedString(s[:1]))
assert.Equal(t, "a", tryFixTruncatedString(s[:2]))
assert.Equal(t, "a", tryFixTruncatedString(s[:3]))
assert.Equal(t, "a", tryFixTruncatedString(s[:4]))
assert.Equal(t, "a🌞", tryFixTruncatedString(s[:5]))
assert.Equal(t, "a🌞", tryFixTruncatedString(s[:6]))
})
t.Run("Non-UTF8", func(t *testing.T) {
s := "\xff\xee\xff\xeeb\xff\xee\xff\xee"
assert.Equal(t, "\xff", tryFixTruncatedString(s[:1]))
assert.Equal(t, "\xff\xee", tryFixTruncatedString(s[:2]))
assert.Equal(t, "\xff\xee\xff", tryFixTruncatedString(s[:3]))
assert.Equal(t, "\xff\xee\xff\xee", tryFixTruncatedString(s[:4]))
assert.Equal(t, "\xff\xee\xff\xeeb", tryFixTruncatedString(s[:5]))
assert.Equal(t, "\xff\xee\xff\xeeb", tryFixTruncatedString(s[:6]))
assert.Equal(t, "\xff\xee\xff\xeeb", tryFixTruncatedString(s[:7]))
assert.Equal(t, "\xff\xee\xff\xeeb", tryFixTruncatedString(s[:8]))
assert.Equal(t, "\xff\xee\xff\xeeb\xff\xee\xff\xee", tryFixTruncatedString(s[:9]))
})
}
+101 -28
View File
@@ -19,7 +19,6 @@ import (
"gitea.dev/modules/json" "gitea.dev/modules/json"
"gitea.dev/modules/setting" "gitea.dev/modules/setting"
"gitea.dev/modules/translation" "gitea.dev/modules/translation"
"gitea.dev/modules/util"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@@ -203,7 +202,7 @@ func TestParsePatch_singlefile(t *testing.T) {
tests := []testcase{ tests := []testcase{
{ {
name: "readme.md2readme.md", name: "same name",
gitdiff: `diff --git "\\a/README.md" "\\b/README.md" gitdiff: `diff --git "\\a/README.md" "\\b/README.md"
--- "\\a/README.md" --- "\\a/README.md"
+++ "\\b/README.md" +++ "\\b/README.md"
@@ -222,7 +221,7 @@ func TestParsePatch_singlefile(t *testing.T) {
oldFilename: "README.md", oldFilename: "README.md",
}, },
{ {
name: "A \\ B", name: `A \ B`,
gitdiff: `diff --git "a/A \\ B" "b/A \\ B" gitdiff: `diff --git "a/A \\ B" "b/A \\ B"
--- "a/A \\ B" --- "a/A \\ B"
+++ "b/A \\ B" +++ "b/A \\ B"
@@ -236,8 +235,8 @@ func TestParsePatch_singlefile(t *testing.T) {
+ cut off`, + cut off`,
addition: 4, addition: 4,
deletion: 1, deletion: 1,
filename: "A \\ B", filename: `A \ B`,
oldFilename: "A \\ B", oldFilename: `A \ B`,
}, },
{ {
name: "really weird filename", name: "really weird filename",
@@ -327,7 +326,7 @@ index 0000000..92e798b
deletion: 0, deletion: 0,
}, },
{ {
name: "rename", name: "ambiguous rename 1",
gitdiff: `diff --git a/b b/b b/b b/b b/b b/b gitdiff: `diff --git a/b b/b b/b b/b b/b b/b
similarity index 100% similarity index 100%
rename from b b/b b/b b/b b/b rename from b b/b b/b b/b b/b
@@ -337,17 +336,7 @@ rename to b
filename: "b", filename: "b",
}, },
{ {
name: "ambiguous 1", name: "ambiguous rename 2",
gitdiff: `diff --git a/b b/b b/b b/b b/b b/b
similarity index 100%
rename from b b/b b/b b/b b/b
rename to b
`,
oldFilename: "b b/b b/b b/b b/b",
filename: "b",
},
{
name: "ambiguous 2",
gitdiff: `diff --git a/b b/b b/b b/b b/b b/b gitdiff: `diff --git a/b b/b b/b b/b b/b b/b
similarity index 100% similarity index 100%
rename from b b/b b/b b/b rename from b b/b b/b b/b
@@ -442,8 +431,8 @@ index 0000000..6bb8f39
if err != nil { if err != nil {
t.Errorf("There should not be an error: %v", err) t.Errorf("There should not be an error: %v", err)
} }
if !result.Files[0].IsIncomplete { if result.Files[0].IsIncomplete {
t.Errorf("Files should be incomplete! %v", result.Files[0]) t.Errorf("Files should not be incomplete! %v", result.Files[0])
} }
// Test max characters // Test max characters
@@ -480,8 +469,8 @@ index 0000000..6bb8f39
if err != nil { if err != nil {
t.Errorf("There should not be an error: %v", err) t.Errorf("There should not be an error: %v", err)
} }
if !result.Files[0].IsIncomplete { if result.Files[0].IsIncomplete {
t.Errorf("Files should be incomplete! %v", result.Files[0]) t.Errorf("Files should not be incomplete! %v", result.Files[0])
} }
diff = `diff --git "a/README.md" "b/README.md" diff = `diff --git "a/README.md" "b/README.md"
@@ -549,6 +538,65 @@ index 0000000..6bb8f39
} }
} }
func TestParsePatchLongLines(t *testing.T) {
overSizedContent := strings.Repeat("a", defaultDiffLineBufferSize*2)
for _, test := range []struct {
name, full string
limit int
result string
}{
{name: "below", limit: 8, full: "short", result: "short"},
{name: "exact", limit: 8, full: "1234567", result: "1234567"},
{name: "above", limit: 8, full: "12345678", result: "1234567"},
{name: "multiple fragments", limit: 101, full: overSizedContent, result: overSizedContent[:100]},
{name: "cutoff-truncate", limit: 8, full: "a🙂b🙂", result: "a🙂b"},
{name: "cutoff-no-truncate", limit: 12, full: "a🙂b🙂", result: "a🙂b🙂"},
} {
for _, eol := range []string{"\n", "\r\n"} {
t.Run(test.name+"/"+strconv.Quote(eol), func(t *testing.T) {
patch := strings.Join([]string{
"diff --git a/first b/first",
"--- a/first",
"+++ b/first",
"@@ -1,3 +1,3 @@",
"-" + test.full,
"+" + test.full,
" " + test.full,
" short",
"@@ -10 +10 @@",
" next",
"diff --git a/second b/second",
"--- a/second", "+++ b/second",
"@@ -1 +1 @@",
" final",
"",
}, eol)
maxLines, maxFiles, skipToFile := 20, 10, ""
diff, err := ParsePatch(t.Context(), maxLines, test.limit, maxFiles, strings.NewReader(patch), skipToFile)
require.NoError(t, err)
require.Len(t, diff.Files, 2)
file := diff.Files[0]
assert.False(t, file.IsIncomplete)
assert.Equal(t, test.result != test.full, file.HasTruncatedLines)
require.Len(t, file.Sections, 2)
lines := file.Sections[0].Lines
require.Len(t, lines, 5)
for i, marker := range []string{"-", "+", " "} {
assert.Equal(t, marker+test.result, lines[i+1].Content)
assert.Equal(t, test.result != test.full, lines[i+1].IsTruncated)
}
assert.Equal(t, 2, lines[1].Match)
assert.Equal(t, 1, lines[2].Match)
assert.Equal(t, &DiffLine{Type: DiffLinePlain, LeftIdx: 3, RightIdx: 3, Content: " short"}, lines[4])
assert.Equal(t, &DiffLine{Type: DiffLinePlain, LeftIdx: 10, RightIdx: 10, Content: " next"}, file.Sections[1].Lines[1])
assert.False(t, diff.Files[1].IsIncomplete)
assert.False(t, diff.Files[1].HasTruncatedLines)
assert.Equal(t, &DiffLine{Type: DiffLinePlain, LeftIdx: 1, RightIdx: 1, Content: " final"}, diff.Files[1].Sections[0].Lines[1])
})
}
}
}
func TestParsePatchExactLineLimit(t *testing.T) { func TestParsePatchExactLineLimit(t *testing.T) {
for _, test := range []struct { for _, test := range []struct {
name, hunk string name, hunk string
@@ -563,6 +611,7 @@ func TestParsePatchExactLineLimit(t *testing.T) {
{name: "deletion", limit: 1, lines: 1, hunk: "@@ -1 +0,0 @@\n-one\n"}, {name: "deletion", limit: 1, lines: 1, hunk: "@@ -1 +0,0 @@\n-one\n"},
{name: "marker has no cost", limit: 1, lines: 1, hunk: "@@ -1 +1 @@\n line\n\\ No newline at end of file\n"}, {name: "marker has no cost", limit: 1, lines: 1, hunk: "@@ -1 +1 @@\n line\n\\ No newline at end of file\n"},
{name: "hunk at capacity", limit: 1, lines: 1, hunk: "@@ -1 +1 @@\n one\n@@ -3 +3 @@\n three\n", incomplete: true}, {name: "hunk at capacity", limit: 1, lines: 1, hunk: "@@ -1 +1 @@\n one\n@@ -3 +3 @@\n three\n", incomplete: true},
{name: "long line after capacity", limit: 1, lines: 1, hunk: "@@ -0,0 +1,3 @@\n+one\n+" + strings.Repeat("x", 10000) + "\n+three\n", incomplete: true},
} { } {
t.Run(test.name, func(t *testing.T) { t.Run(test.name, func(t *testing.T) {
patch := "diff --git a/file b/file\n--- a/file\n+++ b/file\n" + test.hunk patch := "diff --git a/file b/file\n--- a/file\n+++ b/file\n" + test.hunk
@@ -570,17 +619,12 @@ func TestParsePatchExactLineLimit(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
require.Len(t, diff.Files, 1) require.Len(t, diff.Files, 1)
diffFile := diff.Files[0] diffFile := diff.Files[0]
assert.Equal(t, test.incomplete, diffFile.IsIncomplete)
if test.limit == 0 { if test.limit == 0 {
require.Len(t, diffFile.Sections, 0) require.Len(t, diffFile.Sections, 0)
} else { } else {
require.Len(t, diffFile.Sections, 1) require.Len(t, diffFile.Sections, 1)
diffSection := diffFile.Sections[0] assert.Len(t, diffFile.Sections[0].Lines, test.lines+1) // lines with a section
lineSecCount := 0
for _, line := range diffSection.Lines {
lineSecCount += util.Iif(line.Type == DiffLineSection, 1, 0)
}
assert.Equal(t, test.lines, len(diffSection.Lines)-lineSecCount) // actual diff lines
assert.Equal(t, test.incomplete, diffFile.IsIncomplete)
} }
}) })
} }
@@ -1165,6 +1209,22 @@ func TestDiffSection_GetComputedInlineDiffFor(t *testing.T) {
assert.True(t, diffInline.EscapeStatus.Escaped) assert.True(t, diffInline.EscapeStatus.Escaped)
assert.Equal(t, `@@ -1,3 +1,3 @@ func <span class="escaped-code-point" data-escaped="[U+202E]"><span class="char">`+"\u202e"+`</span></span>name() &lt;b&gt;`, string(diffInline.Content)) assert.Equal(t, `@@ -1,3 +1,3 @@ func <span class="escaped-code-point" data-escaped="[U+202E]"><span class="char">`+"\u202e"+`</span></span>name() &lt;b&gt;`, string(diffInline.Content))
}) })
t.Run("ShortLineUseHighlight", func(t *testing.T) {
diffFile := &DiffFile{}
diffFile.highlightedRightLines.value = map[int]template.HTML{0: "highlighted short line"}
diffLine := &DiffLine{Type: DiffLinePlain, RightIdx: 1, Content: " short line", IsTruncated: false}
inline := newDiffSectionForDiffFile(diffFile).GetComputedInlineDiffFor(diffLine, translation.MockLocale{})
assert.Equal(t, template.HTML("highlighted short line"), inline.Content)
assert.False(t, inline.IsTruncated)
})
t.Run("LongLineTruncated", func(t *testing.T) {
diffFile := &DiffFile{}
diffFile.highlightedRightLines.value = map[int]template.HTML{0: "highlighted line"}
diffLine := &DiffLine{Type: DiffLinePlain, RightIdx: 1, Content: " truncated line", IsTruncated: true}
inline := newDiffSectionForDiffFile(diffFile).GetComputedInlineDiffFor(diffLine, translation.MockLocale{})
assert.Equal(t, template.HTML(`truncated line<span class="ui label diff-line-truncated">repo.diff.line_truncated</span>`), inline.Content)
assert.True(t, inline.IsTruncated)
})
} }
func TestHighlightCodeLines(t *testing.T) { func TestHighlightCodeLines(t *testing.T) {
@@ -1214,6 +1274,19 @@ func TestHighlightCodeLines(t *testing.T) {
assert.Equal(t, "a␍b\n", string(ret[0])) assert.Equal(t, "a␍b\n", string(ret[0]))
assert.Equal(t, `c`, string(ret[1])) assert.Equal(t, `c`, string(ret[1]))
}) })
t.Run("LongLineTruncated", func(t *testing.T) {
diffFile := &DiffFile{
Name: "a.c",
Sections: []*DiffSection{
{
Lines: []*DiffLine{{LeftIdx: 1, IsTruncated: true}},
},
},
}
ret := highlightCodeLinesForDiffFile(diffFile, true, []byte("// anything"))
assert.Empty(t, ret)
})
} }
func TestSyncUserSpecificDiff_UpdatedFiles(t *testing.T) { func TestSyncUserSpecificDiff_UpdatedFiles(t *testing.T) {
+3 -7
View File
@@ -90,7 +90,7 @@
{{/*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*/}} {{/*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*/}}
{{$isImage:= $file.IsBlobTypeImage}} {{$isImage:= $file.IsBlobTypeImage}}
{{$isCsv := $file.IsBlobTypeCsv}} {{$isCsv := $file.IsBlobTypeCsv}}
{{$showFileViewToggle := or $isImage (and (not $file.IsIncomplete) $isCsv)}} {{$showFileViewToggle := $file.CanShowFileViewToggle}}
{{$isReviewFile := and $.IsSigned $.PageIsPullFiles (not $.Repository.IsArchived) $.IsShowingAllCommits}} {{$isReviewFile := and $.IsSigned $.PageIsPullFiles (not $.Repository.IsArchived) $.IsShowingAllCommits}}
<div class="diff-file-box file-content {{TabSizeClass $.Editorconfig $file.Name}} tw-mt-0" id="diff-{{$file.NameHash}}" data-old-filename="{{$file.OldName}}" data-new-filename="{{$file.Name}}" {{if $file.ShouldBeHidden}}data-folded="true"{{end}}> <div class="diff-file-box file-content {{TabSizeClass $.Editorconfig $file.Name}} tw-mt-0" id="diff-{{$file.NameHash}}" data-old-filename="{{$file.OldName}}" data-new-filename="{{$file.Name}}" {{if $file.ShouldBeHidden}}data-folded="true"{{end}}>
<div class="diff-file-header sticky-2nd-row ui top attached header"> <div class="diff-file-header sticky-2nd-row ui top attached header">
@@ -173,12 +173,8 @@
{{if or $file.IsIncomplete $file.IsBin}} {{if or $file.IsIncomplete $file.IsBin}}
<div class="tw-p-3"> <div class="tw-p-3">
{{if $file.IsIncomplete}} {{if $file.IsIncomplete}}
{{if $file.IsIncompleteLineTooLong}} {{ctx.Locale.Tr "repo.diff.file_suppressed"}}
{{ctx.Locale.Tr "repo.diff.file_suppressed_line_too_long"}} <a class="ui basic tiny button" data-global-click="diffLoadFileBody" data-href="?file-only=true&files={{$file.Name}}&files={{$file.OldName}}">{{ctx.Locale.Tr "repo.diff.load"}}</a>
{{else}}
{{ctx.Locale.Tr "repo.diff.file_suppressed"}}
<a class="ui basic tiny button" data-global-click="diffLoadFileBody" data-href="?file-only=true&files={{$file.Name}}&files={{$file.OldName}}">{{ctx.Locale.Tr "repo.diff.load"}}</a>
{{end}}
{{else}} {{else}}
{{ctx.Locale.Tr "repo.diff.bin_not_shown"}} {{ctx.Locale.Tr "repo.diff.bin_not_shown"}}
{{end}} {{end}}
+65
View File
@@ -5,12 +5,19 @@ package integration
import ( import (
"net/http" "net/http"
"strings"
"testing" "testing"
repo_model "gitea.dev/models/repo"
"gitea.dev/models/unittest"
"gitea.dev/modules/git"
"gitea.dev/modules/setting"
"gitea.dev/modules/test"
"gitea.dev/tests" "gitea.dev/tests"
"github.com/PuerkitoBio/goquery" "github.com/PuerkitoBio/goquery"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
) )
func TestPullDiff(t *testing.T) { func TestPullDiff(t *testing.T) {
@@ -65,3 +72,61 @@ func testPullDiffAssertPage(t *testing.T, prDiffURL string, reviewBtnDisabled bo
// Ensure the review button is enabled for full PR reviews // Ensure the review button is enabled for full PR reviews
assert.Equal(t, reviewBtnDisabled, doc.Find(".js-btn-review").HasClass("disabled")) assert.Equal(t, reviewBtnDisabled, doc.Find(".js-btn-review").HasClass("disabled"))
} }
func TestLongLineDiffRendering(t *testing.T) {
defer tests.PrepareTestEnv(t)()
defer test.MockVariableValue(&setting.Git.MaxGitDiffLineCharacters, 32)()
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
const suffix = "DISCARDED_SUFFIX"
const short = "following short line"
contextLine := strings.Repeat("context ", 8) + suffix + "\n"
before := git.FastImportCommit{Ref: "refs/heads/long-line-before"}
after := git.FastImportCommit{Ref: "refs/heads/long-line-after"}
for _, name := range []string{"long.txt", "long.csv"} {
before.Files = append(before.Files, git.FastImportFile{Path: name, Content: strings.Repeat("old ", 16) + suffix + "\n" + contextLine + short + "\n"})
after.Files = append(after.Files, git.FastImportFile{Path: name, Content: strings.Repeat("new ", 16) + suffix + "\n" + contextLine + short + "\n"})
}
outsideHunk := "header\n" + contextLine + strings.Repeat("unchanged\n", 6)
before.Files = append(before.Files, git.FastImportFile{Path: "outside.csv", Content: outsideHunk + "old\n"})
after.Files = append(after.Files, git.FastImportFile{Path: "outside.csv", Content: outsideHunk + "new\n"})
require.NoError(t, git.ForceFastImport(t.Context(), repo.CodeStorageRepo(), []git.FastImportCommit{before, after}))
oldPrefix := strings.Repeat("old ", 8)[:31]
newPrefix := strings.Repeat("new ", 8)[:31]
contextPrefix := contextLine[:31]
for style, want := range map[string][]string{
"unified": {oldPrefix, newPrefix, contextPrefix, short},
"split": {oldPrefix, newPrefix, contextPrefix, contextPrefix, short, short},
} {
t.Run(style, func(t *testing.T) {
req := NewRequest(t, "GET", "/user2/repo1/compare/long-line-before..long-line-after?style="+style)
resp := MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
for _, filename := range []string{"long.txt", "long.csv"} {
t.Run(filename, func(t *testing.T) {
diffFileBox := doc.Find(`.diff-file-box[data-new-filename="` + filename + `"]`)
diffBody := diffFileBox.Find(".code-diff-" + style)
require.Equal(t, 1, diffBody.Length())
assert.False(t, diffBody.HasClass("tw-hidden"))
assert.Empty(t, diffFileBox.Find(".file-view-toggle, .data-table").Nodes)
assert.NotContains(t, diffFileBox.Text(), suffix)
got := diffBody.Find("tr:not(.tag-code) code.code-inner").Map(func(_ int, s *goquery.Selection) string {
markerText := s.Find(".diff-line-truncated").Text()
textContent := strings.TrimSpace(strings.TrimSuffix(strings.TrimSpace(s.Text()), markerText))
if textContent == short {
assert.Empty(t, markerText)
} else {
assert.Equal(t, "Line truncated", markerText)
}
return textContent
})
assert.Equal(t, want, got)
})
}
outside := doc.Find(`.diff-file-box[data-new-filename="outside.csv"]`)
assert.Equal(t, 2, outside.Find(".file-view-toggle").Length())
assert.Equal(t, 1, outside.Find(".data-table").Length())
})
}
}
+11
View File
@@ -686,6 +686,17 @@ overflow-menu .ui.label:empty {
line-height: inherit; /* needed for inline code preview in markup */ line-height: inherit; /* needed for inline code preview in markup */
} }
.code-inner:has(.diff-line-truncated) {
text-decoration: underline dashed 1px;
text-underline-offset: 4px;
}
.code-inner .diff-line-truncated {
margin-left: var(--gap-inline);
padding: 0 4px;
user-select: none;
}
.blame .code-inner { .blame .code-inner {
white-space: pre-wrap; white-space: pre-wrap;
overflow-wrap: anywhere; overflow-wrap: anywhere;