Skip to content

Default to creating pull request for base repository on file edit #34239

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

Closed
wants to merge 2 commits into from
Closed

Default to creating pull request for base repository on file edit #34239

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6838b29
Default to creating pull request for base repository on file edit
brechtvl Apr 18, 2025
49da6e9
Add comments
brechtvl Apr 22, 2025
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
30 changes: 16 additions & 14 deletions routers/web/repo/editor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ func canCreateBasePullRequest(ctx *context.Context) bool {
return baseRepo != nil && baseRepo.UnitEnabled(ctx, unit.TypePullRequests)
}

// defaultCreateNewBranch checks if the default commit choice should be to create new branch.
// As a heuristic, always do this the default branch on forks, which is the right choice for
// pull requests, and making further edits to that pull request branch.
func defaultCreateNewBranch(ctx *context.Context) bool {
baseRepo := ctx.Repo.Repository.BaseRepo
return canCreateBasePullRequest(ctx) && baseRepo != nil && ctx.Repo.BranchName == baseRepo.DefaultBranch
}

func renderCommitRights(ctx *context.Context) bool {
canCommitToBranch, err := ctx.Repo.CanCommitToBranch(ctx, ctx.Doer)
if err != nil {
Expand All @@ -64,13 +72,15 @@ func redirectForCommitChoice(ctx *context.Context, commitChoice, newBranchName,
repo := ctx.Repo.Repository
baseBranch := ctx.Repo.BranchName
headBranch := newBranchName
if repo.UnitEnabled(ctx, unit.TypePullRequests) {
redirectToPullRequest = true
} else if canCreateBasePullRequest(ctx) {
// Prefer to create a pull request to the base repository if possible, matching
// the behavior when creating a pull request from a branch elsewhere in the UI.
if canCreateBasePullRequest(ctx) {
redirectToPullRequest = true
baseBranch = repo.BaseRepo.DefaultBranch
headBranch = repo.Owner.Name + "/" + repo.Name + ":" + headBranch
repo = repo.BaseRepo
} else if repo.UnitEnabled(ctx, unit.TypePullRequests) {
redirectToPullRequest = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand the change, and it doesn't match the comment "// Redirect to a pull request when possible" above now.

Please add more comments for the details and it's better to have some tests to clarify the behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some comments now.

Note that whether or not it redirects to the pull request is not changed by this. Just which repository and branch is the default choice in the pull request.

For testing I'll have a look later.

}

if redirectToPullRequest {
Expand Down Expand Up @@ -189,7 +199,7 @@ func editFile(ctx *context.Context, isNewFile bool) {
ctx.Data["TreePaths"] = treePaths
ctx.Data["commit_summary"] = ""
ctx.Data["commit_message"] = ""
ctx.Data["commit_choice"] = util.Iif(canCommit, frmCommitChoiceDirect, frmCommitChoiceNewBranch)
ctx.Data["commit_choice"] = util.Iif(canCommit && !defaultCreateNewBranch(ctx), frmCommitChoiceDirect, frmCommitChoiceNewBranch)
ctx.Data["new_branch_name"] = GetUniquePatchBranchName(ctx)
ctx.Data["last_commit"] = ctx.Repo.CommitID

Expand Down Expand Up @@ -446,11 +456,7 @@ func DeleteFile(ctx *context.Context) {
ctx.Data["commit_summary"] = ""
ctx.Data["commit_message"] = ""
ctx.Data["last_commit"] = ctx.Repo.CommitID
if canCommit {
ctx.Data["commit_choice"] = frmCommitChoiceDirect
} else {
ctx.Data["commit_choice"] = frmCommitChoiceNewBranch
}
ctx.Data["commit_choice"] = util.Iif(canCommit && !defaultCreateNewBranch(ctx), frmCommitChoiceDirect, frmCommitChoiceNewBranch)
ctx.Data["new_branch_name"] = GetUniquePatchBranchName(ctx)

ctx.HTML(http.StatusOK, tplDeleteFile)
Expand Down Expand Up @@ -620,11 +626,7 @@ func UploadFile(ctx *context.Context) {
ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.RefTypeNameSubURL()
ctx.Data["commit_summary"] = ""
ctx.Data["commit_message"] = ""
if canCommit {
ctx.Data["commit_choice"] = frmCommitChoiceDirect
} else {
ctx.Data["commit_choice"] = frmCommitChoiceNewBranch
}
ctx.Data["commit_choice"] = util.Iif(canCommit && !defaultCreateNewBranch(ctx), frmCommitChoiceDirect, frmCommitChoiceNewBranch)
ctx.Data["new_branch_name"] = GetUniquePatchBranchName(ctx)

ctx.HTML(http.StatusOK, tplUploadFile)
Expand Down