Skip to content

Commit

Permalink
Add buttons to allow loading of incomplete diffs
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.

Signed-off-by: Andrew Thornton <art27@cantab.net>
  • Loading branch information
zeripath committed Aug 31, 2021
1 parent d217024 commit d918330
Show file tree
Hide file tree
Showing 11 changed files with 224 additions and 133 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) (_ *CompareInfo, err error) {
func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string, fileOnly bool) (_ *CompareInfo, err error) {
var (
remoteBranch string
tmpRemote string
Expand Down Expand Up @@ -80,13 +80,17 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string)
compareInfo.BaseCommitID = remoteBranch
}
// We have a common base - therefore we know that ... should work
logs, err := NewCommand("log", compareInfo.MergeBase+"..."+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", compareInfo.MergeBase+"..."+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 @@ -2009,7 +2009,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.comment.placeholder = Leave a comment
diff.comment.markdown_info = Styling with markdown is supported.
diff.comment.add_single_comment = Add single 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 @@ -1031,7 +1031,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)
compareInfo, err := headGitRepo.GetCompareInfo(models.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseBranch, headBranch, false)
if err != nil {
headGitRepo.Close()
ctx.Error(http.StatusInternalServerError, "GetCompareInfo", err)
Expand Down Expand Up @@ -1198,9 +1198,9 @@ func GetPullRequestCommits(ctx *context.APIContext) {
}
defer baseGitRepo.Close()
if pr.HasMerged {
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName())
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName(), false)
} else {
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName())
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName(), false)
}
if err != nil {
ctx.ServerError("GetCompareInfo", err)
Expand Down
19 changes: 9 additions & 10 deletions routers/web/repo/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,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 @@ -287,16 +289,8 @@ func Diff(ctx *context.Context) {
commitID = commit.ID.String()
}

statuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository.ID, commitID, models.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)))
if err != nil {
Expand Down Expand Up @@ -331,10 +325,15 @@ 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
}

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
71 changes: 51 additions & 20 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 @@ -149,6 +150,8 @@ func setCsvCompareContext(ctx *context.Context) {
func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, *git.Repository, *git.CompareInfo, string, string) {
baseRepo := ctx.Repo.Repository

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

// Get compared branches information
// A full compare url is of the form:
//
Expand Down Expand Up @@ -395,15 +398,26 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, *
if rootRepo != nil &&
rootRepo.ID != headRepo.ID &&
rootRepo.ID != baseRepo.ID {
perm, branches, tags, err := getBranchesAndTagsForRepo(ctx.User, rootRepo)
if err != nil {
ctx.ServerError("GetBranchesForRepo", err)
return nil, nil, nil, nil, "", ""
}
if perm {
ctx.Data["RootRepo"] = rootRepo
ctx.Data["RootRepoBranches"] = branches
ctx.Data["RootRepoTags"] = tags
if !fileOnly {
perm, branches, tags, err := getBranchesAndTagsForRepo(ctx.User, rootRepo)
if err != nil {
ctx.ServerError("GetBranchesForRepo", err)
return nil, nil, nil, nil, "", ""
}
if perm {
ctx.Data["RootRepo"] = rootRepo
ctx.Data["RootRepoBranches"] = branches
ctx.Data["RootRepoTags"] = tags
}
} else {
perm, err := models.GetUserRepoPermission(rootRepo, ctx.User)
if err != nil {
ctx.ServerError("GetUserRepoPermission", err)
return nil, nil, nil, nil, "", ""
}
if !perm.CanRead(models.UnitTypeCode) {
ctx.Data["RootRepo"] = rootRepo
}
}
}

Expand All @@ -416,15 +430,26 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, *
ownForkRepo.ID != 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, nil, nil, nil, "", ""
}
if perm {
ctx.Data["OwnForkRepo"] = ownForkRepo
ctx.Data["OwnForkRepoBranches"] = branches
ctx.Data["OwnForkRepoTags"] = tags
if !fileOnly {
perm, branches, tags, err := getBranchesAndTagsForRepo(ctx.User, ownForkRepo)
if err != nil {
ctx.ServerError("GetBranchesForRepo", err)
return nil, nil, nil, nil, "", ""
}
if perm {
ctx.Data["OwnForkRepo"] = ownForkRepo
ctx.Data["OwnForkRepoBranches"] = branches
ctx.Data["OwnForkRepoTags"] = tags
}
} else {
perm, err := models.GetUserRepoPermission(rootRepo, ctx.User)
if err != nil {
ctx.ServerError("GetUserRepoPermission", err)
return nil, nil, nil, nil, "", ""
}
if !perm.CanRead(models.UnitTypeCode) {
ctx.Data["RootRepo"] = rootRepo
}
}
}

Expand Down Expand Up @@ -476,7 +501,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, *
headBranchRef = git.TagPrefix + headBranch
}

compareInfo, err := headGitRepo.GetCompareInfo(baseRepo.RepoPath(), baseBranchRef, headBranchRef)
compareInfo, err := headGitRepo.GetCompareInfo(baseRepo.RepoPath(), baseBranchRef, headBranchRef, fileOnly)
if err != nil {
ctx.ServerError("GetCompareInfo", err)
return nil, nil, nil, nil, "", ""
Expand Down Expand Up @@ -527,7 +552,7 @@ func PrepareCompareDiff(
}

diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(headGitRepo,
compareInfo.MergeBase, headCommitID, setting.Git.MaxGitDiffLines,
compareInfo.MergeBase, headCommitID, ctx.FormString("skip-to"), setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, whitespaceBehavior)
if err != nil {
ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err)
Expand Down Expand Up @@ -639,6 +664,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 := 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 @@ -317,7 +317,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())
pull.MergeBase, pull.GetGitRefName(), 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 @@ -400,7 +400,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
}

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

compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(),
git.BranchPrefix+pull.BaseBranch, pull.GetGitRefName())
git.BranchPrefix+pull.BaseBranch, pull.GetGitRefName(), false)
if err != nil {
if strings.Contains(err.Error(), "fatal: Not a valid object name") {
ctx.Data["IsPullRequestBroken"] = true
Expand Down Expand Up @@ -632,7 +632,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)))
if err != nil {
Expand Down
32 changes: 24 additions & 8 deletions services/gitdiff/gitdiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,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 @@ -715,6 +716,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(ioutil.Discard, reader)
if err != nil {
Expand Down Expand Up @@ -1209,7 +1213,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) (*Diff, error) {
func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID, afterCommitID, skipTo string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string) (*Diff, error) {
repoPath := gitRepo.Path

commit, err := gitRepo.GetCommit(afterCommitID)
Expand All @@ -1220,31 +1224,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 @@ -1264,6 +1279,7 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID,
if err != nil {
return nil, fmt.Errorf("ParsePatch: %v", err)
}
diff.Start = skipTo
for _, diffFile := range diff.Files {
tailSection := diffFile.GetTailSection(gitRepo, beforeCommitID, afterCommitID)
if tailSection != nil {
Expand Down Expand Up @@ -1295,8 +1311,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) (*Diff, error) {
return GetDiffRangeWithWhitespaceBehavior(gitRepo, "", commitID, maxLines, maxLineCharacters, maxFiles, whitespaceBehavior)
func GetDiffCommitWithWhitespaceBehavior(gitRepo *git.Repository, commitID, skipTo string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string) (*Diff, error) {
return GetDiffRangeWithWhitespaceBehavior(gitRepo, "", commitID, skipTo, maxLines, maxLineCharacters, maxFiles, whitespaceBehavior)
}

// CommentAsDiff returns c.Patch as *Diff
Expand Down
2 changes: 1 addition & 1 deletion services/gitdiff/gitdiff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) {
}
defer gitRepo.Close()
for _, behavior := range []string{"-w", "--ignore-space-at-eol", "-b", ""} {
diffs, err := GetDiffRangeWithWhitespaceBehavior(gitRepo, "559c156f8e0178b71cb44355428f24001b08fc68", "bd7063cc7c04689c4d082183d32a604ed27a24f9",
diffs, err := GetDiffRangeWithWhitespaceBehavior(gitRepo, "559c156f8e0178b71cb44355428f24001b08fc68", "bd7063cc7c04689c4d082183d32a604ed27a24f9", "",
setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles, behavior)
assert.NoError(t, err, fmt.Sprintf("Error when diff with %s", behavior))
for _, f := range diffs.Files {
Expand Down
2 changes: 1 addition & 1 deletion services/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int6
defer baseGitRepo.Close()

compareInfo, err := baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(),
git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName())
git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName(), false)
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit d918330

Please sign in to comment.