diff --git a/modules/git/ref.go b/modules/git/ref.go
index 56b2db858a..7b63d06b38 100644
--- a/modules/git/ref.go
+++ b/modules/git/ref.go
@@ -220,3 +220,14 @@ func (ref RefName) RefWebLinkPath() string {
}
return string(refType) + "/" + util.PathEscapeSegments(ref.ShortName())
}
+
+func ParseRefSuffix(ref string) (string, string) {
+ // Partially support https://git-scm.com/docs/gitrevisions
+ if idx := strings.Index(ref, "@{"); idx != -1 {
+ return ref[:idx], ref[idx:]
+ }
+ if idx := strings.Index(ref, "^"); idx != -1 {
+ return ref[:idx], ref[idx:]
+ }
+ return ref, ""
+}
diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json
index 0fb95606b3..480aafe879 100644
--- a/options/locale/locale_en-US.json
+++ b/options/locale/locale_en-US.json
@@ -1736,8 +1736,11 @@
"repo.issues.reference_link": "Reference: %s",
"repo.compare.compare_base": "base",
"repo.compare.compare_head": "compare",
+ "repo.compare.title": "Comparing changes",
+ "repo.compare.description": "Choose two branches or tags to see what’s changed or to start a new pull request.",
"repo.pulls.desc": "Enable pull requests and code reviews.",
"repo.pulls.new": "New Pull Request",
+ "repo.pulls.new.description": "Discuss and review the changes in this comparison with others.",
"repo.pulls.new.blocked_user": "Cannot create pull request because you are blocked by the repository owner.",
"repo.pulls.new.must_collaborator": "You must be a collaborator to create pull request.",
"repo.pulls.new.already_existed": "A pull request between these branches already exists",
@@ -1747,7 +1750,6 @@
"repo.pulls.allow_edits_from_maintainers": "Allow edits from maintainers",
"repo.pulls.allow_edits_from_maintainers_desc": "Users with write access to the base branch can also push to this branch",
"repo.pulls.allow_edits_from_maintainers_err": "Updating failed",
- "repo.pulls.compare_changes_desc": "Select the branch to merge into and the branch to pull from.",
"repo.pulls.has_viewed_file": "Viewed",
"repo.pulls.has_changed_since_last_review": "Changed since your last review",
"repo.pulls.viewed_files_label": "%[1]d / %[2]d files viewed",
diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go
index 209647e7d7..9b1eb3fc85 100644
--- a/routers/api/v1/repo/pull.go
+++ b/routers/api/v1/repo/pull.go
@@ -1062,19 +1062,11 @@ func MergePullRequest(ctx *context.APIContext) {
// parseCompareInfo returns non-nil if it succeeds, it always writes to the context and returns nil if it fails
func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *git_service.CompareInfo, closer func()) {
baseRepo := ctx.Repo.Repository
- compareReq, err := common.ParseCompareRouterParam(compareParam)
- switch {
- case errors.Is(err, util.ErrInvalidArgument):
- ctx.APIError(http.StatusBadRequest, err.Error())
- return nil, nil
- case err != nil:
- ctx.APIErrorInternal(err)
- return nil, nil
- }
+ compareReq := common.ParseCompareRouterParam(compareParam)
// remove the check when we support compare with carets
- if compareReq.CaretTimes > 0 {
- ctx.APIError(http.StatusBadRequest, "Unsupported compare syntax with carets")
+ if compareReq.BaseOriRefSuffix != "" {
+ ctx.APIError(http.StatusBadRequest, "Unsupported comparison syntax: ref with suffix")
return nil, nil
}
diff --git a/routers/common/compare.go b/routers/common/compare.go
index fa9e4e668e..5b6fdba4e0 100644
--- a/routers/common/compare.go
+++ b/routers/common/compare.go
@@ -9,31 +9,28 @@ import (
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
+ "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/util"
)
type CompareRouterReq struct {
- BaseOriRef string
+ BaseOriRef string
+ BaseOriRefSuffix string
+
+ CompareSeparator string
+
HeadOwner string
HeadRepoName string
HeadOriRef string
- CaretTimes int // ^ times after base ref
- DotTimes int
}
func (cr *CompareRouterReq) DirectComparison() bool {
- return cr.DotTimes == 2 || cr.DotTimes == 0
+ // FIXME: the design of "DirectComparison" is wrong, it loses the information of `^`
+ // To correctly handle the comparison, developers should use `ci.CompareSeparator` directly, all "DirectComparison" related code should be rewritten.
+ return cr.CompareSeparator == ".."
}
-func parseBase(base string) (string, int) {
- parts := strings.SplitN(base, "^", 2)
- if len(parts) == 1 {
- return base, 0
- }
- return parts[0], len(parts[1]) + 1
-}
-
-func parseHead(head string) (string, string, string) {
+func parseHead(head string) (headOwnerName, headRepoName, headRef string) {
paths := strings.SplitN(head, ":", 2)
if len(paths) == 1 {
return "", "", paths[0]
@@ -48,6 +45,7 @@ func parseHead(head string) (string, string, string) {
// ParseCompareRouterParam Get compare information from the router parameter.
// A full compare url is of the form:
//
+// 0. /{:baseOwner}/{:baseRepoName}/compare
// 1. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headBranch}
// 2. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}:{:headBranch}
// 3. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}/{:headRepoName}:{:headBranch}
@@ -70,45 +68,31 @@ func parseHead(head string) (string, string, string) {
// format: ...[
:]
// base<-head: master...head:feature
// same repo: master...feature
-func ParseCompareRouterParam(routerParam string) (*CompareRouterReq, error) {
+func ParseCompareRouterParam(routerParam string) *CompareRouterReq {
if routerParam == "" {
- return &CompareRouterReq{}, nil
+ return &CompareRouterReq{}
}
- var basePart, headPart string
- dotTimes := 3
- parts := strings.Split(routerParam, "...")
- if len(parts) > 2 {
- return nil, util.NewInvalidArgumentErrorf("invalid compare router: %s", routerParam)
- }
- if len(parts) != 2 {
- parts = strings.Split(routerParam, "..")
- if len(parts) == 1 {
+ sep := "..."
+ basePart, headPart, ok := strings.Cut(routerParam, sep)
+ if !ok {
+ sep = ".."
+ basePart, headPart, ok = strings.Cut(routerParam, sep)
+ if !ok {
headOwnerName, headRepoName, headRef := parseHead(routerParam)
return &CompareRouterReq{
- HeadOriRef: headRef,
- HeadOwner: headOwnerName,
- HeadRepoName: headRepoName,
- DotTimes: dotTimes,
- }, nil
- } else if len(parts) > 2 {
- return nil, util.NewInvalidArgumentErrorf("invalid compare router: %s", routerParam)
+ HeadOriRef: headRef,
+ HeadOwner: headOwnerName,
+ HeadRepoName: headRepoName,
+ CompareSeparator: "...",
+ }
}
- dotTimes = 2
}
- basePart, headPart = parts[0], parts[1]
- baseRef, caretTimes := parseBase(basePart)
- headOwnerName, headRepoName, headRef := parseHead(headPart)
-
- return &CompareRouterReq{
- BaseOriRef: baseRef,
- HeadOriRef: headRef,
- HeadOwner: headOwnerName,
- HeadRepoName: headRepoName,
- CaretTimes: caretTimes,
- DotTimes: dotTimes,
- }, nil
+ ci := &CompareRouterReq{CompareSeparator: sep}
+ ci.BaseOriRef, ci.BaseOriRefSuffix = git.ParseRefSuffix(basePart)
+ ci.HeadOwner, ci.HeadRepoName, ci.HeadOriRef = parseHead(headPart)
+ return ci
}
// maxForkTraverseLevel defines the maximum levels to traverse when searching for the head repository.
diff --git a/routers/common/compare_test.go b/routers/common/compare_test.go
index a55f6607ae..e4e24a03cf 100644
--- a/routers/common/compare_test.go
+++ b/routers/common/compare_test.go
@@ -6,146 +6,100 @@ package common
import (
"testing"
- "code.gitea.io/gitea/models/unittest"
-
"github.com/stretchr/testify/assert"
)
func TestCompareRouterReq(t *testing.T) {
- unittest.PrepareTestEnv(t)
-
- kases := []struct {
- router string
+ cases := []struct {
+ input string
CompareRouterReq *CompareRouterReq
}{
{
- router: "",
+ input: "",
+ CompareRouterReq: &CompareRouterReq{},
+ },
+ {
+ input: "v1.0...v1.1",
CompareRouterReq: &CompareRouterReq{
- BaseOriRef: "",
- HeadOriRef: "",
- DotTimes: 0,
+ BaseOriRef: "v1.0",
+ CompareSeparator: "...",
+ HeadOriRef: "v1.1",
},
},
{
- router: "main...develop",
+ input: "main..develop",
CompareRouterReq: &CompareRouterReq{
- BaseOriRef: "main",
- HeadOriRef: "develop",
- DotTimes: 3,
+ BaseOriRef: "main",
+ CompareSeparator: "..",
+ HeadOriRef: "develop",
},
},
{
- router: "main..develop",
+ input: "main^...develop",
CompareRouterReq: &CompareRouterReq{
- BaseOriRef: "main",
- HeadOriRef: "develop",
- DotTimes: 2,
+ BaseOriRef: "main",
+ BaseOriRefSuffix: "^",
+ CompareSeparator: "...",
+ HeadOriRef: "develop",
},
},
{
- router: "main^...develop",
+ input: "main^^^^^...develop",
CompareRouterReq: &CompareRouterReq{
- BaseOriRef: "main",
- HeadOriRef: "develop",
- CaretTimes: 1,
- DotTimes: 3,
+ BaseOriRef: "main",
+ BaseOriRefSuffix: "^^^^^",
+ CompareSeparator: "...",
+ HeadOriRef: "develop",
},
},
{
- router: "main^^^^^...develop",
+ input: "develop",
CompareRouterReq: &CompareRouterReq{
- BaseOriRef: "main",
- HeadOriRef: "develop",
- CaretTimes: 5,
- DotTimes: 3,
+ CompareSeparator: "...",
+ HeadOriRef: "develop",
},
},
{
- router: "develop",
+ input: "teabot:feature1",
CompareRouterReq: &CompareRouterReq{
- HeadOriRef: "develop",
- DotTimes: 3,
+ CompareSeparator: "...",
+ HeadOwner: "teabot",
+ HeadOriRef: "feature1",
},
},
{
- router: "lunny/forked_repo:develop",
+ input: "lunny/forked_repo:develop",
CompareRouterReq: &CompareRouterReq{
- HeadOwner: "lunny",
- HeadRepoName: "forked_repo",
- HeadOriRef: "develop",
- DotTimes: 3,
+ CompareSeparator: "...",
+ HeadOwner: "lunny",
+ HeadRepoName: "forked_repo",
+ HeadOriRef: "develop",
},
},
{
- router: "main...lunny/forked_repo:develop",
+ input: "main...lunny/forked_repo:develop",
CompareRouterReq: &CompareRouterReq{
- BaseOriRef: "main",
- HeadOwner: "lunny",
- HeadRepoName: "forked_repo",
- HeadOriRef: "develop",
- DotTimes: 3,
+ BaseOriRef: "main",
+ CompareSeparator: "...",
+ HeadOwner: "lunny",
+ HeadRepoName: "forked_repo",
+ HeadOriRef: "develop",
},
},
{
- router: "main...lunny/forked_repo:develop",
+ input: "main^...lunny/forked_repo:develop",
CompareRouterReq: &CompareRouterReq{
- BaseOriRef: "main",
- HeadOwner: "lunny",
- HeadRepoName: "forked_repo",
- HeadOriRef: "develop",
- DotTimes: 3,
- },
- },
- {
- router: "main^...lunny/forked_repo:develop",
- CompareRouterReq: &CompareRouterReq{
- BaseOriRef: "main",
- HeadOwner: "lunny",
- HeadRepoName: "forked_repo",
- HeadOriRef: "develop",
- DotTimes: 3,
- CaretTimes: 1,
- },
- },
- {
- router: "v1.0...v1.1",
- CompareRouterReq: &CompareRouterReq{
- BaseOriRef: "v1.0",
- HeadOriRef: "v1.1",
- DotTimes: 3,
- },
- },
- {
- router: "teabot-patch-1...v0.0.1",
- CompareRouterReq: &CompareRouterReq{
- BaseOriRef: "teabot-patch-1",
- HeadOriRef: "v0.0.1",
- DotTimes: 3,
- },
- },
- {
- router: "teabot:feature1",
- CompareRouterReq: &CompareRouterReq{
- HeadOwner: "teabot",
- HeadOriRef: "feature1",
- DotTimes: 3,
- },
- },
- {
- router: "8eb19a5ae19abae15c0666d4ab98906139a7f439...283c030497b455ecfa759d4649f9f8b45158742e",
- CompareRouterReq: &CompareRouterReq{
- BaseOriRef: "8eb19a5ae19abae15c0666d4ab98906139a7f439",
- HeadOriRef: "283c030497b455ecfa759d4649f9f8b45158742e",
- DotTimes: 3,
+ BaseOriRef: "main",
+ BaseOriRefSuffix: "^",
+ CompareSeparator: "...",
+ HeadOwner: "lunny",
+ HeadRepoName: "forked_repo",
+ HeadOriRef: "develop",
},
},
}
- for _, kase := range kases {
- t.Run(kase.router, func(t *testing.T) {
- r, err := ParseCompareRouterParam(kase.router)
- assert.NoError(t, err)
- assert.Equal(t, kase.CompareRouterReq, r)
- })
+ for _, c := range cases {
+ assert.Equal(t, c.CompareRouterReq, ParseCompareRouterParam(c.input), "input: %s", c.input)
}
}
diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go
index 29a82b5dfc..bbe1ed3b5e 100644
--- a/routers/web/repo/compare.go
+++ b/routers/web/repo/compare.go
@@ -196,21 +196,16 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
baseRepo := ctx.Repo.Repository
fileOnly := ctx.FormBool("file-only")
- compareReq, err := common.ParseCompareRouterParam(ctx.PathParam("*"))
- switch {
- case errors.Is(err, util.ErrInvalidArgument):
- ctx.HTTPError(http.StatusBadRequest, err.Error())
- return nil
- case err != nil:
- ctx.ServerError("ParseCompareRouterParam", err)
- return nil
- }
+ // 1 Parse compare router param
+ compareReq := common.ParseCompareRouterParam(ctx.PathParam("*"))
+
// remove the check when we support compare with carets
- if compareReq.CaretTimes > 0 {
- ctx.HTTPError(http.StatusBadRequest, "Unsupported compare syntax with carets")
+ if compareReq.BaseOriRefSuffix != "" {
+ ctx.HTTPError(http.StatusBadRequest, "Unsupported comparison syntax: ref with suffix")
return nil
}
+ // 2 get repository and owner for head
headOwner, headRepo, err := common.GetHeadOwnerAndRepo(ctx, baseRepo, compareReq)
switch {
case errors.Is(err, util.ErrInvalidArgument):
@@ -224,45 +219,66 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
return nil
}
- baseBranch := util.IfZero(compareReq.BaseOriRef, baseRepo.DefaultBranch)
- headBranch := util.IfZero(compareReq.HeadOriRef, headRepo.DefaultBranch)
isSameRepo := baseRepo.ID == headRepo.ID
- ctx.Data["BaseName"] = baseRepo.OwnerName
- ctx.Data["BaseBranch"] = baseBranch
- ctx.Data["HeadUser"] = headOwner
- ctx.Data["HeadBranch"] = headBranch
- ctx.Repo.PullRequest.SameRepo = isSameRepo
+ // 3 permission check
+ // base repository's code unit read permission check has been done on web.go
+ permBase := ctx.Repo.Permission
- // Check if base branch is valid.
- baseIsCommit := ctx.Repo.GitRepo.IsCommitExist(baseBranch)
- baseIsBranch, _ := git_model.IsBranchExist(ctx, ctx.Repo.Repository.ID, baseBranch)
- baseIsTag := gitrepo.IsTagExist(ctx, ctx.Repo.Repository, 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
- baseIsCommit = true
- } else if baseBranch == ctx.Repo.GetObjectFormat().EmptyObjectID().String() {
- if isSameRepo {
- ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + util.PathEscapeSegments(headBranch))
- } else {
- ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + util.PathEscapeSegments(headRepo.FullName()) + ":" + util.PathEscapeSegments(headBranch))
- }
+ // If we're not merging from the same repo:
+ if !isSameRepo {
+ // Assert ctx.Doer has permission to read headRepo's codes
+ permHead, err := access_model.GetUserRepoPermission(ctx, headRepo, ctx.Doer)
+ if err != nil {
+ ctx.ServerError("GetUserRepoPermission", err)
return nil
- } else {
+ }
+ if !permHead.CanRead(unit.TypeCode) {
+ if log.IsTrace() {
+ log.Trace("Permission Denied: User: %-v cannot read code in Repo: %-v\nUser in headRepo has Permissions: %-+v",
+ ctx.Doer,
+ headRepo,
+ permHead)
+ }
ctx.NotFound(nil)
return nil
}
+ ctx.Data["CanWriteToHeadRepo"] = permHead.CanWrite(unit.TypeCode)
}
- ctx.Data["BaseIsCommit"] = baseIsCommit
- ctx.Data["BaseIsBranch"] = baseIsBranch
- ctx.Data["BaseIsTag"] = baseIsTag
- ctx.Data["IsPull"] = true
- // Now we have the repository that represents the base
+ // 4 get base and head refs
+ baseRefName := util.IfZero(compareReq.BaseOriRef, baseRepo.DefaultBranch)
+ headRefName := util.IfZero(compareReq.HeadOriRef, headRepo.DefaultBranch)
+
+ baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(baseRefName)
+ if baseRef == "" {
+ ctx.NotFound(nil)
+ return nil
+ }
+ var headGitRepo *git.Repository
+ if isSameRepo {
+ headGitRepo = ctx.Repo.GitRepo
+ } else {
+ headGitRepo, err = gitrepo.OpenRepository(ctx, headRepo)
+ if err != nil {
+ ctx.ServerError("OpenRepository", err)
+ return nil
+ }
+ defer headGitRepo.Close()
+ }
+ headRef := headGitRepo.UnstableGuessRefByShortName(headRefName)
+ if headRef == "" {
+ ctx.NotFound(nil)
+ return nil
+ }
+
+ ctx.Data["BaseName"] = baseRepo.OwnerName
+ ctx.Data["BaseBranch"] = baseRef.ShortName() // for legacy templates
+ ctx.Data["HeadUser"] = headOwner
+ ctx.Data["HeadBranch"] = headRef.ShortName() // for legacy templates
+ ctx.Repo.PullRequest.SameRepo = isSameRepo
+
+ ctx.Data["IsPull"] = true
// The current base and head repositories and branches may not
// actually be the intended branches that the user wants to
@@ -331,64 +347,9 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
ctx.Data["PageIsComparePull"] = false
}
- // 8. Finally open the git repo
- var headGitRepo *git.Repository
- if isSameRepo {
- headGitRepo = ctx.Repo.GitRepo
- } else if has {
- headGitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, headRepo)
- if err != nil {
- ctx.ServerError("RepositoryFromRequestContextOrOpen", err)
- return nil
- }
- } else {
- ctx.NotFound(nil)
- return nil
- }
-
ctx.Data["HeadRepo"] = headRepo
ctx.Data["BaseCompareRepo"] = ctx.Repo.Repository
- // Now we need to assert that the ctx.Doer has permission to read
- // the baseRepo's code and pulls
- // (NOT headRepo's)
- permBase, err := access_model.GetUserRepoPermission(ctx, baseRepo, ctx.Doer)
- if err != nil {
- ctx.ServerError("GetUserRepoPermission", err)
- return nil
- }
- if !permBase.CanRead(unit.TypeCode) {
- if log.IsTrace() {
- log.Trace("Permission Denied: User: %-v cannot read code in Repo: %-v\nUser in baseRepo has Permissions: %-+v",
- ctx.Doer,
- baseRepo,
- permBase)
- }
- ctx.NotFound(nil)
- return nil
- }
-
- // If we're not merging from the same repo:
- if !isSameRepo {
- // Assert ctx.Doer has permission to read headRepo's codes
- permHead, err := access_model.GetUserRepoPermission(ctx, headRepo, ctx.Doer)
- if err != nil {
- ctx.ServerError("GetUserRepoPermission", err)
- return nil
- }
- if !permHead.CanRead(unit.TypeCode) {
- if log.IsTrace() {
- log.Trace("Permission Denied: User: %-v cannot read code in Repo: %-v\nUser in headRepo has Permissions: %-+v",
- ctx.Doer,
- headRepo,
- permHead)
- }
- ctx.NotFound(nil)
- return nil
- }
- ctx.Data["CanWriteToHeadRepo"] = permHead.CanWrite(unit.TypeCode)
- }
-
// If we have a rootRepo and it's different from:
// 1. the computed base
// 2. the computed head
@@ -436,28 +397,9 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
}
}
- // Check if head branch is valid.
- headIsCommit := headGitRepo.IsCommitExist(headBranch)
- headIsBranch, _ := git_model.IsBranchExist(ctx, headRepo.ID, headBranch)
- headIsTag := gitrepo.IsTagExist(ctx, headRepo, 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
- headIsCommit = true
- } else {
- ctx.NotFound(nil)
- return nil
- }
- }
- ctx.Data["HeadIsCommit"] = headIsCommit
- ctx.Data["HeadIsBranch"] = headIsBranch
- ctx.Data["HeadIsTag"] = headIsTag
-
// Treat as pull request if both references are branches
if ctx.Data["PageIsComparePull"] == nil {
- ctx.Data["PageIsComparePull"] = headIsBranch && baseIsBranch && permBase.CanReadIssuesOrPulls(true)
+ ctx.Data["PageIsComparePull"] = baseRef.IsBranch() && headRef.IsBranch() && permBase.CanReadIssuesOrPulls(true)
}
if ctx.Data["PageIsComparePull"] == true && !permBase.CanReadIssuesOrPulls(true) {
@@ -471,20 +413,7 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
return nil
}
- baseBranchRef := git.RefName(baseBranch)
- if baseIsBranch {
- baseBranchRef = git.RefNameFromBranch(baseBranch)
- } else if baseIsTag {
- baseBranchRef = git.RefNameFromTag(baseBranch)
- }
- headBranchRef := git.RefName(headBranch)
- if headIsBranch {
- headBranchRef = git.RefNameFromBranch(headBranch)
- } else if headIsTag {
- headBranchRef = git.RefNameFromTag(headBranch)
- }
-
- compareInfo, err := git_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseBranchRef, headBranchRef, compareReq.DirectComparison(), fileOnly)
+ compareInfo, err := git_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef, headRef, compareReq.DirectComparison(), fileOnly)
if err != nil {
ctx.ServerError("GetCompareInfo", err)
return nil
@@ -517,7 +446,7 @@ func PrepareCompareDiff(
ctx.Data["TitleQuery"] = newPrFormTitle
ctx.Data["BodyQuery"] = newPrFormBody
- if (headCommitID == ci.MergeBase && !ci.DirectComparison) ||
+ if (headCommitID == ci.MergeBase && !ci.DirectComparison()) ||
headCommitID == ci.BaseCommitID {
ctx.Data["IsNothingToCompare"] = true
if unit, err := repo.GetUnit(ctx, unit.TypePullRequests); err == nil {
@@ -534,7 +463,7 @@ func PrepareCompareDiff(
}
beforeCommitID := ci.MergeBase
- if ci.DirectComparison {
+ if ci.DirectComparison() {
beforeCommitID = ci.BaseCommitID
}
@@ -555,7 +484,7 @@ func PrepareCompareDiff(
MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters,
MaxFiles: maxFiles,
WhitespaceBehavior: whitespaceBehavior,
- DirectComparison: ci.DirectComparison,
+ DirectComparison: ci.DirectComparison(),
}, ctx.FormStrings("files")...)
if err != nil {
ctx.ServerError("GetDiff", err)
@@ -668,13 +597,7 @@ func CompareDiff(ctx *context.Context) {
ctx.Data["PageIsViewCode"] = true
ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes
- ctx.Data["DirectComparison"] = ci.DirectComparison
- ctx.Data["OtherCompareSeparator"] = ".."
- ctx.Data["CompareSeparator"] = "..."
- if ci.DirectComparison {
- ctx.Data["CompareSeparator"] = ".."
- ctx.Data["OtherCompareSeparator"] = "..."
- }
+ ctx.Data["CompareInfo"] = ci
nothingToCompare := PrepareCompareDiff(ctx, ci, gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
if ctx.Written() {
@@ -751,11 +674,7 @@ func CompareDiff(ctx *context.Context) {
beforeCommitID := ctx.Data["BeforeCommitID"].(string)
afterCommitID := ctx.Data["AfterCommitID"].(string)
- separator := "..."
- if ci.DirectComparison {
- separator = ".."
- }
- ctx.Data["Title"] = "Comparing " + base.ShortSha(beforeCommitID) + separator + base.ShortSha(afterCommitID)
+ ctx.Data["Title"] = "Comparing " + base.ShortSha(beforeCommitID) + ci.CompareSeparator + base.ShortSha(afterCommitID)
ctx.Data["IsDiffCompare"] = true
diff --git a/services/git/compare.go b/services/git/compare.go
index e4996f357a..ee397fe947 100644
--- a/services/git/compare.go
+++ b/services/git/compare.go
@@ -14,6 +14,7 @@ import (
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/graceful"
logger "code.gitea.io/gitea/modules/log"
+ "code.gitea.io/gitea/modules/util"
)
// CompareInfo represents needed information for comparing references.
@@ -25,7 +26,7 @@ type CompareInfo struct {
HeadGitRepo *git.Repository
HeadRef git.RefName
HeadCommitID string
- DirectComparison bool
+ CompareSeparator string
MergeBase string
Commits []*git.Commit
NumFiles int
@@ -39,6 +40,12 @@ func (ci *CompareInfo) IsSameRef() bool {
return ci.IsSameRepository() && ci.BaseRef == ci.HeadRef
}
+func (ci *CompareInfo) DirectComparison() bool {
+ // FIXME: the design of "DirectComparison" is wrong, it loses the information of `^`
+ // To correctly handle the comparison, developers should use `ci.CompareSeparator` directly, all "DirectComparison" related code should be rewritten.
+ return ci.CompareSeparator == ".."
+}
+
// GetCompareInfo generates and returns compare information between base and head branches of repositories.
func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Repository, headGitRepo *git.Repository, baseRef, headRef git.RefName, directComparison, fileOnly bool) (_ *CompareInfo, err error) {
var (
@@ -66,7 +73,7 @@ func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Reposito
HeadRepo: headRepo,
HeadGitRepo: headGitRepo,
HeadRef: headRef,
- DirectComparison: directComparison,
+ CompareSeparator: util.Iif(directComparison, "..", "..."),
}
compareInfo.HeadCommitID, err = gitrepo.GetFullCommitID(ctx, headRepo, headRef.String())
diff --git a/templates/repo/commits_ref_name.tmpl b/templates/repo/commits_ref_name.tmpl
new file mode 100644
index 0000000000..94e1be9de3
--- /dev/null
+++ b/templates/repo/commits_ref_name.tmpl
@@ -0,0 +1,9 @@
+{{- /* Template Argument: git.RefName */ -}}
+{{- $refName := . -}}
+{{- if $refName.IsBranch -}}
+ {{svg "octicon-git-branch"}} {{$refName.ShortName}}
+{{- else if $refName.IsTag -}}
+ {{svg "octicon-tag"}} {{$refName.ShortName}}
+{{- else -}}
+ {{ShortSha $refName.ShortName}}
+{{- end -}}
diff --git a/templates/repo/commits_table.tmpl b/templates/repo/commits_table.tmpl
index a0c5eacdd4..c8ae535a18 100644
--- a/templates/repo/commits_table.tmpl
+++ b/templates/repo/commits_table.tmpl
@@ -10,9 +10,9 @@
{{if .IsDiffCompare}}
{{end}}
diff --git a/templates/repo/diff/compare.tmpl b/templates/repo/diff/compare.tmpl
index 4e8ad1326c..41d0a198f4 100644
--- a/templates/repo/diff/compare.tmpl
+++ b/templates/repo/diff/compare.tmpl
@@ -2,11 +2,13 @@
{{template "repo/header" .}}
+ {{$compareSeparator := $.CompareInfo.CompareSeparator}}
+ {{$compareSeparatorSwitch := Iif $.CompareInfo.DirectComparison "..." ".."}}