Repository: cli/cli
Pull request #3344 (2021-04-01 17:05:51)

Allow editing commit subject when squash-merging a PR
Ref. https://github.com/cli/cli/issues/1023#issuecomment-801379487

This PR adds support for editing the commit subject line when squash-merging a PR.



masoodaam: https://github.com/masoodaam/1NeoArmnGyWHKfbje9JNWqw3tquMY7jHCw
masoodaam: 1NeoArmnGyWHKfbje9JNWqw3tquMY7jHCw
mgabeler-lee-6rs: I & my team greatly miss this feature for our workflow, is there anything I can do to help this along?
cristiand391: @mgabeler-lee-6rs will see if I can update this PR this weekend :)

bernhardkaindl: @cristiand391 Your current commit 6e5db7 from April 1st on the branch of this PR didn't work for me. I
```py
git describe --tags
v0.12.0-1076-g6e5db706
make install prefix=$HOME/.local
$HOME/.local/bin/gh pr merge --squash --auto --subject 'py-ipykernel: Add version 6.0.2 (#26501)' --body ''
```
resulted in this commit: https://github.com/spack/spack/commit/2cdfa336775bf58b1d8c0e16d9d6c76a10cbd8c9
The merged commit message subject was the commit message subject of the submitter of the PR, which was not what I expected to see using the command above to do the auto-merge.

Patch:
diff --git a/pkg/cmd/pr/merge/http.go b/pkg/cmd/pr/merge/http.go
index 74a575a64..217290d9d 100644
--- a/pkg/cmd/pr/merge/http.go
+++ b/pkg/cmd/pr/merge/http.go
@@ -20,14 +20,13 @@ const (
 )
 
 type mergePayload struct {
-	repo             ghrepo.Interface
-	pullRequestID    string
-	method           PullRequestMergeMethod
-	auto             bool
-	commitSubject    string
-	setCommitSubject bool
-	commitBody       string
-	setCommitBody    bool
+	repo          ghrepo.Interface
+	pullRequestID string
+	method        PullRequestMergeMethod
+	auto          bool
+	commitSubject string
+	commitBody    string
+	setCommitBody bool
 }
 
 // TODO: drop after githubv4 gets updated
@@ -52,7 +51,7 @@ func mergePullRequest(client *http.Client, payload mergePayload) error {
 		input.MergeMethod = &m
 	}
 
-	if payload.setCommitSubject {
+	if payload.commitSubject != "" {
 		commitHeadline := githubv4.String(payload.commitSubject)
 		input.CommitHeadline = &commitHeadline
 	}
@@ -100,7 +99,7 @@ func disableAutoMerge(client *http.Client, repo ghrepo.Interface, prID string) e
 	return gql.MutateNamed(context.Background(), "PullRequestAutoMergeDisable", &mutation, variables)
 }
 
-func getMergeText(client *http.Client, repo ghrepo.Interface, prID string, mergeMethod PullRequestMergeMethod) (string, error) {
+func getMergeText(client *http.Client, repo ghrepo.Interface, prID string, mergeMethod PullRequestMergeMethod) (string, string, error) {
 	var method githubv4.PullRequestMergeMethod
 	switch mergeMethod {
 	case PullRequestMergeMethodMerge:
@@ -114,7 +113,8 @@ func getMergeText(client *http.Client, repo ghrepo.Interface, prID string, merge
 	var query struct {
 		Node struct {
 			PullRequest struct {
-				ViewerMergeBodyText string `graphql:"viewerMergeBodyText(mergeType: $method)"`
+				ViewerMergeHeadlineText string `graphql:"viewerMergeHeadlineText(mergeType: $method)"`
+				ViewerMergeBodyText     string `graphql:"viewerMergeBodyText(mergeType: $method)"`
 			} `graphql:"...on PullRequest"`
 		} `graphql:"node(id: $prID)"`
 	}
@@ -128,11 +128,12 @@ func getMergeText(client *http.Client, repo ghrepo.Interface, prID string, merge
 	err := gql.QueryNamed(context.Background(), "PullRequestMergeText", &query, variables)
 	if err != nil {
 		// Tolerate this API missing in older GitHub Enterprise
-		if strings.Contains(err.Error(), "Field 'viewerMergeBodyText' doesn't exist") {
-			return "", nil
+		if strings.Contains(err.Error(), "Field 'viewerMergeHeadlineText' doesn't exist") ||
+			strings.Contains(err.Error(), "Field 'viewerMergeBodyText' doesn't exist") {
+			return "", "", nil
 		}
-		return "", err
+		return "", "", err
 	}
 
-	return query.Node.PullRequest.ViewerMergeBodyText, nil
+	return query.Node.PullRequest.ViewerMergeHeadlineText, query.Node.PullRequest.ViewerMergeBodyText, nil
 }
diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go
index b2185fab2..d7a0c085d 100644
--- a/pkg/cmd/pr/merge/merge.go
+++ b/pkg/cmd/pr/merge/merge.go
@@ -41,6 +41,7 @@ type MergeOptions struct {
 
 	Body    string
 	BodySet bool
+	Subject string
 	Editor  editor
 
 	UseAdmin                bool
@@ -158,6 +159,7 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm
 	cmd.Flags().BoolVarP(&opts.DeleteBranch, "delete-branch", "d", false, "Delete the local and remote branch after merge")
 	cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "Body `text` for the merge commit")
 	cmd.Flags().StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file` (use \"-\" to read from standard input)")
+	cmd.Flags().StringVarP(&opts.Subject, "subject", "t", "", "Subject `text` for the merge commit")
 	cmd.Flags().BoolVarP(&flagMerge, "merge", "m", false, "Merge the commits with the base branch")
 	cmd.Flags().BoolVarP(&flagRebase, "rebase", "r", false, "Rebase the commits onto the base branch")
 	cmd.Flags().BoolVarP(&flagSquash, "squash", "s", false, "Squash the commits into one commit and merge it into the base branch")
@@ -231,6 +233,7 @@ func mergeRun(opts *MergeOptions) error {
 			pullRequestID: pr.ID,
 			method:        opts.MergeMethod,
 			auto:          autoMerge,
+			commitSubject: opts.Subject,
 			commitBody:    opts.Body,
 			setCommitBody: opts.BodySet,
 		}
@@ -251,34 +254,20 @@ func mergeRun(opts *MergeOptions) error {
 
 			allowEditMsg := payload.method != PullRequestMergeMethodRebase
 
-			action, err := confirmSurvey(allowEditMsg)
-			if err != nil {
-				return fmt.Errorf("unable to confirm: %w", err)
-			}
-
-			if action == shared.EditCommitMessageAction {
-				if !payload.setCommitBody {
-					payload.commitBody, err = getMergeText(httpClient, baseRepo, pr.ID, payload.method)
-					if err != nil {
-						return err
-					}
+			for {
+				action, err := confirmSurvey(allowEditMsg)
+				if err != nil {
+					return fmt.Errorf("unable to confirm: %w", err)
 				}
 
-				payload.commitBody, err = opts.Editor.Edit("*.md", payload.commitBody)
+				submit, err := confirmSubmission(httpClient, opts, action, &payload)
 				if err != nil {
 					return err
 				}
-				payload.setCommitBody = true
-
-				action, err = confirmSurvey(false)
-				if err != nil {
-					return fmt.Errorf("unable to confirm: %w", err)
+				if submit {
+					break
 				}
 			}
-			if action == shared.CancelAction {
-				fmt.Fprintln(opts.IO.ErrOut, "Cancelled.")
-				return cmdutil.CancelError
-			}
 		}
 
 		err = mergePullRequest(httpClient, payload)
@@ -452,14 +441,15 @@ func deleteBranchSurvey(opts *MergeOptions, crossRepoPR, localBranchExists bool)
 
 func confirmSurvey(allowEditMsg bool) (shared.Action, error) {
 	const (
-		submitLabel        = "Submit"
-		editCommitMsgLabel = "Edit commit message"
-		cancelLabel        = "Cancel"
+		submitLabel            = "Submit"
+		editCommitSubjectLabel = "Edit commit subject"
+		editCommitMsgLabel     = "Edit commit message"
+		cancelLabel            = "Cancel"
 	)
 
 	options := []string{submitLabel}
 	if allowEditMsg {
-		options = append(options, editCommitMsgLabel)
+		options = append(options, editCommitSubjectLabel, editCommitMsgLabel)
 	}
 	options = append(options, cancelLabel)
 
@@ -476,6 +466,8 @@ func confirmSurvey(allowEditMsg bool) (shared.Action, error) {
 	switch result {
 	case submitLabel:
 		return shared.SubmitAction, nil
+	case editCommitSubjectLabel:
+		return shared.EditCommitSubjectAction, nil
 	case editCommitMsgLabel:
 		return shared.EditCommitMessageAction, nil
 	default:
@@ -483,6 +475,53 @@ func confirmSurvey(allowEditMsg bool) (shared.Action, error) {
 	}
 }
 
+func confirmSubmission(client *http.Client, opts *MergeOptions, action shared.Action, payload *mergePayload) (bool, error) {
+	var err error
+
+	switch action {
+	case shared.EditCommitMessageAction:
+		if !payload.setCommitBody {
+			_, payload.commitBody, err = getMergeText(client, payload.repo, payload.pullRequestID, payload.method)
+			if err != nil {
+				return false, err
+			}
+		}
+
+		payload.commitBody, err = opts.Editor.Edit("*.md", payload.commitBody)
+		if err != nil {
+			return false, err
+		}
+		payload.setCommitBody = true
+
+		return false, nil
+
+	case shared.EditCommitSubjectAction:
+		if payload.commitSubject == "" {
+			payload.commitSubject, _, err = getMergeText(client, payload.repo, payload.pullRequestID, payload.method)
+			if err != nil {
+				return false, err
+			}
+		}
+
+		payload.commitSubject, err = opts.Editor.Edit("*.md", payload.commitSubject)
+		if err != nil {
+			return false, err
+		}
+
+		return false, nil
+
+	case shared.CancelAction:
+		fmt.Fprintln(opts.IO.ErrOut, "Cancelled.")
+		return false, cmdutil.CancelError
+
+	case shared.SubmitAction:
+		return true, nil
+
+	default:
+		return false, fmt.Errorf("unable to confirm: %w", err)
+	}
+}
+
 type userEditor struct {
 	io     *iostreams.IOStreams
 	config func() (config.Config, error)
diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go
index 5b04801ae..aaaaffaa8 100644
--- a/pkg/cmd/pr/shared/survey.go
+++ b/pkg/cmd/pr/shared/survey.go
@@ -22,6 +22,7 @@ const (
 	CancelAction
 	MetadataAction
 	EditCommitMessageAction
+	EditCommitSubjectAction
 
 	noMilestone = "(none)"
 )


Test patch:
diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go
index 0826e200e..679142b82 100644
--- a/pkg/cmd/pr/merge/merge_test.go
+++ b/pkg/cmd/pr/merge/merge_test.go
@@ -371,6 +371,45 @@ func TestPrMerge_nontty(t *testing.T) {
 	assert.Equal(t, "", output.Stderr())
 }
 
+func TestPrMerge_editMessage_nontty(t *testing.T) {
+	http := initFakeHTTP()
+	defer http.Verify(t)
+
+	shared.RunCommandFinder(
+		"1",
+		&api.PullRequest{
+			ID:               "THE-ID",
+			Number:           1,
+			State:            "OPEN",
+			Title:            "The title of the PR",
+			MergeStateStatus: "CLEAN",
+		},
+		baseRepo("OWNER", "REPO", "master"),
+	)
+
+	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, "MERGE", input["mergeMethod"].(string))
+			assert.Equal(t, "mytitle", input["commitHeadline"].(string))
+			assert.Equal(t, "mybody", input["commitBody"].(string))
+		}))
+
+	cs, cmdTeardown := run.Stub()
+	defer cmdTeardown(t)
+
+	cs.Register(`git rev-parse --verify refs/heads/`, 0, "")
+
+	output, err := runCommand(http, "master", false, "pr merge 1 --merge -t mytitle -b mybody")
+	if err != nil {
+		t.Fatalf("error running command `pr merge`: %v", err)
+	}
+
+	assert.Equal(t, "", output.String())
+	assert.Equal(t, "", output.Stderr())
+}
+
 func TestPrMerge_withRepoFlag(t *testing.T) {
 	http := initFakeHTTP()
 	defer http.Verify(t)
@@ -884,7 +923,7 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) {
 	`), output.Stderr())
 }
 
-func TestPRMerge_interactiveSquashEditCommitMsg(t *testing.T) {
+func TestPRMerge_interactiveSquashEditCommitMsgAndSubject(t *testing.T) {
 	io, _, stdout, stderr := iostreams.Test()
 	io.SetStdoutTTY(true)
 	io.SetStderrTTY(true)
@@ -903,6 +942,14 @@ func TestPRMerge_interactiveSquashEditCommitMsg(t *testing.T) {
 		httpmock.GraphQL(`query PullRequestMergeText\b`),
 		httpmock.StringResponse(`
 		{ "data": { "node": {
+			"viewerMergeHeadlineText": "default headline text",
+			"viewerMergeBodyText": "default body text"
+		} } }`))
+	tr.Register(
+		httpmock.GraphQL(`query PullRequestMergeText\b`),
+		httpmock.StringResponse(`
+		{ "data": { "node": {
+			"viewerMergeHeadlineText": "default headline text",
 			"viewerMergeBodyText": "default body text"
 		} } }`))
 	tr.Register(
@@ -910,6 +957,7 @@ func TestPRMerge_interactiveSquashEditCommitMsg(t *testing.T) {
 		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, "DEFAULT HEADLINE TEXT", input["commitHeadline"].(string))
 			assert.Equal(t, "DEFAULT BODY TEXT", input["commitBody"].(string))
 		}))
 
@@ -921,6 +969,7 @@ func TestPRMerge_interactiveSquashEditCommitMsg(t *testing.T) {
 
 	as.StubOne(2)                     // Merge method survey
 	as.StubOne(false)                 // Delete branch survey
+	as.StubOne("Edit commit subject") // Confirm submit survey
 	as.StubOne("Edit commit message") // Confirm submit survey
 	as.StubOne("Submit")              // Confirm submit survey