Repository: cli/cli
Pull request #3018 (2021-02-23 07:05:31)

Add `pr create --body-file` flag
The `--body-file` flag helps to create long bodies or multi-line bodies.
In particular, it avoids the "filename or extension is too long" problem in Windows environments.

I refer to the following comments.
https://github.com/cli/cli/issues/595#issuecomment-608368316


Patch:
diff --git a/pkg/cmd/auth/shared/login_flow.go b/pkg/cmd/auth/shared/login_flow.go
index 6f75a784f..fbff9882e 100644
--- a/pkg/cmd/auth/shared/login_flow.go
+++ b/pkg/cmd/auth/shared/login_flow.go
@@ -9,7 +9,6 @@ import (
 	"github.com/MakeNowJust/heredoc"
 	"github.com/cli/cli/api"
 	"github.com/cli/cli/internal/authflow"
-	"github.com/cli/cli/internal/ghinstance"
 	"github.com/cli/cli/pkg/iostreams"
 	"github.com/cli/cli/pkg/prompt"
 )
@@ -126,7 +125,7 @@ func Login(opts *LoginOptions) error {
 		fmt.Fprint(opts.IO.ErrOut, heredoc.Docf(`
 			Tip: you can generate a Personal Access Token here https://%s/settings/tokens
 			The minimum required scopes are %s.
-		`, hostname, scopesSentence(minimumScopes, ghinstance.IsEnterprise(hostname))))
+		`, hostname, scopesSentence(minimumScopes)))
 
 		err := prompt.SurveyAskOne(&survey.Password{
 			Message: "Paste your authentication token:",
@@ -194,14 +193,11 @@ func Login(opts *LoginOptions) error {
 	return nil
 }
 
-func scopesSentence(scopes []string, isEnterprise bool) string {
+func scopesSentence(scopes []string) string {
 	quoted := make([]string, len(scopes))
 	for i, s := range scopes {
 		quoted[i] = fmt.Sprintf("'%s'", s)
-		if s == "workflow" && isEnterprise {
-			// remove when GHE 2.x reaches EOL
-			quoted[i] += " (GHE 3.0+)"
-		}
 	}
+	// TODO: insert an "and" before the last element
 	return strings.Join(quoted, ", ")
 }
diff --git a/pkg/cmd/auth/shared/oauth_scopes.go b/pkg/cmd/auth/shared/oauth_scopes.go
index 20dbf9a83..c99b5543d 100644
--- a/pkg/cmd/auth/shared/oauth_scopes.go
+++ b/pkg/cmd/auth/shared/oauth_scopes.go
@@ -40,7 +40,7 @@ func HasMinimumScopes(httpClient httpClient, hostname, authToken string) error {
 		return err
 	}
 
-	req.Header.Set("Authorization", "token "+authToken)
+	req.Header.Set("Autorization", "token "+authToken)
 
 	res, err := httpClient.Do(req)
 	if err != nil {
diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go
index 0b2a93cd4..6bf9356c6 100644
--- a/pkg/cmd/issue/create/create.go
+++ b/pkg/cmd/issue/create/create.go
@@ -46,6 +46,8 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
 		Config:     f.Config,
 	}
 
+	var bodyFile string
+
 	cmd := &cobra.Command{
 		Use:   "create",
 		Short: "Create a new issue",
@@ -61,19 +63,27 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
 		RunE: func(cmd *cobra.Command, args []string) error {
 			// support `-R, --repo` override
 			opts.BaseRepo = f.BaseRepo
+			opts.HasRepoOverride = cmd.Flags().Changed("repo")
 
 			titleProvided := cmd.Flags().Changed("title")
 			bodyProvided := cmd.Flags().Changed("body")
-			opts.HasRepoOverride = cmd.Flags().Changed("repo")
+			if bodyFile != "" {
+				b, err := cmdutil.ReadFile(bodyFile, opts.IO.In)
+				if err != nil {
+					return err
+				}
+				opts.Body = string(b)
+				bodyProvided = true
+			}
 
 			if !opts.IO.CanPrompt() && opts.RecoverFile != "" {
-				return &cmdutil.FlagError{Err: errors.New("--recover only supported when running interactively")}
+				return &cmdutil.FlagError{Err: errors.New("`--recover` only supported when running interactively")}
 			}
 
 			opts.Interactive = !(titleProvided && bodyProvided)
 
 			if opts.Interactive && !opts.IO.CanPrompt() {
-				return &cmdutil.FlagError{Err: errors.New("must provide --title and --body when not running interactively")}
+				return &cmdutil.FlagError{Err: errors.New("must provide title and body when not running interactively")}
 			}
 
 			if runF != nil {
@@ -85,6 +95,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
 
 	cmd.Flags().StringVarP(&opts.Title, "title", "t", "", "Supply a title. Will prompt for one otherwise.")
 	cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "Supply a body. Will prompt for one otherwise.")
+	cmd.Flags().StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file`")
 	cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the browser to create an issue")
 	cmd.Flags().StringSliceVarP(&opts.Assignees, "assignee", "a", nil, "Assign people by their `login`. Use \"@me\" to self-assign.")
 	cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Add labels by `name`")
diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go
index 52539b8f5..08ea04d94 100644
--- a/pkg/cmd/pr/create/create.go
+++ b/pkg/cmd/pr/create/create.go
@@ -81,6 +81,8 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
 		Branch:     f.Branch,
 	}
 
