diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index f2923573c..67498b726 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/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) } diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index 90e8d69c2..dd4ff80cc 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/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 +}