From 8f77b4a23ffc6d1f90add52013e617d7229a9b8a Mon Sep 17 00:00:00 2001 From: notomo Date: Sun, 19 Mar 2023 15:28:38 +0900 Subject: [PATCH 1/6] Add `issue create --editor` --- pkg/cmd/issue/create/create.go | 55 ++++++++--- pkg/cmd/issue/create/create_test.go | 101 +++++++++++++++++++++ pkg/cmd/pr/shared/survey.go | 20 ++++ pkg/cmd/pr/shared/templates.go | 18 +++- pkg/cmd/pr/shared/templates_test.go | 6 +- pkg/githubtemplate/github_template.go | 15 +++ pkg/githubtemplate/github_template_test.go | 61 +++++++++++++ 7 files changed, 260 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index c359df2a0..a83e74617 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -18,16 +18,18 @@ import ( ) type CreateOptions struct { - HttpClient func() (*http.Client, error) - Config func() (gh.Config, error) - IO *iostreams.IOStreams - BaseRepo func() (ghrepo.Interface, error) - Browser browser.Browser - Prompter prShared.Prompt + HttpClient func() (*http.Client, error) + Config func() (gh.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + Browser browser.Browser + Prompter prShared.Prompt + TitledEditSurvey func(string, string) (string, string, error) RootDirOverride string HasRepoOverride bool + EditorMode bool WebMode bool RecoverFile string @@ -44,11 +46,12 @@ type CreateOptions struct { func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { opts := &CreateOptions{ - IO: f.IOStreams, - HttpClient: f.HttpClient, - Config: f.Config, - Browser: f.Browser, - Prompter: f.Prompter, + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + Browser: f.Browser, + Prompter: f.Prompter, + TitledEditSurvey: prShared.TitledEditSurvey(f.Config, f.IOStreams), } var bodyFile string @@ -96,12 +99,20 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return errors.New("`--template` is not supported when using `--body` or `--body-file`") } - opts.Interactive = !(titleProvided && bodyProvided) + opts.Interactive = !opts.EditorMode && !(titleProvided && bodyProvided) if opts.Interactive && !opts.IO.CanPrompt() { return cmdutil.FlagErrorf("must provide `--title` and `--body` when not running interactively") } + if err := cmdutil.MutuallyExclusive( + "specify only one of `--editor` or `--web`", + opts.EditorMode, + opts.WebMode, + ); err != nil { + return err + } + if runF != nil { return runF(opts) } @@ -112,6 +123,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.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`") @@ -285,6 +297,25 @@ func createRun(opts *CreateOptions) (err error) { return } } else { + if opts.EditorMode { + if opts.Template != "" { + var template prShared.Template + template, err = tpl.Select(opts.Template) + if err != nil { + return + } + if tb.Title == "" { + tb.Title = template.Title() + } + templateNameForSubmit = template.NameForSubmit() + tb.Body = string(template.Body()) + } + + tb.Title, tb.Body, err = opts.TitledEditSurvey(tb.Title, tb.Body) + if err != nil { + return + } + } if tb.Title == "" { err = fmt.Errorf("title can't be blank") return diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index c70507327..476faa9ec 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -125,6 +125,41 @@ func TestNewCmdCreate(t *testing.T) { cli: `-t mytitle --template "bug report" --body-file "body_file.md"`, wantsErr: true, }, + { + name: "editor", + tty: false, + cli: "--editor", + wantsErr: false, + wantsOpts: CreateOptions{ + Title: "", + Body: "", + RecoverFile: "", + WebMode: false, + EditorMode: true, + Interactive: false, + }, + }, + { + name: "editor and template", + tty: false, + cli: `--editor --template "bug report"`, + wantsErr: false, + wantsOpts: CreateOptions{ + Title: "", + Body: "", + RecoverFile: "", + WebMode: false, + EditorMode: true, + Template: "bug report", + Interactive: false, + }, + }, + { + name: "editor and web", + tty: false, + cli: "--editor --web", + wantsErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -310,6 +345,72 @@ func Test_createRun(t *testing.T) { }, wantsErr: "cannot open in browser: maximum URL length exceeded", }, + { + name: "editor", + httpStubs: func(r *httpmock.Registry) { + r.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } }`)) + r.Register( + httpmock.GraphQL(`mutation IssueCreate\b`), + httpmock.GraphQLMutation(` + { "data": { "createIssue": { "issue": { + "URL": "https://github.com/OWNER/REPO/issues/12" + } } } } + `, func(inputs map[string]interface{}) { + assert.Equal(t, "title", inputs["title"]) + assert.Equal(t, "body", inputs["body"]) + })) + }, + opts: CreateOptions{ + EditorMode: true, + TitledEditSurvey: func(string, string) (string, string, error) { return "title", "body", nil }, + }, + wantsStdout: "https://github.com/OWNER/REPO/issues/12\n", + wantsStderr: "\nCreating issue in OWNER/REPO\n\n", + }, + { + name: "editor and template", + httpStubs: func(r *httpmock.Registry) { + r.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } }`)) + r.Register( + httpmock.GraphQL(`query IssueTemplates\b`), + httpmock.StringResponse(` + { "data": { "repository": { "issueTemplates": [ + { "name": "Bug report", + "title": "bug: ", + "body": "Does not work :((" } + ] } } }`), + ) + r.Register( + httpmock.GraphQL(`mutation IssueCreate\b`), + httpmock.GraphQLMutation(` + { "data": { "createIssue": { "issue": { + "URL": "https://github.com/OWNER/REPO/issues/12" + } } } } + `, func(inputs map[string]interface{}) { + assert.Equal(t, "bug: ", inputs["title"]) + assert.Equal(t, "Does not work :((", inputs["body"]) + })) + }, + opts: CreateOptions{ + EditorMode: true, + Template: "Bug report", + TitledEditSurvey: func(title string, body string) (string, string, error) { return title, body, nil }, + }, + wantsStdout: "https://github.com/OWNER/REPO/issues/12\n", + wantsStderr: "\nCreating issue in OWNER/REPO\n\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 393b87757..8396a4f16 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -5,8 +5,11 @@ import ( "strings" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/pkg/surveyext" ) type Action int @@ -317,3 +320,20 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface return nil } + +func TitledEditSurvey(cf func() (gh.Config, error), io *iostreams.IOStreams) func(string, string) (string, string, error) { + return func(initialTitle, initialBody string) (string, string, error) { + editorCommand, err := cmdutil.DetermineEditor(cf) + if err != nil { + return "", "", err + } + initialValue := initialTitle + "\n" + initialBody + titleAndBody, err := surveyext.Edit(editorCommand, "*.md", initialValue, io.In, io.Out, io.ErrOut) + if err != nil { + return "", "", err + } + titleAndBody = strings.ReplaceAll(titleAndBody, "\r\n", "\n") + title, body, _ := strings.Cut(titleAndBody, "\n") + return title, body, nil + } +} diff --git a/pkg/cmd/pr/shared/templates.go b/pkg/cmd/pr/shared/templates.go index f46a9f1d4..a3ffa13be 100644 --- a/pkg/cmd/pr/shared/templates.go +++ b/pkg/cmd/pr/shared/templates.go @@ -16,8 +16,9 @@ import ( ) type issueTemplate struct { - Gname string `graphql:"name"` - Gbody string `graphql:"body"` + Gname string `graphql:"name"` + Gbody string `graphql:"body"` + Gtitle string `graphql:"title"` } type pullRequestTemplate struct { @@ -37,6 +38,10 @@ func (t *issueTemplate) Body() []byte { return []byte(t.Gbody) } +func (t *issueTemplate) Title() string { + return t.Gtitle +} + func (t *pullRequestTemplate) Name() string { return t.Gname } @@ -49,6 +54,10 @@ func (t *pullRequestTemplate) Body() []byte { return []byte(t.Gbody) } +func (t *pullRequestTemplate) Title() string { + return "" +} + func listIssueTemplates(httpClient *http.Client, repo ghrepo.Interface) ([]Template, error) { var query struct { Repository struct { @@ -109,6 +118,7 @@ type Template interface { Name() string NameForSubmit() string Body() []byte + Title() string } type iprompter interface { @@ -294,3 +304,7 @@ func (t *filesystemTemplate) NameForSubmit() string { func (t *filesystemTemplate) Body() []byte { return githubtemplate.ExtractContents(t.path) } + +func (t *filesystemTemplate) Title() string { + return githubtemplate.ExtractTitle(t.path) +} diff --git a/pkg/cmd/pr/shared/templates_test.go b/pkg/cmd/pr/shared/templates_test.go index 319d9daa5..cb3dbb704 100644 --- a/pkg/cmd/pr/shared/templates_test.go +++ b/pkg/cmd/pr/shared/templates_test.go @@ -27,8 +27,8 @@ func TestTemplateManager_hasAPI(t *testing.T) { httpmock.GraphQL(`query IssueTemplates\b`), httpmock.StringResponse(`{"data":{"repository":{ "issueTemplates": [ - {"name": "Bug report", "body": "I found a problem"}, - {"name": "Feature request", "body": "I need a feature"} + {"name": "Bug report", "body": "I found a problem", "title": "bug: "}, + {"name": "Feature request", "body": "I need a feature", "title": "request: "} ] }}}`)) @@ -62,6 +62,7 @@ func TestTemplateManager_hasAPI(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "Feature request", tpl.NameForSubmit()) assert.Equal(t, "I need a feature", string(tpl.Body())) + assert.Equal(t, "request: ", tpl.Title()) } func TestTemplateManager_hasAPI_PullRequest(t *testing.T) { @@ -112,6 +113,7 @@ func TestTemplateManager_hasAPI_PullRequest(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "", tpl.NameForSubmit()) assert.Equal(t, "I fixed a problem", string(tpl.Body())) + assert.Equal(t, "", tpl.Title()) } func TestTemplateManagerSelect(t *testing.T) { diff --git a/pkg/githubtemplate/github_template.go b/pkg/githubtemplate/github_template.go index c52c3e4b4..4eb8b64e9 100644 --- a/pkg/githubtemplate/github_template.go +++ b/pkg/githubtemplate/github_template.go @@ -98,6 +98,21 @@ func ExtractName(filePath string) string { return path.Base(filePath) } +// ExtractTitle returns the title of the template from YAML front-matter +func ExtractTitle(filePath string) string { + contents, err := os.ReadFile(filePath) + frontmatterBoundaries := detectFrontmatter(contents) + if err == nil && frontmatterBoundaries[0] == 0 { + templateData := struct { + Title string + }{} + if err := yaml.Unmarshal(contents[0:frontmatterBoundaries[1]], &templateData); err == nil && templateData.Title != "" { + return templateData.Title + } + } + return "" +} + // ExtractContents returns the template contents without the YAML front-matter func ExtractContents(filePath string) []byte { contents, err := os.ReadFile(filePath) diff --git a/pkg/githubtemplate/github_template_test.go b/pkg/githubtemplate/github_template_test.go index eb31484a5..20709f7e6 100644 --- a/pkg/githubtemplate/github_template_test.go +++ b/pkg/githubtemplate/github_template_test.go @@ -319,6 +319,67 @@ about: This is how you report bugs } } +func TestExtractTitle(t *testing.T) { + tmpfile, err := os.CreateTemp(t.TempDir(), "gh-cli") + if err != nil { + t.Fatal(err) + } + defer tmpfile.Close() + + type args struct { + filePath string + } + tests := []struct { + name string + prepare string + args args + want string + }{ + { + name: "Complete front-matter", + prepare: `--- +name: Bug Report +title: 'bug: ' +about: This is how you report bugs +--- + +**Template contents** +`, + args: args{ + filePath: tmpfile.Name(), + }, + want: "bug: ", + }, + { + name: "Incomplete front-matter", + prepare: `--- +about: This is how you report bugs +--- +`, + args: args{ + filePath: tmpfile.Name(), + }, + want: "", + }, + { + name: "No front-matter", + prepare: `name: This is not yaml!`, + args: args{ + filePath: tmpfile.Name(), + }, + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _ = os.WriteFile(tmpfile.Name(), []byte(tt.prepare), 0600) + if got := ExtractTitle(tt.args.filePath); got != tt.want { + t.Errorf("ExtractTitle() = %v, want %v", got, tt.want) + } + }) + } +} + func TestExtractContents(t *testing.T) { tmpfile, err := os.CreateTemp(t.TempDir(), "gh-cli") if err != nil { From 30b32865333e7236bf5b6d5980697e03b82c5501 Mon Sep 17 00:00:00 2001 From: notomo Date: Wed, 3 Jul 2024 20:27:11 +0900 Subject: [PATCH 2/6] Add prefer_editor_prompt config --- internal/config/config.go | 41 ++++++++++++------ internal/config/stub.go | 3 ++ internal/gh/gh.go | 2 + internal/gh/mock/config.go | 72 +++++++++++++++++++++++++------- pkg/cmd/config/list/list_test.go | 2 + 5 files changed, 94 insertions(+), 26 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index fbdcdef95..29b66b73b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -15,18 +15,19 @@ import ( ) const ( - aliasesKey = "aliases" - browserKey = "browser" - editorKey = "editor" - gitProtocolKey = "git_protocol" - hostsKey = "hosts" - httpUnixSocketKey = "http_unix_socket" - oauthTokenKey = "oauth_token" - pagerKey = "pager" - promptKey = "prompt" - userKey = "user" - usersKey = "users" - versionKey = "version" + aliasesKey = "aliases" + browserKey = "browser" + editorKey = "editor" + gitProtocolKey = "git_protocol" + hostsKey = "hosts" + httpUnixSocketKey = "http_unix_socket" + oauthTokenKey = "oauth_token" + pagerKey = "pager" + promptKey = "prompt" + preferEditorPromptKey = "prefer_editor_prompt" + userKey = "user" + usersKey = "users" + versionKey = "version" ) func NewConfig() (gh.Config, error) { @@ -137,6 +138,11 @@ func (c *cfg) Prompt(hostname string) gh.ConfigEntry { return c.GetOrDefault(hostname, promptKey).Unwrap() } +func (c *cfg) PreferEditorPrompt(hostname string) gh.ConfigEntry { + // Intentionally panic if there is no user provided value or default value (which would be a programmer error) + return c.GetOrDefault(hostname, preferEditorPromptKey).Unwrap() +} + func (c *cfg) Version() o.Option[string] { return c.get("", versionKey) } @@ -509,6 +515,8 @@ git_protocol: https editor: # When to interactively prompt. This is a global config that cannot be overridden by hostname. Supported values: enabled, disabled prompt: enabled +# Preference for editor-based interactive prompting. This is a global config that cannot be overridden by hostname. Supported values: enabled, disabled +prefer_editor_prompt: disabled # A pager program to send command output to, e.g. "less". If blank, will refer to environment. Set the value to "cat" to disable the pager. pager: # Aliases allow you to create nicknames for gh commands @@ -555,6 +563,15 @@ var Options = []ConfigOption{ return c.Prompt(hostname).Value }, }, + { + Key: preferEditorPromptKey, + Description: "toggle preference for editor-based interactive prompting in the terminal", + DefaultValue: "disabled", + AllowedValues: []string{"enabled", "disabled"}, + CurrentValue: func(c gh.Config, hostname string) string { + return c.PreferEditorPrompt(hostname).Value + }, + }, { Key: pagerKey, Description: "the terminal pager program to send standard output to", diff --git a/internal/config/stub.go b/internal/config/stub.go index c89868721..71d44556d 100644 --- a/internal/config/stub.go +++ b/internal/config/stub.go @@ -70,6 +70,9 @@ func NewFromString(cfgStr string) *ghmock.ConfigMock { mock.PromptFunc = func(hostname string) gh.ConfigEntry { return cfg.Prompt(hostname) } + mock.PreferEditorPromptFunc = func(hostname string) gh.ConfigEntry { + return cfg.PreferEditorPrompt(hostname) + } mock.VersionFunc = func() o.Option[string] { return cfg.Version() } diff --git a/internal/gh/gh.go b/internal/gh/gh.go index a6db43d66..c39734075 100644 --- a/internal/gh/gh.go +++ b/internal/gh/gh.go @@ -47,6 +47,8 @@ type Config interface { Pager(hostname string) ConfigEntry // Prompt returns the configured prompt, optionally scoped by host. Prompt(hostname string) ConfigEntry + // PreferEditorPrompt returns the configured editor-based prompt, optionally scoped by host. + PreferEditorPrompt(hostname string) ConfigEntry // Aliases provides persistent storage and modification of command aliases. Aliases() AliasConfig diff --git a/internal/gh/mock/config.go b/internal/gh/mock/config.go index 502047240..569af1fac 100644 --- a/internal/gh/mock/config.go +++ b/internal/gh/mock/config.go @@ -49,6 +49,9 @@ var _ gh.Config = &ConfigMock{} // PagerFunc: func(hostname string) gh.ConfigEntry { // panic("mock out the Pager method") // }, +// PreferEditorPromptFunc: func(hostname string) gh.ConfigEntry { +// panic("mock out the PreferEditorPrompt method") +// }, // PromptFunc: func(hostname string) gh.ConfigEntry { // panic("mock out the Prompt method") // }, @@ -98,6 +101,9 @@ type ConfigMock struct { // PagerFunc mocks the Pager method. PagerFunc func(hostname string) gh.ConfigEntry + // PreferEditorPromptFunc mocks the PreferEditorPrompt method. + PreferEditorPromptFunc func(hostname string) gh.ConfigEntry + // PromptFunc mocks the Prompt method. PromptFunc func(hostname string) gh.ConfigEntry @@ -158,6 +164,11 @@ type ConfigMock struct { // Hostname is the hostname argument value. Hostname string } + // PreferEditorPrompt holds details about calls to the PreferEditorPrompt method. + PreferEditorPrompt []struct { + // Hostname is the hostname argument value. + Hostname string + } // Prompt holds details about calls to the Prompt method. Prompt []struct { // Hostname is the hostname argument value. @@ -179,20 +190,21 @@ type ConfigMock struct { Write []struct { } } - lockAliases sync.RWMutex - lockAuthentication sync.RWMutex - lockBrowser sync.RWMutex - lockCacheDir sync.RWMutex - lockEditor sync.RWMutex - lockGetOrDefault sync.RWMutex - lockGitProtocol sync.RWMutex - lockHTTPUnixSocket sync.RWMutex - lockMigrate sync.RWMutex - lockPager sync.RWMutex - lockPrompt sync.RWMutex - lockSet sync.RWMutex - lockVersion sync.RWMutex - lockWrite sync.RWMutex + lockAliases sync.RWMutex + lockAuthentication sync.RWMutex + lockBrowser sync.RWMutex + lockCacheDir sync.RWMutex + lockEditor sync.RWMutex + lockGetOrDefault sync.RWMutex + lockGitProtocol sync.RWMutex + lockHTTPUnixSocket sync.RWMutex + lockMigrate sync.RWMutex + lockPager sync.RWMutex + lockPreferEditorPrompt sync.RWMutex + lockPrompt sync.RWMutex + lockSet sync.RWMutex + lockVersion sync.RWMutex + lockWrite sync.RWMutex } // Aliases calls AliasesFunc. @@ -504,6 +516,38 @@ func (mock *ConfigMock) PagerCalls() []struct { return calls } +// PreferEditorPrompt calls PreferEditorPromptFunc. +func (mock *ConfigMock) PreferEditorPrompt(hostname string) gh.ConfigEntry { + if mock.PreferEditorPromptFunc == nil { + panic("ConfigMock.PreferEditorPromptFunc: method is nil but Config.PreferEditorPrompt was just called") + } + callInfo := struct { + Hostname string + }{ + Hostname: hostname, + } + mock.lockPreferEditorPrompt.Lock() + mock.calls.PreferEditorPrompt = append(mock.calls.PreferEditorPrompt, callInfo) + mock.lockPreferEditorPrompt.Unlock() + return mock.PreferEditorPromptFunc(hostname) +} + +// PreferEditorPromptCalls gets all the calls that were made to PreferEditorPrompt. +// Check the length with: +// +// len(mockedConfig.PreferEditorPromptCalls()) +func (mock *ConfigMock) PreferEditorPromptCalls() []struct { + Hostname string +} { + var calls []struct { + Hostname string + } + mock.lockPreferEditorPrompt.RLock() + calls = mock.calls.PreferEditorPrompt + mock.lockPreferEditorPrompt.RUnlock() + return calls +} + // Prompt calls PromptFunc. func (mock *ConfigMock) Prompt(hostname string) gh.ConfigEntry { if mock.PromptFunc == nil { diff --git a/pkg/cmd/config/list/list_test.go b/pkg/cmd/config/list/list_test.go index b6a9d0fca..65f83d659 100644 --- a/pkg/cmd/config/list/list_test.go +++ b/pkg/cmd/config/list/list_test.go @@ -84,6 +84,7 @@ func Test_listRun(t *testing.T) { cfg.Set("HOST", "git_protocol", "ssh") cfg.Set("HOST", "editor", "/usr/bin/vim") cfg.Set("HOST", "prompt", "disabled") + cfg.Set("HOST", "prefer_editor_prompt", "enabled") cfg.Set("HOST", "pager", "less") cfg.Set("HOST", "http_unix_socket", "") cfg.Set("HOST", "browser", "brave") @@ -93,6 +94,7 @@ func Test_listRun(t *testing.T) { stdout: `git_protocol=ssh editor=/usr/bin/vim prompt=disabled +prefer_editor_prompt=enabled pager=less http_unix_socket= browser=brave From 50d0cb72799807aa1456b04d5d3b22222e2c1e10 Mon Sep 17 00:00:00 2001 From: notomo Date: Thu, 4 Jul 2024 20:31:24 +0900 Subject: [PATCH 3/6] Use prefer_editor_prompt config by `issue create` --- pkg/cmd/issue/create/create.go | 6 ++++++ pkg/cmd/issue/create/create_test.go | 24 +++++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index a83e74617..dcf75e040 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -80,6 +80,12 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co opts.BaseRepo = f.BaseRepo opts.HasRepoOverride = cmd.Flags().Changed("repo") + config, err := f.Config() + if err != nil { + return err + } + opts.EditorMode = opts.EditorMode || config.PreferEditorPrompt("").Value == "enabled" + titleProvided := cmd.Flags().Changed("title") bodyProvided := cmd.Flags().Changed("body") if bodyFile != "" { diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 476faa9ec..74c9d0d9a 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -38,6 +38,7 @@ func TestNewCmdCreate(t *testing.T) { tty bool stdin string cli string + config string wantsErr bool wantsOpts CreateOptions }{ @@ -126,7 +127,7 @@ func TestNewCmdCreate(t *testing.T) { wantsErr: true, }, { - name: "editor", + name: "editor by cli", tty: false, cli: "--editor", wantsErr: false, @@ -139,6 +140,21 @@ func TestNewCmdCreate(t *testing.T) { Interactive: false, }, }, + { + name: "editor by config", + tty: false, + cli: "", + config: "prefer_editor_prompt: enabled", + wantsErr: false, + wantsOpts: CreateOptions{ + Title: "", + Body: "", + RecoverFile: "", + WebMode: false, + EditorMode: true, + Interactive: false, + }, + }, { name: "editor and template", tty: false, @@ -173,6 +189,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 From 0d37174076f0a3d78921cc20ab08a7c9f1493c89 Mon Sep 17 00:00:00 2001 From: notomo Date: Tue, 9 Jul 2024 08:26:10 +0900 Subject: [PATCH 4/6] Add editor hint message --- pkg/cmd/issue/create/create.go | 2 +- pkg/cmd/pr/shared/survey.go | 36 +++++++++++++++++++++++++------- pkg/cmd/pr/shared/survey_test.go | 35 +++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index dcf75e040..220906340 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -51,7 +51,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co Config: f.Config, Browser: f.Browser, Prompter: f.Prompter, - TitledEditSurvey: prShared.TitledEditSurvey(f.Config, f.IOStreams), + TitledEditSurvey: prShared.TitledEditSurvey(&prShared.UserEditor{Config: f.Config, IO: f.IOStreams}), } var bodyFile string diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 8396a4f16..5f4bf7dd9 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -321,19 +321,39 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface return nil } -func TitledEditSurvey(cf func() (gh.Config, error), io *iostreams.IOStreams) func(string, string) (string, string, error) { +type Editor interface { + Edit(filename, initialValue string) (string, error) +} + +type UserEditor struct { + IO *iostreams.IOStreams + Config func() (gh.Config, error) +} + +func (e *UserEditor) Edit(filename, initialValue string) (string, error) { + editorCommand, err := cmdutil.DetermineEditor(e.Config) + if err != nil { + return "", err + } + return surveyext.Edit(editorCommand, filename, initialValue, e.IO.In, e.IO.Out, e.IO.ErrOut) +} + +const editorHintMarker = "------------------------ >8 ------------------------" +const editorHint = ` +Please Enter the title on the first line and the body on subsequent lines. +Lines below dotted lines will be ignored, and an empty title aborts the creation process.` + +func TitledEditSurvey(editor Editor) func(string, string) (string, string, error) { return func(initialTitle, initialBody string) (string, string, error) { - editorCommand, err := cmdutil.DetermineEditor(cf) - if err != nil { - return "", "", err - } - initialValue := initialTitle + "\n" + initialBody - titleAndBody, err := surveyext.Edit(editorCommand, "*.md", initialValue, io.In, io.Out, io.ErrOut) + initialValue := strings.Join([]string{initialTitle, initialBody, editorHintMarker, editorHint}, "\n") + titleAndBody, err := editor.Edit("*.md", initialValue) if err != nil { return "", "", err } + titleAndBody = strings.ReplaceAll(titleAndBody, "\r\n", "\n") + titleAndBody, _, _ = strings.Cut(titleAndBody, editorHintMarker) title, body, _ := strings.Cut(titleAndBody, "\n") - return title, body, nil + return title, strings.TrimSuffix(body, "\n"), nil } } diff --git a/pkg/cmd/pr/shared/survey_test.go b/pkg/cmd/pr/shared/survey_test.go index 4653ce08b..8df492785 100644 --- a/pkg/cmd/pr/shared/survey_test.go +++ b/pkg/cmd/pr/shared/survey_test.go @@ -123,3 +123,38 @@ func TestMetadataSurvey_keepExisting(t *testing.T) { assert.Equal(t, []string{"good first issue"}, state.Labels) assert.Equal(t, []string{"The road to 1.0"}, state.Projects) } + +func TestTitledEditSurvey_cleanupHint(t *testing.T) { + var editorInitialText string + editor := &testEditor{ + edit: func(s string) (string, error) { + editorInitialText = s + return `editedTitle +editedBody +------------------------ >8 ------------------------ + +Please Enter the title on the first line and the body on subsequent lines. +Lines below dotted lines will be ignored, and an empty title aborts the creation process.`, nil + }, + } + + title, body, err := TitledEditSurvey(editor)("initialTitle", "initialBody") + assert.NoError(t, err) + + assert.Equal(t, `initialTitle +initialBody +------------------------ >8 ------------------------ + +Please Enter the title on the first line and the body on subsequent lines. +Lines below dotted lines will be ignored, and an empty title aborts the creation process.`, editorInitialText) + assert.Equal(t, "editedTitle", title) + assert.Equal(t, "editedBody", body) +} + +type testEditor struct { + edit func(string) (string, error) +} + +func (e testEditor) Edit(filename, text string) (string, error) { + return e.edit(text) +} From 0b32e66bba57587d5a2399dd9a9405d9f32c198d Mon Sep 17 00:00:00 2001 From: notomo Date: Tue, 9 Jul 2024 20:42:50 +0900 Subject: [PATCH 5/6] Enable to use --web even though editor is enabled by config --- pkg/cmd/issue/create/create.go | 18 +++++++++--------- pkg/cmd/issue/create/create_test.go | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 220906340..98b05a5e3 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -80,11 +80,19 @@ 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() if err != nil { return err } - opts.EditorMode = opts.EditorMode || config.PreferEditorPrompt("").Value == "enabled" + opts.EditorMode = !opts.WebMode && (opts.EditorMode || config.PreferEditorPrompt("").Value == "enabled") titleProvided := cmd.Flags().Changed("title") bodyProvided := cmd.Flags().Changed("body") @@ -111,14 +119,6 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return cmdutil.FlagErrorf("must provide `--title` and `--body` when not running interactively") } - if err := cmdutil.MutuallyExclusive( - "specify only one of `--editor` or `--web`", - opts.EditorMode, - opts.WebMode, - ); err != nil { - return err - } - if runF != nil { return runF(opts) } diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 74c9d0d9a..bd544990f 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -176,6 +176,21 @@ func TestNewCmdCreate(t *testing.T) { cli: "--editor --web", wantsErr: true, }, + { + name: "can use web even though editor is enabled by config", + tty: false, + cli: `--web --title mytitle --body "issue body"`, + config: "prefer_editor_prompt: enabled", + wantsErr: false, + wantsOpts: CreateOptions{ + Title: "mytitle", + Body: "issue body", + RecoverFile: "", + WebMode: true, + EditorMode: false, + Interactive: false, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 28cc56cd456b51b3858e0556228ba26373777513 Mon Sep 17 00:00:00 2001 From: notomo Date: Sat, 13 Jul 2024 11:01:48 +0900 Subject: [PATCH 6/6] Raise error if editor is used in non-tty mode --- pkg/cmd/issue/create/create.go | 3 +++ pkg/cmd/issue/create/create_test.go | 16 +++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 98b05a5e3..6de78f2e1 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -118,6 +118,9 @@ 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) diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index bd544990f..9ae959c57 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -128,7 +128,7 @@ func TestNewCmdCreate(t *testing.T) { }, { name: "editor by cli", - tty: false, + tty: true, cli: "--editor", wantsErr: false, wantsOpts: CreateOptions{ @@ -142,7 +142,7 @@ func TestNewCmdCreate(t *testing.T) { }, { name: "editor by config", - tty: false, + tty: true, cli: "", config: "prefer_editor_prompt: enabled", wantsErr: false, @@ -157,7 +157,7 @@ func TestNewCmdCreate(t *testing.T) { }, { name: "editor and template", - tty: false, + tty: true, cli: `--editor --template "bug report"`, wantsErr: false, wantsOpts: CreateOptions{ @@ -172,13 +172,13 @@ func TestNewCmdCreate(t *testing.T) { }, { name: "editor and web", - tty: false, + tty: true, cli: "--editor --web", wantsErr: true, }, { name: "can use web even though editor is enabled by config", - tty: false, + tty: true, cli: `--web --title mytitle --body "issue body"`, config: "prefer_editor_prompt: enabled", wantsErr: false, @@ -191,6 +191,12 @@ func TestNewCmdCreate(t *testing.T) { Interactive: false, }, }, + { + name: "editor with non-tty", + tty: false, + cli: "--editor", + wantsErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {