diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 160249e09..82d4ab133 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -81,19 +81,11 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co opts.BaseRepo = f.BaseRepo opts.HasRepoOverride = cmd.Flags().Changed("repo") - if err := cmdutil.MutuallyExclusive( - "specify only one of `--editor` or `--web`", - opts.EditorMode, - opts.WebMode, - ); err != nil { - return err - } - - config, err := f.Config() + var err error + opts.EditorMode, err = prShared.InitEditorMode(f, opts.EditorMode, opts.WebMode, opts.IO.CanPrompt()) if err != nil { return err } - opts.EditorMode = !opts.WebMode && (opts.EditorMode || config.PreferEditorPrompt("").Value == "enabled") titleProvided := cmd.Flags().Changed("title") bodyProvided := cmd.Flags().Changed("body") @@ -119,9 +111,6 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co if opts.Interactive && !opts.IO.CanPrompt() { return cmdutil.FlagErrorf("must provide `--title` and `--body` when not running interactively") } - if opts.EditorMode && !opts.IO.CanPrompt() { - return errors.New("--editor or enabled prefer_editor_prompt configuration are not supported in non-tty mode") - } if runF != nil { return runF(opts) @@ -133,7 +122,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` (use \"-\" to read from standard input)") - cmd.Flags().BoolVarP(&opts.EditorMode, "editor", "e", false, "Skip prompts and open the text editor to write the title and body in. The first line is the title and the rest text is the body.") + cmd.Flags().BoolVarP(&opts.EditorMode, "editor", "e", false, "Skip prompts and open the text editor to write the title and body in. The first line is the title and the remaining text is the body.") 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 c6c49ac32..7e893395c 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -30,15 +30,16 @@ import ( type CreateOptions struct { // This struct stores user input and factory functions - HttpClient func() (*http.Client, error) - GitClient *git.Client - Config func() (gh.Config, error) - IO *iostreams.IOStreams - Remotes func() (ghContext.Remotes, error) - Branch func() (string, error) - Browser browser.Browser - Prompter shared.Prompt - Finder shared.PRFinder + HttpClient func() (*http.Client, error) + GitClient *git.Client + Config func() (gh.Config, error) + IO *iostreams.IOStreams + Remotes func() (ghContext.Remotes, error) + Branch func() (string, error) + Browser browser.Browser + Prompter shared.Prompt + Finder shared.PRFinder + TitledEditSurvey func(string, string) (string, string, error) TitleProvided bool BodyProvided bool @@ -49,6 +50,7 @@ type CreateOptions struct { Autofill bool FillVerbose bool FillFirst bool + EditorMode bool WebMode bool RecoverFile string @@ -88,14 +90,15 @@ type CreateContext struct { func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { opts := &CreateOptions{ - IO: f.IOStreams, - HttpClient: f.HttpClient, - GitClient: f.GitClient, - Config: f.Config, - Remotes: f.Remotes, - Branch: f.Branch, - Browser: f.Browser, - Prompter: f.Prompter, + IO: f.IOStreams, + HttpClient: f.HttpClient, + GitClient: f.GitClient, + Config: f.Config, + Remotes: f.Remotes, + Branch: f.Branch, + Browser: f.Browser, + Prompter: f.Prompter, + TitledEditSurvey: shared.TitledEditSurvey(&shared.UserEditor{Config: f.Config, IO: f.IOStreams}), } var bodyFile string @@ -177,6 +180,20 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return cmdutil.FlagErrorf("`--fill-verbose` is not supported with `--fill`") } + if err := cmdutil.MutuallyExclusive( + "specify only one of `--editor` or `--web`", + opts.EditorMode, + opts.WebMode, + ); err != nil { + return err + } + + var err error + opts.EditorMode, err = shared.InitEditorMode(f, opts.EditorMode, opts.WebMode, opts.IO.CanPrompt()) + if err != nil { + return err + } + opts.BodyProvided = cmd.Flags().Changed("body") if bodyFile != "" { b, err := cmdutil.ReadFile(bodyFile, opts.IO.In) @@ -213,6 +230,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co fl.StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file` (use \"-\" to read from standard input)") 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.EditorMode, "editor", "e", false, "Skip prompts and open the text editor to write the title and body in. The first line is the title and the remaining text is the body.") fl.BoolVarP(&opts.WebMode, "web", "w", false, "Open the web browser to create a pull request") fl.BoolVarP(&opts.FillVerbose, "fill-verbose", "", false, "Use commits msg+body for description") fl.BoolVarP(&opts.Autofill, "fill", "f", false, "Use commit info for title and body") @@ -315,7 +333,7 @@ func createRun(opts *CreateOptions) (err error) { ghrepo.FullName(ctx.BaseRepo)) } - if opts.FillVerbose || opts.Autofill || opts.FillFirst || (opts.TitleProvided && opts.BodyProvided) { + if !opts.EditorMode && (opts.FillVerbose || opts.Autofill || opts.FillFirst || (opts.TitleProvided && opts.BodyProvided)) { err = handlePush(*opts, *ctx) if err != nil { return @@ -330,71 +348,101 @@ func createRun(opts *CreateOptions) (err error) { } } - if !opts.TitleProvided { - err = shared.TitleSurvey(opts.Prompter, state) - if err != nil { - return - } + action := shared.SubmitAction + if opts.IsDraft { + action = shared.SubmitDraftAction } - defer shared.PreserveInput(opts.IO, state, &err)() + tpl := shared.NewTemplateManager(client.HTTP(), ctx.BaseRepo, opts.Prompter, opts.RootDirOverride, opts.RepoOverride == "", true) - if !opts.BodyProvided { - templateContent := "" - if opts.RecoverFile == "" { - tpl := shared.NewTemplateManager(client.HTTP(), ctx.BaseRepo, opts.Prompter, opts.RootDirOverride, opts.RepoOverride == "", true) + if opts.EditorMode { + if opts.Template != "" { var template shared.Template - - if opts.Template != "" { - template, err = tpl.Select(opts.Template) - if err != nil { - return - } - } else { - template, err = tpl.Choose() - if err != nil { - return - } + template, err = tpl.Select(opts.Template) + if err != nil { + return } + if state.Title == "" { + state.Title = template.Title() + } + state.Body = string(template.Body()) + } - if template != nil { - templateContent = string(template.Body()) + state.Title, state.Body, err = opts.TitledEditSurvey(state.Title, state.Body) + if err != nil { + return + } + if state.Title == "" { + err = fmt.Errorf("title can't be blank") + return + } + } else { + + if !opts.TitleProvided { + err = shared.TitleSurvey(opts.Prompter, state) + if err != nil { + return } } - err = shared.BodySurvey(opts.Prompter, state, templateContent) - if err != nil { - return + defer shared.PreserveInput(opts.IO, state, &err)() + + if !opts.BodyProvided { + templateContent := "" + if opts.RecoverFile == "" { + var template shared.Template + + if opts.Template != "" { + template, err = tpl.Select(opts.Template) + if err != nil { + return + } + } else { + template, err = tpl.Choose() + if err != nil { + return + } + } + + if template != nil { + templateContent = string(template.Body()) + } + } + + err = shared.BodySurvey(opts.Prompter, state, templateContent) + if err != nil { + return + } } - } - openURL, err = generateCompareURL(*ctx, *state) - if err != nil { - return - } - - allowPreview := !state.HasMetadata() && shared.ValidURL(openURL) && !opts.DryRun - allowMetadata := ctx.BaseRepo.ViewerCanTriage() - action, err := shared.ConfirmPRSubmission(opts.Prompter, allowPreview, allowMetadata, state.Draft) - if err != nil { - return fmt.Errorf("unable to confirm: %w", err) - } - - if action == shared.MetadataAction { - fetcher := &shared.MetadataFetcher{ - IO: opts.IO, - APIClient: client, - Repo: ctx.BaseRepo, - State: state, - } - err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.BaseRepo, fetcher, state) + openURL, err = generateCompareURL(*ctx, *state) if err != nil { return } - action, err = shared.ConfirmPRSubmission(opts.Prompter, !state.HasMetadata() && !opts.DryRun, false, state.Draft) + allowPreview := !state.HasMetadata() && shared.ValidURL(openURL) && !opts.DryRun + allowMetadata := ctx.BaseRepo.ViewerCanTriage() + action, err = shared.ConfirmPRSubmission(opts.Prompter, allowPreview, allowMetadata, state.Draft) if err != nil { - return + return fmt.Errorf("unable to confirm: %w", err) + } + + if action == shared.MetadataAction { + fetcher := &shared.MetadataFetcher{ + IO: opts.IO, + APIClient: client, + Repo: ctx.BaseRepo, + State: state, + } + err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.BaseRepo, fetcher, state) + if err != nil { + return + } + + action, err = shared.ConfirmPRSubmission(opts.Prompter, !state.HasMetadata() && !opts.DryRun, false, state.Draft) + if err != nil { + return + } } } diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 3b64688c3..e0347945c 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -40,6 +40,7 @@ func TestNewCmdCreate(t *testing.T) { tty bool stdin string cli string + config string wantsErr bool wantsOpts CreateOptions }{ @@ -202,6 +203,64 @@ func TestNewCmdCreate(t *testing.T) { cli: "--web --dry-run", wantsErr: true, }, + { + name: "editor by cli", + tty: true, + cli: "--editor", + wantsErr: false, + wantsOpts: CreateOptions{ + Title: "", + Body: "", + RecoverFile: "", + WebMode: false, + EditorMode: true, + MaintainerCanModify: true, + }, + }, + { + name: "editor by config", + tty: true, + cli: "", + config: "prefer_editor_prompt: enabled", + wantsErr: false, + wantsOpts: CreateOptions{ + Title: "", + Body: "", + RecoverFile: "", + WebMode: false, + EditorMode: true, + MaintainerCanModify: true, + }, + }, + { + name: "editor and web", + tty: true, + cli: "--editor --web", + wantsErr: true, + }, + { + name: "can use web even though editor is enabled by config", + tty: true, + cli: `--web --title mytitle --body "issue body"`, + config: "prefer_editor_prompt: enabled", + wantsErr: false, + wantsOpts: CreateOptions{ + Title: "mytitle", + Body: "issue body", + TitleProvided: true, + BodyProvided: true, + RecoverFile: "", + WebMode: true, + EditorMode: false, + MaintainerCanModify: true, + }, + }, + { + name: "editor with non-tty", + tty: false, + cli: "--editor", + wantsErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -215,6 +274,12 @@ func TestNewCmdCreate(t *testing.T) { f := &cmdutil.Factory{ IOStreams: ios, + Config: func() (gh.Config, error) { + if tt.config != "" { + return config.NewFromString(tt.config), nil + } + return config.NewBlankConfig(), nil + }, } var opts *CreateOptions @@ -1372,6 +1437,33 @@ func Test_createRun(t *testing.T) { expectedOut: "https://github.com/OWNER/REPO/pull/12\n", expectedErrOut: "\nCreating pull request for feature into master in OWNER/REPO\n\n", }, + { + name: "editor", + httpStubs: func(r *httpmock.Registry, t *testing.T) { + r.Register( + httpmock.GraphQL(`mutation PullRequestCreate\b`), + httpmock.GraphQLMutation(` + { + "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } + } + `, func(inputs map[string]interface{}) { + assert.Equal(t, "title", inputs["title"]) + assert.Equal(t, "body", inputs["body"]) + })) + }, + setup: func(opts *CreateOptions, t *testing.T) func() { + opts.EditorMode = true + opts.HeadBranch = "feature" + opts.TitledEditSurvey = func(string, string) (string, string, error) { return "title", "body", nil } + return func() {} + }, + cmdStubs: func(cs *run.CommandStubber) { + cs.Register("git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b%x00 --cherry origin/master...feature", 0, "") + }, + expectedOut: "https://github.com/OWNER/REPO/pull/12\n", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 5f4bf7dd9..5b8bde0eb 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -1,6 +1,7 @@ package shared import ( + "errors" "fmt" "strings" @@ -357,3 +358,26 @@ func TitledEditSurvey(editor Editor) func(string, string) (string, string, error return title, strings.TrimSuffix(body, "\n"), nil } } + +func InitEditorMode(f *cmdutil.Factory, editorMode bool, webMode bool, canPrompt bool) (bool, error) { + if err := cmdutil.MutuallyExclusive( + "specify only one of `--editor` or `--web`", + editorMode, + webMode, + ); err != nil { + return false, err + } + + config, err := f.Config() + if err != nil { + return false, err + } + + editorMode = !webMode && (editorMode || config.PreferEditorPrompt("").Value == "enabled") + + if editorMode && !canPrompt { + return false, errors.New("--editor or enabled prefer_editor_prompt configuration are not supported in non-tty mode") + } + + return editorMode, nil +}