+	var bodyFile string
+
 	cmd := &cobra.Command{
 		Use:   "create",
 		Short: "Create a pull request",
@@ -110,7 +112,6 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
 		Args: cmdutil.NoArgsQuoteReminder,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			opts.TitleProvided = cmd.Flags().Changed("title")
-			opts.BodyProvided = cmd.Flags().Changed("body")
 			opts.RepoOverride, _ = cmd.Flags().GetString("repo")
 			noMaintainerEdit, _ := cmd.Flags().GetBool("no-maintainer-edit")
 			opts.MaintainerCanModify = !noMaintainerEdit
@@ -133,6 +134,16 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
 				return errors.New("the `--no-maintainer-edit` flag is not supported with `--web`")
 			}
 
+			opts.BodyProvided = cmd.Flags().Changed("body")
+			if bodyFile != "" {
+				b, err := cmdutil.ReadFile(bodyFile, opts.IO.In)
+				if err != nil {
+					return err
+				}
+				opts.Body = string(b)
+				opts.BodyProvided = true
+			}
+
 			if runF != nil {
 				return runF(opts)
 			}
@@ -144,6 +155,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
 	fl.BoolVarP(&opts.IsDraft, "draft", "d", false, "Mark pull request as a draft")
 	fl.StringVarP(&opts.Title, "title", "t", "", "Title for the pull request")
 	fl.StringVarP(&opts.Body, "body", "b", "", "Body for the pull request")
