Skip to content

Refactor compare diff/pull page (1) #37481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bircni merged 3 commits into from
Apr 29, 2026
Merged

Refactor compare diff/pull page (1) #37481

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f9eaf63
fix
wxiaoguang Apr 29, 2026
6860d5e
fix
wxiaoguang Apr 29, 2026
a8bf1d1
Merge branch 'main' into refactor-compare-pull
bircni Apr 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions routers/api/v1/repo/pull.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ func CreatePullRequest(ctx *context.APIContext) {
BaseBranch: compareResult.BaseRef.ShortName(),
HeadRepo: compareResult.HeadRepo,
BaseRepo: repo,
MergeBase: compareResult.MergeBase,
MergeBase: compareResult.CompareBase,
Type: issues_model.PullRequestGitea,
}

Expand Down Expand Up @@ -1569,7 +1569,7 @@ func GetPullRequestFiles(ctx *context.APIContext) {
return
}

startCommitID := compareInfo.MergeBase
startCommitID := compareInfo.CompareBase
endCommitID := headCommitID

maxLines := setting.Git.MaxGitDiffLines
Expand Down
13 changes: 1 addition & 12 deletions routers/web/repo/commit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@ func Commits(ctx *context.Context) {
} else {
ctx.Data["CommitsTagsMap"] = commitsTagsMap
}
ctx.Data["Username"] = ctx.Repo.Owner.Name
ctx.Data["Reponame"] = ctx.Repo.Repository.Name
ctx.Data["CommitCount"] = commitsCount

