diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 5d1208aab1..866b3d9133 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -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 @@ -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 } @@ -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 } @@ -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. diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 3d43b99c31..e6a29a5c8a 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -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. diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index b9767b413d..1c28363e8a 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -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) @@ -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) diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 61435527da..c5f3f70c1c 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -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 diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 0a0b38f078..71ca4df93e 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -145,9 +145,21 @@ func setCsvCompareContext(ctx *context.Context) { } } +// CompareInfo represents the collected results from ParseCompareInfo +type CompareInfo struct { + HeadUser *models.User + HeadRepo *models.Repository + HeadGitRepo *git.Repository + CompareInfo *git.CompareInfo + BaseBranch string + HeadBranch string + DirectComparison bool +} + // ParseCompareInfo parse compare info between two commit for preparing comparing references -func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, *git.Repository, *git.CompareInfo, string, string) { +func ParseCompareInfo(ctx *context.Context) *CompareInfo { baseRepo := ctx.Repo.Repository + ci := &CompareInfo{} // Get compared branches information // A full compare url is of the form: @@ -173,92 +185,97 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * // same repo: master...feature var ( - headUser *models.User - headRepo *models.Repository - headBranch string isSameRepo bool infoPath string err error ) + infoPath = ctx.Params("*") infos := strings.SplitN(infoPath, "...", 2) + + if len(infos) != 2 { + infos = strings.SplitN(infoPath, "..", 2) + ci.DirectComparison = true + ctx.Data["PageIsComparePull"] = false + } + if len(infos) != 2 { log.Trace("ParseCompareInfo[%d]: not enough compared branches information %s", baseRepo.ID, infos) ctx.NotFound("CompareAndPullRequest", nil) - return nil, nil, nil, nil, "", "" + return nil } ctx.Data["BaseName"] = baseRepo.OwnerName - baseBranch := infos[0] - ctx.Data["BaseBranch"] = baseBranch + ci.BaseBranch = infos[0] + ctx.Data["BaseBranch"] = ci.BaseBranch // If there is no head repository, it means compare between same repository. headInfos := strings.Split(infos[1], ":") if len(headInfos) == 1 { isSameRepo = true - headUser = ctx.Repo.Owner - headBranch = headInfos[0] + ci.HeadUser = ctx.Repo.Owner + ci.HeadBranch = headInfos[0] } else if len(headInfos) == 2 { headInfosSplit := strings.Split(headInfos[0], "/") if len(headInfosSplit) == 1 { - headUser, err = models.GetUserByName(headInfos[0]) + ci.HeadUser, err = models.GetUserByName(headInfos[0]) if err != nil { if models.IsErrUserNotExist(err) { ctx.NotFound("GetUserByName", nil) } else { ctx.ServerError("GetUserByName", err) } - return nil, nil, nil, nil, "", "" + return nil } - headBranch = headInfos[1] - isSameRepo = headUser.ID == ctx.Repo.Owner.ID + ci.HeadBranch = headInfos[1] + isSameRepo = ci.HeadUser.ID == ctx.Repo.Owner.ID if isSameRepo { - headRepo = baseRepo + ci.HeadRepo = baseRepo } } else { - headRepo, err = models.GetRepositoryByOwnerAndName(headInfosSplit[0], headInfosSplit[1]) + ci.HeadRepo, err = models.GetRepositoryByOwnerAndName(headInfosSplit[0], headInfosSplit[1]) if err != nil { if models.IsErrRepoNotExist(err) { ctx.NotFound("GetRepositoryByOwnerAndName", nil) } else { ctx.ServerError("GetRepositoryByOwnerAndName", err) } - return nil, nil, nil, nil, "", "" + return nil } - if err := headRepo.GetOwner(); err != nil { + if err := ci.HeadRepo.GetOwner(); err != nil { if models.IsErrUserNotExist(err) { ctx.NotFound("GetUserByName", nil) } else { ctx.ServerError("GetUserByName", err) } - return nil, nil, nil, nil, "", "" + return nil } - headBranch = headInfos[1] - headUser = headRepo.Owner - isSameRepo = headRepo.ID == ctx.Repo.Repository.ID + ci.HeadBranch = headInfos[1] + ci.HeadUser = ci.HeadRepo.Owner + isSameRepo = ci.HeadRepo.ID == ctx.Repo.Repository.ID } } else { ctx.NotFound("CompareAndPullRequest", nil) - return nil, nil, nil, nil, "", "" + return nil } - ctx.Data["HeadUser"] = headUser - ctx.Data["HeadBranch"] = headBranch + ctx.Data["HeadUser"] = ci.HeadUser + ctx.Data["HeadBranch"] = ci.HeadBranch ctx.Repo.PullRequest.SameRepo = isSameRepo // Check if base branch is valid. - baseIsCommit := ctx.Repo.GitRepo.IsCommitExist(baseBranch) - baseIsBranch := ctx.Repo.GitRepo.IsBranchExist(baseBranch) - baseIsTag := ctx.Repo.GitRepo.IsTagExist(baseBranch) + baseIsCommit := ctx.Repo.GitRepo.IsCommitExist(ci.BaseBranch) + baseIsBranch := ctx.Repo.GitRepo.IsBranchExist(ci.BaseBranch) + baseIsTag := ctx.Repo.GitRepo.IsTagExist(ci.BaseBranch) if !baseIsCommit && !baseIsBranch && !baseIsTag { // Check if baseBranch is short sha commit hash - if baseCommit, _ := ctx.Repo.GitRepo.GetCommit(baseBranch); baseCommit != nil { - baseBranch = baseCommit.ID.String() - ctx.Data["BaseBranch"] = baseBranch + if baseCommit, _ := ctx.Repo.GitRepo.GetCommit(ci.BaseBranch); baseCommit != nil { + ci.BaseBranch = baseCommit.ID.String() + ctx.Data["BaseBranch"] = ci.BaseBranch baseIsCommit = true } else { ctx.NotFound("IsRefExist", nil) - return nil, nil, nil, nil, "", "" + return nil } } ctx.Data["BaseIsCommit"] = baseIsCommit @@ -284,7 +301,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * if err != nil { if !models.IsErrRepoNotExist(err) { ctx.ServerError("Unable to find root repo", err) - return nil, nil, nil, nil, "", "" + return nil } } else { rootRepo = baseRepo.BaseRepo @@ -303,29 +320,29 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * } } - has := headRepo != nil + has := ci.HeadRepo != nil // 3. If the base is a forked from "RootRepo" and the owner of // the "RootRepo" is the :headUser - set headRepo to that - if !has && rootRepo != nil && rootRepo.OwnerID == headUser.ID { - headRepo = rootRepo + if !has && rootRepo != nil && rootRepo.OwnerID == ci.HeadUser.ID { + ci.HeadRepo = rootRepo has = true } // 4. If the ctx.User has their own fork of the baseRepo and the headUser is the ctx.User // set the headRepo to the ownFork - if !has && ownForkRepo != nil && ownForkRepo.OwnerID == headUser.ID { - headRepo = ownForkRepo + if !has && ownForkRepo != nil && ownForkRepo.OwnerID == ci.HeadUser.ID { + ci.HeadRepo = ownForkRepo has = true } // 5. If the headOwner has a fork of the baseRepo - use that if !has { - headRepo, has = models.HasForkedRepo(headUser.ID, baseRepo.ID) + ci.HeadRepo, has = models.HasForkedRepo(ci.HeadUser.ID, baseRepo.ID) } // 6. If the baseRepo is a fork and the headUser has a fork of that use that if !has && baseRepo.IsFork { - headRepo, has = models.HasForkedRepo(headUser.ID, baseRepo.ForkID) + ci.HeadRepo, has = models.HasForkedRepo(ci.HeadUser.ID, baseRepo.ForkID) } // 7. Otherwise if we're not the same repo and haven't found a repo give up @@ -334,20 +351,19 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * } // 8. Finally open the git repo - var headGitRepo *git.Repository if isSameRepo { - headRepo = ctx.Repo.Repository - headGitRepo = ctx.Repo.GitRepo + ci.HeadRepo = ctx.Repo.Repository + ci.HeadGitRepo = ctx.Repo.GitRepo } else if has { - headGitRepo, err = git.OpenRepository(headRepo.RepoPath()) + ci.HeadGitRepo, err = git.OpenRepository(ci.HeadRepo.RepoPath()) if err != nil { ctx.ServerError("OpenRepository", err) - return nil, nil, nil, nil, "", "" + return nil } - defer headGitRepo.Close() + defer ci.HeadGitRepo.Close() } - ctx.Data["HeadRepo"] = headRepo + ctx.Data["HeadRepo"] = ci.HeadRepo // Now we need to assert that the ctx.User has permission to read // the baseRepo's code and pulls @@ -355,7 +371,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * permBase, err := models.GetUserRepoPermission(baseRepo, ctx.User) if err != nil { ctx.ServerError("GetUserRepoPermission", err) - return nil, nil, nil, nil, "", "" + return nil } if !permBase.CanRead(models.UnitTypeCode) { if log.IsTrace() { @@ -365,26 +381,26 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * permBase) } ctx.NotFound("ParseCompareInfo", nil) - return nil, nil, nil, nil, "", "" + return nil } // If we're not merging from the same repo: if !isSameRepo { // Assert ctx.User has permission to read headRepo's codes - permHead, err := models.GetUserRepoPermission(headRepo, ctx.User) + permHead, err := models.GetUserRepoPermission(ci.HeadRepo, ctx.User) if err != nil { ctx.ServerError("GetUserRepoPermission", err) - return nil, nil, nil, nil, "", "" + return nil } if !permHead.CanRead(models.UnitTypeCode) { if log.IsTrace() { log.Trace("Permission Denied: User: %-v cannot read code in Repo: %-v\nUser in headRepo has Permissions: %-+v", ctx.User, - headRepo, + ci.HeadRepo, permHead) } ctx.NotFound("ParseCompareInfo", nil) - return nil, nil, nil, nil, "", "" + return nil } } @@ -393,12 +409,12 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * // 2. the computed head // then get the branches of it if rootRepo != nil && - rootRepo.ID != headRepo.ID && + 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, nil, nil, nil, "", "" + return nil } if perm { ctx.Data["RootRepo"] = rootRepo @@ -413,13 +429,13 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * // 3. The rootRepo (if we have one) // then get the branches from it. if ownForkRepo != nil && - ownForkRepo.ID != headRepo.ID && + 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, nil, nil, nil, "", "" + return nil } if perm { ctx.Data["OwnForkRepo"] = ownForkRepo @@ -429,18 +445,18 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * } // Check if head branch is valid. - headIsCommit := headGitRepo.IsCommitExist(headBranch) - headIsBranch := headGitRepo.IsBranchExist(headBranch) - headIsTag := headGitRepo.IsTagExist(headBranch) + headIsCommit := ci.HeadGitRepo.IsCommitExist(ci.HeadBranch) + headIsBranch := ci.HeadGitRepo.IsBranchExist(ci.HeadBranch) + headIsTag := ci.HeadGitRepo.IsTagExist(ci.HeadBranch) if !headIsCommit && !headIsBranch && !headIsTag { // Check if headBranch is short sha commit hash - if headCommit, _ := headGitRepo.GetCommit(headBranch); headCommit != nil { - headBranch = headCommit.ID.String() - ctx.Data["HeadBranch"] = headBranch + if headCommit, _ := ci.HeadGitRepo.GetCommit(ci.HeadBranch); headCommit != nil { + ci.HeadBranch = headCommit.ID.String() + ctx.Data["HeadBranch"] = ci.HeadBranch headIsCommit = true } else { ctx.NotFound("IsRefExist", nil) - return nil, nil, nil, nil, "", "" + return nil } } ctx.Data["HeadIsCommit"] = headIsCommit @@ -460,40 +476,36 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * permBase) } ctx.NotFound("ParseCompareInfo", nil) - return nil, nil, nil, nil, "", "" + return nil } - baseBranchRef := baseBranch + baseBranchRef := ci.BaseBranch if baseIsBranch { - baseBranchRef = git.BranchPrefix + baseBranch + baseBranchRef = git.BranchPrefix + ci.BaseBranch } else if baseIsTag { - baseBranchRef = git.TagPrefix + baseBranch + baseBranchRef = git.TagPrefix + ci.BaseBranch } - headBranchRef := headBranch + headBranchRef := ci.HeadBranch if headIsBranch { - headBranchRef = git.BranchPrefix + headBranch + headBranchRef = git.BranchPrefix + ci.HeadBranch } else if headIsTag { - headBranchRef = git.TagPrefix + headBranch + headBranchRef = git.TagPrefix + ci.HeadBranch } - compareInfo, err := headGitRepo.GetCompareInfo(baseRepo.RepoPath(), baseBranchRef, headBranchRef) + ci.CompareInfo, err = ci.HeadGitRepo.GetCompareInfo(baseRepo.RepoPath(), baseBranchRef, headBranchRef, ci.DirectComparison) if err != nil { ctx.ServerError("GetCompareInfo", err) - return nil, nil, nil, nil, "", "" + return nil } - ctx.Data["BeforeCommitID"] = compareInfo.MergeBase + ctx.Data["BeforeCommitID"] = ci.CompareInfo.MergeBase - return headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch + return ci } // PrepareCompareDiff renders compare diff page func PrepareCompareDiff( ctx *context.Context, - headUser *models.User, - headRepo *models.Repository, - headGitRepo *git.Repository, - compareInfo *git.CompareInfo, - baseBranch, headBranch string, + ci *CompareInfo, whitespaceBehavior string) bool { var ( @@ -503,19 +515,20 @@ func PrepareCompareDiff( ) // Get diff information. - ctx.Data["CommitRepoLink"] = headRepo.Link() + ctx.Data["CommitRepoLink"] = ci.HeadRepo.Link() - headCommitID := compareInfo.HeadCommitID + headCommitID := ci.CompareInfo.HeadCommitID ctx.Data["AfterCommitID"] = headCommitID - if headCommitID == compareInfo.MergeBase { + if (headCommitID == ci.CompareInfo.MergeBase && !ci.DirectComparison) || + headCommitID == ci.CompareInfo.BaseCommitID { ctx.Data["IsNothingToCompare"] = true if unit, err := repo.GetUnit(models.UnitTypePullRequests); err == nil { config := unit.PullRequestsConfig() if !config.AutodetectManualMerge { - allowEmptyPr := !(baseBranch == headBranch && ctx.Repo.Repository.Name == headRepo.Name) + allowEmptyPr := !(ci.BaseBranch == ci.HeadBranch && ctx.Repo.Repository.Name == ci.HeadRepo.Name) ctx.Data["AllowEmptyPr"] = allowEmptyPr return !allowEmptyPr @@ -526,9 +539,14 @@ func PrepareCompareDiff( return true } - diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(headGitRepo, - compareInfo.MergeBase, headCommitID, setting.Git.MaxGitDiffLines, - setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, whitespaceBehavior) + beforeCommitID := ci.CompareInfo.MergeBase + if ci.DirectComparison { + beforeCommitID = ci.CompareInfo.BaseCommitID + } + + diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(ci.HeadGitRepo, + beforeCommitID, headCommitID, setting.Git.MaxGitDiffLines, + setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, whitespaceBehavior, ci.DirectComparison) if err != nil { ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err) return false @@ -536,14 +554,14 @@ func PrepareCompareDiff( ctx.Data["Diff"] = diff ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0 - headCommit, err := headGitRepo.GetCommit(headCommitID) + headCommit, err := ci.HeadGitRepo.GetCommit(headCommitID) if err != nil { ctx.ServerError("GetCommit", err) return false } baseGitRepo := ctx.Repo.GitRepo - baseCommitID := compareInfo.BaseCommitID + baseCommitID := ci.CompareInfo.BaseCommitID baseCommit, err := baseGitRepo.GetCommit(baseCommitID) if err != nil { @@ -551,7 +569,7 @@ func PrepareCompareDiff( return false } - commits := models.ConvertFromGitCommit(compareInfo.Commits, headRepo) + commits := models.ConvertFromGitCommit(ci.CompareInfo.Commits, ci.HeadRepo) ctx.Data["Commits"] = commits ctx.Data["CommitCount"] = len(commits) @@ -564,7 +582,7 @@ func PrepareCompareDiff( ctx.Data["content"] = strings.Join(body[1:], "\n") } } else { - title = headBranch + title = ci.HeadBranch } if len(title) > 255 { var trailer string @@ -579,10 +597,10 @@ func PrepareCompareDiff( } ctx.Data["title"] = title - ctx.Data["Username"] = headUser.Name - ctx.Data["Reponame"] = headRepo.Name + ctx.Data["Username"] = ci.HeadUser.Name + ctx.Data["Reponame"] = ci.HeadRepo.Name - headTarget := path.Join(headUser.Name, repo.Name) + headTarget := path.Join(ci.HeadUser.Name, repo.Name) setCompareContext(ctx, baseCommit, headCommit, headTarget) return false @@ -615,17 +633,25 @@ func getBranchesAndTagsForRepo(user *models.User, repo *models.Repository) (bool // CompareDiff show different from one commit to another commit func CompareDiff(ctx *context.Context) { - headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch := ParseCompareInfo(ctx) + ci := ParseCompareInfo(ctx) defer func() { - if headGitRepo != nil { - headGitRepo.Close() + if ci.HeadGitRepo != nil { + ci.HeadGitRepo.Close() } }() if ctx.Written() { return } - nothingToCompare := PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch, + ctx.Data["DirectComparison"] = ci.DirectComparison + ctx.Data["OtherCompareSeparator"] = ".." + ctx.Data["CompareSeparator"] = "..." + if ci.DirectComparison { + ctx.Data["CompareSeparator"] = ".." + ctx.Data["OtherCompareSeparator"] = "..." + } + + nothingToCompare := PrepareCompareDiff(ctx, ci, gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string))) if ctx.Written() { return @@ -639,14 +665,14 @@ func CompareDiff(ctx *context.Context) { } ctx.Data["Tags"] = baseTags - headBranches, _, err := headGitRepo.GetBranches(0, 0) + headBranches, _, err := ci.HeadGitRepo.GetBranches(0, 0) if err != nil { ctx.ServerError("GetBranches", err) return } ctx.Data["HeadBranches"] = headBranches - headTags, err := headGitRepo.GetTags(0, 0) + headTags, err := ci.HeadGitRepo.GetTags(0, 0) if err != nil { ctx.ServerError("GetTags", err) return @@ -654,7 +680,7 @@ func CompareDiff(ctx *context.Context) { ctx.Data["HeadTags"] = headTags if ctx.Data["PageIsComparePull"] == true { - pr, err := models.GetUnmergedPullRequest(headRepo.ID, ctx.Repo.Repository.ID, headBranch, baseBranch, models.PullRequestFlowGithub) + pr, err := models.GetUnmergedPullRequest(ci.HeadRepo.ID, ctx.Repo.Repository.ID, ci.HeadBranch, ci.BaseBranch, models.PullRequestFlowGithub) if err != nil { if !models.IsErrPullRequestNotExist(err) { ctx.ServerError("GetUnmergedPullRequest", err) @@ -678,7 +704,11 @@ func CompareDiff(ctx *context.Context) { beforeCommitID := ctx.Data["BeforeCommitID"].(string) afterCommitID := ctx.Data["AfterCommitID"].(string) - ctx.Data["Title"] = "Comparing " + base.ShortSha(beforeCommitID) + "..." + base.ShortSha(afterCommitID) + separator := "..." + if ci.DirectComparison { + separator = ".." + } + ctx.Data["Title"] = "Comparing " + base.ShortSha(beforeCommitID) + separator + base.ShortSha(afterCommitID) ctx.Data["IsRepoToolbarCommits"] = true ctx.Data["IsDiffCompare"] = true diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index c370e7f04d..e6d67f3fcc 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -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()) + pull.MergeBase, pull.GetGitRefName(), true) 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 @@ -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()) + pull.MergeBase, pull.GetGitRefName(), true) if err != nil { if strings.Contains(err.Error(), "fatal: Not a valid object name") { ctx.Data["IsPullRequestBroken"] = true @@ -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()) + git.BranchPrefix+pull.BaseBranch, pull.GetGitRefName(), true) if err != nil { if strings.Contains(err.Error(), "fatal: Not a valid object name") { ctx.Data["IsPullRequestBroken"] = true @@ -635,7 +635,7 @@ func ViewPullFiles(ctx *context.Context) { diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(gitRepo, startCommitID, endCommitID, 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.ServerError("GetDiffRangeWithWhitespaceBehavior", err) return @@ -1041,10 +1041,10 @@ func CompareAndPullRequestPost(ctx *context.Context) { attachments []string ) - headUser, headRepo, headGitRepo, prInfo, baseBranch, headBranch := ParseCompareInfo(ctx) + ci := ParseCompareInfo(ctx) defer func() { - if headGitRepo != nil { - headGitRepo.Close() + if ci.HeadGitRepo != nil { + ci.HeadGitRepo.Close() } }() if ctx.Written() { @@ -1065,7 +1065,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { // This stage is already stop creating new pull request, so it does not matter if it has // something to compare or not. - PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, prInfo, baseBranch, headBranch, + PrepareCompareDiff(ctx, ci, gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string))) if ctx.Written() { return @@ -1084,7 +1084,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { } if util.IsEmptyString(form.Title) { - PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, prInfo, baseBranch, headBranch, + PrepareCompareDiff(ctx, ci, gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string))) if ctx.Written() { return @@ -1104,13 +1104,13 @@ func CompareAndPullRequestPost(ctx *context.Context) { Content: form.Content, } pullRequest := &models.PullRequest{ - HeadRepoID: headRepo.ID, + HeadRepoID: ci.HeadRepo.ID, BaseRepoID: repo.ID, - HeadBranch: headBranch, - BaseBranch: baseBranch, - HeadRepo: headRepo, + HeadBranch: ci.HeadBranch, + BaseBranch: ci.BaseBranch, + HeadRepo: ci.HeadRepo, BaseRepo: repo, - MergeBase: prInfo.MergeBase, + MergeBase: ci.CompareInfo.MergeBase, Type: models.PullRequestGitea, } // FIXME: check error in the case two people send pull request at almost same time, give nice error prompt diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index ac5e947d15..ef66675c5b 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1217,7 +1217,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 string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string, directComparison bool) (*Diff, error) { repoPath := gitRepo.Path commit, err := gitRepo.GetCommit(afterCommitID) @@ -1357,7 +1357,12 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID, return nil, fmt.Errorf("Wait: %v", err) } - shortstatArgs := []string{beforeCommitID + "..." + afterCommitID} + separator := "..." + if directComparison { + separator = ".." + } + + shortstatArgs := []string{beforeCommitID + separator + afterCommitID} if len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA { shortstatArgs = []string{git.EmptyTreeSHA, afterCommitID} } @@ -1377,8 +1382,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 string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string, directComparison bool) (*Diff, error) { + return GetDiffRangeWithWhitespaceBehavior(gitRepo, "", commitID, maxLines, maxLineCharacters, maxFiles, whitespaceBehavior, directComparison) } // CommentAsDiff returns c.Patch as *Diff diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go index 0d8d60d94f..0c216ccb54 100644 --- a/services/gitdiff/gitdiff_test.go +++ b/services/gitdiff/gitdiff_test.go @@ -523,7 +523,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", - setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles, behavior) + setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles, behavior, false) 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/services/pull/pull.go b/services/pull/pull.go index f7e231379b..f7d154cfd0 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -80,7 +80,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(), true) if err != nil { return err } diff --git a/templates/repo/diff/compare.tmpl b/templates/repo/diff/compare.tmpl index 2beca57960..54ea461587 100644 --- a/templates/repo/diff/compare.tmpl +++ b/templates/repo/diff/compare.tmpl @@ -33,7 +33,7 @@ {{- end -}} {{- end -}}
- {{svg "octicon-git-compare"}} + {{svg "octicon-git-compare"}}
{{if $.PageIsComparePull}}{{.i18n.Tr "repo.pulls.compare_base"}}{{else}}{{.i18n.Tr "repo.compare.compare_base"}}{{end}}: {{$BaseCompareName}}:{{$.BaseBranch}} @@ -62,47 +62,47 @@
{{range .Branches}} -
{{$BaseCompareName}}:{{.}}
+
{{$BaseCompareName}}:{{.}}
{{end}} {{if not .PullRequestCtx.SameRepo}} {{range .HeadBranches}} -
{{$HeadCompareName}}:{{.}}
+
{{$HeadCompareName}}:{{.}}
{{end}} {{end}} {{if .OwnForkRepo}} {{range .OwnForkRepoBranches}} -
{{$OwnForkCompareName}}:{{.}}
+
{{$OwnForkCompareName}}:{{.}}
{{end}} {{end}} {{if .RootRepo}} {{range .RootRepoBranches}} -
{{$RootRepoCompareName}}:{{.}}
+
{{$RootRepoCompareName}}:{{.}}
{{end}} {{end}}
- ... + {{.CompareSeparator}}
{{if $.PageIsComparePull}}{{.i18n.Tr "repo.pulls.compare_compare"}}{{else}}{{.i18n.Tr "repo.compare.compare_head"}}{{end}}: {{$HeadCompareName}}:{{$.HeadBranch}} @@ -131,41 +131,41 @@
{{range .HeadBranches}} -
{{$HeadCompareName}}:{{.}}
+
{{$HeadCompareName}}:{{.}}
{{end}} {{if not .PullRequestCtx.SameRepo}} {{range .Branches}} -
{{$BaseCompareName}}:{{.}}
+
{{$BaseCompareName}}:{{.}}
{{end}} {{end}} {{if .OwnForkRepo}} {{range .OwnForkRepoBranches}} -
{{$OwnForkCompareName}}:{{.}}
+
{{$OwnForkCompareName}}:{{.}}
{{end}} {{end}} {{if .RootRepo}} {{range .RootRepoBranches}} -
{{$RootRepoCompareName}}:{{.}}
+
{{$RootRepoCompareName}}:{{.}}
{{end}} {{end}}