+	fl.StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file`")
 	fl.StringVarP(&opts.BaseBranch, "base", "B", "", "The `branch` into which you want your code merged")
 	fl.StringVarP(&opts.HeadBranch, "head", "H", "", "The `branch` that contains commits for your pull request (default: current branch)")
 	fl.BoolVarP(&opts.WebMode, "web", "w", false, "Open the web browser to create a pull request")
diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go
index 9b87b8845..cab6292c7 100644
--- a/pkg/cmd/release/create/create.go
+++ b/pkg/cmd/release/create/create.go
@@ -3,7 +3,6 @@ package create
 import (
 	"bytes"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 	"os"
 	"strings"
@@ -106,12 +105,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
 
 			opts.BodyProvided = cmd.Flags().Changed("notes")
 			if notesFile != "" {
-				var b []byte
-				if notesFile == "-" {
-					b, err = ioutil.ReadAll(opts.IO.In)
-				} else {
-					b, err = ioutil.ReadFile(notesFile)
-				}
+				b, err := cmdutil.ReadFile(notesFile, opts.IO.In)
 				if err != nil {
 					return err
 				}
diff --git a/pkg/cmdutil/file_input.go b/pkg/cmdutil/file_input.go
new file mode 100644
index 000000000..32322bc0f
--- /dev/null
+++ b/pkg/cmdutil/file_input.go
@@ -0,0 +1,16 @@
+package cmdutil
+
+import (
+	"io"
+	"io/ioutil"
+)
+
+func ReadFile(filename string, stdin io.ReadCloser) ([]byte, error) {
+	if filename == "-" {
+		b, err := ioutil.ReadAll(stdin)
+		_ = stdin.Close()
+		return b, err
+	}
+
+	return ioutil.ReadFile(filename)
+}


Test patch:
diff --git a/pkg/cmd/auth/shared/login_flow_test.go b/pkg/cmd/auth/shared/login_flow_test.go
index 521616dfa..c37d89b46 100644
--- a/pkg/cmd/auth/shared/login_flow_test.go
+++ b/pkg/cmd/auth/shared/login_flow_test.go
@@ -101,55 +101,3 @@ func TestLogin_ssh(t *testing.T) {
 	assert.Equal(t, "ATOKEN", cfg["example.com:oauth_token"])
 	assert.Equal(t, "ssh", cfg["example.com:git_protocol"])
 }
-
-func Test_scopesSentence(t *testing.T) {
-	type args struct {
-		scopes       []string
-		isEnterprise bool
-	}
-	tests := []struct {
-		name string
-		args args
-		want string
-	}{
-		{
-			name: "basic scopes",
-			args: args{
-				scopes:       []string{"repo", "read:org"},
-				isEnterprise: false,
-			},
-			want: "'repo', 'read:org'",
-		},
-		{
-			name: "empty",
-			args: args{
-				scopes:       []string(nil),
-				isEnterprise: false,
-			},
-			want: "",
-		},
-		{
-			name: "workflow scope for dotcom",
-			args: args{
-				scopes:       []string{"repo", "workflow"},
-				isEnterprise: false,
-			},
-			want: "'repo', 'workflow'",
-		},
-		{
-			name: "workflow scope for GHE",
-			args: args{
-				scopes:       []string{"repo", "workflow"},
-				isEnterprise: true,
-			},
-			want: "'repo', 'workflow' (GHE 3.0+)",
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := scopesSentence(tt.args.scopes, tt.args.isEnterprise); got != tt.want {
-				t.Errorf("scopesSentence() = %q, want %q", got, tt.want)
-			}
-		})
-	}
-}
diff --git a/pkg/cmd/auth/shared/oauth_scopes_test.go b/pkg/cmd/auth/shared/oauth_scopes_test.go
index 0f6bd9f32..3b9766a95 100644
--- a/pkg/cmd/auth/shared/oauth_scopes_test.go
+++ b/pkg/cmd/auth/shared/oauth_scopes_test.go
@@ -52,9 +52,7 @@ func Test_HasMinimumScopes(t *testing.T) {
 			fakehttp := &httpmock.Registry{}
 			defer fakehttp.Verify(t)
 
-			var gotAuthorization string
 			fakehttp.Register(httpmock.REST("GET", ""), func(req *http.Request) (*http.Response, error) {
-				gotAuthorization = req.Header.Get("authorization")
 				return &http.Response{
 					Request:    req,
 					StatusCode: 200,
@@ -72,7 +70,6 @@ func Test_HasMinimumScopes(t *testing.T) {
 			} else {
 				assert.NoError(t, err)
 			}
-			assert.Equal(t, gotAuthorization, "token ATOKEN")
 		})
 	}
 
diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go
index abffff3fa..13fd3b98c 100644
--- a/pkg/cmd/issue/create/create_test.go
+++ b/pkg/cmd/issue/create/create_test.go
@@ -7,6 +7,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -22,8 +23,118 @@ import (
 	"github.com/cli/cli/test"
 	"github.com/google/shlex"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+func TestNewCmdCreate(t *testing.T) {
+	tmpFile := filepath.Join(t.TempDir(), "my-body.md")
+	err := ioutil.WriteFile(tmpFile, []byte("a body from file"), 0600)
+	require.NoError(t, err)
+
+	tests := []struct {
+		name      string
+		tty       bool
+		stdin     string
+		cli       string
+		wantsErr  bool
+		wantsOpts CreateOptions
+	}{
+		{
+			name:     "empty non-tty",
+			tty:      false,
+			cli:      "",
+			wantsErr: true,
+		},
+		{
+			name:     "only title non-tty",
+			tty:      false,
+			cli:      "-t mytitle",
+			wantsErr: true,
+		},
+		{
+			name:     "empty tty",
+			tty:      true,
+			cli:      "",
+			wantsErr: false,
+			wantsOpts: CreateOptions{
+				Title:       "",
+				Body:        "",
+				RecoverFile: "",
+				WebMode:     false,
+				Interactive: true,
+			},
+		},
+		{
+			name:     "body from stdin",
+			tty:      false,
+			stdin:    "this is on standard input",
+			cli:      "-t mytitle -F -",
+			wantsErr: false,
+			wantsOpts: CreateOptions{
+				Title:       "mytitle",
+				Body:        "this is on standard input",
+				RecoverFile: "",
+				WebMode:     false,
+				Interactive: false,
+			},
+		},
+		{
+			name:     "body from file",
+			tty:      false,
+			cli:      fmt.Sprintf("-t mytitle -F '%s'", tmpFile),
+			wantsErr: false,
+			wantsOpts: CreateOptions{
+				Title:       "mytitle",
+				Body:        "a body from file",
+				RecoverFile: "",
+				WebMode:     false,
+				Interactive: false,
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			io, stdin, stdout, stderr := iostreams.Test()
+			if tt.stdin != "" {
+				_, _ = stdin.WriteString(tt.stdin)
+			} else if tt.tty {
+				io.SetStdinTTY(true)
+				io.SetStdoutTTY(true)
+			}
+
+			f := &cmdutil.Factory{
+				IOStreams: io,
+			}
+
+			var opts *CreateOptions
+			cmd := NewCmdCreate(f, func(o *CreateOptions) error {
+				opts = o
+				return nil
+			})
+
+			args, err := shlex.Split(tt.cli)
+			require.NoError(t, err)
+			cmd.SetArgs(args)
+			_, err = cmd.ExecuteC()
+			if tt.wantsErr {
+				assert.Error(t, err)
+				return
+			} else {
+				require.NoError(t, err)
+			}
+
+			assert.Equal(t, "", stdout.String())
+			assert.Equal(t, "", stderr.String())
+
+			assert.Equal(t, tt.wantsOpts.Body, opts.Body)
+			assert.Equal(t, tt.wantsOpts.Title, opts.Title)
+			assert.Equal(t, tt.wantsOpts.RecoverFile, opts.RecoverFile)
+			assert.Equal(t, tt.wantsOpts.WebMode, opts.WebMode)
+			assert.Equal(t, tt.wantsOpts.Interactive, opts.Interactive)
+		})
+	}
+}
+
 func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) {
 	return runCommandWithRootDirOverridden(rt, isTTY, cli, "")
 }
@@ -69,14 +180,6 @@ func runCommandWithRootDirOverridden(rt http.RoundTripper, isTTY bool, cli strin
 	}, err
 }
 
-func TestIssueCreate_nontty_error(t *testing.T) {
-	http := &httpmock.Registry{}
-	defer http.Verify(t)
-
-	_, err := runCommand(http, false, `-t hello`)
-	assert.EqualError(t, err, "must provide --title and --body when not running interactively")
-}
-
 func TestIssueCreate(t *testing.T) {
 	http := &httpmock.Registry{}
 	defer http.Verify(t)
diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go
index c95b86368..7e94cf084 100644
--- a/pkg/cmd/pr/create/create_test.go
+++ b/pkg/cmd/pr/create/create_test.go
@@ -7,6 +7,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -28,6 +29,133 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestNewCmdCreate(t *testing.T) {
+	tmpFile := filepath.Join(t.TempDir(), "my-body.md")
+	err := ioutil.WriteFile(tmpFile, []byte("a body from file"), 0600)
+	require.NoError(t, err)
+
+	tests := []struct {
+		name      string
+		tty       bool
+		stdin     string
+		cli       string
+		wantsErr  bool
+		wantsOpts CreateOptions
+	}{
+		{
+			name:     "empty non-tty",
+			tty:      false,
+			cli:      "",
+			wantsErr: true,
+		},
+		{
+			name:     "empty tty",
+			tty:      true,
+			cli:      "",
+			wantsErr: false,
+			wantsOpts: CreateOptions{
+				Title:               "",
+				TitleProvided:       false,
+				Body:                "",
+				BodyProvided:        false,
+				Autofill:            false,
+				RecoverFile:         "",
+				WebMode:             false,
+				IsDraft:             false,
+				BaseBranch:          "",
+				HeadBranch:          "",
+				MaintainerCanModify: true,
+			},
+		},
+		{
+			name:     "body from stdin",
+			tty:      false,
+			stdin:    "this is on standard input",
+			cli:      "-t mytitle -F -",
+			wantsErr: false,
+			wantsOpts: CreateOptions{
+				Title:               "mytitle",
+				TitleProvided:       true,
+				Body:                "this is on standard input",
+				BodyProvided:        true,
+				Autofill:            false,
+				RecoverFile:         "",
+				WebMode:             false,
+				IsDraft:             false,
+				BaseBranch:          "",
+				HeadBranch:          "",
+				MaintainerCanModify: true,
+			},
+		},
+		{
+			name:     "body from file",
+			tty:      false,
+			cli:      fmt.Sprintf("-t mytitle -F '%s'", tmpFile),
+			wantsErr: false,
+			wantsOpts: CreateOptions{
+				Title:               "mytitle",
+				TitleProvided:       true,
+				Body:                "a body from file",
+				BodyProvided:        true,
+				Autofill:            false,
+				RecoverFile:         "",
+				WebMode:             false,
+				IsDraft:             false,
+				BaseBranch:          "",
+				HeadBranch:          "",
+				MaintainerCanModify: true,
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			io, stdin, stdout, stderr := iostreams.Test()
+			if tt.stdin != "" {
+				_, _ = stdin.WriteString(tt.stdin)
+			} else if tt.tty {
+				io.SetStdinTTY(true)
+				io.SetStdoutTTY(true)
+			}
+
+			f := &cmdutil.Factory{
+				IOStreams: io,
+			}
+
+			var opts *CreateOptions
+			cmd := NewCmdCreate(f, func(o *CreateOptions) error {
+				opts = o
+				return nil
+			})
+
+			args, err := shlex.Split(tt.cli)
+			require.NoError(t, err)
+			cmd.SetArgs(args)
+			_, err = cmd.ExecuteC()
+			if tt.wantsErr {
+				assert.Error(t, err)
+				return
+			} else {
+				require.NoError(t, err)
+			}
+
+			assert.Equal(t, "", stdout.String())
+			assert.Equal(t, "", stderr.String())
+
+			assert.Equal(t, tt.wantsOpts.Body, opts.Body)
+			assert.Equal(t, tt.wantsOpts.BodyProvided, opts.BodyProvided)
+			assert.Equal(t, tt.wantsOpts.Title, opts.Title)
+			assert.Equal(t, tt.wantsOpts.TitleProvided, opts.TitleProvided)
+			assert.Equal(t, tt.wantsOpts.Autofill, opts.Autofill)
+			assert.Equal(t, tt.wantsOpts.WebMode, opts.WebMode)
+			assert.Equal(t, tt.wantsOpts.RecoverFile, opts.RecoverFile)
+			assert.Equal(t, tt.wantsOpts.IsDraft, opts.IsDraft)
+			assert.Equal(t, tt.wantsOpts.MaintainerCanModify, opts.MaintainerCanModify)
+			assert.Equal(t, tt.wantsOpts.BaseBranch, opts.BaseBranch)
+			assert.Equal(t, tt.wantsOpts.HeadBranch, opts.HeadBranch)
+		})
+	}
+}
+
 func runCommand(rt http.RoundTripper, remotes context.Remotes, branch string, isTTY bool, cli string) (*test.CmdOut, error) {
 	return runCommandWithRootDirOverridden(rt, remotes, branch, isTTY, cli, "")
 }
@@ -115,16 +243,6 @@ func TestPRCreate_nontty_web(t *testing.T) {
 	assert.Equal(t, "", output.Stderr())
 }
 
-func TestPRCreate_nontty_insufficient_flags(t *testing.T) {
-	http := initFakeHTTP()
-	defer http.Verify(t)
-
-	output, err := runCommand(http, nil, "feature", false, "")
-	assert.EqualError(t, err, "`--title` or `--fill` required when not running interactively")
-
-	assert.Equal(t, "", output.String())
-}
-
 func TestPRCreate_recover(t *testing.T) {
 	http := initFakeHTTP()
 	defer http.Verify(t)