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/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.go b/pkg/cmd/pr/create/create.go index 36ad7e9b2..08ea04d94 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -3,7 +3,6 @@ package create import ( "errors" "fmt" - "io/ioutil" "net/http" "net/url" "regexp" @@ -113,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 @@ -136,14 +134,9 @@ 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 != "" { - var b []byte - var err error - if bodyFile == "-" { - b, err = ioutil.ReadAll(opts.IO.In) - } else { - b, err = ioutil.ReadFile(bodyFile) - } + b, err := cmdutil.ReadFile(bodyFile, opts.IO.In) if err != nil { return err } @@ -162,7 +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 from file. Use - to read the body from the standard input.") + 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/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) 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) +}