diff --git a/api/queries_pr.go b/api/queries_pr.go index 4c35a153a..1743031ee 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -88,6 +88,9 @@ type PullRequest struct { ReactionGroups ReactionGroups Reviews PullRequestReviews ReviewRequests ReviewRequests + + ViewerMergeBodyText string + ViewerMergeHeadlineText string } type ReviewRequests struct { @@ -584,6 +587,8 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu milestone{ title } + viewerMergeBodyText + viewerMergeHeadlineText ` + commentsFragment() + ` ` + reactionGroupsFragment() + ` } diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index 4c8c946c0..e5d3cb274 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -16,6 +16,7 @@ import ( "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/prompt" + "github.com/cli/cli/pkg/surveyext" "github.com/spf13/cobra" ) @@ -154,6 +155,8 @@ func mergeRun(opts *MergeOptions) error { if !isPRAlreadyMerged { mergeMethod := opts.MergeMethod + action := shared.SubmitAction + if opts.InteractiveMode { r, err := api.GitHubRepo(apiClient, baseRepo) if err != nil { @@ -167,19 +170,52 @@ func mergeRun(opts *MergeOptions) error { if err != nil { return err } - confirm, err := confirmSurvey() + + allowEditMsg := mergeMethod != api.PullRequestMergeMethodRebase + + action, err = confirmSubmission(allowEditMsg) if err != nil { - return err + return fmt.Errorf("unable to confirm: %w", err) } - if !confirm { + + if action == shared.EditCommitMessageAction { + var editorCommand string + editorCommand, err = cmdutil.DetermineEditor(opts.Config) + if err != nil { + return err + } + + if opts.Body == nil { + var body string + + if mergeMethod == api.PullRequestMergeMethodSquash { + body = fmt.Sprintf("%s\n\n%s", pr.ViewerMergeHeadlineText, pr.ViewerMergeBodyText) + } + + opts.Body = &body + } + + err := commitMsgSurvey(opts.Body, editorCommand) + if err != nil { + return err + } + + action, err = confirmSubmission(false) + if err != nil { + return fmt.Errorf("unable to confirm: %w", err) + } + } + + if action == shared.CancelAction { fmt.Fprintln(opts.IO.ErrOut, "Cancelled.") return cmdutil.SilentError } } - - err = api.PullRequestMerge(apiClient, baseRepo, pr, mergeMethod, opts.Body) - if err != nil { - return err + if action == shared.SubmitAction { + err = api.PullRequestMerge(apiClient, baseRepo, pr, mergeMethod, opts.Body) + if err != nil { + return err + } } if isTerminal { @@ -316,12 +352,69 @@ func deleteBranchSurvey(opts *MergeOptions, crossRepoPR bool) (bool, error) { return opts.DeleteBranch, nil } -func confirmSurvey() (bool, error) { - var confirm bool - submit := &survey.Confirm{ - Message: "Submit?", - Default: true, +func confirmSubmission(allowEditMsg bool) (shared.Action, error) { + const ( + submitLabel = "Submit" + editCommitMsgLabel = "Edit Commit Message" + cancelLabel = "Cancel" + ) + options := []string{submitLabel} + if allowEditMsg { + options = append(options, editCommitMsgLabel) + } + options = append(options, cancelLabel) + + confirmAnswers := struct { + Confirmation int + }{} + + confirmQs := []*survey.Question{ + { + Name: "confirmation", + Prompt: &survey.Select{ + Message: "What's next?", + Options: options, + }, + }, + } + + err := prompt.SurveyAsk(confirmQs, &confirmAnswers) + if err != nil { + return -1, fmt.Errorf("could not prompt: %w", err) + } + + switch options[confirmAnswers.Confirmation] { + case submitLabel: + return shared.SubmitAction, nil + case editCommitMsgLabel: + return shared.EditCommitMessageAction, nil + case cancelLabel: + return shared.CancelAction, nil + default: + return -1, fmt.Errorf("invalid index: %d", confirmAnswers.Confirmation) } - err := prompt.SurveyAskOne(submit, &confirm) - return confirm, err +} + +func commitMsgSurvey(body *string, editorCommand string) error { + qs := []*survey.Question{ + { + Name: "body", + Prompt: &surveyext.GhEditor{ + BlankAllowed: true, + EditorCommand: editorCommand, + Editor: &survey.Editor{ + Message: "Body", + AppendDefault: true, + Default: *body, + FileName: "*.md", + }, + }, + }, + } + err := prompt.SurveyAsk(qs, body) + if err != nil { + return err + } + + return nil } diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index ce772f1e6..ba31544f6 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -626,9 +626,14 @@ func TestPRMerge_interactive(t *testing.T) { as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown() - as.StubOne(0) // Merge method survey - as.StubOne(true) // Delete branch survey - as.StubOne(true) // Confirm submit survey + as.StubOne(0) // Merge method survey + as.StubOne(true) // Delete branch survey + as.Stub([]*prompt.QuestionStub{ // Confirm submit survey + { + Name: "confirmation", + Value: 0, + }, + }) output, err := runCommand(http, "blueberries", true, "") if err != nil { @@ -682,8 +687,14 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown() - as.StubOne(0) // Merge method survey - as.StubOne(true) // Confirm submit survey + as.StubOne(0) // Merge method survey + as.StubOne(true) // Confirm submit survey + as.Stub([]*prompt.QuestionStub{ // Confirm submit survey + { + Name: "confirmation", + Value: 0, + }, + }) output, err := runCommand(http, "blueberries", true, "-d") if err != nil { @@ -694,6 +705,138 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { test.ExpectLines(t, output.Stderr(), "Merged pull request #3", "Deleted branch blueberries and switched to branch master") } +func TestPRMerge_interactiveSquashEditCommitMsg(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes": [{ + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"}, + "id": "THE-ID", + "number": 3 + }] } } } }`)) + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "mergeCommitAllowed": true, + "rebaseMergeAllowed": true, + "squashMergeAllowed": true + } } }`)) + http.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)) + })) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + 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.Stub([]*prompt.QuestionStub{ // Confirm submit survey + { + Name: "confirmation", + Value: 1, + }, + }) + as.Stub([]*prompt.QuestionStub{ // Edit Commit Msg editor survey + { + Name: "body", + Value: "cool story", + }, + }) + as.Stub([]*prompt.QuestionStub{ // Confirm submit survey + { + Name: "confirmation", + Value: 0, + }, + }) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + assert.Equal(t, "✔ Squashed and merged pull request #3 ()\n", output.Stderr()) +} + +func TestPRMerge_interactiveSquashEditCommitMsgDefaultBody(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes": [{ + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"}, + "id": "THE-ID", + "number": 3, + "viewerMergeBodyText": "Co-authored by: Octocat ", + "viewerMergeHeadlineText": "Add new feat (#33)" + }] } } } }`)) + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "mergeCommitAllowed": true, + "rebaseMergeAllowed": true, + "squashMergeAllowed": true + } } }`)) + http.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, "Add new feat (#33)\n\nCo-authored by: Octocat ", input["commitBody"]) + })) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + 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.Stub([]*prompt.QuestionStub{ // Confirm submit survey + { + Name: "confirmation", + Value: 1, + }, + }) + as.Stub([]*prompt.QuestionStub{ // Edit Commit Msg editor survey + { + Name: "body", + Default: true, + }, + }) + as.Stub([]*prompt.QuestionStub{ // Confirm submit survey + { + Name: "confirmation", + Value: 0, + }, + }) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + assert.Equal(t, "✔ Squashed and merged pull request #3 ()\n", output.Stderr()) +} + func TestPRMerge_interactiveCancelled(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) @@ -724,9 +867,14 @@ func TestPRMerge_interactiveCancelled(t *testing.T) { as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown() - as.StubOne(0) // Merge method survey - as.StubOne(true) // Delete branch survey - as.StubOne(false) // Confirm submit survey + as.StubOne(0) // Merge method survey + as.StubOne(true) // Delete branch survey + as.Stub([]*prompt.QuestionStub{ // Confirm submit survey + { + Name: "confirmation", + Value: 2, + }, + }) output, err := runCommand(http, "blueberries", true, "") if !errors.Is(err, cmdutil.SilentError) { diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 8ef40d047..836f83889 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -21,6 +21,7 @@ const ( PreviewAction CancelAction MetadataAction + EditCommitMessageAction noMilestone = "(none)" )