Skip to content

Commit

Permalink
Support direct comparison (git diff a..b) as well merge comparison (a…
Browse files Browse the repository at this point in the history
…...b) (#16635)

This PR changes the compare page to make the "..." in the between branches a clickable
link. This changes the comparison type from "..." to "..". Similarly it makes the
initial compare icon clickable to switch the head and base branches.

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

Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
  • Loading branch information
3 people authored Sep 27, 2021
1 parent 123f0ae commit 920608e
Show file tree
Hide file tree
Showing 10 changed files with 198 additions and 148 deletions.
22 changes: 17 additions & 5 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, directComparison bool) (_ *CompareInfo, err error) {
var (
remoteBranch string
tmpRemote string
Expand Down Expand Up @@ -79,8 +79,15 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string)
if err != nil {
compareInfo.BaseCommitID = remoteBranch
}
separator := "..."
baseCommitID := compareInfo.MergeBase
if directComparison {
separator = ".."
baseCommitID = compareInfo.BaseCommitID
}

// We have a common base - therefore we know that ... should work
logs, err := NewCommand("log", compareInfo.MergeBase+"..."+headBranch, prettyLogFormat).RunInDirBytes(repo.Path)
logs, err := NewCommand("log", baseCommitID+separator+headBranch, prettyLogFormat).RunInDirBytes(repo.Path)
if err != nil {
return nil, err
}
Expand All @@ -100,7 +107,7 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string)
// Count number of changed files.
// This probably should be removed as we need to use shortstat elsewhere
// Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly
compareInfo.NumFiles, err = repo.GetDiffNumChangedFiles(remoteBranch, headBranch)
compareInfo.NumFiles, err = repo.GetDiffNumChangedFiles(remoteBranch, headBranch, directComparison)
if err != nil {
return nil, err
}
Expand All @@ -120,12 +127,17 @@ func (l *lineCountWriter) Write(p []byte) (n int, err error) {

// GetDiffNumChangedFiles counts the number of changed files
// This is substantially quicker than shortstat but...
func (repo *Repository) GetDiffNumChangedFiles(base, head string) (int, error) {
func (repo *Repository) GetDiffNumChangedFiles(base, head string, directComparison bool) (int, error) {
// Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly
w := &lineCountWriter{}
stderr := new(bytes.Buffer)

if err := NewCommand("diff", "-z", "--name-only", base+"..."+head).
separator := "..."
if directComparison {
separator = ".."
}

if err := NewCommand("diff", "-z", "--name-only", base+separator+head).
RunInDirPipeline(repo.Path, w, stderr); err != nil {
if strings.Contains(stderr.String(), "no merge base") {
// git >= 2.28 now returns an error if base and head have become unrelated.
Expand Down
2 changes: 2 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,8 @@ pulls.compare_changes = New Pull Request
pulls.compare_changes_desc = Select the branch to merge into and the branch to pull from.
pulls.compare_base = merge into
pulls.compare_compare = pull from
pulls.switch_comparison_type = Switch comparison type
pulls.switch_head_and_base = Switch head and base
pulls.filter_branch = Filter branch
pulls.no_results = No results found.
pulls.nothing_to_compare = These branches are equal. There is no need to create a pull request.
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 @@ -1006,7 +1006,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, true)
if err != nil {
headGitRepo.Close()
ctx.Error(http.StatusInternalServerError, "GetCompareInfo", err)
Expand Down Expand Up @@ -1183,9 +1183,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(), true)
} else {
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName())
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName(), true)
}
if err != nil {
ctx.ServerError("GetCompareInfo", err)
Expand Down
3 changes: 2 additions & 1 deletion routers/web/repo/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,8 @@ func Diff(ctx *context.Context) {
diff, err := gitdiff.GetDiffCommitWithWhitespaceBehavior(gitRepo,
commitID, setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),
false)
if err != nil {
ctx.NotFound("GetDiffCommitWithWhitespaceBehavior", err)
return
Expand Down
Loading

0 comments on commit 920608e

Please sign in to comment.