Repository: cli/cli
Pull request #899 (2020-05-11 19:13:58)

Interactive merge
This PR adds two features:

1. Calling `gh merge` with no merge method flags (--merge | --rebase | --squash) enters you into interactive mode.
2. Adds the ability to delete the PR's branch after a merge using a flag (--delete) or as a choice in interactive mode.

Here is how it looks:
<img width="599" alt="CleanShot 2020-05-12 at 09 49 05@2x" src="https://user-images.githubusercontent.com/596/81722184-dc64ea00-9435-11ea-9214-82a8d8c36b9e.png">
![CleanShot 2020-05-12 at 09 51 39](https://user-images.githubusercontent.com/596/81722558-644af400-9436-11ea-9000-0e925c8a35e0.gif)


Closes https://github.com/cli/cli/issues/373


Patch:
diff --git a/api/queries_pr.go b/api/queries_pr.go
index 6ee26176c..64925fae5 100644
--- a/api/queries_pr.go
+++ b/api/queries_pr.go
@@ -48,6 +48,7 @@ type PullRequest struct {
 	BaseRefName string
 	HeadRefName string
 	Body        string
+	Mergeable   string
 
 	Author struct {
 		Login string
@@ -264,6 +265,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
 		state
 		url
 		headRefName
+		mergeable
 		headRepositoryOwner {
 			login
 		}
@@ -418,6 +420,7 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu
 				state
 				closed
 				body
+				mergeable
 				author {
 				  login
 				}
@@ -526,6 +529,7 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea
 					title
 					state
 					body
+					mergeable
 					author {
 						login
 					}
diff --git a/api/queries_repo.go b/api/queries_repo.go
index c8ba31348..22d42cfbb 100644
--- a/api/queries_repo.go
+++ b/api/queries_repo.go
@@ -84,6 +84,9 @@ func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
 			hasIssuesEnabled
 			description
 			viewerPermission
+			defaultBranchRef {
+				name
+			}
 		}
 	}`
 	variables := map[string]interface{}{
diff --git a/command/pr.go b/command/pr.go
index 83ddf7d13..c740b3968 100644
--- a/command/pr.go
+++ b/command/pr.go
@@ -1,6 +1,7 @@
 package command
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"regexp"
@@ -8,6 +9,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/AlecAivazis/survey/v2"
 	"github.com/cli/cli/api"
 	"github.com/cli/cli/context"
 	"github.com/cli/cli/git"
@@ -26,7 +28,8 @@ func init() {
 	prCmd.AddCommand(prCloseCmd)
 	prCmd.AddCommand(prReopenCmd)
 	prCmd.AddCommand(prMergeCmd)
-	prMergeCmd.Flags().BoolP("merge", "m", true, "Merge the commits with the base branch")
+	prMergeCmd.Flags().BoolP("delete-branch", "d", true, "Delete the local branch after merge")
+	prMergeCmd.Flags().BoolP("merge", "m", false, "Merge the commits with the base branch")
 	prMergeCmd.Flags().BoolP("rebase", "r", false, "Rebase the commits onto the base branch")
 	prMergeCmd.Flags().BoolP("squash", "s", false, "Squash the commits into one commit and merge it into the base branch")
 	prCmd.AddCommand(prReadyCmd)
@@ -451,7 +454,15 @@ func prMerge(cmd *cobra.Command, args []string) error {
 
 	var pr *api.PullRequest
 	if len(args) > 0 {
-		pr, err = prFromArg(apiClient, baseRepo, args[0])
+		var prNumber string
+		n, _ := prFromURL(args[0])
+		if n != "" {
+			prNumber = n
+		} else {
+			prNumber = args[0]
+		}
+
+		pr, err = prFromArg(apiClient, baseRepo, prNumber)
 		if err != nil {
 			return err
 		}
@@ -471,41 +482,146 @@ func prMerge(cmd *cobra.Command, args []string) error {
 		}
 	}
 
-	if pr.State == "MERGED" {
+	if pr.Mergeable == "CONFLICTING" {
+		err := fmt.Errorf("%s Pull request #%d has conflicts and isn't mergeable ", utils.Red("!"), pr.Number)
+		return err
+	} else if pr.Mergeable == "UNKNOWN" {
+		err := fmt.Errorf("%s Pull request #%d can't be merged right now; try again in a few seconds", utils.Red("!"), pr.Number)
+		return err
+	} else if pr.State == "MERGED" {
 		err := fmt.Errorf("%s Pull request #%d was already merged", utils.Red("!"), pr.Number)
 		return err
 	}
 
-	rebase, err := cmd.Flags().GetBool("rebase")
+	var mergeMethod api.PullRequestMergeMethod
+	deleteBranch, err := cmd.Flags().GetBool("delete-branch")
 	if err != nil {
 		return err
 	}
-	squash, err := cmd.Flags().GetBool("squash")
-	if err != nil {
-		return err
+
+	// Ensure only one merge method is specified
+	enabledFlagCount := 0
+	isInteractive := false
+	if b, _ := cmd.Flags().GetBool("merge"); b {
+		enabledFlagCount++
+		mergeMethod = api.PullRequestMergeMethodMerge
+	}
+	if b, _ := cmd.Flags().GetBool("rebase"); b {
+		enabledFlagCount++
+		mergeMethod = api.PullRequestMergeMethodRebase
+	}
+	if b, _ := cmd.Flags().GetBool("squash"); b {
+		enabledFlagCount++
+		mergeMethod = api.PullRequestMergeMethodSquash
+	}
+
+	if enabledFlagCount == 0 {
+		isInteractive = true
+	} else if enabledFlagCount > 1 {
+		return errors.New("expected exactly one of --merge, --rebase, or --squash to be true")
+	}
+
+	if isInteractive {
+		mergeMethod, deleteBranch, err = prInteractiveMerge()
+		if err != nil {
+			return nil
+		}
 	}
 
-	var output string
-	if rebase {
-		output = fmt.Sprintf("%s Rebased and merged pull request #%d\n", utils.Green("✔"), pr.Number)
+	var action string
+	if mergeMethod == api.PullRequestMergeMethodRebase {
+		action = "Rebased and merged"
 		err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodRebase)
-	} else if squash {
-		output = fmt.Sprintf("%s Squashed and merged pull request #%d\n", utils.Green("✔"), pr.Number)
+	} else if mergeMethod == api.PullRequestMergeMethodSquash {
+		action = "Squashed and merged"
 		err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodSquash)
-	} else {
-		output = fmt.Sprintf("%s Merged pull request #%d\n", utils.Green("✔"), pr.Number)
+	} 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 err != nil {
 		return fmt.Errorf("API call failed: %w", err)
 	}
 
-	fmt.Fprint(colorableOut(cmd), output)
+	fmt.Fprintf(colorableOut(cmd), "%s %s pull request #%d\n", utils.Magenta("✔"), action, pr.Number)
+
+	if deleteBranch && !cmd.Flags().Changed("repo") {
+		repo, err := api.GitHubRepo(apiClient, baseRepo)
+		if err != nil {
+			return err
+		}
+
+		currentBranch, err := ctx.Branch()
+		if err != nil {
+			return err
+		}
+
+		if currentBranch == pr.HeadRefName {
+			err = git.CheckoutBranch(repo.DefaultBranchRef.Name)
+			if err != nil {
+				return err
+			}
+		}
+
+		err = git.DeleteLocalBranch(pr.HeadRefName)
+		if err != nil {
+			fmt.Fprintf(colorableErr(cmd), "%s Could not delete local branch %s: %s\n", utils.Red("!"), utils.Cyan(pr.HeadRefName), err)
+			return err
+		}
+		fmt.Fprintf(colorableOut(cmd), "%s Deleted local branch %s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName))
+	}
 
 	return nil
 }
 
+func prInteractiveMerge() (api.PullRequestMergeMethod, bool, error) {
+	mergeMethodQuestion := &survey.Question{
+		Name: "mergeMethod",
+		Prompt: &survey.Select{
+			Message: "What merge method would you like to use?",
+			Options: []string{"Create a merge commit", "Rebase and merge", "Squash and merge"},
+			Default: "Create a merge commit",
+		},
+	}
+
+	deleteBranchQuestion := &survey.Question{
+		Name: "deleteBranch",
+		Prompt: &survey.Confirm{
+			Message: "Delete the branch locally?",
+			Default: true,
+		},
+	}
+
+	qs := []*survey.Question{mergeMethodQuestion, deleteBranchQuestion}
+
+	answers := struct {
+		MergeMethod  int
+		DeleteBranch bool
+	}{}
+
+	err := SurveyAsk(qs, &answers)
+	if err != nil {
+		return 0, false, fmt.Errorf("could not prompt: %w", err)
+	}
+
+	var mergeMethod api.PullRequestMergeMethod
+	switch answers.MergeMethod {
+	case 0:
+		mergeMethod = api.PullRequestMergeMethodMerge
+	case 1:
+		mergeMethod = api.PullRequestMergeMethodRebase
+	case 2:
+		mergeMethod = api.PullRequestMergeMethodSquash
+	}
+
+	deleteBranch := answers.DeleteBranch
+	return mergeMethod, deleteBranch, nil
+}
+
 func printPrPreview(out io.Writer, pr *api.PullRequest) error {
 	// Header (Title and State)
 	fmt.Fprintln(out, utils.Bold(pr.Title))
diff --git a/git/git.go b/git/git.go
index 360917eab..634b9e084 100644
--- a/git/git.go
+++ b/git/git.go
@@ -203,6 +203,18 @@ func ReadBranchConfig(branch string) (cfg BranchConfig) {
 	return
 }
 
+func DeleteLocalBranch(branch string) error {
+	branchCmd := GitCommand("branch", "-D", branch)
+	err := run.PrepareCmd(branchCmd).Run()
+	return err
+}
+
+func CheckoutBranch(branch string) error {
+	configCmd := GitCommand("checkout", branch)
+	err := run.PrepareCmd(configCmd).Run()
+	return err
+}
+
 func isFilesystemPath(p string) bool {
 	return p == "." || strings.HasPrefix(p, "./") || strings.HasPrefix(p, "/")
 }


Test patch:
diff --git a/command/pr_test.go b/command/pr_test.go
index 24fce9d63..5f5cdf6b8 100644
--- a/command/pr_test.go
+++ b/command/pr_test.go
@@ -982,10 +982,10 @@ func initWithStubs(branch string, stubs ...stubResponse) {
 	initBlankContext("", "OWNER/REPO", branch)
 	http := initFakeHTTP()
 	http.StubRepoResponse("OWNER", "REPO")
-
 	for _, s := range stubs {
 		http.StubResponse(s.ResponseCode, s.ResponseBody)
 	}
+	http.StubRepoResponse("OWNER", "REPO")
 }
 
 func TestPrMerge(t *testing.T) {
@@ -996,7 +996,41 @@ func TestPrMerge(t *testing.T) {
 		stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)},
 	)
 
-	output, err := RunCommand("pr merge 1")
+	cs, cmdTeardown := test.InitCmdStubber()
+	defer cmdTeardown()
+
+	cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge)
+	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("")
+
+	output, err := RunCommand("pr merge 1 --merge")
+	if err != nil {
+		t.Fatalf("error running command `pr merge`: %v", err)
+	}
+
+	r := regexp.MustCompile(`Merged pull request #1`)
+
+	if !r.MatchString(output.String()) {
+		t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr())
+	}
+}
+
+func TestPrMerge_withRepoFlag(t *testing.T) {
+	initBlankContext("", "OWNER/REPO", "master")
+	http := initFakeHTTP()
+	http.StubResponse(200, bytes.NewBufferString(`{ "data": { "repository": {
+			"pullRequest": { "number": 1, "closed": false, "state": "OPEN"}
+		} } }`))
+	http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`))
+
+	cs, cmdTeardown := test.InitCmdStubber()
+	defer cmdTeardown()
+
+	eq(t, len(cs.Calls), 0)
+
+	output, err := RunCommand("pr merge 1 --merge -R stinky/boi")
 	if err != nil {
 		t.Fatalf("error running command `pr merge`: %v", err)
 	}
@@ -1008,11 +1042,63 @@ func TestPrMerge(t *testing.T) {
 	}
 }
 
+func TestPrMerge_deleteBranch(t *testing.T) {
+	initWithStubs("blueberries",
+		stubResponse{200, bytes.NewBufferString(`
+		{ "data": { "repository": { "pullRequests": { "nodes": [
+			{ "headRefName": "blueberries", "id": "THE-ID", "number": 3}
+		] } } } }`)},
+		stubResponse{200, bytes.NewBufferString(`{ "data": {} }`)})
+
+	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
+
+	output, err := RunCommand(`pr merge --merge --delete-branch`)
+	if err != nil {
+		t.Fatalf("Got unexpected error running `pr merge` %s", err)
+	}
+
+	test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted local branch")
+}
+
+func TestPrMerge_deleteNonCurrentBranch(t *testing.T) {
+	initBlankContext("", "OWNER/REPO", "another-branch")
+	http := initFakeHTTP()
+	http.StubRepoResponse("OWNER", "REPO")
+	http.StubResponse(200, bytes.NewBufferString(`
+	{ "data": { "repository": { "pullRequests": { "nodes": [
+		{ "headRefName": "blueberries", "id": "THE-ID", "number": 3}
+	] } } } }`))
+	http.StubResponse(200, bytes.NewBufferString(`{ "data": {} }`))
+	http.StubRepoResponse("OWNER", "REPO")
+
+	cs, cmdTeardown := test.InitCmdStubber()
+	defer cmdTeardown()
+	// We don't expect the default branch to be checked out, just that blueberries is deleted
+	cs.Stub("") // git branch -d blueberries
+
+	output, err := RunCommand(`pr merge --merge --delete-branch blueberries`)
+	if err != nil {
+		t.Fatalf("Got unexpected error running `pr merge` %s", err)
+	}
+
+	test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted local branch")
+}
+
 func TestPrMerge_noPrNumberGiven(t *testing.T) {
 	cs, cmdTeardown := test.InitCmdStubber()
 	defer cmdTeardown()
 
 	cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge)
+	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
 
 	jsonFile, _ := os.Open("../test/fixtures/prViewPreviewWithMetadataByBranch.json")
 	defer jsonFile.Close()
@@ -1022,7 +1108,7 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) {
 		stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)},
 	)
 
-	output, err := RunCommand("pr merge")
+	output, err := RunCommand("pr merge --merge")
 	if err != nil {
 		t.Fatalf("error running command `pr merge`: %v", err)
 	}
@@ -1042,6 +1128,14 @@ func TestPrMerge_rebase(t *testing.T) {
 		stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)},
 	)
 
+	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
+
 	output, err := RunCommand("pr merge 2 --rebase")
 	if err != nil {
 		t.Fatalf("error running command `pr merge`: %v", err)
@@ -1062,6 +1156,14 @@ func TestPrMerge_squash(t *testing.T) {
 		stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)},
 	)
 
+	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
+
 	output, err := RunCommand("pr merge 3 --squash")
 	if err != nil {
 		t.Fatalf("error running command `pr merge`: %v", err)
@@ -1082,6 +1184,14 @@ func TestPrMerge_alreadyMerged(t *testing.T) {
 		stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)},
 	)
 
+	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
+
 	output, err := RunCommand("pr merge 4")
 	if err == nil {
 		t.Fatalf("expected an error running command `pr merge`: %v", err)
@@ -1094,6 +1204,58 @@ func TestPrMerge_alreadyMerged(t *testing.T) {
 	}
 }
 
+func TestPRMerge_interactive(t *testing.T) {
+	initWithStubs("blueberries",
+		stubResponse{200, bytes.NewBufferString(`
+		{ "data": { "repository": { "pullRequests": { "nodes": [
+			{ "headRefName": "blueberries", "id": "THE-ID", "number": 3}
+		] } } } }`)},
+		stubResponse{200, bytes.NewBufferString(`{ "data": {} }`)})
+
+	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
+
+	as, surveyTeardown := initAskStubber()
+	defer surveyTeardown()
+
+	as.Stub([]*QuestionStub{
+		{
+			Name:  "mergeMethod",
+			Value: 0,
+		},
+		{
+			Name:  "deleteBranch",
+			Value: true,
+		},
+	})
+
+	output, err := RunCommand(`pr merge`)
+	if err != nil {
+		t.Fatalf("Got unexpected error running `pr merge` %s", err)
+	}
+
+	test.ExpectLines(t, output.String(), "Merged pull request #3", "Deleted local branch")
+}
+
+func TestPrMerge_multipleMergeMethods(t *testing.T) {
+	initWithStubs("master",
+		stubResponse{200, bytes.NewBufferString(`{ "data": { "repository": {
+			"pullRequest": { "number": 1, "closed": false, "state": "OPEN"}
+		} } }`)},
+		stubResponse{200, bytes.NewBufferString(`{"id": "THE-ID"}`)},
+	)
+
+	_, err := RunCommand("pr merge 1 --merge --squash")
+	if err == nil {
+		t.Fatal("expected error running `pr merge` with multiple merge methods")
+	}
+}
+
 func TestPRReady(t *testing.T) {
 	initBlankContext("", "OWNER/REPO", "master")
 	http := initFakeHTTP()
diff --git a/test/helpers.go b/test/helpers.go
index c8b0efe53..c2d4e2f37 100644
--- a/test/helpers.go
+++ b/test/helpers.go
@@ -68,6 +68,7 @@ func createStubbedPrepareCmd(cs *CmdStubber) func(*exec.Cmd) run.Runnable {
 		if call >= len(cs.Stubs) {
 			panic(fmt.Sprintf("more execs than stubs. most recent call: %v", cmd))
 		}
+		// fmt.Printf("Called stub for `%v`\n", cmd) // Helpful for debugging
 		return cs.Stubs[call]
 	}
 }