Skip to content
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

Jump to bottom

Add prompt to delete local branch when attempting to merge a PR that … #2789

Merged
merged 4 commits into from Jan 18, 2021
Merged

Add prompt to delete local branch when attempting to merge a PR that … #2789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
85e0e44
Add prompt to delete local branch when attempting to merge a PR that …
dpromanko Jan 15, 2021
2c35eb0
address pr comments
dpromanko Jan 15, 2021
df31fae
remove prompt for deleting branches on pr merge in interactive mode w…
dpromanko Jan 15, 2021
66546e2
When `pr merge --delete-branch` flag is supplied, avoid prompting for it
mislav Jan 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
165 changes: 86 additions & 79 deletions pkg/cmd/pr/merge/merge.go
View file
Open in desktop
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -123,107 +126,110 @@ func mergeRun(opts *MergeOptions) error {
if pr.Mergeable == "CONFLICTING" {
err := fmt.Errorf("%s Pull request #%d (%s) has conflicts and isn't mergeable ", cs.Red("!"), pr.Number, pr.Title)
return err
} else if pr.State == "MERGED" {
err := fmt.Errorf("%s Pull request #%d (%s) was already merged", cs.Red("!"), pr.Number, pr.Title)
return err
}

mergeMethod := opts.MergeMethod
deleteBranch := opts.DeleteBranch
crossRepoPR := pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner()
isTerminal := opts.IO.IsStdoutTTY()

if opts.InteractiveMode {
mergeMethod, deleteBranch, err = prInteractiveMerge(opts.DeleteLocalBranch, crossRepoPR)
if err != nil {
if errors.Is(err, cancelError) {
fmt.Fprintln(opts.IO.ErrOut, "Cancelled.")
return cmdutil.SilentError
isPRAlreadyMerged := pr.State == "MERGED"
if !isPRAlreadyMerged {
mergeMethod := opts.MergeMethod

if opts.InteractiveMode {
mergeMethod, deleteBranch, err = prInteractiveMerge(opts, crossRepoPR)
if err != nil {
if errors.Is(err, cancelError) {
fmt.Fprintln(opts.IO.ErrOut, "Cancelled.")
return cmdutil.SilentError
}
return err
}
}

err = api.PullRequestMerge(apiClient, baseRepo, pr, mergeMethod)
if err != nil {
return err
}
}

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)
return 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.IsDeleteBranchIndicated && opts.InteractiveMode {
err := prompt.SurveyAskOne(&survey.Confirm{
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 err != nil {
return fmt.Errorf("API call failed: %w", err)
if !deleteBranch {
return nil
}

isTerminal := opts.IO.IsStdoutTTY()

if isTerminal {
fmt.Fprintf(opts.IO.ErrOut, "%s %s pull request #%d (%s)\n", cs.Magenta("✔"), action, pr.Number, pr.Title)
}
branchSwitchString := ""

if deleteBranch {
branchSwitchString := ""
if opts.CanDeleteLocalBranch && !crossRepoPR {
currentBranch, err := opts.Branch()
if err != nil {
return err
}

if opts.DeleteLocalBranch && !crossRepoPR {
currentBranch, err := opts.Branch()
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))
err = git.CheckoutBranch(branchToSwitchTo)
if err != nil {
return err
}
}

if !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)
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 isTerminal {
fmt.Fprintf(opts.IO.ErrOut, "%s Deleted branch %s%s\n", cs.Red("✔"), cs.Cyan(pr.HeadRefName), branchSwitchString)
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
}

var cancelError = errors.New("cancelError")

func prInteractiveMerge(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{
Expand All @@ -235,9 +241,9 @@ func prInteractiveMerge(deleteLocalBranch bool, crossRepoPR bool) (api.PullReque

qs := []*survey.Question{mergeMethodQuestion}

if !crossRepoPR {
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?"
Expand Down Expand Up @@ -265,7 +271,9 @@ func prInteractiveMerge(deleteLocalBranch bool, crossRepoPR bool) (api.PullReque
MergeMethod int
DeleteBranch bool
IsConfirmed bool
}{}
}{
DeleteBranch: opts.DeleteBranch,
}

err := prompt.SurveyAsk(qs, &answers)
if err != nil {
Expand All @@ -285,6 +293,5 @@ func prInteractiveMerge(deleteLocalBranch bool, crossRepoPR bool) (api.PullReque
mergeMethod = api.PullRequestMergeMethodSquash
}

deleteBranch := answers.DeleteBranch
return mergeMethod, deleteBranch, nil
return mergeMethod, answers.DeleteBranch, nil
}