Allow Loading of Diffs that are too large (#17739) * 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 <art27@cantab.net>

Changes

8 changed files (+146/-62)

• 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
• 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,23 @@ 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, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles + files := ctx.FormStrings("files") + if fileOnly && (len(files) == 2 || len(files) == 1) { + maxLines, maxFiles = -1, -1 + } + + diff, err := gitdiff.GetDiff(gitRepo, &gitdiff.DiffOptions{ + AfterCommitID: commitID, + SkipTo: ctx.FormString("skip-to"), + MaxLines: maxLines, + MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters, + MaxFiles: maxFiles, + WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), + }, files...) if err != nil { - ctx.NotFound("GetDiffCommitWithWhitespaceBehavior", err) + ctx.NotFound("GetDiff", err) return }
  • @@ -325,10 +333,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 {
• routers/web/repo/compare.go
  • @@ -569,9 +569,23 @@ 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, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles + files := ctx.FormStrings("files") + if len(files) == 2 || len(files) == 1 { + maxLines, maxFiles = -1, -1 + } + + diff, err := gitdiff.GetDiff(ci.HeadGitRepo, + &gitdiff.DiffOptions{ + BeforeCommitID: beforeCommitID, + AfterCommitID: headCommitID, + SkipTo: ctx.FormString("skip-to"), + MaxLines: maxLines, + MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters, + MaxFiles: maxFiles, + WhitespaceBehavior: whitespaceBehavior, + DirectComparison: ci.DirectComparison, + }, ctx.FormStrings("files")...) if err != nil { ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err) return false
• routers/web/repo/pull.go
  • @@ -633,10 +633,24 @@ 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, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles + files := ctx.FormStrings("files") + if fileOnly && (len(files) == 2 || len(files) == 1) { + maxLines, maxFiles = -1, -1 + } + + diff, err := gitdiff.GetDiff(gitRepo, + &gitdiff.DiffOptions{ + BeforeCommitID: startCommitID, + AfterCommitID: endCommitID, + SkipTo: ctx.FormString("skip-to"), + MaxLines: maxLines, + MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters, + MaxFiles: maxFiles, + WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), + }, ctx.FormStrings("files")...) if err != nil { ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err) return
• 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,
• 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))
• 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"}} + <a class="ui basic tiny button diff-show-more-button" data-href="{{$.Link}}?file-only=true&files={{$file.Name}}&files={{$file.OldName}}">{{$.i18n.Tr "repo.diff.load"}}</a> {{end}} {{else}} {{$.i18n.Tr "repo.diff.bin_not_shown"}}
• web_src/js/features/repo-diff.js
  • @@ -94,13 +94,40 @@ 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) => { + 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) { + $target.removeClass('disabled'); + return; + } + + $target.parent().replaceWith($(resp).find('#diff-file-boxes .diff-file-body .file-body').children()); + }).fail(() => { + $target.removeClass('disabled'); }); }); }