pager := context.NewPagination(commitsCount, pageSize, page, 5)
Expand Down Expand Up @@ -163,9 +161,6 @@ func Graph(ctx *context.Context) {

ctx.Data["AllRefs"] = gitRefs

ctx.Data["Username"] = ctx.Repo.Owner.Name
ctx.Data["Reponame"] = ctx.Repo.Repository.Name

divOnly := ctx.FormBool("div-only")
queryParams := ctx.Req.URL.Query()
queryParams.Del("div-only")
Expand Down Expand Up @@ -209,8 +204,6 @@ func SearchCommits(ctx *context.Context) {
if all {
ctx.Data["All"] = true
}
ctx.Data["Username"] = ctx.Repo.Owner.Name
ctx.Data["Reponame"] = ctx.Repo.Repository.Name
ctx.HTML(http.StatusOK, tplCommits)
}

Expand Down Expand Up @@ -248,8 +241,6 @@ func FileHistory(ctx *context.Context) {
return
}

ctx.Data["Username"] = ctx.Repo.Owner.Name
ctx.Data["Reponame"] = ctx.Repo.Repository.Name
ctx.Data["FileTreePath"] = ctx.Repo.TreePath
ctx.Data["CommitCount"] = commitsCount

Expand Down Expand Up @@ -321,7 +312,7 @@ func Diff(ctx *context.Context) {
MaxLines: maxLines,
MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters,
MaxFiles: maxFiles,
WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),
WhitespaceBehavior: gitdiff.GetWhitespaceFlag(GetWhitespaceBehavior(ctx)),
}, files...)
if err != nil {
ctx.NotFound(err)
Expand All @@ -346,8 +337,6 @@ func Diff(ctx *context.Context) {

ctx.Data["CommitID"] = commitID
ctx.Data["AfterCommitID"] = commitID
ctx.Data["Username"] = userName
ctx.Data["Reponame"] = repoName

var parentCommit *git.Commit
var parentCommitID string
Expand Down
122 changes: 59 additions & 63 deletions routers/web/repo/compare.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,18 @@ func setCsvCompareContext(ctx *context.Context) {
}
}

type comparePageInfoType struct {
compareInfo *git_service.CompareInfo
nothingToCompare bool
allowCreatePull bool
}

func newComparePageInfo() *comparePageInfoType {
return &comparePageInfoType{}
}

// parseCompareInfo parse compare info between two commit for preparing comparing references
func parseCompareInfo(ctx *context.Context) (*git_service.CompareInfo, error) {
func (cpi *comparePageInfoType) parseCompareInfo(ctx *context.Context) error {
baseRepo := ctx.Repo.Repository
fileOnly := ctx.FormBool("file-only")

Expand All @@ -200,13 +210,13 @@ func parseCompareInfo(ctx *context.Context) (*git_service.CompareInfo, error) {

// remove the check when we support compare with carets
if compareReq.BaseOriRefSuffix != "" {
return nil, util.NewInvalidArgumentErrorf("unsupported comparison syntax: ref with suffix")
return util.NewInvalidArgumentErrorf("unsupported comparison syntax: ref with suffix")
}

// 2 get repository and owner for head
headOwner, headRepo, err := common.GetHeadOwnerAndRepo(ctx, baseRepo, compareReq)
if err != nil {
return nil, err
return err
}

// 3 permission check
Expand All @@ -219,10 +229,10 @@ func parseCompareInfo(ctx *context.Context) (*git_service.CompareInfo, error) {
// Assert ctx.Doer has permission to read headRepo's codes
permHead, err := access_model.GetDoerRepoPermission(ctx, headRepo, ctx.Doer)
if err != nil {
return nil, err
return err
}
if !permHead.CanRead(unit.TypeCode) {
return nil, util.NewNotExistErrorf("") // permission: no error message for end users
return util.NewNotExistErrorf("") // permission: no error message for end users
}
ctx.Data["CanWriteToHeadRepo"] = permHead.CanWrite(unit.TypeCode)
}
Expand All @@ -233,17 +243,16 @@ func parseCompareInfo(ctx *context.Context) (*git_service.CompareInfo, error) {

baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(baseRefName)
if baseRef == "" {
return nil, util.NewNotExistErrorf("no base ref: %s", baseRefName)
return util.NewNotExistErrorf("no base ref: %s", baseRefName)
}
headGitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, headRepo)
if err != nil {
ctx.ServerError("OpenRepository", err)
return nil, err
return err
}

headRef := headGitRepo.UnstableGuessRefByShortName(headRefName)
if headRef == "" {
return nil, util.NewNotExistErrorf("no head ref: %s", headRefName)
return util.NewNotExistErrorf("no head ref: %s", headRefName)
}

ctx.Data["BaseName"] = baseRepo.OwnerName
Expand All @@ -268,7 +277,7 @@ func parseCompareInfo(ctx *context.Context) (*git_service.CompareInfo, error) {
if baseRepo.IsFork {
err = baseRepo.GetBaseRepo(ctx)
if err != nil && !repo_model.IsErrRepoNotExist(err) {
return nil, err
return err
} else if err == nil {
rootRepo = baseRepo.BaseRepo
}
Expand Down Expand Up @@ -302,7 +311,7 @@ func parseCompareInfo(ctx *context.Context) (*git_service.CompareInfo, error) {
if !fileOnly {
branches, tags, err := getBranchesAndTagsForRepo(ctx, rootRepo)
if err != nil {
return nil, err
return err
}
ctx.Data["RootRepoBranches"] = branches
ctx.Data["RootRepoTags"] = tags
Expand All @@ -325,7 +334,7 @@ func parseCompareInfo(ctx *context.Context) (*git_service.CompareInfo, error) {
if !fileOnly {
branches, tags, err := getBranchesAndTagsForRepo(ctx, ownForkRepo)
if err != nil {
return nil, err
return err
}
ctx.Data["OwnForkRepoBranches"] = branches
ctx.Data["OwnForkRepoTags"] = tags
Expand All @@ -335,19 +344,14 @@ func parseCompareInfo(ctx *context.Context) (*git_service.CompareInfo, error) {

compareInfo, err := git_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef, headRef, compareReq.DirectComparison(), fileOnly)
if err != nil {
return nil, err
return err
}

// Treat as pull request if both references are branches
allowCreatePullRequest := baseRef.IsBranch() && headRef.IsBranch() && permBase.CanReadIssuesOrPulls(true)
allowCreatePullRequest = allowCreatePullRequest && compareInfo.MergeBase != ""
ctx.Data["PageIsComparePull"] = allowCreatePullRequest
if compareReq.DirectComparison() {
ctx.Data["BeforeCommitID"] = compareInfo.BaseCommitID
} else {
ctx.Data["BeforeCommitID"] = compareInfo.MergeBase
}
return &compareInfo, nil
cpi.allowCreatePull = baseRef.IsBranch() && headRef.IsBranch() && permBase.CanReadIssuesOrPulls(true)
cpi.allowCreatePull = cpi.allowCreatePull && compareInfo.CompareBase != ""
cpi.compareInfo = &compareInfo
return nil
}

// autoTitleFromBranchName humanizes a branch name into a PR title.
Expand Down Expand Up @@ -414,14 +418,17 @@ func prepareNewPullRequestTitleContent(ci *git_service.CompareInfo, commits []*g
}

// prepareCompareDiff renders compare diff page. TODO: need to refactor it and other "compare diff" related functions together
func prepareCompareDiff(ctx *context.Context, ci *git_service.CompareInfo, whitespaceBehavior gitcmd.TrustedCmdArgs) (nothingToCompare bool) {
if ci.MergeBase == "" {
return true
func (cpi *comparePageInfoType) prepareCompareDiff(ctx *context.Context, whitespaceBehavior gitcmd.TrustedCmdArgs) {
ci := cpi.compareInfo
if ci.CompareBase == "" {
cpi.nothingToCompare = true
return
}
repo := ctx.Repo.Repository
headCommitID := ci.HeadCommitID

ctx.Data["CommitRepoLink"] = ci.HeadRepo.Link()
ctx.Data["BeforeCommitID"] = ci.CompareBase
ctx.Data["AfterCommitID"] = headCommitID

// follow GitHub's behavior: autofill the form and expand
Expand All @@ -431,26 +438,20 @@ func prepareCompareDiff(ctx *context.Context, ci *git_service.CompareInfo, white
ctx.Data["TitleQuery"] = newPrFormTitle
ctx.Data["BodyQuery"] = newPrFormBody

if (headCommitID == ci.MergeBase && !ci.DirectComparison()) ||
if (headCommitID == ci.CompareBase && !ci.DirectComparison()) ||
headCommitID == ci.BaseCommitID {
ctx.Data["IsNothingToCompare"] = true
if unit, err := repo.GetUnit(ctx, unit.TypePullRequests); err == nil {
config := unit.PullRequestsConfig()

if !config.AutodetectManualMerge {
ctx.Data["AllowEmptyPr"] = !ci.IsSameRef()
return ci.IsSameRef()
}
config := repo.MustGetUnit(ctx, unit.TypePullRequests).PullRequestsConfig()

ctx.Data["AllowEmptyPr"] = false
ctx.Data["AllowEmptyPr"] = false
if !config.AutodetectManualMerge {
ctx.Data["AllowEmptyPr"] = !ci.IsSameRef()
cpi.nothingToCompare = ci.IsSameRef()
}
Comment thread
wxiaoguang marked this conversation as resolved.
Outdated
return true
return
}

beforeCommitID := ci.MergeBase
if ci.DirectComparison() {
beforeCommitID = ci.BaseCommitID
}
beforeCommitID := ci.CompareBase

maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles
files := ctx.FormStrings("files")
Expand All @@ -473,12 +474,12 @@ func prepareCompareDiff(ctx *context.Context, ci *git_service.CompareInfo, white
}, ctx.FormStrings("files")...)
if err != nil {
ctx.ServerError("GetDiff", err)
return false
return
}
diffShortStat, err := gitdiff.GetDiffShortStat(ctx, ci.HeadRepo, ci.HeadGitRepo, beforeCommitID, headCommitID)
if err != nil {
ctx.ServerError("GetDiffShortStat", err)
return false
return
}
ctx.Data["DiffShortStat"] = diffShortStat
ctx.Data["Diff"] = diff
Expand All @@ -493,7 +494,7 @@ func prepareCompareDiff(ctx *context.Context, ci *git_service.CompareInfo, white
diffTree, err := gitdiff.GetDiffTree(ctx, ci.HeadGitRepo, false, beforeCommitID, headCommitID)
if err != nil {
ctx.ServerError("GetDiffTree", err)
return false
return
}

renderedIconPool := fileicon.NewRenderedIconPool()
Expand All @@ -506,32 +507,28 @@ func prepareCompareDiff(ctx *context.Context, ci *git_service.CompareInfo, white
headCommit, err := ci.HeadGitRepo.GetCommit(headCommitID)
if err != nil {
ctx.ServerError("GetCommit", err)
return false
return
}

baseGitRepo := ctx.Repo.GitRepo

beforeCommit, err := baseGitRepo.GetCommit(beforeCommitID)
if err != nil {
ctx.ServerError("GetCommit", err)
return false
return
}

commits, err := processGitCommits(ctx, ci.Commits)
if err != nil {
ctx.ServerError("processGitCommits", err)
return false
return
}
ctx.Data["Commits"] = commits
ctx.Data["CommitCount"] = len(commits)

ctx.Data["title"], ctx.Data["content"] = prepareNewPullRequestTitleContent(ci, commits, setting.Repository.PullRequest.DefaultTitleSource)
ctx.Data["Username"] = ci.HeadRepo.OwnerName
ctx.Data["Reponame"] = ci.HeadRepo.Name

setCompareContext(ctx, beforeCommit, headCommit, ci.HeadRepo.OwnerName, repo.Name)

return false
}

func getBranchesAndTagsForRepo(ctx gocontext.Context, repo *repo_model.Repository) (branches, tags []string, err error) {
Expand All @@ -552,24 +549,22 @@ func getBranchesAndTagsForRepo(ctx gocontext.Context, repo *repo_model.Repositor

// CompareDiff show different from one commit to another commit
func CompareDiff(ctx *context.Context) {
ci, err := parseCompareInfo(ctx)
if ctx.Written() {
return
}
comparePageInfo := newComparePageInfo()
err := comparePageInfo.parseCompareInfo(ctx)
if errors.Is(err, util.ErrNotExist) || errors.Is(err, util.ErrInvalidArgument) {
ctx.NotFound(nil)
return
} else if err != nil {
ctx.ServerError("ParseCompareInfo", err)
return
}

ci := comparePageInfo.compareInfo
ctx.Data["PageIsViewCode"] = true
ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes
ctx.Data["CompareInfo"] = ci

// TODO: need to refactor "prepare compare" related functions together
nothingToCompare := prepareCompareDiff(ctx, ci, gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
comparePageInfo.prepareCompareDiff(ctx, gitdiff.GetWhitespaceFlag(GetWhitespaceBehavior(ctx)))
if ctx.Written() {
return
}
Expand Down Expand Up @@ -601,21 +596,22 @@ func CompareDiff(ctx *context.Context) {
return
}

if ci.MergeBase != "" {
prepareCreatePullRequestPage(ctx, ci, nothingToCompare)
if ci.CompareBase != "" {
comparePageInfo.prepareCreatePullRequestPage(ctx)
if ctx.Written() {
return
}
} else {
ctx.Flash.Error(ctx.Tr("repo.pulls.no_common_history"), true)
ctx.Data["PageIsComparePull"] = false
ctx.Data["CommitCount"] = 0
}
ctx.Data["PageIsComparePull"] = comparePageInfo.allowCreatePull
ctx.HTML(http.StatusOK, tplCompare)
}

func prepareCreatePullRequestPage(ctx *context.Context, ci *git_service.CompareInfo, nothingToCompare bool) {
if ctx.Data["PageIsComparePull"] == true {
func (cpi *comparePageInfoType) prepareCreatePullRequestPage(ctx *context.Context) {
ci := cpi.compareInfo
if cpi.allowCreatePull {
pr, err := issues_model.GetUnmergedPullRequest(ctx, ci.HeadRepo.ID, ctx.Repo.Repository.ID, ci.HeadRef.ShortName(), ci.BaseRef.ShortName(), issues_model.PullRequestFlowGithub)
if err != nil {
if !issues_model.IsErrPullRequestNotExist(err) {
Expand All @@ -633,7 +629,7 @@ func prepareCreatePullRequestPage(ctx *context.Context, ci *git_service.CompareI
return
}

if !nothingToCompare {
if !cpi.nothingToCompare {
// Setup information for new form.
pageMetaData := retrieveRepoIssueMetaData(ctx, ctx.Repo.Repository, nil, true)
if ctx.Written() {
Expand All @@ -645,8 +641,8 @@ func prepareCreatePullRequestPage(ctx *context.Context, ci *git_service.CompareI
}
}
}
beforeCommitID := ctx.Data["BeforeCommitID"].(string)
afterCommitID := ctx.Data["AfterCommitID"].(string)
beforeCommitID := cpi.compareInfo.CompareBase
afterCommitID := cpi.compareInfo.HeadCommitID

ctx.Data["Title"] = "Comparing " + base.ShortSha(beforeCommitID) + ci.CompareSeparator + base.ShortSha(afterCommitID)

Expand Down
Loading