When pr merge --delete-branch flag is supplied, avoid prompting for it
This commit is contained in:
Mislav Marohnić
parent
df31fae9c6
commit
66546e2245
2 changed files with 102 additions and 107 deletions
|
|
@ -27,11 +27,13 @@ type MergeOptions struct {
|
|||
Remotes func() (context.Remotes, error)
|
||||
Branch func() (string, error)
|
||||
|
||||
SelectorArg string
|
||||
DeleteBranch bool
|
||||
DeleteLocalBranch bool
|
||||
MergeMethod api.PullRequestMergeMethod
|
||||
InteractiveMode bool
|
||||
SelectorArg string
|
||||
DeleteBranch bool
|
||||
MergeMethod api.PullRequestMergeMethod
|
||||
|
||||
IsDeleteBranchIndicated bool
|
||||
CanDeleteLocalBranch bool
|
||||
InteractiveMode bool
|
||||
}
|
||||
|
||||
func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Command {
|
||||
|
|
@ -90,7 +92,8 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm
|
|||
return &cmdutil.FlagError{Err: errors.New("only one of --merge, --rebase, or --squash can be enabled")}
|
||||
}
|
||||
|
||||
opts.DeleteLocalBranch = !cmd.Flags().Changed("repo")
|
||||
opts.IsDeleteBranchIndicated = cmd.Flags().Changed("delete-branch")
|
||||
opts.CanDeleteLocalBranch = !cmd.Flags().Changed("repo")
|
||||
|
||||
if runF != nil {
|
||||
return runF(opts)
|
||||
|
|
@ -125,22 +128,16 @@ func mergeRun(opts *MergeOptions) error {
|
|||
return err
|
||||
}
|
||||
|
||||
isPRAlreadyMerged := pr.State == "MERGED"
|
||||
|
||||
if isPRAlreadyMerged && !opts.InteractiveMode && !opts.DeleteBranch {
|
||||
err := fmt.Errorf("%s Pull request #%d (%s) was already merged", cs.Red("!"), pr.Number, pr.Title)
|
||||
return err
|
||||
}
|
||||
|
||||
deleteBranch := opts.DeleteBranch
|
||||
crossRepoPR := pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner()
|
||||
isTerminal := opts.IO.IsStdoutTTY()
|
||||
|
||||
isPRAlreadyMerged := pr.State == "MERGED"
|
||||
if !isPRAlreadyMerged {
|
||||
mergeMethod := opts.MergeMethod
|
||||
|
||||
if opts.InteractiveMode {
|
||||
mergeMethod, deleteBranch, err = prInteractiveMerge(opts.DeleteBranch, opts.DeleteLocalBranch, crossRepoPR)
|
||||
mergeMethod, deleteBranch, err = prInteractiveMerge(opts, crossRepoPR)
|
||||
if err != nil {
|
||||
if errors.Is(err, cancelError) {
|
||||
fmt.Fprintln(opts.IO.ErrOut, "Cancelled.")
|
||||
|
|
@ -150,87 +147,81 @@ func mergeRun(opts *MergeOptions) error {
|
|||
}
|
||||
}
|
||||
|
||||
var action string
|
||||
if mergeMethod == api.PullRequestMergeMethodRebase {
|
||||
action = "Rebased and merged"
|
||||
err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodRebase)
|
||||
} else if mergeMethod == api.PullRequestMergeMethodSquash {
|
||||
action = "Squashed and merged"
|
||||
err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodSquash)
|
||||
} else if mergeMethod == api.PullRequestMergeMethodMerge {
|
||||
action = "Merged"
|
||||
err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodMerge)
|
||||
} else {
|
||||
err = fmt.Errorf("unknown merge method (%d) used", mergeMethod)
|
||||
err = api.PullRequestMerge(apiClient, baseRepo, pr, mergeMethod)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
return fmt.Errorf("API call failed: %w", err)
|
||||
}
|
||||
|
||||
if isTerminal {
|
||||
action := "Merged"
|
||||
switch mergeMethod {
|
||||
case api.PullRequestMergeMethodRebase:
|
||||
action = "Rebased and merged"
|
||||
case api.PullRequestMergeMethodSquash:
|
||||
action = "Squashed and merged"
|
||||
}
|
||||
fmt.Fprintf(opts.IO.ErrOut, "%s %s pull request #%d (%s)\n", cs.Magenta("✔"), action, pr.Number, pr.Title)
|
||||
}
|
||||
} else if !opts.DeleteBranch {
|
||||
} else if !opts.IsDeleteBranchIndicated && opts.InteractiveMode {
|
||||
err := prompt.SurveyAskOne(&survey.Confirm{
|
||||
Message: fmt.Sprintf("Pull request #%d (%s) was already merged. Delete the branch locally and switch to default branch?", pr.Number, pr.Title),
|
||||
Message: fmt.Sprintf("Pull request #%d was already merged. Delete the branch locally?", pr.Number),
|
||||
Default: false,
|
||||
}, &deleteBranch)
|
||||
|
||||
if err != nil {
|
||||
return fmt.Errorf("could not prompt: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
if deleteBranch {
|
||||
branchSwitchString := ""
|
||||
if !deleteBranch {
|
||||
return nil
|
||||
}
|
||||
|
||||
if opts.DeleteLocalBranch && !crossRepoPR {
|
||||
currentBranch, err := opts.Branch()
|
||||
branchSwitchString := ""
|
||||
|
||||
if opts.CanDeleteLocalBranch && !crossRepoPR {
|
||||
currentBranch, err := opts.Branch()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
var branchToSwitchTo string
|
||||
if currentBranch == pr.HeadRefName {
|
||||
branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
var branchToSwitchTo string
|
||||
if currentBranch == pr.HeadRefName {
|
||||
branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
err = git.CheckoutBranch(branchToSwitchTo)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
localBranchExists := git.HasLocalBranch(pr.HeadRefName)
|
||||
if localBranchExists {
|
||||
err = git.DeleteLocalBranch(pr.HeadRefName)
|
||||
if err != nil {
|
||||
err = fmt.Errorf("failed to delete local branch %s: %w", cs.Cyan(pr.HeadRefName), err)
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
if branchToSwitchTo != "" {
|
||||
branchSwitchString = fmt.Sprintf(" and switched to branch %s", cs.Cyan(branchToSwitchTo))
|
||||
}
|
||||
}
|
||||
|
||||
if !isPRAlreadyMerged && !crossRepoPR {
|
||||
err = api.BranchDeleteRemote(apiClient, baseRepo, pr.HeadRefName)
|
||||
var httpErr api.HTTPError
|
||||
// The ref might have already been deleted by GitHub
|
||||
if err != nil && (!errors.As(err, &httpErr) || httpErr.StatusCode != 422) {
|
||||
err = fmt.Errorf("failed to delete remote branch %s: %w", cs.Cyan(pr.HeadRefName), err)
|
||||
err = git.CheckoutBranch(branchToSwitchTo)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
if isTerminal {
|
||||
fmt.Fprintf(opts.IO.ErrOut, "%s Deleted branch %s%s\n", cs.Red("✔"), cs.Cyan(pr.HeadRefName), branchSwitchString)
|
||||
localBranchExists := git.HasLocalBranch(pr.HeadRefName)
|
||||
if localBranchExists {
|
||||
err = git.DeleteLocalBranch(pr.HeadRefName)
|
||||
if err != nil {
|
||||
err = fmt.Errorf("failed to delete local branch %s: %w", cs.Cyan(pr.HeadRefName), err)
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
if branchToSwitchTo != "" {
|
||||
branchSwitchString = fmt.Sprintf(" and switched to branch %s", cs.Cyan(branchToSwitchTo))
|
||||
}
|
||||
}
|
||||
|
||||
if !isPRAlreadyMerged && !crossRepoPR {
|
||||
err = api.BranchDeleteRemote(apiClient, baseRepo, pr.HeadRefName)
|
||||
var httpErr api.HTTPError
|
||||
// The ref might have already been deleted by GitHub
|
||||
if err != nil && (!errors.As(err, &httpErr) || httpErr.StatusCode != 422) {
|
||||
err = fmt.Errorf("failed to delete remote branch %s: %w", cs.Cyan(pr.HeadRefName), err)
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
if isTerminal {
|
||||
fmt.Fprintf(opts.IO.ErrOut, "%s Deleted branch %s%s\n", cs.Red("✔"), cs.Cyan(pr.HeadRefName), branchSwitchString)
|
||||
}
|
||||
|
||||
return nil
|
||||
|
|
@ -238,7 +229,7 @@ func mergeRun(opts *MergeOptions) error {
|
|||
|
||||
var cancelError = errors.New("cancelError")
|
||||
|
||||
func prInteractiveMerge(deleteBranch bool, deleteLocalBranch bool, crossRepoPR bool) (api.PullRequestMergeMethod, bool, error) {
|
||||
func prInteractiveMerge(opts *MergeOptions, crossRepoPR bool) (api.PullRequestMergeMethod, bool, error) {
|
||||
mergeMethodQuestion := &survey.Question{
|
||||
Name: "mergeMethod",
|
||||
Prompt: &survey.Select{
|
||||
|
|
@ -250,9 +241,9 @@ func prInteractiveMerge(deleteBranch bool, deleteLocalBranch bool, crossRepoPR b
|
|||
|
||||
qs := []*survey.Question{mergeMethodQuestion}
|
||||
|
||||
if !crossRepoPR && !deleteBranch {
|
||||
if !crossRepoPR && !opts.IsDeleteBranchIndicated {
|
||||
var message string
|
||||
if deleteLocalBranch {
|
||||
if opts.CanDeleteLocalBranch {
|
||||
message = "Delete the branch locally and on GitHub?"
|
||||
} else {
|
||||
message = "Delete the branch on GitHub?"
|
||||
|
|
@ -280,7 +271,9 @@ func prInteractiveMerge(deleteBranch bool, deleteLocalBranch bool, crossRepoPR b
|
|||
MergeMethod int
|
||||
DeleteBranch bool
|
||||
IsConfirmed bool
|
||||
}{}
|
||||
}{
|
||||
DeleteBranch: opts.DeleteBranch,
|
||||
}
|
||||
|
||||
err := prompt.SurveyAsk(qs, &answers)
|
||||
if err != nil {
|
||||
|
|
@ -300,9 +293,5 @@ func prInteractiveMerge(deleteBranch bool, deleteLocalBranch bool, crossRepoPR b
|
|||
mergeMethod = api.PullRequestMergeMethodSquash
|
||||
}
|
||||
|
||||
if !deleteBranch {
|
||||
deleteBranch = answers.DeleteBranch
|
||||
}
|
||||
|
||||
return mergeMethod, deleteBranch, nil
|
||||
return mergeMethod, answers.DeleteBranch, nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import (
|
|||
"github.com/cli/cli/git"
|
||||
"github.com/cli/cli/internal/config"
|
||||
"github.com/cli/cli/internal/ghrepo"
|
||||
"github.com/cli/cli/internal/run"
|
||||
"github.com/cli/cli/pkg/cmdutil"
|
||||
"github.com/cli/cli/pkg/httpmock"
|
||||
"github.com/cli/cli/pkg/iostreams"
|
||||
|
|
@ -37,11 +38,25 @@ func Test_NewCmdMerge(t *testing.T) {
|
|||
args: "123",
|
||||
isTTY: true,
|
||||
want: MergeOptions{
|
||||
SelectorArg: "123",
|
||||
DeleteBranch: false,
|
||||
DeleteLocalBranch: true,
|
||||
MergeMethod: api.PullRequestMergeMethodMerge,
|
||||
InteractiveMode: true,
|
||||
SelectorArg: "123",
|
||||
DeleteBranch: false,
|
||||
IsDeleteBranchIndicated: false,
|
||||
CanDeleteLocalBranch: true,
|
||||
MergeMethod: api.PullRequestMergeMethodMerge,
|
||||
InteractiveMode: true,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "delete-branch specified",
|
||||
args: "--delete-branch=false",
|
||||
isTTY: true,
|
||||
want: MergeOptions{
|
||||
SelectorArg: "",
|
||||
DeleteBranch: false,
|
||||
IsDeleteBranchIndicated: true,
|
||||
CanDeleteLocalBranch: true,
|
||||
MergeMethod: api.PullRequestMergeMethodMerge,
|
||||
InteractiveMode: true,
|
||||
},
|
||||
},
|
||||
{
|
||||
|
|
@ -105,7 +120,7 @@ func Test_NewCmdMerge(t *testing.T) {
|
|||
|
||||
assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg)
|
||||
assert.Equal(t, tt.want.DeleteBranch, opts.DeleteBranch)
|
||||
assert.Equal(t, tt.want.DeleteLocalBranch, opts.DeleteLocalBranch)
|
||||
assert.Equal(t, tt.want.CanDeleteLocalBranch, opts.CanDeleteLocalBranch)
|
||||
assert.Equal(t, tt.want.MergeMethod, opts.MergeMethod)
|
||||
assert.Equal(t, tt.want.InteractiveMode, opts.InteractiveMode)
|
||||
})
|
||||
|
|
@ -498,13 +513,12 @@ func TestPrMerge_alreadyMerged(t *testing.T) {
|
|||
}
|
||||
} } }`))
|
||||
|
||||
cs, cmdTeardown := test.InitCmdStubber()
|
||||
defer cmdTeardown()
|
||||
cs, cmdTeardown := run.Stub()
|
||||
defer cmdTeardown(t)
|
||||
|
||||
cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$
|
||||
cs.Stub("") // git symbolic-ref --quiet --short HEAD
|
||||
cs.Stub("") // git checkout master
|
||||
cs.Stub("") // git branch -d
|
||||
cs.Register(`git checkout master`, 0, "")
|
||||
cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "")
|
||||
cs.Register(`git branch -D blueberries`, 0, "")
|
||||
|
||||
as, surveyTeardown := prompt.InitAskStubber()
|
||||
defer surveyTeardown()
|
||||
|
|
@ -528,24 +542,16 @@ func TestPrMerge_alreadyMerged_nonInteractive(t *testing.T) {
|
|||
"pullRequest": { "number": 4, "title": "The title of the PR", "state": "MERGED"}
|
||||
} } }`))
|
||||
|
||||
cs, cmdTeardown := test.InitCmdStubber()
|
||||
defer cmdTeardown()
|
||||
|
||||
cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$
|
||||
cs.Stub("") // git symbolic-ref --quiet --short HEAD
|
||||
cs.Stub("") // git checkout master
|
||||
cs.Stub("") // git branch -d
|
||||
_, cmdTeardown := run.Stub()
|
||||
defer cmdTeardown(t)
|
||||
|
||||
output, err := runCommand(http, "blueberries", true, "pr merge 4 --merge")
|
||||
if err == nil {
|
||||
t.Fatalf("expected an error running command `pr merge`: %v", err)
|
||||
if err != nil {
|
||||
t.Fatalf("Got unexpected error running `pr merge` %s", err)
|
||||
}
|
||||
|
||||
r := regexp.MustCompile(`Pull request #4 \(The title of the PR\) was already merged`)
|
||||
|
||||
if !r.MatchString(err.Error()) {
|
||||
t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr())
|
||||
}
|
||||
assert.Equal(t, "", output.String())
|
||||
assert.Equal(t, "", output.Stderr())
|
||||
}
|
||||
|
||||
func TestPRMerge_interactive(t *testing.T) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue