From 1e13badb390e8e9ebf2ff3f40c151f8f8154664f Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Sat, 12 Sep 2026 13:56:25 +0200 Subject: [PATCH] enhance: truncate but show long lines in diffs (#39279) Co-authored-by: wxiaoguang --- modules/charset/escape.go | 17 +- modules/git/diff.go | 10 +- modules/htmlutil/html.go | 9 +- modules/htmlutil/html_test.go | 2 +- modules/markup/jupyter/jupyter.go | 26 +- options/locale/locale_en-US.json | 2 +- services/gitdiff/gitdiff.go | 319 ++++++++++++------------- services/gitdiff/gitdiff_excerpt.go | 4 +- services/gitdiff/gitdiff_parse_test.go | 48 ++++ services/gitdiff/gitdiff_test.go | 129 +++++++--- templates/repo/diff/box.tmpl | 10 +- tests/integration/pull_diff_test.go | 65 +++++ web_src/css/base.css | 11 + 13 files changed, 430 insertions(+), 222 deletions(-) create mode 100644 services/gitdiff/gitdiff_parse_test.go diff --git a/modules/charset/escape.go b/modules/charset/escape.go index 596bde8e74b..ce3df61b93b 100644 --- a/modules/charset/escape.go +++ b/modules/charset/escape.go @@ -8,6 +8,7 @@ import ( "io" "strings" + "gitea.dev/modules/htmlutil" "gitea.dev/modules/setting" "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 func EscapeControlHTML(html template.HTML, locale translation.Locale, opts ...EscapeOptions) (escaped *EscapeStatus, output template.HTML) { - if !setting.UI.AmbiguousUnicodeDetection { - return &EscapeStatus{}, html - } - sb := &strings.Builder{} - escaped, _ = EscapeControlReader(strings.NewReader(string(html)), sb, locale, opts...) // err has been handled in EscapeControlReader + sb, w := htmlutil.NewHTMLStringWriter() + escaped = EscapeControlHTMLTo(html, locale, w, opts...) return escaped, template.HTML(sb.String()) } diff --git a/modules/git/diff.go b/modules/git/diff.go index 86b25cc8860..8f49fb837a0 100644 --- a/modules/git/diff.go +++ b/modules/git/diff.go @@ -140,6 +140,14 @@ func isHeader(lof string, inHunk bool) bool { 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 // it also recalculates hunks and adds the appropriate headers to the new diff. // Warning: Only one-file diffs are allowed. @@ -149,7 +157,7 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi return "", nil } - scanner := bufio.NewScanner(originalDiff) + scanner := NewGitDiffScanner(originalDiff) hunk := make([]string, 0) // begin is the start of the hunk containing searched line diff --git a/modules/htmlutil/html.go b/modules/htmlutil/html.go index cba73e1fb83..f1e3a87b08a 100644 --- a/modules/htmlutil/html.go +++ b/modules/htmlutil/html.go @@ -93,7 +93,7 @@ type HTMLWriter interface { OriginWriter() io.Writer WriteString(s string) HTMLWriter WriteHTML(s template.HTML) HTMLWriter - WriteFormat(fmt template.HTML, args ...any) HTMLWriter + WriteFormatf(fmt template.HTML, args ...any) HTMLWriter Err() error } @@ -120,7 +120,7 @@ func (h *htmlWriter) WriteHTML(s template.HTML) HTMLWriter { 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 { h.errs = append(h.errs, err) } @@ -135,6 +135,11 @@ func NewHTMLWriter(w io.Writer) HTMLWriter { return &htmlWriter{w: w} } +func NewHTMLStringWriter() (*strings.Builder, HTMLWriter) { + sb := &strings.Builder{} + return sb, &htmlWriter{w: sb} +} + type HTMLBuilder struct { sb strings.Builder } diff --git a/modules/htmlutil/html_test.go b/modules/htmlutil/html_test.go index a499bfba8a1..7175b129f74 100644 --- a/modules/htmlutil/html_test.go +++ b/modules/htmlutil/html_test.go @@ -34,7 +34,7 @@ func TestHTMLBuilder(t *testing.T) { func TestHTMLWriter(t *testing.T) { sb := new(strings.Builder) w := NewHTMLWriter(sb) - w.WriteString("<").WriteHTML("
").WriteFormat("%s%s", ">", EscapeString(">")) + w.WriteString("<").WriteHTML("
").WriteFormatf("%s%s", ">", EscapeString(">")) assert.Equal(t, "<
>>", sb.String()) assert.NoError(t, w.Err()) } diff --git a/modules/markup/jupyter/jupyter.go b/modules/markup/jupyter/jupyter.go index a8b56250f57..0d5825fc32a 100644 --- a/modules/markup/jupyter/jupyter.go +++ b/modules/markup/jupyter/jupyter.go @@ -39,18 +39,18 @@ type mimeHandler struct { } func renderCellCodeOutputTextPlain(w htmlutil.HTMLWriter, text string) error { - w.WriteFormat(`
%s
`, text) + w.WriteFormatf(`
%s
`, text) return w.Err() } func renderCellCodeOutputUnsupported(w htmlutil.HTMLWriter, message string) error { - w.WriteFormat(`
%s
`, message) + w.WriteFormatf(`
%s
`, message) return w.Err() } var dataMimeHandlers = sync.OnceValue(func() []mimeHandler { renderImage := func(w htmlutil.HTMLWriter, subtype, payload string) error { - w.WriteFormat(`
`, subtype, payload) + w.WriteFormatf(`
`, subtype, payload) return w.Err() } 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, // 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. - w.WriteFormat(`
%s
`, markup.Sanitize(d)) + w.WriteFormatf(`
%s
`, markup.Sanitize(d)) return w.Err() }}, {"text/latex", func(w htmlutil.HTMLWriter, d string) error { - w.WriteFormat(`
%s
`, trimMathDelimiters(d)) + w.WriteFormatf(`
%s
`, trimMathDelimiters(d)) return w.Err() }}, {"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 var notebook Notebook if err := json.NewDecoder(input).Decode(¬ebook); err != nil { - htmlWriter.WriteFormat(`
Failed to parse notebook JSON: %v
`, err) + htmlWriter.WriteFormatf(`
Failed to parse notebook JSON: %v
`, err) return htmlWriter.Err() } // Check nbformat version 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) - htmlWriter.WriteFormat(`
%s
`, msg) + htmlWriter.WriteFormatf(`
%s
`, msg) return htmlWriter.Err() } @@ -205,7 +205,7 @@ func renderCellCode(output htmlutil.HTMLWriter, cell Cell, language string) erro output.WriteHTML(`
`) { if executionCount != nil { - output.WriteFormat(`
In [%d]:
`, *executionCount) + output.WriteFormatf(`
In [%d]:
`, *executionCount) } else { output.WriteHTML(`
In [ ]:
`) } @@ -213,7 +213,7 @@ func renderCellCode(output htmlutil.HTMLWriter, cell Cell, language string) erro // Highlight code preAttrs, codeAttrs := highlight.CodeBlockAttributes(language) lexer := highlight.DetectChromaLexerByFileName("", language) - output.WriteFormat(`
`, preAttrs, codeAttrs)
+		output.WriteFormatf(`
`, preAttrs, codeAttrs)
 		output.WriteHTML(highlight.RenderCodeByLexer(lexer, source))
 		output.WriteHTML("
") } @@ -232,7 +232,7 @@ func renderCellCode(output htmlutil.HTMLWriter, cell Cell, language string) erro output.WriteHTML(`
`) { if hasExecutionResult && executionCount != nil { - output.WriteFormat(`
Out [%d]:
`, *executionCount) + output.WriteFormatf(`
Out [%d]:
`, *executionCount) } else { output.WriteHTML(`
`) } @@ -250,7 +250,7 @@ func renderCellCode(output htmlutil.HTMLWriter, cell Cell, language string) erro } func renderCellPrompt(output htmlutil.HTMLWriter, left, right template.HTML) { - output.WriteFormat(` + output.WriteFormatf(`
%s
@@ -335,7 +335,7 @@ func renderCellCodeOutput(output htmlutil.HTMLWriter, out Output) { // Stream output if out.OutputType == "stream" && out.Text != nil { streamName := util.Iif(out.Name == "stderr", "stderr", "stdout") - output.WriteFormat(`
%s
`, streamName, joinSource(out.Text)) + output.WriteFormatf(`
%s
`, streamName, joinSource(out.Text)) return } @@ -352,7 +352,7 @@ func renderCellCodeOutput(output htmlutil.HTMLWriter, out Output) { if traceback == "" && out.Ename != "" { traceback = fmt.Sprintf("%s: %s", out.Ename, out.Evalue) } - output.WriteFormat(`
%s
`, traceback) + output.WriteFormatf(`
%s
`, traceback) return } diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 0c53db9629b..dca46b34dbc 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -2608,8 +2608,8 @@ "repo.diff.file_image_width": "Width", "repo.diff.file_image_height": "Height", "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_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.show_more": "Show more", "repo.diff.load": "Load diff", diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index d0c80c78c3f..505560dcc19 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -9,15 +9,16 @@ import ( "bytes" "context" "fmt" - "html" "html/template" "io" "net/url" "path" + "slices" "sort" "strconv" "strings" "time" + "unicode/utf8" "gitea.dev/models/db" git_model "gitea.dev/models/git" @@ -79,8 +80,9 @@ type DiffLine struct { Content string Comments issues_model.CommentList // related PR code comments SectionInfo *DiffLineSectionInfo + IsTruncated bool - cachedDiffInline *DiffInline + cachedDiffInline *DiffInlineComputed } // DiffLineSectionInfo represents diff line section metadata @@ -327,28 +329,32 @@ func defaultDiffMatchPatch() *diffmatchpatch.DiffMatchPatch { return dmp } -// DiffInline is a struct that has a content and escape status -type DiffInline struct { +// DiffInlineComputed is the final computed diff line for template rendering +type DiffInlineComputed struct { EscapeStatus *charset.EscapeStatus Content template.HTML + IsTruncated bool } -// diffInlineWithUnicodeEscape makes a DiffInline with hidden Unicode characters escaped -func diffInlineWithUnicodeEscape(s template.HTML, locale translation.Locale) DiffInline { - status, content := charset.EscapeControlHTML(s, locale) - return DiffInline{EscapeStatus: status, Content: content} +// computeDiffInline makes a DiffInline with computed content, e.g.: Unicode escaping, truncation hint, etc +func computeDiffInline(s template.HTML, isTruncated bool, locale translation.Locale) DiffInlineComputed { + sb, w := htmlutil.NewHTMLStringWriter() + status := charset.EscapeControlHTMLTo(s, locale, w) + if isTruncated { + w.WriteFormatf(`%s`, 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 { - h, ok := highlightLines[lineIdx-1] - if ok { + if h, ok := highlightLines[lineIdx-1]; ok && !diffLine.IsTruncated { return h } if diffLine.Content == "" { return "" } if setting.Git.DisableDiffHighlight { - return template.HTML(html.EscapeString(diffLine.Content[1:])) + return htmlutil.EscapeString(diffLine.Content[1:]) } if diffSection.highlightLexer.value == nil { 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:]) } -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 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 // caller always uses the "right side" for this type lineHTML := diffSection.getLineContentForRender(rightLine.RightIdx, rightLine, fileLanguage, highlightedRightLines) - return diffInlineWithUnicodeEscape(lineHTML, locale) + return computeDiffInline(lineHTML, rightLine.IsTruncated, locale) } var diffs [2]template.HTML + var truncations [2]bool if leftLine != nil { diffs[0] = diffSection.getLineContentForRender(leftLine.LeftIdx, leftLine, fileLanguage, highlightedLeftLines) + truncations[0] = leftLine.IsTruncated } if rightLine != nil { 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". // "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, // so here only diff them once, then next render can just use the cached result, no need to "diff" again. hcd := newHighlightCodeDiff() lineHTMLDel, lineHTMLAdd := hcd.diffLineWithHighlight(diffs[0], diffs[1]) - leftLine.cachedDiffInline = new(diffInlineWithUnicodeEscape(lineHTMLDel, locale)) - rightLine.cachedDiffInline = new(diffInlineWithUnicodeEscape(lineHTMLAdd, locale)) + leftLine.cachedDiffInline = new(computeDiffInline(lineHTMLDel, truncations[0], locale)) + rightLine.cachedDiffInline = new(computeDiffInline(lineHTMLAdd, truncations[1], locale)) 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. - // the tmpl code already adds background colors for these cases. - return diffInlineWithUnicodeEscape(diffs[sideIdx], locale) + // if left is empty or right is empty (a line is fully deleted or added), or either side is truncated (too long), + // then we do not need to diff anymore, the tmpl code already adds background colors for these cases. + return computeDiffInline(diffs[sideIdx], truncations[sideIdx], locale) } // 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() { 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 @@ -416,7 +425,7 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine, loc switch diffLine.Type { 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 - return diffInlineWithUnicodeEscape(htmlutil.EscapeString(diffLine.Content), locale) + return computeDiffInline(htmlutil.EscapeString(diffLine.Content), diffLine.IsTruncated, locale) case DiffLineAdd: compareDiffLine := diffSection.GetLine(diffLine.Match) return diffSection.getDiffLineForRender(DiffLineAdd, compareDiffLine, diffLine, locale) @@ -456,9 +465,9 @@ type DiffFile struct { IsRenamed bool IsSubmodule bool // basic fields but for render purpose only - Sections []*DiffSection - IsIncomplete bool - IsIncompleteLineTooLong bool + Sections []*DiffSection + IsIncomplete bool // file is too large + HasTruncatedLines bool // some lines are too long // will be filled by the extra loop in GitDiffForRender IsGenerated bool @@ -492,6 +501,10 @@ func (diffFile *DiffFile) GetType() int { return int(diffFile.Type) } +func (diffFile *DiffFile) CanShowFileViewToggle() bool { + return diffFile.IsBlobTypeImage || (diffFile.IsBlobTypeCsv && !diffFile.IsIncomplete && !diffFile.HasTruncatedLines) +} + type DiffRenderDetail struct { needTailSection bool leftLineCount, rightLineCount int @@ -685,62 +698,64 @@ func (diff *Diff) LoadComments(ctx context.Context, issue *issues_model.Issue, c const cmdDiffHead = "diff --git " -// ParsePatch builds a Diff object from a io.Reader and some parameters. -func ParsePatch(ctx context.Context, maxLines, maxLineCharacters, maxFiles int, reader io.Reader, skipToFile string) (*Diff, error) { - log.Debug("ParsePatch(%d, %d, %d, ..., %s)", maxLines, maxLineCharacters, maxFiles, skipToFile) - var curFile *DiffFile +// to correctly parse a diff line, the input buffer size should be large enough to read a full line for git diff headers +var defaultDiffLineBufferSize = 8 * 1024 - 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)} - - 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) + readerSize := max(maxLineCharacters, defaultDiffLineBufferSize) input := bufio.NewReaderSize(reader, readerSize) line, err := input.ReadString('\n') if err != nil { - if err == io.EOF { - return diff, nil - } - return diff, err + return diff, util.Iif(err == io.EOF, nil, 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)) } -parsingLoop: - for { + { // 1. A patch file always begins with `diff --git ` + `a/path b/path` (possibly quoted) // if it does not we have bad input! 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 { lastFile := createDiffFile(line) diff.End = lastFile.Name diff.IsIncomplete = true - break parsingLoop + return "", nil } - curFile = createDiffFile(line) - if skipping { + curFile := createDiffFile(line) + if *skipping { if curFile.Name != skipToFile { - line, err = skipToNextDiffHead(input) - if err != nil { - if err == io.EOF { - return diff, nil - } - return diff, err - } - continue + return skipToNextDiffHead(input) } - skipping = false + *skipping = false } diff.Files = append(diff.Files, curFile) @@ -783,27 +798,23 @@ parsingLoop: // Binary files a/ and b/ differ // // but one of a/ and b/ could be /dev/null. - curFileLoop: for { line, err = input.ReadString('\n') if err != nil { - if err != io.EOF { - return diff, err - } - break parsingLoop + return line, err } switch { case strings.HasPrefix(line, cmdDiffHead): - break curFileLoop + return line, nil case strings.HasPrefix(line, "old mode ") || strings.HasPrefix(line, "new 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 ") { - curFile.EntryMode = prepareValue(line, "new mode ") + curFile.EntryMode = extractGitDiffHead(line, "new mode ") } if strings.HasSuffix(line, " 160000\n") { curFile.IsSubmodule, curFile.SubmoduleDiffInfo = true, &SubmoduleDiffInfo{} @@ -812,33 +823,33 @@ parsingLoop: curFile.IsRenamed = true curFile.Type = DiffFileRename if curFile.isAmbiguous { - curFile.OldName = prepareValue(line, "rename from ") + curFile.OldName = extractGitDiffHead(line, "rename from ") } case strings.HasPrefix(line, "rename to "): curFile.IsRenamed = true curFile.Type = DiffFileRename if curFile.isAmbiguous { - curFile.Name = prepareValue(line, "rename to ") + curFile.Name = extractGitDiffHead(line, "rename to ") curFile.isAmbiguous = false } case strings.HasPrefix(line, "copy from "): curFile.IsRenamed = true curFile.Type = DiffFileCopy if curFile.isAmbiguous { - curFile.OldName = prepareValue(line, "copy from ") + curFile.OldName = extractGitDiffHead(line, "copy from ") } case strings.HasPrefix(line, "copy to "): curFile.IsRenamed = true curFile.Type = DiffFileCopy if curFile.isAmbiguous { - curFile.Name = prepareValue(line, "copy to ") + curFile.Name = extractGitDiffHead(line, "copy to ") curFile.isAmbiguous = false } case strings.HasPrefix(line, "new file"): curFile.Type = DiffFileAdd curFile.IsCreated = true 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") { curFile.IsSubmodule, curFile.SubmoduleDiffInfo = true, &SubmoduleDiffInfo{} @@ -892,31 +903,16 @@ parsingLoop: curFile.isAmbiguous = false } // Otherwise do nothing with this line, but now switch to parsing hunks - lineBytes, isFragment, err := parseHunks(ctx, curFile, maxLines, maxLineCharacters, input) - if err != nil { - if err != io.EOF { - return diff, err - } - 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 + nextLine, err := parseHunks(ctx, curFile, maxLines, maxLineCharacters, input) + return string(nextLine), err + default: + // ignore other extended header lines } } } +} +func (diff *Diff) postProcessFiles() { // TODO: There are numerous issues with this: // - 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 @@ -964,38 +960,18 @@ parsingLoop: } } } - - return diff, nil } func skipToNextDiffHead(input *bufio.Reader) (line string, err error) { - // need to skip until the next cmdDiffHead - var isFragment, wasFragment bool - var lineBytes []byte for { - lineBytes, isFragment, err = input.ReadLine() + lineBytes, _, err := readGitDiffLineWithDiscard(input) if err != nil { return "", err } - if wasFragment { - wasFragment = isFragment - continue - } 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 { @@ -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) { - sb := strings.Builder{} +func readGitDiffLineWithDiscard(r *bufio.Reader) (_ []byte, truncated bool, _ error) { + // 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 curFileLFSPrefix := false @@ -1022,27 +1048,12 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact } for { - for isFragment { - 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() + lineBytes, truncated, err := readGitDiffLineWithDiscard(input) if err != nil { - if err == io.EOF { - return lineBytes, isFragment, err - } - err = fmt.Errorf("unable to ReadLine: %w", err) - return nil, false, err + return lineBytes, err } - if lineBytes[0] == 'd' { - // End of hunks - return lineBytes, isFragment, err + if bytes.HasPrefix(lineBytes, []byte(cmdDiffHead)) { + return lineBytes, err } switch lineBytes[0] { @@ -1051,19 +1062,7 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact curFile.IsIncomplete = true continue } - - _, _ = 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() + line := string(lineBytes) // Create a new section to represent this hunk curSection = newDiffSectionForDiffFile(curFile) @@ -1086,7 +1085,7 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact case '\\': // 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")) { - 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! // 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) default: // 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) - if isFragment { - 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) - } - } - } + curFile.HasTruncatedLines = curFile.HasTruncatedLines || truncated if len(line) > maxLineCharacters { - curFile.IsIncomplete = true - curFile.IsIncompleteLineTooLong = true 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 if line[1:] == lfs.MetaFileIdentifier { @@ -1489,7 +1484,7 @@ func highlightCodeLines(name, lang string, sections []*DiffSection, isLeft bool, } if lineIdx >= 1 { idx := lineIdx - 1 - if idx < len(unsafeLines) { + if idx < len(unsafeLines) && !ln.IsTruncated { lines[idx] = template.HTML(util.UnsafeBytesToString(unsafeLines[idx])) } } diff --git a/services/gitdiff/gitdiff_excerpt.go b/services/gitdiff/gitdiff_excerpt.go index 6604795abac..94752884103 100644 --- a/services/gitdiff/gitdiff_excerpt.go +++ b/services/gitdiff/gitdiff_excerpt.go @@ -4,12 +4,12 @@ package gitdiff import ( - "bufio" "bytes" "fmt" "html/template" "io" + "gitea.dev/modules/git" "gitea.dev/modules/setting" "github.com/alecthomas/chroma/v2" @@ -29,7 +29,7 @@ type BlobExcerptOptions struct { func (diffSection *DiffSection) fillExcerptLines(reader io.Reader, leftStart, rightStart, chunkSize int) error { buf := &bytes.Buffer{} - scanner := bufio.NewScanner(reader) + scanner := git.NewGitDiffScanner(reader) var diffLines []*DiffLine for rightLineIdx := 1; rightLineIdx < rightStart+chunkSize; rightLineIdx++ { if ok := scanner.Scan(); !ok { diff --git a/services/gitdiff/gitdiff_parse_test.go b/services/gitdiff/gitdiff_parse_test.go new file mode 100644 index 00000000000..7c751c61d22 --- /dev/null +++ b/services/gitdiff/gitdiff_parse_test.go @@ -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])) + }) +} diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go index c1dd5615823..10dbcfc0424 100644 --- a/services/gitdiff/gitdiff_test.go +++ b/services/gitdiff/gitdiff_test.go @@ -19,7 +19,6 @@ import ( "gitea.dev/modules/json" "gitea.dev/modules/setting" "gitea.dev/modules/translation" - "gitea.dev/modules/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -203,7 +202,7 @@ func TestParsePatch_singlefile(t *testing.T) { tests := []testcase{ { - name: "readme.md2readme.md", + name: "same name", gitdiff: `diff --git "\\a/README.md" "\\b/README.md" --- "\\a/README.md" +++ "\\b/README.md" @@ -222,7 +221,7 @@ func TestParsePatch_singlefile(t *testing.T) { oldFilename: "README.md", }, { - name: "A \\ B", + name: `A \ B`, gitdiff: `diff --git "a/A \\ B" "b/A \\ B" --- "a/A \\ B" +++ "b/A \\ B" @@ -236,8 +235,8 @@ func TestParsePatch_singlefile(t *testing.T) { + cut off`, addition: 4, deletion: 1, - filename: "A \\ B", - oldFilename: "A \\ B", + filename: `A \ B`, + oldFilename: `A \ B`, }, { name: "really weird filename", @@ -327,7 +326,7 @@ index 0000000..92e798b deletion: 0, }, { - name: "rename", + name: "ambiguous rename 1", 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 @@ -337,17 +336,7 @@ rename to b filename: "b", }, { - name: "ambiguous 1", - 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", + 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 @@ -442,8 +431,8 @@ index 0000000..6bb8f39 if err != nil { t.Errorf("There should not be an error: %v", err) } - if !result.Files[0].IsIncomplete { - t.Errorf("Files should be incomplete! %v", result.Files[0]) + if result.Files[0].IsIncomplete { + t.Errorf("Files should not be incomplete! %v", result.Files[0]) } // Test max characters @@ -480,8 +469,8 @@ index 0000000..6bb8f39 if err != nil { t.Errorf("There should not be an error: %v", err) } - if !result.Files[0].IsIncomplete { - t.Errorf("Files should be incomplete! %v", result.Files[0]) + if result.Files[0].IsIncomplete { + t.Errorf("Files should not be incomplete! %v", result.Files[0]) } 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) { for _, test := range []struct { 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: "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: "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) { 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.Len(t, diff.Files, 1) diffFile := diff.Files[0] + assert.Equal(t, test.incomplete, diffFile.IsIncomplete) if test.limit == 0 { require.Len(t, diffFile.Sections, 0) } else { require.Len(t, diffFile.Sections, 1) - diffSection := diffFile.Sections[0] - 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) + assert.Len(t, diffFile.Sections[0].Lines, test.lines+1) // lines with a section } }) } @@ -1165,6 +1209,22 @@ func TestDiffSection_GetComputedInlineDiffFor(t *testing.T) { assert.True(t, diffInline.EscapeStatus.Escaped) assert.Equal(t, `@@ -1,3 +1,3 @@ func `+"\u202e"+`name() <b>`, 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 linerepo.diff.line_truncated`), inline.Content) + assert.True(t, inline.IsTruncated) + }) } 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, `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) { diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 298e8c905f7..9145c09c216 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -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*/}} {{$isImage:= $file.IsBlobTypeImage}} {{$isCsv := $file.IsBlobTypeCsv}} - {{$showFileViewToggle := or $isImage (and (not $file.IsIncomplete) $isCsv)}} + {{$showFileViewToggle := $file.CanShowFileViewToggle}} {{$isReviewFile := and $.IsSigned $.PageIsPullFiles (not $.Repository.IsArchived) $.IsShowingAllCommits}}
@@ -173,12 +173,8 @@ {{if or $file.IsIncomplete $file.IsBin}}
{{if $file.IsIncomplete}} - {{if $file.IsIncompleteLineTooLong}} - {{ctx.Locale.Tr "repo.diff.file_suppressed_line_too_long"}} - {{else}} - {{ctx.Locale.Tr "repo.diff.file_suppressed"}} - {{ctx.Locale.Tr "repo.diff.load"}} - {{end}} + {{ctx.Locale.Tr "repo.diff.file_suppressed"}} + {{ctx.Locale.Tr "repo.diff.load"}} {{else}} {{ctx.Locale.Tr "repo.diff.bin_not_shown"}} {{end}} diff --git a/tests/integration/pull_diff_test.go b/tests/integration/pull_diff_test.go index ea97a134b24..be300ab6a92 100644 --- a/tests/integration/pull_diff_test.go +++ b/tests/integration/pull_diff_test.go @@ -5,12 +5,19 @@ package integration import ( "net/http" + "strings" "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" "github.com/PuerkitoBio/goquery" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) 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 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()) + }) + } +} diff --git a/web_src/css/base.css b/web_src/css/base.css index 815efde74c8..6fa5260d93e 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -686,6 +686,17 @@ overflow-menu .ui.label:empty { 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 { white-space: pre-wrap; overflow-wrap: anywhere;