X Tutup
Skip to content

Commit d57cb56

Browse files
cristiand391mislav
cristiand391
authored and
mislav
committed
Allow editing commit msg when squash merging a PR
1 parent 4e5aa91 commit d57cb56

File tree

4 files changed

+269
-22
lines changed

4 files changed

+269
-22
lines changed

‎api/queries_pr.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,9 @@ type PullRequest struct {
8888
ReactionGroups ReactionGroups
8989
Reviews PullRequestReviews
9090
ReviewRequests ReviewRequests
91+
92+
ViewerMergeBodyText string
93+
ViewerMergeHeadlineText string
9194
}
9295

9396
type ReviewRequests struct {
@@ -584,6 +587,8 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu
584587
milestone{
585588
title
586589
}
590+
viewerMergeBodyText
591+
viewerMergeHeadlineText
587592
` + commentsFragment() + `
588593
` + reactionGroupsFragment() + `
589594
}

‎pkg/cmd/pr/merge/merge.go‎

Lines changed: 107 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/cli/cli/pkg/cmdutil"
1717
"github.com/cli/cli/pkg/iostreams"
1818
"github.com/cli/cli/pkg/prompt"
19+
"github.com/cli/cli/pkg/surveyext"
1920
"github.com/spf13/cobra"
2021
)
2122

