mirror of
https://github.com/go-gitea/gitea.git
synced 2026-08-10 05:24:18 +09:00
fix(gitdiff): render exact-limit diffs and zero-limit comments (#38838)
This commit fixes the treatment of various edge cases related to diff limits. - Exact-limit diffs: a file containing exactly the configured maximum was incorrectly treated as too large and hidden behind “Load diff.” It now renders normally. Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
co-authored by
wxiaoguang
parent
ad6107ab88
commit
79535f4e01
+18
-18
@@ -1010,15 +1010,17 @@ 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 parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio.Reader) (lineBytes []byte, isFragment bool, err error) {
|
||||||
sb := strings.Builder{}
|
sb := strings.Builder{}
|
||||||
|
|
||||||
var (
|
var curSection *DiffSection
|
||||||
curSection *DiffSection
|
curFileLFSPrefix := false
|
||||||
curFileLinesCount int
|
|
||||||
curFileLFSPrefix bool
|
|
||||||
)
|
|
||||||
|
|
||||||
lastLeftIdx := -1
|
lastLeftIdx := -1
|
||||||
leftLine, rightLine := 1, 1
|
leftLine, rightLine := 1, 1
|
||||||
|
|
||||||
|
curFileLinesCount := 0
|
||||||
|
curFileLineReachesLimit := func() bool {
|
||||||
|
return maxLines > -1 && curFileLinesCount >= maxLines
|
||||||
|
}
|
||||||
|
|
||||||
for {
|
for {
|
||||||
for isFragment {
|
for isFragment {
|
||||||
curFile.IsIncomplete = true
|
curFile.IsIncomplete = true
|
||||||
@@ -1045,7 +1047,7 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
|
|||||||
|
|
||||||
switch lineBytes[0] {
|
switch lineBytes[0] {
|
||||||
case '@':
|
case '@':
|
||||||
if maxLines > -1 && curFileLinesCount >= maxLines {
|
if curFileLineReachesLimit() {
|
||||||
curFile.IsIncomplete = true
|
curFile.IsIncomplete = true
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
@@ -1068,8 +1070,8 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
|
|||||||
lastLeftIdx = -1
|
lastLeftIdx = -1
|
||||||
curFile.Sections = append(curFile.Sections, curSection)
|
curFile.Sections = append(curFile.Sections, curSection)
|
||||||
|
|
||||||
// FIXME: the "-1" can't be right, these "line idx" are all 1-based, maybe there are other bugs that covers this bug.
|
// use "idx-1" as "last idx" (the last line before this hunk)
|
||||||
lineSectionInfo := newDiffLineSectionInfo(curFile, line, leftLine-1, rightLine-1)
|
lineSectionInfo := newDiffLineSectionInfo(curFile, line, leftLine-1 /*lastLeftIdx*/, rightLine-1 /*lastRightIdx*/)
|
||||||
diffLine := &DiffLine{
|
diffLine := &DiffLine{
|
||||||
Type: DiffLineSection,
|
Type: DiffLineSection,
|
||||||
Content: line,
|
Content: line,
|
||||||
@@ -1082,10 +1084,6 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
|
|||||||
rightLine = lineSectionInfo.RightIdx
|
rightLine = lineSectionInfo.RightIdx
|
||||||
continue
|
continue
|
||||||
case '\\':
|
case '\\':
|
||||||
if maxLines > -1 && curFileLinesCount >= maxLines {
|
|
||||||
curFile.IsIncomplete = true
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
// 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, false, fmt.Errorf("unexpected line in hunk: %s", string(lineBytes))
|
||||||
@@ -1094,12 +1092,13 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
|
|||||||
// 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
|
||||||
continue
|
continue
|
||||||
case '+':
|
case '+':
|
||||||
curFileLinesCount++
|
|
||||||
curFile.Addition++
|
curFile.Addition++
|
||||||
if maxLines > -1 && curFileLinesCount >= maxLines {
|
if curFileLineReachesLimit() {
|
||||||
curFile.IsIncomplete = true
|
curFile.IsIncomplete = true
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
curFileLinesCount++
|
||||||
|
|
||||||
diffLine := &DiffLine{Type: DiffLineAdd, RightIdx: rightLine, Match: -1}
|
diffLine := &DiffLine{Type: DiffLineAdd, RightIdx: rightLine, Match: -1}
|
||||||
rightLine++
|
rightLine++
|
||||||
if curSection == nil {
|
if curSection == nil {
|
||||||
@@ -1125,12 +1124,13 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
case '-':
|
case '-':
|
||||||
curFileLinesCount++
|
|
||||||
curFile.Deletion++
|
curFile.Deletion++
|
||||||
if maxLines > -1 && curFileLinesCount >= maxLines {
|
if curFileLineReachesLimit() {
|
||||||
curFile.IsIncomplete = true
|
curFile.IsIncomplete = true
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
curFileLinesCount++
|
||||||
|
|
||||||
diffLine := &DiffLine{Type: DiffLineDel, LeftIdx: leftLine, Match: -1}
|
diffLine := &DiffLine{Type: DiffLineDel, LeftIdx: leftLine, Match: -1}
|
||||||
if leftLine > 0 {
|
if leftLine > 0 {
|
||||||
leftLine++
|
leftLine++
|
||||||
@@ -1153,11 +1153,11 @@ func parseHunks(ctx context.Context, curFile *DiffFile, maxLines, maxLineCharact
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
case ' ':
|
case ' ':
|
||||||
curFileLinesCount++
|
if curFileLineReachesLimit() {
|
||||||
if maxLines > -1 && curFileLinesCount >= maxLines {
|
|
||||||
curFile.IsIncomplete = true
|
curFile.IsIncomplete = true
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
curFileLinesCount++
|
||||||
diffLine := &DiffLine{Type: DiffLinePlain, LeftIdx: leftLine, RightIdx: rightLine}
|
diffLine := &DiffLine{Type: DiffLinePlain, LeftIdx: leftLine, RightIdx: rightLine}
|
||||||
leftLine++
|
leftLine++
|
||||||
rightLine++
|
rightLine++
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ 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"
|
||||||
@@ -548,6 +549,43 @@ index 0000000..6bb8f39
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestParsePatchExactLineLimit(t *testing.T) {
|
||||||
|
for _, test := range []struct {
|
||||||
|
name, hunk string
|
||||||
|
limit, lines int
|
||||||
|
incomplete bool
|
||||||
|
}{
|
||||||
|
{name: "zero", limit: 0, hunk: "@@ -1,3 +1,3 @@\n one\n two\n three\n", incomplete: true},
|
||||||
|
{name: "one", limit: 1, lines: 1, hunk: "@@ -1,3 +1,3 @@\n one\n two\n three\n", incomplete: true},
|
||||||
|
{name: "N plus one", limit: 2, lines: 2, hunk: "@@ -1,3 +1,3 @@\n one\n two\n three\n", incomplete: true},
|
||||||
|
{name: "N", limit: 3, lines: 3, hunk: "@@ -1,3 +1,3 @@\n one\n two\n three\n"},
|
||||||
|
{name: "addition", limit: 1, lines: 1, hunk: "@@ -0,0 +1 @@\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: "hunk at capacity", limit: 1, lines: 1, hunk: "@@ -1 +1 @@\n one\n@@ -3 +3 @@\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
|
||||||
|
diff, err := ParsePatch(t.Context(), test.limit, 5000, 10, strings.NewReader(patch), "")
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Len(t, diff.Files, 1)
|
||||||
|
diffFile := diff.Files[0]
|
||||||
|
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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func setupDefaultDiff() *Diff {
|
func setupDefaultDiff() *Diff {
|
||||||
return &Diff{
|
return &Diff{
|
||||||
Files: []*DiffFile{
|
Files: []*DiffFile{
|
||||||
|
|||||||
Reference in New Issue
Block a user