From 60468394e509008a36cf56792307ed02ae426cc8 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 17 Oct 2021 20:59:39 +0100 Subject: [PATCH 1/3] Allow Loading of Diffs that are too large This PR allows the loading of diffs that are suppressed because the file is too large. It does not handle diffs of files which have lines which are too long. Fix #17738 Signed-off-by: Andrew Thornton --- options/locale/locale_en-US.ini | 1 + routers/web/repo/commit.go | 31 +++++++---- routers/web/repo/compare.go | 23 +++++++- routers/web/repo/pull.go | 25 +++++++-- services/gitdiff/gitdiff.go | 96 +++++++++++++++++++------------- services/gitdiff/gitdiff_test.go | 11 +++- templates/repo/diff/box.tmpl | 1 + web_src/js/features/repo-diff.js | 23 ++++++++ 8 files changed, 150 insertions(+), 61 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 624f1b4d1b9f4..6e39b4b03d833 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2043,6 +2043,7 @@ diff.file_suppressed = File diff suppressed because it is too large diff.file_suppressed_line_too_long = File diff suppressed because one or more lines are too long diff.too_many_files = Some files were not shown because too many files have changed in this diff diff.show_more = Show More +diff.load = Load Diff diff.generated = generated diff.vendored = vendored diff.comment.placeholder = Leave a comment diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index c9e2b94f239ac..7f7091bbddf21 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -262,8 +262,6 @@ func Diff(ctx *context.Context) { err error ) - fileOnly := ctx.FormBool("file-only") - if ctx.Data["PageIsWiki"] != nil { gitRepo, err = git.OpenRepository(ctx.Repo.Repository.WikiPath()) if err != nil { @@ -288,13 +286,26 @@ func Diff(ctx *context.Context) { commitID = commit.ID.String() } - diff, err := gitdiff.GetDiffCommitWithWhitespaceBehavior(gitRepo, - commitID, ctx.FormString("skip-to"), setting.Git.MaxGitDiffLines, - setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, - gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), - false) + fileOnly := ctx.FormBool("file-only") + maxLines, maxLineCharacters, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles + files := ctx.FormStrings("files") + if fileOnly && (len(files) == 2 || len(files) == 1) { + maxLines, maxLineCharacters, maxFiles = -1, 4096, -1 + if setting.Git.MaxGitDiffLineCharacters > maxLineCharacters { + maxLineCharacters = setting.Git.MaxGitDiffLineCharacters + } + } + + diff, err := gitdiff.GetDiff(gitRepo, gitdiff.DiffOptions{ + AfterCommitID: commitID, + SkipTo: ctx.FormString("skip-to"), + MaxLines: maxLines, + MaxLineCharacters: maxLineCharacters, + MaxFiles: maxFiles, + WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), + }, files...) if err != nil { - ctx.NotFound("GetDiffCommitWithWhitespaceBehavior", err) + ctx.NotFound("GetDiff", err) return } @@ -325,10 +336,6 @@ func Diff(ctx *context.Context) { ctx.Data["Title"] = commit.Summary() + " ยท " + base.ShortSha(commitID) ctx.Data["Commit"] = commit ctx.Data["Diff"] = diff - if fileOnly { - ctx.HTML(http.StatusOK, tplDiffBox) - return - } statuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository.ID, commitID, db.ListOptions{}) if err != nil { diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 3d55e1e8ff396..f4a3facc57b62 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -569,9 +569,26 @@ func PrepareCompareDiff( beforeCommitID = ci.CompareInfo.BaseCommitID } - diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(ci.HeadGitRepo, - beforeCommitID, headCommitID, ctx.FormString("skip-to"), setting.Git.MaxGitDiffLines, - setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, whitespaceBehavior, ci.DirectComparison) + maxLines, maxLineCharacters, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles + files := ctx.FormStrings("files") + if len(files) == 2 || len(files) == 1 { + maxLines, maxLineCharacters, maxFiles = -1, 4096, -1 + if setting.Git.MaxGitDiffLineCharacters > maxLineCharacters { + maxLineCharacters = setting.Git.MaxGitDiffLineCharacters + } + } + + diff, err := gitdiff.GetDiff(ci.HeadGitRepo, + gitdiff.DiffOptions{ + BeforeCommitID: beforeCommitID, + AfterCommitID: headCommitID, + SkipTo: ctx.FormString("skip-to"), + MaxLines: maxLines, + MaxLineCharacters: maxLineCharacters, + MaxFiles: maxFiles, + WhitespaceBehavior: whitespaceBehavior, + DirectComparison: ci.DirectComparison, + }, ctx.FormStrings("files")...) if err != nil { ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err) return false diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index d5aa480d1f706..a7127a71d1c33 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -633,10 +633,27 @@ func ViewPullFiles(ctx *context.Context) { ctx.Data["Reponame"] = ctx.Repo.Repository.Name ctx.Data["AfterCommitID"] = endCommitID - diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(gitRepo, - startCommitID, endCommitID, ctx.FormString("skip-to"), setting.Git.MaxGitDiffLines, - setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, - gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), false) + fileOnly := ctx.FormBool("file-only") + + maxLines, maxLineCharacters, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles + files := ctx.FormStrings("files") + if fileOnly && (len(files) == 2 || len(files) == 1) { + maxLines, maxLineCharacters, maxFiles = -1, 4096, -1 + if setting.Git.MaxGitDiffLineCharacters > maxLineCharacters { + maxLineCharacters = setting.Git.MaxGitDiffLineCharacters + } + } + + diff, err := gitdiff.GetDiff(gitRepo, + gitdiff.DiffOptions{ + BeforeCommitID: startCommitID, + AfterCommitID: endCommitID, + SkipTo: ctx.FormString("skip-to"), + MaxLines: maxLines, + MaxLineCharacters: maxLineCharacters, + MaxFiles: maxFiles, + WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), + }, ctx.FormStrings("files")...) if err != nil { ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err) return diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index c6d11ca89e803..6dece90ead37d 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -695,6 +695,7 @@ const cmdDiffHead = "diff --git " // ParsePatch builds a Diff object from a io.Reader and some parameters. func ParsePatch(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 skipping := skipToFile != "" @@ -726,7 +727,7 @@ parsingLoop: return diff, fmt.Errorf("invalid first file line: %s", line) } - if len(diff.Files) >= maxFiles { + if maxFiles > -1 && len(diff.Files) >= maxFiles { lastFile := createDiffFile(diff, line) diff.End = lastFile.Name diff.IsIncomplete = true @@ -1038,7 +1039,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio switch lineBytes[0] { case '@': - if curFileLinesCount >= maxLines { + if maxLines > -1 && curFileLinesCount >= maxLines { curFile.IsIncomplete = true continue } @@ -1075,7 +1076,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio rightLine = lineSectionInfo.RightIdx continue case '\\': - if curFileLinesCount >= maxLines { + if maxLines > -1 && curFileLinesCount >= maxLines { curFile.IsIncomplete = true continue } @@ -1090,7 +1091,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio case '+': curFileLinesCount++ curFile.Addition++ - if curFileLinesCount >= maxLines { + if maxLines > -1 && curFileLinesCount >= maxLines { curFile.IsIncomplete = true continue } @@ -1114,7 +1115,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio case '-': curFileLinesCount++ curFile.Deletion++ - if curFileLinesCount >= maxLines { + if maxLines > -1 && curFileLinesCount >= maxLines { curFile.IsIncomplete = true continue } @@ -1134,7 +1135,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio curSection.Lines = append(curSection.Lines, diffLine) case ' ': curFileLinesCount++ - if curFileLinesCount >= maxLines { + if maxLines > -1 && curFileLinesCount >= maxLines { curFile.IsIncomplete = true continue } @@ -1278,13 +1279,25 @@ func readFileName(rd *strings.Reader) (string, bool) { return name[2:], ambiguity } -// GetDiffRangeWithWhitespaceBehavior builds a Diff between two commits of a repository. +// DiffOptions represents the options for a DiffRange +type DiffOptions struct { + BeforeCommitID string + AfterCommitID string + SkipTo string + MaxLines int + MaxLineCharacters int + MaxFiles int + WhitespaceBehavior string + DirectComparison bool +} + +// GetDiff builds a Diff between two commits of a repository. // Passing the empty string as beforeCommitID returns a diff from the parent commit. // The whitespaceBehavior is either an empty string or a git flag -func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID, afterCommitID, skipTo string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string, directComparison bool) (*Diff, error) { +func GetDiff(gitRepo *git.Repository, opts DiffOptions, files ...string) (*Diff, error) { repoPath := gitRepo.Path - commit, err := gitRepo.GetCommit(afterCommitID) + commit, err := gitRepo.GetCommit(opts.AfterCommitID) if err != nil { return nil, err } @@ -1293,45 +1306,54 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID, defer cancel() argsLength := 6 - if len(whitespaceBehavior) > 0 { + if len(opts.WhitespaceBehavior) > 0 { argsLength++ } - if len(skipTo) > 0 { + if len(opts.SkipTo) > 0 { argsLength++ } + if len(files) > 0 { + argsLength += len(files) + 1 + } diffArgs := make([]string, 0, argsLength) - if (len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA) && commit.ParentCount() == 0 { + if (len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == git.EmptySHA) && commit.ParentCount() == 0 { diffArgs = append(diffArgs, "diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M") - if len(whitespaceBehavior) != 0 { - diffArgs = append(diffArgs, whitespaceBehavior) + if len(opts.WhitespaceBehavior) != 0 { + diffArgs = append(diffArgs, opts.WhitespaceBehavior) } // append empty tree ref diffArgs = append(diffArgs, "4b825dc642cb6eb9a060e54bf8d69288fbee4904") - diffArgs = append(diffArgs, afterCommitID) + diffArgs = append(diffArgs, opts.AfterCommitID) } else { - actualBeforeCommitID := beforeCommitID + actualBeforeCommitID := opts.BeforeCommitID if len(actualBeforeCommitID) == 0 { parentCommit, _ := commit.Parent(0) actualBeforeCommitID = parentCommit.ID.String() } diffArgs = append(diffArgs, "diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M") - if len(whitespaceBehavior) != 0 { - diffArgs = append(diffArgs, whitespaceBehavior) + if len(opts.WhitespaceBehavior) != 0 { + diffArgs = append(diffArgs, opts.WhitespaceBehavior) } diffArgs = append(diffArgs, actualBeforeCommitID) - diffArgs = append(diffArgs, afterCommitID) - beforeCommitID = actualBeforeCommitID + diffArgs = append(diffArgs, opts.AfterCommitID) + opts.BeforeCommitID = actualBeforeCommitID } // In git 2.31, git diff learned --skip-to which we can use to shortcut skip to file // so if we are using at least this version of git we don't have to tell ParsePatch to do // the skipping for us - parsePatchSkipToFile := skipTo - if skipTo != "" && git.CheckGitVersionAtLeast("2.31") == nil { - diffArgs = append(diffArgs, "--skip-to="+skipTo) + parsePatchSkipToFile := opts.SkipTo + if opts.SkipTo != "" && git.CheckGitVersionAtLeast("2.31") == nil { + diffArgs = append(diffArgs, "--skip-to="+opts.SkipTo) parsePatchSkipToFile = "" } + + if len(files) > 0 { + diffArgs = append(diffArgs, "--") + diffArgs = append(diffArgs, files...) + } + cmd := exec.CommandContext(ctx, git.GitExecutable, diffArgs...) cmd.Dir = repoPath @@ -1349,16 +1371,16 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID, pid := process.GetManager().Add(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath), cancel) defer process.GetManager().Remove(pid) - diff, err := ParsePatch(maxLines, maxLineCharacters, maxFiles, stdout, parsePatchSkipToFile) + diff, err := ParsePatch(opts.MaxLines, opts.MaxLineCharacters, opts.MaxFiles, stdout, parsePatchSkipToFile) if err != nil { return nil, fmt.Errorf("unable to ParsePatch: %w", err) } - diff.Start = skipTo + diff.Start = opts.SkipTo var checker *git.CheckAttributeReader if git.CheckGitVersionAtLeast("1.7.8") == nil { - indexFilename, worktree, deleteTemporaryFile, err := gitRepo.ReadTreeToTemporaryIndex(afterCommitID) + indexFilename, worktree, deleteTemporaryFile, err := gitRepo.ReadTreeToTemporaryIndex(opts.AfterCommitID) if err == nil { defer deleteTemporaryFile() @@ -1370,12 +1392,12 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID, } ctx, cancel := context.WithCancel(git.DefaultContext) if err := checker.Init(ctx); err != nil { - log.Error("Unable to open checker for %s. Error: %v", afterCommitID, err) + log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) } else { go func() { err := checker.Run() if err != nil && err != ctx.Err() { - log.Error("Unable to open checker for %s. Error: %v", afterCommitID, err) + log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) } cancel() }() @@ -1426,7 +1448,7 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID, diffFile.IsGenerated = analyze.IsGenerated(diffFile.Name) } - tailSection := diffFile.GetTailSection(gitRepo, beforeCommitID, afterCommitID) + tailSection := diffFile.GetTailSection(gitRepo, opts.BeforeCommitID, opts.AfterCommitID) if tailSection != nil { diffFile.Sections = append(diffFile.Sections, tailSection) } @@ -1437,19 +1459,19 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID, } separator := "..." - if directComparison { + if opts.DirectComparison { separator = ".." } - shortstatArgs := []string{beforeCommitID + separator + afterCommitID} - if len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA { - shortstatArgs = []string{git.EmptyTreeSHA, afterCommitID} + shortstatArgs := []string{opts.BeforeCommitID + separator + opts.AfterCommitID} + if len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == git.EmptySHA { + shortstatArgs = []string{git.EmptyTreeSHA, opts.AfterCommitID} } diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(repoPath, shortstatArgs...) if err != nil && strings.Contains(err.Error(), "no merge base") { // git >= 2.28 now returns an error if base and head have become unrelated. // previously it would return the results of git diff --shortstat base head so let's try that... - shortstatArgs = []string{beforeCommitID, afterCommitID} + shortstatArgs = []string{opts.BeforeCommitID, opts.AfterCommitID} diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(repoPath, shortstatArgs...) } if err != nil { @@ -1459,12 +1481,6 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID, return diff, nil } -// GetDiffCommitWithWhitespaceBehavior builds a Diff representing the given commitID. -// The whitespaceBehavior is either an empty string or a git flag -func GetDiffCommitWithWhitespaceBehavior(gitRepo *git.Repository, commitID, skipTo string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string, directComparison bool) (*Diff, error) { - return GetDiffRangeWithWhitespaceBehavior(gitRepo, "", commitID, skipTo, maxLines, maxLineCharacters, maxFiles, whitespaceBehavior, directComparison) -} - // CommentAsDiff returns c.Patch as *Diff func CommentAsDiff(c *models.Comment) (*Diff, error) { diff, err := ParsePatch(setting.Git.MaxGitDiffLines, diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go index 76a8b4e7caf54..72adf33f555db 100644 --- a/services/gitdiff/gitdiff_test.go +++ b/services/gitdiff/gitdiff_test.go @@ -693,8 +693,15 @@ func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) { } defer gitRepo.Close() for _, behavior := range []string{"-w", "--ignore-space-at-eol", "-b", ""} { - diffs, err := GetDiffRangeWithWhitespaceBehavior(gitRepo, "559c156f8e0178b71cb44355428f24001b08fc68", "bd7063cc7c04689c4d082183d32a604ed27a24f9", "", - setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles, behavior, false) + diffs, err := GetDiff(gitRepo, + DiffOptions{ + AfterCommitID: "bd7063cc7c04689c4d082183d32a604ed27a24f9", + BeforeCommitID: "559c156f8e0178b71cb44355428f24001b08fc68", + MaxLines: setting.Git.MaxGitDiffLines, + MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters, + MaxFiles: setting.Git.MaxGitDiffFiles, + WhitespaceBehavior: behavior, + }) assert.NoError(t, err, fmt.Sprintf("Error when diff with %s", behavior)) for _, f := range diffs.Files { assert.True(t, len(f.Sections) > 0, fmt.Sprintf("%s should have sections", f.Name)) diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 2d910c924b93c..3ab7a11bbd1fa 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -112,6 +112,7 @@ {{$.i18n.Tr "repo.diff.file_suppressed_line_too_long"}} {{else}} {{$.i18n.Tr "repo.diff.file_suppressed"}} + {{$.i18n.Tr "repo.diff.load"}} {{end}} {{else}} {{$.i18n.Tr "repo.diff.bin_not_shown"}} diff --git a/web_src/js/features/repo-diff.js b/web_src/js/features/repo-diff.js index 9606e3baad770..98cae4a9bf705 100644 --- a/web_src/js/features/repo-diff.js +++ b/web_src/js/features/repo-diff.js @@ -103,4 +103,27 @@ export function initRepoDiffShowMore() { $('#diff-incomplete').replaceWith($(resp).find('#diff-file-boxes').children()); }); }); + $(document).on('click', 'a.diff-show-more-button', (e) => { + e.preventDefault(); + const $target = $(e.target); + + if ($target.hasClass('disabled')) { + return; + } + + $target.addClass('disabled'); + + const url = $target.data('href'); + $.ajax({ + type: 'GET', + url, + }).done((resp) => { + if (!resp || resp.html === '' || resp.empty) { + $target.removeClass('disabled'); + return; + } + + $target.parent().replaceWith($(resp).find('#diff-file-boxes .diff-file-body .file-body').children()); + }); + }); } From c97011a1d42b68061baaa03a86cd6ae1dd1783fa Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 20 Nov 2021 20:15:28 +0000 Subject: [PATCH 2/3] as per gusted Signed-off-by: Andrew Thornton --- routers/web/repo/commit.go | 11 ++++------- routers/web/repo/compare.go | 11 ++++------- routers/web/repo/pull.go | 11 ++++------- services/gitdiff/gitdiff.go | 2 +- services/gitdiff/gitdiff_test.go | 2 +- 5 files changed, 14 insertions(+), 23 deletions(-) diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 7f7091bbddf21..ecb5107a3d9b7 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -287,20 +287,17 @@ func Diff(ctx *context.Context) { } fileOnly := ctx.FormBool("file-only") - maxLines, maxLineCharacters, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles + maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles files := ctx.FormStrings("files") if fileOnly && (len(files) == 2 || len(files) == 1) { - maxLines, maxLineCharacters, maxFiles = -1, 4096, -1 - if setting.Git.MaxGitDiffLineCharacters > maxLineCharacters { - maxLineCharacters = setting.Git.MaxGitDiffLineCharacters - } + maxLines, maxFiles = -1, -1 } - diff, err := gitdiff.GetDiff(gitRepo, gitdiff.DiffOptions{ + diff, err := gitdiff.GetDiff(gitRepo, &gitdiff.DiffOptions{ AfterCommitID: commitID, SkipTo: ctx.FormString("skip-to"), MaxLines: maxLines, - MaxLineCharacters: maxLineCharacters, + MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters, MaxFiles: maxFiles, WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), }, files...) diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index f4a3facc57b62..fdaf6fc6c231e 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -569,22 +569,19 @@ func PrepareCompareDiff( beforeCommitID = ci.CompareInfo.BaseCommitID } - maxLines, maxLineCharacters, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles + maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles files := ctx.FormStrings("files") if len(files) == 2 || len(files) == 1 { - maxLines, maxLineCharacters, maxFiles = -1, 4096, -1 - if setting.Git.MaxGitDiffLineCharacters > maxLineCharacters { - maxLineCharacters = setting.Git.MaxGitDiffLineCharacters - } + maxLines, maxFiles = -1, -1 } diff, err := gitdiff.GetDiff(ci.HeadGitRepo, - gitdiff.DiffOptions{ + &gitdiff.DiffOptions{ BeforeCommitID: beforeCommitID, AfterCommitID: headCommitID, SkipTo: ctx.FormString("skip-to"), MaxLines: maxLines, - MaxLineCharacters: maxLineCharacters, + MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters, MaxFiles: maxFiles, WhitespaceBehavior: whitespaceBehavior, DirectComparison: ci.DirectComparison, diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index a7127a71d1c33..a7afc3a05c11d 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -635,22 +635,19 @@ func ViewPullFiles(ctx *context.Context) { fileOnly := ctx.FormBool("file-only") - maxLines, maxLineCharacters, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles + maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles files := ctx.FormStrings("files") if fileOnly && (len(files) == 2 || len(files) == 1) { - maxLines, maxLineCharacters, maxFiles = -1, 4096, -1 - if setting.Git.MaxGitDiffLineCharacters > maxLineCharacters { - maxLineCharacters = setting.Git.MaxGitDiffLineCharacters - } + maxLines, maxFiles = -1, -1 } diff, err := gitdiff.GetDiff(gitRepo, - gitdiff.DiffOptions{ + &gitdiff.DiffOptions{ BeforeCommitID: startCommitID, AfterCommitID: endCommitID, SkipTo: ctx.FormString("skip-to"), MaxLines: maxLines, - MaxLineCharacters: maxLineCharacters, + MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters, MaxFiles: maxFiles, WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), }, ctx.FormStrings("files")...) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 6dece90ead37d..c303de0a01574 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1294,7 +1294,7 @@ type DiffOptions struct { // GetDiff builds a Diff between two commits of a repository. // Passing the empty string as beforeCommitID returns a diff from the parent commit. // The whitespaceBehavior is either an empty string or a git flag -func GetDiff(gitRepo *git.Repository, opts DiffOptions, files ...string) (*Diff, error) { +func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff, error) { repoPath := gitRepo.Path commit, err := gitRepo.GetCommit(opts.AfterCommitID) diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go index 72adf33f555db..7d63beffebc68 100644 --- a/services/gitdiff/gitdiff_test.go +++ b/services/gitdiff/gitdiff_test.go @@ -694,7 +694,7 @@ func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) { defer gitRepo.Close() for _, behavior := range []string{"-w", "--ignore-space-at-eol", "-b", ""} { diffs, err := GetDiff(gitRepo, - DiffOptions{ + &DiffOptions{ AfterCommitID: "bd7063cc7c04689c4d082183d32a604ed27a24f9", BeforeCommitID: "559c156f8e0178b71cb44355428f24001b08fc68", MaxLines: setting.Git.MaxGitDiffLines, From b4affc287cbbe3134ce12ed64432a5fef57ad716 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 21 Nov 2021 13:08:49 +0000 Subject: [PATCH 3/3] as per review - add fail handler and simplify result checker Signed-off-by: Andrew Thornton --- web_src/js/features/repo-diff.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/web_src/js/features/repo-diff.js b/web_src/js/features/repo-diff.js index 98cae4a9bf705..6741f277fb619 100644 --- a/web_src/js/features/repo-diff.js +++ b/web_src/js/features/repo-diff.js @@ -94,13 +94,15 @@ export function initRepoDiffShowMore() { type: 'GET', url, }).done((resp) => { - if (!resp || resp.html === '' || resp.empty) { + if (!resp) { $('#diff-show-more-files, #diff-show-more-files-stats').removeClass('disabled'); return; } $('#diff-too-many-files-stats').remove(); $('#diff-files').append($(resp).find('#diff-files li')); $('#diff-incomplete').replaceWith($(resp).find('#diff-file-boxes').children()); + }).fail(() => { + $('#diff-show-more-files, #diff-show-more-files-stats').removeClass('disabled'); }); }); $(document).on('click', 'a.diff-show-more-button', (e) => { @@ -118,12 +120,14 @@ export function initRepoDiffShowMore() { type: 'GET', url, }).done((resp) => { - if (!resp || resp.html === '' || resp.empty) { + if (!resp) { $target.removeClass('disabled'); return; } $target.parent().replaceWith($(resp).find('#diff-file-boxes .diff-file-body .file-body').children()); + }).fail(() => { + $target.removeClass('disabled'); }); }); }