Skip to content

Commit

Permalink
Add buttons to allow loading of incomplete diffs (#16829)
Browse files Browse the repository at this point in the history
This PR adds two buttons to the stats and the end of the diffs list to load the (some of) the remaining incomplete diff sections.

Contains #16775
    
Signed-off-by: Andrew Thornton <art27@cantab.net>


## Screenshots

### Show more button at the end of the diff
![Screenshot from 2021-09-04 11-12-37](https://user-images.githubusercontent.com/1824502/132091009-b1f6113e-2c04-4be5-8a04-b8ecea56887b.png)

### Show more button at the end of the diff stats box
![Screenshot from 2021-09-04 11-14-54](https://user-images.githubusercontent.com/1824502/132091063-86da5a6d-6628-4b82-bea9-3655cd9f40f6.png)
  • Loading branch information
zeripath committed Oct 15, 2021
1 parent bdfd751 commit a889d0c
Show file tree
Hide file tree
Showing 12 changed files with 227 additions and 154 deletions.
20 changes: 12 additions & 8 deletions modules/git/repo_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (repo *Repository) GetMergeBase(tmpRemote string, base, head string) (strin
}

// GetCompareInfo generates and returns compare information between base and head branches of repositories.
func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string, directComparison bool) (_ *CompareInfo, err error) {
func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string, directComparison, fileOnly bool) (_ *CompareInfo, err error) {
var (
remoteBranch string
tmpRemote string
Expand Down Expand Up @@ -87,13 +87,17 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string,
}

// We have a common base - therefore we know that ... should work
logs, err := NewCommand("log", baseCommitID+separator+headBranch, prettyLogFormat).RunInDirBytes(repo.Path)
if err != nil {
return nil, err
}
compareInfo.Commits, err = repo.parsePrettyFormatLogToList(logs)
if err != nil {
return nil, fmt.Errorf("parsePrettyFormatLogToList: %v", err)
if !fileOnly {
logs, err := NewCommand("log", baseCommitID+separator+headBranch, prettyLogFormat).RunInDirBytes(repo.Path)
if err != nil {
return nil, err
}
compareInfo.Commits, err = repo.parsePrettyFormatLogToList(logs)
if err != nil {
return nil, fmt.Errorf("parsePrettyFormatLogToList: %v", err)
}
} else {
compareInfo.Commits = []*Commit{}
}
} else {
compareInfo.Commits = []*Commit{}
Expand Down
3 changes: 2 additions & 1 deletion options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -2027,7 +2027,8 @@ diff.file_image_height = Height
diff.file_byte_size = Size
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 changed in this diff
diff.too_many_files = Some files were not shown because too many files have changed in this diff
diff.show_more = Show More
diff.generated = generated
diff.vendored = vendored
diff.comment.placeholder = Leave a comment
Expand Down
6 changes: 3 additions & 3 deletions routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
return nil, nil, nil, nil, "", ""
}

compareInfo, err := headGitRepo.GetCompareInfo(models.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseBranch, headBranch, true)
compareInfo, err := headGitRepo.GetCompareInfo(models.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseBranch, headBranch, true, false)
if err != nil {
headGitRepo.Close()
ctx.Error(http.StatusInternalServerError, "GetCompareInfo", err)
Expand Down Expand Up @@ -1193,9 +1193,9 @@ func GetPullRequestCommits(ctx *context.APIContext) {
}
defer baseGitRepo.Close()
if pr.HasMerged {
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName(), true)
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName(), true, false)
} else {
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName(), true)
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName(), true, false)
}
if err != nil {
ctx.ServerError("GetCompareInfo", err)
Expand Down
27 changes: 17 additions & 10 deletions routers/web/repo/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,8 @@ 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 {
Expand All @@ -288,16 +290,8 @@ func Diff(ctx *context.Context) {
commitID = commit.ID.String()
}

statuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository.ID, commitID, db.ListOptions{})
if err != nil {
log.Error("GetLatestCommitStatus: %v", err)
}

ctx.Data["CommitStatus"] = models.CalcCommitStatus(statuses)
ctx.Data["CommitStatuses"] = statuses

diff, err := gitdiff.GetDiffCommitWithWhitespaceBehavior(gitRepo,
commitID, setting.Git.MaxGitDiffLines,
commitID, ctx.FormString("skip-to"), setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),
false)
Expand Down Expand Up @@ -333,10 +327,23 @@ func Diff(ctx *context.Context) {
setCompareContext(ctx, parentCommit, commit, headTarget)
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 {
log.Error("GetLatestCommitStatus: %v", err)
}

ctx.Data["CommitStatus"] = models.CalcCommitStatus(statuses)
ctx.Data["CommitStatuses"] = statuses

verification := models.ParseCommitWithSignature(commit)
ctx.Data["Verification"] = verification
ctx.Data["Author"] = models.ValidateCommitWithEmail(commit)
ctx.Data["Diff"] = diff
ctx.Data["Parents"] = parents
ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0

Expand Down
73 changes: 41 additions & 32 deletions routers/web/repo/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
const (
tplCompare base.TplName = "repo/diff/compare"
tplBlobExcerpt base.TplName = "repo/diff/blob_excerpt"
tplDiffBox base.TplName = "repo/diff/box"
)

// setCompareContext sets context data.
Expand Down Expand Up @@ -161,6 +162,8 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo {
baseRepo := ctx.Repo.Repository
ci := &CompareInfo{}

fileOnly := ctx.FormBool("file-only")

// Get compared branches information
// A full compare url is of the form:
//
Expand Down Expand Up @@ -411,15 +414,19 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo {
if rootRepo != nil &&
rootRepo.ID != ci.HeadRepo.ID &&
rootRepo.ID != baseRepo.ID {
perm, branches, tags, err := getBranchesAndTagsForRepo(ctx.User, rootRepo)
if err != nil {
ctx.ServerError("GetBranchesForRepo", err)
return nil
}
if perm {
canRead := rootRepo.CheckUnitUser(ctx.User, models.UnitTypeCode)
if canRead {
ctx.Data["RootRepo"] = rootRepo
ctx.Data["RootRepoBranches"] = branches
ctx.Data["RootRepoTags"] = tags
if !fileOnly {
branches, tags, err := getBranchesAndTagsForRepo(ctx.User, rootRepo)
if err != nil {
ctx.ServerError("GetBranchesForRepo", err)
return nil
}

ctx.Data["RootRepoBranches"] = branches
ctx.Data["RootRepoTags"] = tags
}
}
}

Expand All @@ -432,15 +439,18 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo {
ownForkRepo.ID != ci.HeadRepo.ID &&
ownForkRepo.ID != baseRepo.ID &&
(rootRepo == nil || ownForkRepo.ID != rootRepo.ID) {
perm, branches, tags, err := getBranchesAndTagsForRepo(ctx.User, ownForkRepo)
if err != nil {
ctx.ServerError("GetBranchesForRepo", err)
return nil
}
if perm {
canRead := ownForkRepo.CheckUnitUser(ctx.User, models.UnitTypeCode)
if canRead {
ctx.Data["OwnForkRepo"] = ownForkRepo
ctx.Data["OwnForkRepoBranches"] = branches
ctx.Data["OwnForkRepoTags"] = tags
if !fileOnly {
branches, tags, err := getBranchesAndTagsForRepo(ctx.User, ownForkRepo)
if err != nil {
ctx.ServerError("GetBranchesForRepo", err)
return nil
}
ctx.Data["OwnForkRepoBranches"] = branches
ctx.Data["OwnForkRepoTags"] = tags
}
}
}

Expand Down Expand Up @@ -492,7 +502,7 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo {
headBranchRef = git.TagPrefix + ci.HeadBranch
}

ci.CompareInfo, err = ci.HeadGitRepo.GetCompareInfo(baseRepo.RepoPath(), baseBranchRef, headBranchRef, ci.DirectComparison)
ci.CompareInfo, err = ci.HeadGitRepo.GetCompareInfo(baseRepo.RepoPath(), baseBranchRef, headBranchRef, ci.DirectComparison, fileOnly)
if err != nil {
ctx.ServerError("GetCompareInfo", err)
return nil
Expand Down Expand Up @@ -545,7 +555,7 @@ func PrepareCompareDiff(
}

diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(ci.HeadGitRepo,
beforeCommitID, headCommitID, setting.Git.MaxGitDiffLines,
beforeCommitID, headCommitID, ctx.FormString("skip-to"), setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, whitespaceBehavior, ci.DirectComparison)
if err != nil {
ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err)
Expand Down Expand Up @@ -606,29 +616,22 @@ func PrepareCompareDiff(
return false
}

func getBranchesAndTagsForRepo(user *models.User, repo *models.Repository) (bool, []string, []string, error) {
perm, err := models.GetUserRepoPermission(repo, user)
if err != nil {
return false, nil, nil, err
}
if !perm.CanRead(models.UnitTypeCode) {
return false, nil, nil, nil
}
func getBranchesAndTagsForRepo(user *models.User, repo *models.Repository) (branches, tags []string, err error) {
gitRepo, err := git.OpenRepository(repo.RepoPath())
if err != nil {
return false, nil, nil, err
return nil, nil, err
}
defer gitRepo.Close()

branches, _, err := gitRepo.GetBranches(0, 0)
branches, _, err = gitRepo.GetBranches(0, 0)
if err != nil {
return false, nil, nil, err
return nil, nil, err
}
tags, err := gitRepo.GetTags(0, 0)
tags, err = gitRepo.GetTags(0, 0)
if err != nil {
return false, nil, nil, err
return nil, nil, err
}
return true, branches, tags, nil
return branches, tags, nil
}

// CompareDiff show different from one commit to another commit
Expand Down Expand Up @@ -665,6 +668,12 @@ func CompareDiff(ctx *context.Context) {
}
ctx.Data["Tags"] = baseTags

fileOnly := ctx.FormBool("file-only")
if fileOnly {
ctx.HTML(http.StatusOK, tplDiffBox)
return
}

headBranches, _, err := ci.HeadGitRepo.GetBranches(0, 0)
if err != nil {
ctx.ServerError("GetBranches", err)
Expand Down
8 changes: 4 additions & 4 deletions routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *models.Issue) *git.C
ctx.Data["HasMerged"] = true

compareInfo, err := ctx.Repo.GitRepo.GetCompareInfo(ctx.Repo.Repository.RepoPath(),
pull.MergeBase, pull.GetGitRefName(), true)
pull.MergeBase, pull.GetGitRefName(), true, false)
if err != nil {
if strings.Contains(err.Error(), "fatal: Not a valid object name") || strings.Contains(err.Error(), "unknown revision or path not in the working tree") {
ctx.Data["IsPullRequestBroken"] = true
Expand Down Expand Up @@ -401,7 +401,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
}

compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(),
pull.MergeBase, pull.GetGitRefName(), true)
pull.MergeBase, pull.GetGitRefName(), true, false)
if err != nil {
if strings.Contains(err.Error(), "fatal: Not a valid object name") {
ctx.Data["IsPullRequestBroken"] = true
Expand Down Expand Up @@ -517,7 +517,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
}

compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(),
git.BranchPrefix+pull.BaseBranch, pull.GetGitRefName(), true)
git.BranchPrefix+pull.BaseBranch, pull.GetGitRefName(), true, false)
if err != nil {
if strings.Contains(err.Error(), "fatal: Not a valid object name") {
ctx.Data["IsPullRequestBroken"] = true
Expand Down Expand Up @@ -633,7 +633,7 @@ func ViewPullFiles(ctx *context.Context) {
ctx.Data["AfterCommitID"] = endCommitID

diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(gitRepo,
startCommitID, endCommitID, setting.Git.MaxGitDiffLines,
startCommitID, endCommitID, ctx.FormString("skip-to"), setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), false)
if err != nil {
Expand Down
34 changes: 25 additions & 9 deletions services/gitdiff/gitdiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,7 @@ func getCommitFileLineCount(commit *git.Commit, filePath string) int {

// Diff represents a difference between two git trees.
type Diff struct {
Start, End string
NumFiles, TotalAddition, TotalDeletion int
Files []*DiffFile
IsIncomplete bool
Expand Down Expand Up @@ -719,6 +720,9 @@ parsingLoop:

// TODO: Handle skipping first n files
if len(diff.Files) >= maxFiles {

lastFile := createDiffFile(diff, line)
diff.End = lastFile.Name
diff.IsIncomplete = true
_, err := io.Copy(io.Discard, reader)
if err != nil {
Expand Down Expand Up @@ -1217,7 +1221,7 @@ func readFileName(rd *strings.Reader) (string, bool) {
// GetDiffRangeWithWhitespaceBehavior 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 string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string, directComparison bool) (*Diff, error) {
func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID, afterCommitID, skipTo string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string, directComparison bool) (*Diff, error) {
repoPath := gitRepo.Path

commit, err := gitRepo.GetCommit(afterCommitID)
Expand All @@ -1228,31 +1232,42 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID,
ctx, cancel := context.WithTimeout(git.DefaultContext, time.Duration(setting.Git.Timeout.Default)*time.Second)
defer cancel()

var cmd *exec.Cmd
argsLength := 6
if len(whitespaceBehavior) > 0 {
argsLength++
}
if len(skipTo) > 0 {
argsLength++
}

diffArgs := make([]string, 0, argsLength)
if (len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA) && commit.ParentCount() == 0 {
diffArgs := []string{"diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M"}
diffArgs = append(diffArgs, "diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M")
if len(whitespaceBehavior) != 0 {
diffArgs = append(diffArgs, whitespaceBehavior)
}
// append empty tree ref
diffArgs = append(diffArgs, "4b825dc642cb6eb9a060e54bf8d69288fbee4904")
diffArgs = append(diffArgs, afterCommitID)
cmd = exec.CommandContext(ctx, git.GitExecutable, diffArgs...)
} else {
actualBeforeCommitID := beforeCommitID
if len(actualBeforeCommitID) == 0 {
parentCommit, _ := commit.Parent(0)
actualBeforeCommitID = parentCommit.ID.String()
}
diffArgs := []string{"diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M"}
diffArgs = append(diffArgs, "diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M")
if len(whitespaceBehavior) != 0 {
diffArgs = append(diffArgs, whitespaceBehavior)
}
diffArgs = append(diffArgs, actualBeforeCommitID)
diffArgs = append(diffArgs, afterCommitID)
cmd = exec.CommandContext(ctx, git.GitExecutable, diffArgs...)
beforeCommitID = actualBeforeCommitID
}
if skipTo != "" {
diffArgs = append(diffArgs, "--skip-to="+skipTo)
}
cmd := exec.CommandContext(ctx, git.GitExecutable, diffArgs...)

cmd.Dir = repoPath
cmd.Stderr = os.Stderr

Expand All @@ -1272,6 +1287,7 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID,
if err != nil {
return nil, fmt.Errorf("ParsePatch: %v", err)
}
diff.Start = skipTo

var checker *git.CheckAttributeReader

Expand Down Expand Up @@ -1299,7 +1315,7 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID,
log.Error("Unable to open checker for %s. Error: %v", afterCommitID, err)
} else {
go func() {
err = checker.Run()
err := checker.Run()
if err != nil && err != ctx.Err() {
log.Error("Unable to open checker for %s. Error: %v", afterCommitID, err)
}
Expand Down Expand Up @@ -1382,8 +1398,8 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID,

// 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 string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string, directComparison bool) (*Diff, error) {
return GetDiffRangeWithWhitespaceBehavior(gitRepo, "", commitID, maxLines, maxLineCharacters, maxFiles, whitespaceBehavior, directComparison)
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
Expand Down
Loading

0 comments on commit a889d0c

Please sign in to comment.