pr merge: avoid prompting to enter editor after editing phase is chosen

When user chooses "Edit commit message", open the editor immediately
instead of showing an additional prompt to open the editor.

pkg/cmd/pr/merge/merge.go

@@ -20,6 +20,10 @@ import (
 	"github.com/spf13/cobra"
 )
 
+type editor interface {
+	Edit(string, string) (string, error)
+}
+
 type MergeOptions struct {
 	HttpClient func() (*http.Client, error)
 	Config     func() (config.Config, error)
@@ -37,6 +41,7 @@ type MergeOptions struct {
 
 	Body    string
 	BodySet bool
+	Editor  editor
 
 	IsDeleteBranchIndicated bool
 	CanDeleteLocalBranch    bool
@@ -103,6 +108,11 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm
 			opts.CanDeleteLocalBranch = !cmd.Flags().Changed("repo")
 			opts.BodySet = cmd.Flags().Changed("body")
 
+			opts.Editor = &userEditor{
+				io:     opts.IO,
+				config: opts.Config,
+			}
+
 			if runF != nil {
 				return runF(opts)
 			}
@@ -198,12 +208,6 @@ func mergeRun(opts *MergeOptions) error {
 			}
 
 			if action == shared.EditCommitMessageAction {
-				var editorCommand string
-				editorCommand, err = cmdutil.DetermineEditor(opts.Config)
-				if err != nil {
-					return err
-				}
-
 				if !payload.setCommitBody {
 					payload.commitBody, err = getMergeText(httpClient, baseRepo, pr.ID, payload.method)
 					if err != nil {
@@ -211,7 +215,7 @@ func mergeRun(opts *MergeOptions) error {
 					}
 				}
 
-				payload.commitBody, err = commitMsgSurvey(payload.commitBody, editorCommand)
+				payload.commitBody, err = opts.Editor.Edit("*.md", payload.commitBody)
 				if err != nil {
 					return err
 				}
@@ -410,17 +414,16 @@ func confirmSurvey(allowEditMsg bool) (shared.Action, error) {
 	}
 }
 
-func commitMsgSurvey(msg string, editorCommand string) (string, error) {
-	var result string
-	q := &surveyext.GhEditor{
-		EditorCommand: editorCommand,
-		Editor: &survey.Editor{
-			Message:       "Body",
-			AppendDefault: true,
-			Default:       msg,
-			FileName:      "*.md",
-		},
-	}
-	err := prompt.SurveyAskOne(q, &result)
-	return result, err
+type userEditor struct {
+	io     *iostreams.IOStreams
+	config func() (config.Config, error)
+}
+
+func (e *userEditor) Edit(filename, startingText string) (string, error) {
+	editorCommand, err := cmdutil.DetermineEditor(e.config)
+	if err != nil {
+		return "", err
+	}
+
+	return surveyext.Edit(editorCommand, filename, startingText, e.io.In, e.io.Out, e.io.ErrOut, nil)
 }

pkg/cmd/pr/merge/merge_test.go

@@ -689,19 +689,23 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) {
 }
 
 func TestPRMerge_interactiveSquashEditCommitMsg(t *testing.T) {
-	http := initFakeHTTP()
-	defer http.Verify(t)
-	http.Register(
-		httpmock.GraphQL(`query PullRequestForBranch\b`),
+	io, _, stdout, stderr := iostreams.Test()
+	io.SetStdoutTTY(true)
+	io.SetStderrTTY(true)
+
+	tr := initFakeHTTP()
+	defer tr.Verify(t)
+
+	tr.Register(
+		httpmock.GraphQL(`query PullRequestByNumber\b`),
 		httpmock.StringResponse(`
-		{ "data": { "repository": { "pullRequests": { "nodes": [{
-			"headRefName": "blueberries",
+		{ "data": { "repository": { "pullRequest": {
 			"headRepositoryOwner": {"login": "OWNER"},
 			"id": "THE-ID",
 			"number": 3,
 			"title": "title"
-		}] } } } }`))
-	http.Register(
+		} } } }`))
+	tr.Register(
 		httpmock.GraphQL(`query RepositoryInfo\b`),
 		httpmock.StringResponse(`
 		{ "data": { "repository": {
@@ -709,41 +713,44 @@ func TestPRMerge_interactiveSquashEditCommitMsg(t *testing.T) {
 			"rebaseMergeAllowed": true,
 			"squashMergeAllowed": true
 		} } }`))
-	http.Register(
+	tr.Register(
 		httpmock.GraphQL(`query PullRequestMergeText\b`),
 		httpmock.StringResponse(`
 		{ "data": { "node": {
-			"viewerMergeBodyText": ""
+			"viewerMergeBodyText": "default body text"
 		} } }`))
-	http.Register(
+	tr.Register(
 		httpmock.GraphQL(`mutation PullRequestMerge\b`),
 		httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) {
 			assert.Equal(t, "THE-ID", input["pullRequestId"].(string))
 			assert.Equal(t, "SQUASH", input["mergeMethod"].(string))
-			assert.Equal(t, "cool story", input["commitBody"].(string))
+			assert.Equal(t, "DEFAULT BODY TEXT", input["commitBody"].(string))
 		}))
 
-	cs, cmdTeardown := run.Stub()
+	_, cmdTeardown := run.Stub()
 	defer cmdTeardown(t)
 
-	cs.Register("git -c log.ShowSignature=false log --pretty=format:%H,%s -1", 0, "")
-	cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "")
-
 	as, surveyTeardown := prompt.InitAskStubber()
 	defer surveyTeardown()
 
 	as.StubOne(2)                     // Merge method survey
 	as.StubOne(false)                 // Delete branch survey
 	as.StubOne("Edit commit message") // Confirm submit survey
-	as.StubOne("cool story")          // Edit commit message survey
 	as.StubOne("Submit")              // Confirm submit survey
 
-	output, err := runCommand(http, "blueberries", true, "")
-	if err != nil {
-		t.Fatalf("Got unexpected error running `pr merge` %s", err)
-	}
+	err := mergeRun(&MergeOptions{
+		IO:     io,
+		Editor: testEditor{},
+		HttpClient: func() (*http.Client, error) {
+			return &http.Client{Transport: tr}, nil
+		},
+		SelectorArg:     "https://github.com/OWNER/REPO/pull/123",
+		InteractiveMode: true,
+	})
+	assert.NoError(t, err)
 
-	assert.Equal(t, "✓ Squashed and merged pull request #3 (title)\n", output.Stderr())
+	assert.Equal(t, "", stdout.String())
+	assert.Equal(t, "✓ Squashed and merged pull request #3 (title)\n", stderr.String())
 }
 
 func TestPRMerge_interactiveCancelled(t *testing.T) {
@@ -885,3 +892,9 @@ func TestMergeRun_disableAutoMerge(t *testing.T) {
 	assert.Equal(t, "", stdout.String())
 	assert.Equal(t, "✓ Auto-merge disabled for pull request #123\n", stderr.String())
 }
+
+type testEditor struct{}
+
+func (e testEditor) Edit(filename, text string) (string, error) {
+	return strings.ToUpper(text), nil
+}