@@ -154,6 +155,8 @@ func mergeRun(opts *MergeOptions) error {
154155
if !isPRAlreadyMerged {
155156
mergeMethod := opts.MergeMethod
156157

158+
action := shared.SubmitAction
159+
157160
if opts.InteractiveMode {
158161
r, err := api.GitHubRepo(apiClient, baseRepo)
159162
if err != nil {
@@ -167,19 +170,52 @@ func mergeRun(opts *MergeOptions) error {
167170
if err != nil {
168171
return err
169172
}
170-
confirm, err := confirmSurvey()
173+
174+
allowEditMsg := mergeMethod != api.PullRequestMergeMethodRebase
175+
176+
action, err = confirmSubmission(allowEditMsg)
171177
if err != nil {
172-
return err
178+
return fmt.Errorf("unable to confirm: %w", err)
179+
}
180+
181+
if action == shared.EditCommitMessageAction {
182+
var editorCommand string
183+
editorCommand, err = cmdutil.DetermineEditor(opts.Config)
184+
if err != nil {
185+
return err
186+
}
187+
188+
if opts.Body == nil {
189+
var body string
190+
191+
if mergeMethod == api.PullRequestMergeMethodSquash {
192+
body = fmt.Sprintf("%s\n\n%s", pr.ViewerMergeHeadlineText, pr.ViewerMergeBodyText)
193+
}
194+
195+
opts.Body = &body
196+
}
197+
198+
err := commitMsgSurvey(opts.Body, editorCommand)
199+
if err != nil {
200+
return err
201+
}
202+
203+
action, err = confirmSubmission(false)
204+
if err != nil {
205+
return fmt.Errorf("unable to confirm: %w", err)
206+
}
173207
}
174-
if !confirm {
208+
209+
if action == shared.CancelAction {
175210
fmt.Fprintln(opts.IO.ErrOut, "Cancelled.")
176211
return cmdutil.SilentError
177212
}
178213
}
179-
180-
err = api.PullRequestMerge(apiClient, baseRepo, pr, mergeMethod, opts.Body)
181-
if err != nil {
182-
return err
214+
if action == shared.SubmitAction {
215+
err = api.PullRequestMerge(apiClient, baseRepo, pr, mergeMethod, opts.Body)
216+
if err != nil {
217+
return err
218+
}
183219
}
184220

185221
if isTerminal {
@@ -316,12 +352,69 @@ func deleteBranchSurvey(opts *MergeOptions, crossRepoPR bool) (bool, error) {
316352
return opts.DeleteBranch, nil
317353
}
318354

319-
func confirmSurvey() (bool, error) {
320-
var confirm bool
321-
submit := &survey.Confirm{
322-
Message: "Submit?",
323-
Default: true,
355+
func confirmSubmission(allowEditMsg bool) (shared.Action, error) {
356+
const (
357+
submitLabel = "Submit"
358+
editCommitMsgLabel = "Edit Commit Message"
359+
cancelLabel = "Cancel"
360+
)
361+
options := []string{submitLabel}
362+
if allowEditMsg {
363+
options = append(options, editCommitMsgLabel)
364+
}
365+
options = append(options, cancelLabel)
366+
367+
confirmAnswers := struct {
368+
Confirmation int
369+
}{}
370+
371+
confirmQs := []*survey.Question{
372+
{
373+
Name: "confirmation",
374+
Prompt: &survey.Select{
375+
Message: "What's next?",
376+
Options: options,
377+
},
378+
},
379+
}
380+
381+
err := prompt.SurveyAsk(confirmQs, &confirmAnswers)
382+
if err != nil {
383+
return -1, fmt.Errorf("could not prompt: %w", err)
384+
}
385+
386+
switch options[confirmAnswers.Confirmation] {
387+
case submitLabel:
388+
return shared.SubmitAction, nil
389+
case editCommitMsgLabel:
390+
return shared.EditCommitMessageAction, nil
391+
case cancelLabel:
392+
return shared.CancelAction, nil
393+
default:
394+
return -1, fmt.Errorf("invalid index: %d", confirmAnswers.Confirmation)
324395
}
325-
err := prompt.SurveyAskOne(submit, &confirm)
326-
return confirm, err
396+
}
397+
398+
func commitMsgSurvey(body *string, editorCommand string) error {
399+
qs := []*survey.Question{
400+
{
401+
Name: "body",
402+
Prompt: &surveyext.GhEditor{
403+
BlankAllowed: true,
404+
EditorCommand: editorCommand,
405+
Editor: &survey.Editor{
406+
Message: "Body",
407+
AppendDefault: true,
408+
Default: *body,
409+
FileName: "*.md",
410+
},
411+
},
412+
},
413+
}
414+
err := prompt.SurveyAsk(qs, body)
415+
if err != nil {
416+
return err
417+
}
418+
419+
return nil
327420
}

‎pkg/cmd/pr/merge/merge_test.go‎

Lines changed: 156 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -626,9 +626,14 @@ func TestPRMerge_interactive(t *testing.T) {
626626
as, surveyTeardown := prompt.InitAskStubber()
627627
defer surveyTeardown()
628628

629-
as.StubOne(0) // Merge method survey
630-
as.StubOne(true) // Delete branch survey
631-
as.StubOne(true) // Confirm submit survey
629+
as.StubOne(0) // Merge method survey
630+
as.StubOne(true) // Delete branch survey
631+
as.Stub([]*prompt.QuestionStub{ // Confirm submit survey
632+
{
633+
Name: "confirmation",
634+
Value: 0,
635+
},
636+
})
632637

633638
output, err := runCommand(http, "blueberries", true, "")
634639
if err != nil {
@@ -682,8 +687,14 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) {
682687
as, surveyTeardown := prompt.InitAskStubber()
683688
defer surveyTeardown()
684689

685-
as.StubOne(0) // Merge method survey
686-
as.StubOne(true) // Confirm submit survey
690+
as.StubOne(0) // Merge method survey
691+
as.StubOne(true) // Confirm submit survey
692+
as.Stub([]*prompt.QuestionStub{ // Confirm submit survey
693+
{
694+
Name: "confirmation",
695+
Value: 0,
696+
},
697+
})
687698

688699
output, err := runCommand(http, "blueberries", true, "-d")
689700
if err != nil {
@@ -694,6 +705,138 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) {
694705
test.ExpectLines(t, output.Stderr(), "Merged pull request #3", "Deleted branch blueberries and switched to branch master")
695706
}
696707

708+
func TestPRMerge_interactiveSquashEditCommitMsg(t *testing.T) {
709+
http := initFakeHTTP()
710+
defer http.Verify(t)
711+
http.Register(
712+
httpmock.GraphQL(`query PullRequestForBranch\b`),
713+
httpmock.StringResponse(`
714+
{ "data": { "repository": { "pullRequests": { "nodes": [{
715+
"headRefName": "blueberries",
716+
"headRepositoryOwner": {"login": "OWNER"},
717+
"id": "THE-ID",
718+
"number": 3
719+
}] } } } }`))
720+
http.Register(
721+
httpmock.GraphQL(`query RepositoryInfo\b`),
722+
httpmock.StringResponse(`
723+
{ "data": { "repository": {
724+
"mergeCommitAllowed": true,
725+
"rebaseMergeAllowed": true,
726+
"squashMergeAllowed": true
727+
} } }`))
728+
http.Register(
729+
httpmock.GraphQL(`mutation PullRequestMerge\b`),
730+
httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) {
731+
assert.Equal(t, "THE-ID", input["pullRequestId"].(string))
732+
assert.Equal(t, "SQUASH", input["mergeMethod"].(string))
733+
assert.Equal(t, "cool story", input["commitBody"].(string))
734+
}))
735+
736+
cs, cmdTeardown := run.Stub()
737+
defer cmdTeardown(t)
738+
739+
cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "")
740+
741+
as, surveyTeardown := prompt.InitAskStubber()
742+
defer surveyTeardown()
743+
744+
as.StubOne(2) // Merge method survey
745+
as.StubOne(false) // Delete branch survey
746+
as.Stub([]*prompt.QuestionStub{ // Confirm submit survey
747+
{
748+
Name: "confirmation",
749+
Value: 1,
750+
},
751+
})
752+
as.Stub([]*prompt.QuestionStub{ // Edit Commit Msg editor survey
753+
{
754+
Name: "body",
755+
Value: "cool story",
756+
},
757+
})
758+
as.Stub([]*prompt.QuestionStub{ // Confirm submit survey
759+
{
760+
Name: "confirmation",
761+
Value: 0,
762+
},
763+
})
764+
765+
output, err := runCommand(http, "blueberries", true, "")
766+
if err != nil {
767+
t.Fatalf("Got unexpected error running `pr merge` %s", err)
768+
}
769+
770+
assert.Equal(t, "✔ Squashed and merged pull request #3 ()\n", output.Stderr())
771+
}
772+
773+
func TestPRMerge_interactiveSquashEditCommitMsgDefaultBody(t *testing.T) {
774+
http := initFakeHTTP()
775+
defer http.Verify(t)
776+
http.Register(
777+
httpmock.GraphQL(`query PullRequestForBranch\b`),
778+
httpmock.StringResponse(`
779+
{ "data": { "repository": { "pullRequests": { "nodes": [{
780+
"headRefName": "blueberries",
781+
"headRepositoryOwner": {"login": "OWNER"},
782+
"id": "THE-ID",
783+
"number": 3,
784+
"viewerMergeBodyText": "Co-authored by: Octocat <octocat@github.com>",
785+
"viewerMergeHeadlineText": "Add new feat (#33)"
786+
}] } } } }`))
787+
http.Register(
788+
httpmock.GraphQL(`query RepositoryInfo\b`),
789+
httpmock.StringResponse(`
790+
{ "data": { "repository": {
791+
"mergeCommitAllowed": true,
792+
"rebaseMergeAllowed": true,
793+
"squashMergeAllowed": true
794+
} } }`))
795+
http.Register(
796+
httpmock.GraphQL(`mutation PullRequestMerge\b`),
797+
httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) {
798+
assert.Equal(t, "THE-ID", input["pullRequestId"].(string))
799+
assert.Equal(t, "SQUASH", input["mergeMethod"].(string))
800+
assert.Equal(t, "Add new feat (#33)\n\nCo-authored by: Octocat <octocat@github.com>", input["commitBody"])
801+
}))
802+
803+
cs, cmdTeardown := run.Stub()
804+
defer cmdTeardown(t)
805+
806+
cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "")
807+
808+
as, surveyTeardown := prompt.InitAskStubber()
809+
defer surveyTeardown()
810+
811+
as.StubOne(2) // Merge method survey
812+
as.StubOne(false) // Delete branch survey
813+
as.Stub([]*prompt.QuestionStub{ // Confirm submit survey
814+
{
815+
Name: "confirmation",
816+
Value: 1,
817+
},
818+
})
819+
as.Stub([]*prompt.QuestionStub{ // Edit Commit Msg editor survey
820+
{
821+
Name: "body",
822+
Default: true,
823+
},
824+
})
825+
as.Stub([]*prompt.QuestionStub{ // Confirm submit survey
826+
{
827+
Name: "confirmation",
828+
Value: 0,
829+
},
830+
})
831+
832+
output, err := runCommand(http, "blueberries", true, "")
833+
if err != nil {
834+
t.Fatalf("Got unexpected error running `pr merge` %s", err)
835+
}
836+
837+
assert.Equal(t, "✔ Squashed and merged pull request #3 ()\n", output.Stderr())
838+
}
839+
697840
func TestPRMerge_interactiveCancelled(t *testing.T) {
698841
http := initFakeHTTP()
699842
defer http.Verify(t)
@@ -724,9 +867,14 @@ func TestPRMerge_interactiveCancelled(t *testing.T) {
724867
as, surveyTeardown := prompt.InitAskStubber()
725868
defer surveyTeardown()
726869

727-
as.StubOne(0) // Merge method survey
728-
as.StubOne(true) // Delete branch survey
729-
as.StubOne(false) // Confirm submit survey
870+
as.StubOne(0) // Merge method survey
871+
as.StubOne(true) // Delete branch survey
872+
as.Stub([]*prompt.QuestionStub{ // Confirm submit survey
873+
{
874+
Name: "confirmation",
875+
Value: 2,
876+
},
877+
})
730878

731879
output, err := runCommand(http, "blueberries", true, "")
732880
if !errors.Is(err, cmdutil.SilentError) {

‎pkg/cmd/pr/shared/survey.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const (
2121
PreviewAction
2222
CancelAction
2323
MetadataAction
24+
EditCommitMessageAction
2425

2526
noMilestone = "(none)"
2627
)

0 commit comments

Comments
 (0)
X Tutup