diff --git a/pkg/cmd/issue/comment/comment.go b/pkg/cmd/issue/comment/comment.go index 3bd9df598..c91cf79c9 100644 --- a/pkg/cmd/issue/comment/comment.go +++ b/pkg/cmd/issue/comment/comment.go @@ -15,7 +15,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e HttpClient: f.HttpClient, EditSurvey: prShared.CommentableEditSurvey(f.Config, f.IOStreams), InteractiveEditSurvey: prShared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams), - ConfirmSubmitSurvey: prShared.CommentableConfirmSubmitSurvey, + ConfirmSubmitSurvey: prShared.CommentableConfirmSubmitSurvey(f.Prompter), OpenInBrowser: f.Browser.Browse, } diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index c6f057388..081e55b20 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -23,6 +23,7 @@ type CreateOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) Browser browser.Browser + Prompter prShared.Prompt RootDirOverride string @@ -46,6 +47,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co HttpClient: f.HttpClient, Config: f.Config, Browser: f.Browser, + Prompter: f.Prompter, } var bodyFile string @@ -152,7 +154,7 @@ func createRun(opts *CreateOptions) (err error) { } } - tpl := shared.NewTemplateManager(httpClient, baseRepo, opts.RootDirOverride, !opts.HasRepoOverride, false) + tpl := shared.NewTemplateManager(httpClient, baseRepo, opts.Prompter, opts.RootDirOverride, !opts.HasRepoOverride, false) if opts.WebMode { var openURL string @@ -194,16 +196,10 @@ func createRun(opts *CreateOptions) (err error) { var openURL string if opts.Interactive { - var editorCommand string - editorCommand, err = cmdutil.DetermineEditor(opts.Config) - if err != nil { - return - } - defer prShared.PreserveInput(opts.IO, &tb, &err)() if opts.Title == "" { - err = prShared.TitleSurvey(&tb) + err = prShared.TitleSurvey(opts.Prompter, &tb) if err != nil { return } @@ -227,7 +223,7 @@ func createRun(opts *CreateOptions) (err error) { } } - err = prShared.BodySurvey(&tb, templateContent, editorCommand) + err = prShared.BodySurvey(opts.Prompter, &tb, templateContent) if err != nil { return } @@ -239,7 +235,7 @@ func createRun(opts *CreateOptions) (err error) { } allowPreview := !tb.HasMetadata() && prShared.ValidURL(openURL) - action, err = prShared.ConfirmIssueSubmission(allowPreview, repo.ViewerCanTriage()) + action, err = prShared.ConfirmIssueSubmission(opts.Prompter, allowPreview, repo.ViewerCanTriage()) if err != nil { err = fmt.Errorf("unable to confirm: %w", err) return @@ -257,7 +253,7 @@ func createRun(opts *CreateOptions) (err error) { return } - action, err = prShared.ConfirmIssueSubmission(!tb.HasMetadata(), false) + action, err = prShared.ConfirmIssueSubmission(opts.Prompter, !tb.HasMetadata(), false) if err != nil { return } diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 9fc9c30ea..9bfaea8b5 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -15,12 +15,12 @@ import ( "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/run" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/cli/cli/v2/test" "github.com/google/shlex" "github.com/stretchr/testify/assert" @@ -288,11 +288,11 @@ func Test_createRun(t *testing.T) { /*** LEGACY TESTS ***/ -func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { - return runCommandWithRootDirOverridden(rt, isTTY, cli, "") +func runCommand(rt http.RoundTripper, isTTY bool, cli string, pm *prompter.PrompterMock) (*test.CmdOut, error) { + return runCommandWithRootDirOverridden(rt, isTTY, cli, "", pm) } -func runCommandWithRootDirOverridden(rt http.RoundTripper, isTTY bool, cli string, rootDir string) (*test.CmdOut, error) { +func runCommandWithRootDirOverridden(rt http.RoundTripper, isTTY bool, cli string, rootDir string, pm *prompter.PrompterMock) (*test.CmdOut, error) { ios, _, stdout, stderr := iostreams.Test() ios.SetStdoutTTY(isTTY) ios.SetStdinTTY(isTTY) @@ -310,7 +310,8 @@ func runCommandWithRootDirOverridden(rt http.RoundTripper, isTTY bool, cli strin BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - Browser: browser, + Browser: browser, + Prompter: pm, } cmd := NewCmdCreate(factory, func(opts *CreateOptions) error { @@ -361,7 +362,7 @@ func TestIssueCreate(t *testing.T) { }), ) - output, err := runCommand(http, true, `-t hello -b "cash rules everything around me"`) + output, err := runCommand(http, true, `-t hello -b "cash rules everything around me"`, nil) if err != nil { t.Errorf("error running command `issue create`: %v", err) } @@ -403,12 +404,28 @@ func TestIssueCreate_recover(t *testing.T) { assert.Equal(t, []interface{}{"BUGID", "TODOID"}, inputs["labelIds"]) })) - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - - as.StubPrompt("Title").AnswerDefault() - as.StubPrompt("Body").AnswerDefault() - as.StubPrompt("What's next?").AnswerWith("Submit") + pm := &prompter.PrompterMock{} + pm.InputFunc = func(p, d string) (string, error) { + if p == "Title" { + return d, nil + } else { + return "", prompter.NoSuchPromptErr(p) + } + } + pm.MarkdownEditorFunc = func(p, d string, ba bool) (string, error) { + if p == "Body" { + return d, nil + } else { + return "", prompter.NoSuchPromptErr(p) + } + } + pm.SelectFunc = func(p, _ string, opts []string) (int, error) { + if p == "What's next?" { + return prompter.IndexFor(opts, "Submit") + } else { + return -1, prompter.NoSuchPromptErr(p) + } + } tmpfile, err := os.CreateTemp(t.TempDir(), "testrecover*") assert.NoError(t, err) @@ -428,7 +445,7 @@ func TestIssueCreate_recover(t *testing.T) { args := fmt.Sprintf("--recover '%s'", tmpfile.Name()) - output, err := runCommandWithRootDirOverridden(http, true, args, "") + output, err := runCommandWithRootDirOverridden(http, true, args, "", pm) if err != nil { t.Errorf("error running command `issue create`: %v", err) } @@ -471,16 +488,26 @@ func TestIssueCreate_nonLegacyTemplate(t *testing.T) { }), ) - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) + pm := &prompter.PrompterMock{} + pm.MarkdownEditorFunc = func(p, d string, ba bool) (string, error) { + if p == "Body" { + return d, nil + } else { + return "", prompter.NoSuchPromptErr(p) + } + } + pm.SelectFunc = func(p, _ string, opts []string) (int, error) { + switch p { + case "What's next?": + return prompter.IndexFor(opts, "Submit") + case "Choose a template": + return prompter.IndexFor(opts, "Submit a request") + default: + return -1, prompter.NoSuchPromptErr(p) + } + } - as.StubPrompt("Choose a template").AnswerWith("Submit a request") - as.StubPrompt("Body").AnswerDefault() - as.StubPrompt("What's next?"). - AssertOptions([]string{"Submit", "Continue in browser", "Cancel"}). - AnswerWith("Submit") - - output, err := runCommandWithRootDirOverridden(http, true, `-t hello`, "./fixtures/repoWithNonLegacyIssueTemplates") + output, err := runCommandWithRootDirOverridden(http, true, `-t hello`, "./fixtures/repoWithNonLegacyIssueTemplates", pm) if err != nil { t.Errorf("error running command `issue create`: %v", err) } @@ -502,16 +529,26 @@ func TestIssueCreate_continueInBrowser(t *testing.T) { } } }`), ) - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - - as.StubPrompt("Title").AnswerWith("hello") - as.StubPrompt("What's next?").AnswerWith("Continue in browser") + pm := &prompter.PrompterMock{} + pm.InputFunc = func(p, d string) (string, error) { + if p == "Title" { + return "hello", nil + } else { + return "", prompter.NoSuchPromptErr(p) + } + } + pm.SelectFunc = func(p, _ string, opts []string) (int, error) { + if p == "What's next?" { + return prompter.IndexFor(opts, "Continue in browser") + } else { + return -1, prompter.NoSuchPromptErr(p) + } + } _, cmdTeardown := run.Stub() defer cmdTeardown(t) - output, err := runCommand(http, true, `-b body`) + output, err := runCommand(http, true, `-b body`, pm) if err != nil { t.Errorf("error running command `issue create`: %v", err) } @@ -596,7 +633,7 @@ func TestIssueCreate_metadata(t *testing.T) { } })) - output, err := runCommand(http, true, `-t TITLE -b BODY -a monalisa -l bug -l todo -p roadmap -m 'big one.oh'`) + output, err := runCommand(http, true, `-t TITLE -b BODY -a monalisa -l bug -l todo -p roadmap -m 'big one.oh'`, nil) if err != nil { t.Errorf("error running command `issue create`: %v", err) } @@ -617,7 +654,7 @@ func TestIssueCreate_disabledIssues(t *testing.T) { } } }`), ) - _, err := runCommand(http, true, `-t heres -b johnny`) + _, err := runCommand(http, true, `-t heres -b johnny`, nil) if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { t.Errorf("error running command `issue create`: %v", err) } @@ -668,7 +705,7 @@ func TestIssueCreate_AtMeAssignee(t *testing.T) { assert.Equal(t, []interface{}{"MONAID", "SOMEID"}, inputs["assigneeIds"]) })) - output, err := runCommand(http, true, `-a @me -a someoneelse -t hello -b "cash rules everything around me"`) + output, err := runCommand(http, true, `-a @me -a someoneelse -t hello -b "cash rules everything around me"`, nil) if err != nil { t.Errorf("error running command `issue create`: %v", err) } diff --git a/pkg/cmd/pr/comment/comment.go b/pkg/cmd/pr/comment/comment.go index 34644f62b..31a2bc25c 100644 --- a/pkg/cmd/pr/comment/comment.go +++ b/pkg/cmd/pr/comment/comment.go @@ -14,7 +14,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*shared.CommentableOptions) err HttpClient: f.HttpClient, EditSurvey: shared.CommentableEditSurvey(f.Config, f.IOStreams), InteractiveEditSurvey: shared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams), - ConfirmSubmitSurvey: shared.CommentableConfirmSubmitSurvey, + ConfirmSubmitSurvey: shared.CommentableConfirmSubmitSurvey(f.Prompter), OpenInBrowser: f.Browser.Browse, } diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 4066767bc..3c020d4a1 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -10,7 +10,6 @@ import ( "strings" "time" - "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" ghContext "github.com/cli/cli/v2/context" @@ -22,14 +21,9 @@ import ( "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/spf13/cobra" ) -type iprompter interface { - Select(string, string, []string) (int, error) -} - type CreateOptions struct { // This struct stores user input and factory functions HttpClient func() (*http.Client, error) @@ -39,7 +33,7 @@ type CreateOptions struct { Remotes func() (ghContext.Remotes, error) Branch func() (string, error) Browser browser.Browser - Prompter iprompter + Prompter shared.Prompt Finder shared.PRFinder TitleProvided bool @@ -276,23 +270,18 @@ func createRun(opts *CreateOptions) (err error) { } if !opts.TitleProvided { - err = shared.TitleSurvey(state) + err = shared.TitleSurvey(opts.Prompter, state) if err != nil { return } } - editorCommand, err := cmdutil.DetermineEditor(opts.Config) - if err != nil { - return - } - defer shared.PreserveInput(opts.IO, state, &err)() if !opts.BodyProvided { templateContent := "" if opts.RecoverFile == "" { - tpl := shared.NewTemplateManager(client.HTTP(), ctx.BaseRepo, opts.RootDirOverride, opts.RepoOverride == "", true) + tpl := shared.NewTemplateManager(client.HTTP(), ctx.BaseRepo, opts.Prompter, opts.RootDirOverride, opts.RepoOverride == "", true) var template shared.Template template, err = tpl.Choose() if err != nil { @@ -306,7 +295,7 @@ func createRun(opts *CreateOptions) (err error) { } } - err = shared.BodySurvey(state, templateContent, editorCommand) + err = shared.BodySurvey(opts.Prompter, state, templateContent) if err != nil { return } @@ -319,7 +308,7 @@ func createRun(opts *CreateOptions) (err error) { allowPreview := !state.HasMetadata() && shared.ValidURL(openURL) allowMetadata := ctx.BaseRepo.ViewerCanTriage() - action, err := shared.ConfirmPRSubmission(allowPreview, allowMetadata, state.Draft) + action, err := shared.ConfirmPRSubmission(opts.Prompter, allowPreview, allowMetadata, state.Draft) if err != nil { return fmt.Errorf("unable to confirm: %w", err) } @@ -336,7 +325,7 @@ func createRun(opts *CreateOptions) (err error) { return } - action, err = shared.ConfirmPRSubmission(!state.HasMetadata(), false, state.Draft) + action, err = shared.ConfirmPRSubmission(opts.Prompter, !state.HasMetadata(), false, state.Draft) if err != nil { return } @@ -582,12 +571,7 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) { pushOptions = append(pushOptions, "Skip pushing the branch") pushOptions = append(pushOptions, "Cancel") - var selectedOption int - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err = prompt.SurveyAskOne(&survey.Select{ - Message: fmt.Sprintf("Where should we push the '%s' branch?", headBranch), - Options: pushOptions, - }, &selectedOption) + selectedOption, err := opts.Prompter.Select(fmt.Sprintf("Where should we push the '%s' branch?", headBranch), "", pushOptions) if err != nil { return nil, err } diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 125c479b3..26e2a71d0 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -16,12 +16,12 @@ import ( "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/run" "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/cli/cli/v2/test" "github.com/google/shlex" "github.com/stretchr/testify/assert" @@ -187,7 +187,7 @@ func Test_createRun(t *testing.T) { name string setup func(*CreateOptions, *testing.T) func() cmdStubs func(*run.CommandStubber) - askStubs func(*prompt.AskStubber) // TODO eventually migrate to PrompterMock + promptStubs func(*prompter.PrompterMock) httpStubs func(*httpmock.Registry, *testing.T) expectedOut string expectedErrOut string @@ -268,8 +268,14 @@ func Test_createRun(t *testing.T) { cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Where should we push the 'feature' branch?").AnswerDefault() + promptStubs: func(pm *prompter.PrompterMock) { + pm.SelectFunc = func(p, _ string, opts []string) (int, error) { + if p == "Where should we push the 'feature' branch?" { + return 0, nil + } else { + return -1, prompter.NoSuchPromptErr(p) + } + } }, expectedOut: "https://github.com/OWNER/REPO/pull/12\n", expectedErrOut: "\nCreating pull request for feature into master in OWNER/REPO\n\n", @@ -309,8 +315,14 @@ func Test_createRun(t *testing.T) { cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Where should we push the 'feature' branch?").AnswerDefault() + promptStubs: func(pm *prompter.PrompterMock) { + pm.SelectFunc = func(p, _ string, opts []string) (int, error) { + if p == "Where should we push the 'feature' branch?" { + return 0, nil + } else { + return -1, prompter.NoSuchPromptErr(p) + } + } }, expectedOut: "https://github.com/OWNER/REPO/pull/12\n", expectedErrOut: "\nCreating pull request for feature into master in OWNER/REPO\n\n", @@ -354,10 +366,14 @@ func Test_createRun(t *testing.T) { cs.Register(`git remote add -f fork https://github.com/monalisa/REPO.git`, 0, "") cs.Register(`git push --set-upstream fork HEAD:feature`, 0, "") }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Where should we push the 'feature' branch?"). - AssertOptions([]string{"OWNER/REPO", "Create a fork of OWNER/REPO", "Skip pushing the branch", "Cancel"}). - AnswerWith("Create a fork of OWNER/REPO") + promptStubs: func(pm *prompter.PrompterMock) { + pm.SelectFunc = func(p, _ string, opts []string) (int, error) { + if p == "Where should we push the 'feature' branch?" { + return prompter.IndexFor(opts, "Create a fork of OWNER/REPO") + } else { + return -1, prompter.NoSuchPromptErr(p) + } + } }, expectedOut: "https://github.com/OWNER/REPO/pull/12\n", expectedErrOut: "\nCreating pull request for monalisa:feature into master in OWNER/REPO\n\n", @@ -482,14 +498,24 @@ func Test_createRun(t *testing.T) { cmdStubs: func(cs *run.CommandStubber) { cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "1234567890,commit 0\n2345678901,commit 1") }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Choose a template"). - AssertOptions([]string{"template1", "template2", "Open a blank pull request"}). - AnswerWith("template1") - as.StubPrompt("Body").AnswerDefault() - as.StubPrompt("What's next?"). - AssertOptions([]string{"Submit", "Submit as draft", "Continue in browser", "Add metadata", "Cancel"}). - AnswerDefault() + promptStubs: func(pm *prompter.PrompterMock) { + pm.MarkdownEditorFunc = func(p, d string, ba bool) (string, error) { + if p == "Body" { + return d, nil + } else { + return "", prompter.NoSuchPromptErr(p) + } + } + pm.SelectFunc = func(p, _ string, opts []string) (int, error) { + switch p { + case "What's next?": + return 0, nil + case "Choose a template": + return prompter.IndexFor(opts, "template1") + default: + return -1, prompter.NoSuchPromptErr(p) + } + } }, expectedOut: "https://github.com/OWNER/REPO/pull/12\n", expectedErrOut: "\nCreating pull request for feature into master in OWNER/REPO\n\n", @@ -636,10 +662,14 @@ func Test_createRun(t *testing.T) { cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Where should we push the 'feature' branch?"). - AssertOptions([]string{"OWNER/REPO", "Skip pushing the branch", "Cancel"}). - AnswerDefault() + promptStubs: func(pm *prompter.PrompterMock) { + pm.SelectFunc = func(p, _ string, opts []string) (int, error) { + if p == "Where should we push the 'feature' branch?" { + return 0, nil + } else { + return -1, prompter.NoSuchPromptErr(p) + } + } }, expectedErrOut: "Opening github.com/OWNER/REPO/compare/master...feature in your browser.\n", expectedBrowse: "https://github.com/OWNER/REPO/compare/master...feature?body=&expand=1", @@ -685,8 +715,14 @@ func Test_createRun(t *testing.T) { cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Where should we push the 'feature' branch?").AnswerDefault() + promptStubs: func(pm *prompter.PrompterMock) { + pm.SelectFunc = func(p, _ string, opts []string) (int, error) { + if p == "Where should we push the 'feature' branch?" { + return 0, nil + } else { + return -1, prompter.NoSuchPromptErr(p) + } + } }, expectedErrOut: "Opening github.com/OWNER/REPO/compare/master...feature in your browser.\n", expectedBrowse: "https://github.com/OWNER/REPO/compare/master...feature?body=&expand=1&projects=ORG%2F1", @@ -725,12 +761,24 @@ func Test_createRun(t *testing.T) { cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H,%s --cherry origin/master...feature`, 0, "") cs.Register(`git rev-parse --show-toplevel`, 0, "") }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Choose a template").AnswerDefault() - as.StubPrompt("Body").AnswerDefault() - as.StubPrompt("What's next?"). - AssertOptions([]string{"Submit", "Submit as draft", "Continue in browser", "Add metadata", "Cancel"}). - AnswerWith("Submit as draft") + promptStubs: func(pm *prompter.PrompterMock) { + pm.MarkdownEditorFunc = func(p, d string, ba bool) (string, error) { + if p == "Body" { + return d, nil + } else { + return "", prompter.NoSuchPromptErr(p) + } + } + pm.SelectFunc = func(p, _ string, opts []string) (int, error) { + switch p { + case "What's next?": + return prompter.IndexFor(opts, "Submit as draft") + case "Choose a template": + return 0, nil + default: + return -1, prompter.NoSuchPromptErr(p) + } + } }, expectedOut: "https://github.com/OWNER/REPO/pull/12\n", expectedErrOut: "\nCreating pull request for feature into master in OWNER/REPO\n\n", @@ -771,10 +819,28 @@ func Test_createRun(t *testing.T) { cmdStubs: func(cs *run.CommandStubber) { cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "") }, - askStubs: func(as *prompt.AskStubber) { - as.StubPrompt("Title").AnswerDefault() - as.StubPrompt("Body").AnswerDefault() - as.StubPrompt("What's next?").AnswerDefault() + promptStubs: func(pm *prompter.PrompterMock) { + pm.InputFunc = func(p, d string) (string, error) { + if p == "Title" { + return d, nil + } else { + return "", prompter.NoSuchPromptErr(p) + } + } + pm.MarkdownEditorFunc = func(p, d string, ba bool) (string, error) { + if p == "Body" { + return d, nil + } else { + return "", prompter.NoSuchPromptErr(p) + } + } + pm.SelectFunc = func(p, _ string, opts []string) (int, error) { + if p == "What's next?" { + return 0, nil + } else { + return -1, prompter.NoSuchPromptErr(p) + } + } }, setup: func(opts *CreateOptions, t *testing.T) func() { tmpfile, err := os.CreateTemp(t.TempDir(), "testrecover*") @@ -848,11 +914,10 @@ func Test_createRun(t *testing.T) { tt.httpStubs(reg, t) } - //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber - ask, cleanupAsk := prompt.InitAskStubber() - defer cleanupAsk() - if tt.askStubs != nil { - tt.askStubs(ask) + pm := &prompter.PrompterMock{} + + if tt.promptStubs != nil { + tt.promptStubs(pm) } cs, cmdTeardown := run.Stub() @@ -864,6 +929,7 @@ func Test_createRun(t *testing.T) { } opts := CreateOptions{} + opts.Prompter = pm ios, _, stdout, stderr := iostreams.Test() // TODO do i need to bother with this @@ -1095,3 +1161,5 @@ func Test_generateCompareURL(t *testing.T) { }) } } + +// TODO interactive metadata tests once: 1) we have test utils for Prompter and 2) metadata questions use Prompter diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index dbf0923f7..1a331597c 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -6,7 +6,6 @@ import ( "fmt" "net/http" - "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" ghContext "github.com/cli/cli/v2/context" @@ -16,7 +15,6 @@ import ( "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/cli/cli/v2/pkg/surveyext" "github.com/spf13/cobra" ) @@ -31,6 +29,7 @@ type MergeOptions struct { IO *iostreams.IOStreams Branch func() (string, error) Remotes func() (ghContext.Remotes, error) + Prompter shared.Prompt Finder shared.PRFinder @@ -65,6 +64,7 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm GitClient: f.GitClient, Branch: f.Branch, Remotes: f.Remotes, + Prompter: f.Prompter, } var ( @@ -309,7 +309,7 @@ func (m *mergeContext) merge() error { return err } - payload.method, err = mergeMethodSurvey(r) + payload.method, err = mergeMethodSurvey(m.opts.Prompter, r) if err != nil { return err } @@ -321,7 +321,7 @@ func (m *mergeContext) merge() error { allowEditMsg := payload.method != PullRequestMergeMethodRebase for { - action, err := confirmSurvey(allowEditMsg) + action, err := confirmSurvey(m.opts.Prompter, allowEditMsg) if err != nil { return fmt.Errorf("unable to confirm: %w", err) } @@ -375,16 +375,12 @@ func (m *mergeContext) deleteLocalBranch() error { } if m.merged { - // prompt for delete if m.opts.IO.CanPrompt() && !m.opts.IsDeleteBranchIndicated { - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err := prompt.SurveyAskOne(&survey.Confirm{ - Message: fmt.Sprintf("Pull request #%d was already merged. Delete the branch locally?", m.pr.Number), - Default: false, - }, &m.deleteBranch) + confirmed, err := m.opts.Prompter.Confirm(fmt.Sprintf("Pull request #%d was already merged. Delete the branch locally?", m.pr.Number), false) if err != nil { return fmt.Errorf("could not prompt: %w", err) } + m.deleteBranch = confirmed } else { _ = m.warnf(fmt.Sprintf("%s Pull request #%d was already merged\n", m.cs.WarningIcon(), m.pr.Number)) } @@ -550,7 +546,7 @@ func mergeRun(opts *MergeOptions) error { return nil } -func mergeMethodSurvey(baseRepo *api.Repository) (PullRequestMergeMethod, error) { +func mergeMethodSurvey(p shared.Prompt, baseRepo *api.Repository) (PullRequestMergeMethod, error) { type mergeOption struct { title string method PullRequestMergeMethod @@ -575,14 +571,8 @@ func mergeMethodSurvey(baseRepo *api.Repository) (PullRequestMergeMethod, error) surveyOpts = append(surveyOpts, v.title) } - mergeQuestion := &survey.Select{ - Message: "What merge method would you like to use?", - Options: surveyOpts, - } + result, err := p.Select("What merge method would you like to use?", "", surveyOpts) - var result int - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err := prompt.SurveyAskOne(mergeQuestion, &result) return mergeOpts[result].method, err } @@ -595,20 +585,13 @@ func deleteBranchSurvey(opts *MergeOptions, crossRepoPR, localBranchExists bool) message = "Delete the branch on GitHub?" } - var result bool - submit := &survey.Confirm{ - Message: message, - Default: false, - } - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err := prompt.SurveyAskOne(submit, &result) - return result, err + return opts.Prompter.Confirm(message, false) } return opts.DeleteBranch, nil } -func confirmSurvey(allowEditMsg bool) (shared.Action, error) { +func confirmSurvey(p shared.Prompt, allowEditMsg bool) (shared.Action, error) { const ( submitLabel = "Submit" editCommitSubjectLabel = "Edit commit subject" @@ -622,18 +605,12 @@ func confirmSurvey(allowEditMsg bool) (shared.Action, error) { } options = append(options, cancelLabel) - var result string - submit := &survey.Select{ - Message: "What's next?", - Options: options, - } - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err := prompt.SurveyAskOne(submit, &result) + selected, err := p.Select("What's next?", "", options) if err != nil { return shared.CancelAction, fmt.Errorf("could not prompt: %w", err) } - switch result { + switch options[selected] { case submitLabel: return shared.SubmitAction, nil case editCommitSubjectLabel: diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index b2a514d82..d2c8a4dcb 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -17,12 +17,12 @@ import ( "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/run" "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/cli/cli/v2/test" "github.com/google/shlex" "github.com/stretchr/testify/assert" @@ -246,7 +246,8 @@ func stubCommit(pr *api.PullRequest, oid string) { }) } -func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*test.CmdOut, error) { +// TODO port to new style tests +func runCommand(rt http.RoundTripper, pm *prompter.PrompterMock, branch string, isTTY bool, cli string) (*test.CmdOut, error) { ios, _, stdout, stderr := iostreams.Test() ios.SetStdoutTTY(isTTY) ios.SetStdinTTY(isTTY) @@ -274,6 +275,7 @@ func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*t GhPath: "some/path/gh", GitPath: "some/path/git", }, + Prompter: pm, } cmd := NewCmdMerge(factory, nil) @@ -330,7 +332,7 @@ func TestPrMerge(t *testing.T) { defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "main", true, "pr merge 1 --merge") + output, err := runCommand(http, nil, "main", true, "pr merge 1 --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -362,7 +364,7 @@ func TestPrMerge_blocked(t *testing.T) { defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "main", true, "pr merge 1 --merge") + output, err := runCommand(http, nil, "main", true, "pr merge 1 --merge") assert.EqualError(t, err, "SilentError") assert.Equal(t, "", output.String()) @@ -395,7 +397,7 @@ func TestPrMerge_dirty(t *testing.T) { defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "main", true, "pr merge 1 --merge") + output, err := runCommand(http, nil, "main", true, "pr merge 1 --merge") assert.EqualError(t, err, "SilentError") assert.Equal(t, "", output.String()) @@ -436,7 +438,7 @@ func TestPrMerge_nontty(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "main", false, "pr merge 1 --merge") + output, err := runCommand(http, nil, "main", false, "pr merge 1 --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -475,7 +477,7 @@ func TestPrMerge_editMessage_nontty(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "main", false, "pr merge 1 --merge -t mytitle -b mybody") + output, err := runCommand(http, nil, "main", false, "pr merge 1 --merge -t mytitle -b mybody") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -511,7 +513,7 @@ func TestPrMerge_withRepoFlag(t *testing.T) { _, cmdTeardown := run.Stub() defer cmdTeardown(t) - output, err := runCommand(http, "main", true, "pr merge 1 --merge -R OWNER/REPO") + output, err := runCommand(http, nil, "main", true, "pr merge 1 --merge -R OWNER/REPO") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -552,7 +554,7 @@ func TestPrMerge_withMatchCommitHeadFlag(t *testing.T) { defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "main", true, "pr merge 1 --merge --match-head-commit 285ed5ab740f53ff6b0b4b629c59a9df23b9c6db") + output, err := runCommand(http, nil, "main", true, "pr merge 1 --merge --match-head-commit 285ed5ab740f53ff6b0b4b629c59a9df23b9c6db") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -594,7 +596,7 @@ func TestPrMerge_withAuthorFlag(t *testing.T) { defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "main", true, "pr merge 1 --merge --author-email octocat@github.com") + output, err := runCommand(http, nil, "main", true, "pr merge 1 --merge --author-email octocat@github.com") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -644,7 +646,7 @@ func TestPrMerge_deleteBranch(t *testing.T) { cs.Register(`git branch -D blueberries`, 0, "") cs.Register(`git pull --ff-only`, 0, "") - output, err := runCommand(http, "blueberries", true, `pr merge --merge --delete-branch`) + output, err := runCommand(http, nil, "blueberries", true, `pr merge --merge --delete-branch`) if err != nil { t.Fatalf("Got unexpected error running `pr merge` %s", err) } @@ -694,7 +696,7 @@ func TestPrMerge_deleteBranch_nonDefault(t *testing.T) { cs.Register(`git branch -D blueberries`, 0, "") cs.Register(`git pull --ff-only`, 0, "") - output, err := runCommand(http, "blueberries", true, `pr merge --merge --delete-branch`) + output, err := runCommand(http, nil, "blueberries", true, `pr merge --merge --delete-branch`) if err != nil { t.Fatalf("Got unexpected error running `pr merge` %s", err) } @@ -744,7 +746,7 @@ func TestPrMerge_deleteBranch_checkoutNewBranch(t *testing.T) { cs.Register(`git branch -D blueberries`, 0, "") cs.Register(`git pull --ff-only`, 0, "") - output, err := runCommand(http, "blueberries", true, `pr merge --merge --delete-branch`) + output, err := runCommand(http, nil, "blueberries", true, `pr merge --merge --delete-branch`) if err != nil { t.Fatalf("Got unexpected error running `pr merge` %s", err) } @@ -790,7 +792,7 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") cs.Register(`git branch -D blueberries`, 0, "") - output, err := runCommand(http, "main", true, `pr merge --merge --delete-branch blueberries`) + output, err := runCommand(http, nil, "main", true, `pr merge --merge --delete-branch blueberries`) if err != nil { t.Fatalf("Got unexpected error running `pr merge` %s", err) } @@ -832,7 +834,7 @@ func Test_nonDivergingPullRequest(t *testing.T) { cs.Register(`git .+ show .+ HEAD`, 0, "COMMITSHA1,title") cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "blueberries", true, "pr merge --merge") + output, err := runCommand(http, nil, "blueberries", true, "pr merge --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -872,7 +874,7 @@ func Test_divergingPullRequestWarning(t *testing.T) { cs.Register(`git .+ show .+ HEAD`, 0, "COMMITSHA2,title") cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "blueberries", true, "pr merge --merge") + output, err := runCommand(http, nil, "blueberries", true, "pr merge --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -912,7 +914,7 @@ func Test_pullRequestWithoutCommits(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "blueberries", true, "pr merge --merge") + output, err := runCommand(http, nil, "blueberries", true, "pr merge --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -951,7 +953,7 @@ func TestPrMerge_rebase(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "main", true, "pr merge 2 --rebase") + output, err := runCommand(http, nil, "main", true, "pr merge 2 --rebase") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -992,7 +994,7 @@ func TestPrMerge_squash(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "main", true, "pr merge 3 --squash") + output, err := runCommand(http, nil, "main", true, "pr merge 3 --squash") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -1029,11 +1031,17 @@ func TestPrMerge_alreadyMerged(t *testing.T) { cs.Register(`git branch -D blueberries`, 0, "") cs.Register(`git pull --ff-only`, 0, "") - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("Pull request #4 was already merged. Delete the branch locally?").AnswerWith(true) + pm := &prompter.PrompterMock{ + ConfirmFunc: func(p string, d bool) (bool, error) { + if p == "Pull request #4 was already merged. Delete the branch locally?" { + return true, nil + } else { + return false, prompter.NoSuchPromptErr(p) + } + }, + } - output, err := runCommand(http, "blueberries", true, "pr merge 4") + output, err := runCommand(http, pm, "blueberries", true, "pr merge 4") assert.NoError(t, err) assert.Equal(t, "", output.String()) assert.Equal(t, "✓ Deleted branch blueberries and switched to branch main\n", output.Stderr()) @@ -1060,7 +1068,7 @@ func TestPrMerge_alreadyMerged_withMergeStrategy(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "blueberries", false, "pr merge 4 --merge") + output, err := runCommand(http, nil, "blueberries", false, "pr merge 4 --merge") if err != nil { t.Fatalf("Got unexpected error running `pr merge` %s", err) } @@ -1091,11 +1099,17 @@ func TestPrMerge_alreadyMerged_withMergeStrategy_TTY(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") cs.Register(`git branch -D `, 0, "") - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("Pull request #4 was already merged. Delete the branch locally?").AnswerWith(true) + pm := &prompter.PrompterMock{ + ConfirmFunc: func(p string, d bool) (bool, error) { + if p == "Pull request #4 was already merged. Delete the branch locally?" { + return true, nil + } else { + return false, prompter.NoSuchPromptErr(p) + } + }, + } - output, err := runCommand(http, "blueberries", true, "pr merge 4 --merge") + output, err := runCommand(http, pm, "blueberries", true, "pr merge 4 --merge") if err != nil { t.Fatalf("Got unexpected error running `pr merge` %s", err) } @@ -1125,7 +1139,7 @@ func TestPrMerge_alreadyMerged_withMergeStrategy_crossRepo(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "blueberries", true, "pr merge 4 --merge") + output, err := runCommand(http, nil, "blueberries", true, "pr merge 4 --merge") if err != nil { t.Fatalf("Got unexpected error running `pr merge` %s", err) } @@ -1171,13 +1185,27 @@ func TestPRMergeTTY(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("What merge method would you like to use?").AnswerDefault() - as.StubPrompt("Delete the branch locally and on GitHub?").AnswerDefault() - as.StubPrompt("What's next?").AnswerWith("Submit") + pm := &prompter.PrompterMock{ + ConfirmFunc: func(p string, d bool) (bool, error) { + if p == "Delete the branch locally and on GitHub?" { + return d, nil + } else { + return false, prompter.NoSuchPromptErr(p) + } + }, + SelectFunc: func(p, d string, opts []string) (int, error) { + switch p { + case "What's next?": + return prompter.IndexFor(opts, "Submit") + case "What merge method would you like to use?": + return 0, nil + default: + return -1, prompter.NoSuchPromptErr(p) + } + }, + } - output, err := runCommand(http, "blueberries", true, "") + output, err := runCommand(http, pm, "blueberries", true, "") if err != nil { t.Fatalf("Got unexpected error running `pr merge` %s", err) } @@ -1233,12 +1261,20 @@ func TestPRMergeTTY_withDeleteBranch(t *testing.T) { cs.Register(`git branch -D blueberries`, 0, "") cs.Register(`git pull --ff-only`, 0, "") - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("What merge method would you like to use?").AnswerDefault() - as.StubPrompt("What's next?").AnswerWith("Submit") + pm := &prompter.PrompterMock{ + SelectFunc: func(p, d string, opts []string) (int, error) { + switch p { + case "What's next?": + return prompter.IndexFor(opts, "Submit") + case "What merge method would you like to use?": + return 0, nil + default: + return -1, prompter.NoSuchPromptErr(p) + } + }, + } - output, err := runCommand(http, "blueberries", true, "-d") + output, err := runCommand(http, pm, "blueberries", true, "-d") if err != nil { t.Fatalf("Got unexpected error running `pr merge` %s", err) } @@ -1293,13 +1329,29 @@ func TestPRMergeTTY_squashEditCommitMsgAndSubject(t *testing.T) { _, cmdTeardown := run.Stub() defer cmdTeardown(t) - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("What merge method would you like to use?").AnswerWith("Squash and merge") - as.StubPrompt("Delete the branch on GitHub?").AnswerDefault() - as.StubPrompt("What's next?").AnswerWith("Edit commit message") - as.StubPrompt("What's next?").AnswerWith("Edit commit subject") - as.StubPrompt("What's next?").AnswerWith("Submit") + selectCount := -1 + answers := []string{"Edit commit message", "Edit commit subject", "Submit"} + + pm := &prompter.PrompterMock{ + ConfirmFunc: func(p string, d bool) (bool, error) { + if p == "Delete the branch on GitHub?" { + return d, nil + } else { + return false, prompter.NoSuchPromptErr(p) + } + }, + SelectFunc: func(p, d string, opts []string) (int, error) { + switch p { + case "What's next?": + selectCount++ + return prompter.IndexFor(opts, answers[selectCount]) + case "What merge method would you like to use?": + return prompter.IndexFor(opts, "Squash and merge") + default: + return -1, prompter.NoSuchPromptErr(p) + } + }, + } err := mergeRun(&MergeOptions{ IO: ios, @@ -1307,6 +1359,7 @@ func TestPRMergeTTY_squashEditCommitMsgAndSubject(t *testing.T) { HttpClient: func() (*http.Client, error) { return &http.Client{Transport: tr}, nil }, + Prompter: pm, SelectorArg: "https://github.com/OWNER/REPO/pull/123", MergeStrategyEmpty: true, Finder: shared.NewMockFinder( @@ -1342,7 +1395,7 @@ func TestPRMergeEmptyStrategyNonTTY(t *testing.T) { defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "blueberries", false, "pr merge 1") + output, err := runCommand(http, nil, "blueberries", false, "pr merge 1") assert.EqualError(t, err, "--merge, --rebase, or --squash required when not running interactively") assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) @@ -1372,13 +1425,27 @@ func TestPRTTY_cancelled(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("What merge method would you like to use?").AnswerDefault() - as.StubPrompt("Delete the branch locally and on GitHub?").AnswerDefault() - as.StubPrompt("What's next?").AnswerWith("Cancel") + pm := &prompter.PrompterMock{ + ConfirmFunc: func(p string, d bool) (bool, error) { + if p == "Delete the branch locally and on GitHub?" { + return d, nil + } else { + return false, prompter.NoSuchPromptErr(p) + } + }, + SelectFunc: func(p, d string, opts []string) (int, error) { + switch p { + case "What's next?": + return prompter.IndexFor(opts, "Cancel") + case "What merge method would you like to use?": + return 0, nil + default: + return -1, prompter.NoSuchPromptErr(p) + } + }, + } - output, err := runCommand(http, "blueberries", true, "") + output, err := runCommand(http, pm, "blueberries", true, "") if !errors.Is(err, cmdutil.CancelError) { t.Fatalf("got error %v", err) } @@ -1392,11 +1459,18 @@ func Test_mergeMethodSurvey(t *testing.T) { RebaseMergeAllowed: true, SquashMergeAllowed: true, } - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("What merge method would you like to use?").AnswerWith("Rebase and merge") - method, err := mergeMethodSurvey(repo) + pm := &prompter.PrompterMock{ + SelectFunc: func(p, d string, opts []string) (int, error) { + if p == "What merge method would you like to use?" { + return prompter.IndexFor(opts, "Rebase and merge") + } else { + return -1, prompter.NoSuchPromptErr(p) + } + }, + } + + method, err := mergeMethodSurvey(pm, repo) assert.Nil(t, err) assert.Equal(t, PullRequestMergeMethodRebase, method) } @@ -1533,7 +1607,7 @@ func TestPrInMergeQueue(t *testing.T) { defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "blueberries", true, "pr merge 1") + output, err := runCommand(http, nil, "blueberries", true, "pr merge 1") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -1572,7 +1646,7 @@ func TestPrAddToMergeQueueWithMergeMethod(t *testing.T) { defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "blueberries", true, "pr merge 1 --merge") + output, err := runCommand(http, nil, "blueberries", true, "pr merge 1 --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -1611,7 +1685,7 @@ func TestPrAddToMergeQueueClean(t *testing.T) { defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "blueberries", true, "pr merge 1") + output, err := runCommand(http, nil, "blueberries", true, "pr merge 1") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -1651,7 +1725,7 @@ func TestPrAddToMergeQueueBlocked(t *testing.T) { defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "blueberries", true, "pr merge 1") + output, err := runCommand(http, nil, "blueberries", true, "pr merge 1") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -1700,13 +1774,27 @@ func TestPrAddToMergeQueueAdmin(t *testing.T) { defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("What merge method would you like to use?").AnswerDefault() - as.StubPrompt("Delete the branch locally and on GitHub?").AnswerDefault() - as.StubPrompt("What's next?").AnswerDefault() + pm := &prompter.PrompterMock{ + ConfirmFunc: func(p string, d bool) (bool, error) { + if p == "Delete the branch locally and on GitHub?" { + return d, nil + } else { + return false, prompter.NoSuchPromptErr(p) + } + }, + SelectFunc: func(p, d string, opts []string) (int, error) { + switch p { + case "What's next?": + return 0, nil + case "What merge method would you like to use?": + return 0, nil + default: + return -1, prompter.NoSuchPromptErr(p) + } + }, + } - output, err := runCommand(http, "blueberries", true, "pr merge 1 --admin") + output, err := runCommand(http, pm, "blueberries", true, "pr merge 1 --admin") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } @@ -1745,7 +1833,7 @@ func TestPrAddToMergeQueueAdminWithMergeStrategy(t *testing.T) { defer cmdTeardown(t) cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, "blueberries", true, "pr merge 1 --admin --merge") + output, err := runCommand(http, nil, "blueberries", true, "pr merge 1 --admin --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) } diff --git a/pkg/cmd/pr/shared/commentable.go b/pkg/cmd/pr/shared/commentable.go index 8f1f89137..808960b24 100644 --- a/pkg/cmd/pr/shared/commentable.go +++ b/pkg/cmd/pr/shared/commentable.go @@ -7,7 +7,6 @@ import ( "io" "net/http" - "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" @@ -201,14 +200,10 @@ func updateComment(commentable Commentable, opts *CommentableOptions) error { return nil } -func CommentableConfirmSubmitSurvey() (bool, error) { - var confirm bool - submit := &survey.Confirm{ - Message: "Submit?", - Default: true, +func CommentableConfirmSubmitSurvey(p Prompt) func() (bool, error) { + return func() (bool, error) { + return p.Confirm("Submit?", true) } - err := survey.AskOne(submit, &confirm) - return confirm, err } func CommentableInteractiveEditSurvey(cf func() (config.Config, error), io *iostreams.IOStreams) func(string) (string, error) { diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index dd4ba085d..17f292a1d 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -9,7 +9,6 @@ import ( "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/iostreams" "github.com/cli/cli/v2/pkg/prompt" - "github.com/cli/cli/v2/pkg/surveyext" ) type Action int @@ -32,15 +31,22 @@ const ( cancelLabel = "Cancel" ) -func ConfirmIssueSubmission(allowPreview bool, allowMetadata bool) (Action, error) { - return confirmSubmission(allowPreview, allowMetadata, false, false) +type Prompt interface { + Input(string, string) (string, error) + Select(string, string, []string) (int, error) + MarkdownEditor(string, string, bool) (string, error) + Confirm(string, bool) (bool, error) } -func ConfirmPRSubmission(allowPreview, allowMetadata, isDraft bool) (Action, error) { - return confirmSubmission(allowPreview, allowMetadata, true, isDraft) +func ConfirmIssueSubmission(p Prompt, allowPreview bool, allowMetadata bool) (Action, error) { + return confirmSubmission(p, allowPreview, allowMetadata, false, false) } -func confirmSubmission(allowPreview, allowMetadata, allowDraft, isDraft bool) (Action, error) { +func ConfirmPRSubmission(p Prompt, allowPreview, allowMetadata, isDraft bool) (Action, error) { + return confirmSubmission(p, allowPreview, allowMetadata, true, isDraft) +} + +func confirmSubmission(p Prompt, allowPreview, allowMetadata, allowDraft, isDraft bool) (Action, error) { var options []string if !isDraft { options = append(options, submitLabel) @@ -56,26 +62,12 @@ func confirmSubmission(allowPreview, allowMetadata, allowDraft, isDraft bool) (A } options = append(options, cancelLabel) - confirmAnswers := struct { - Confirmation int - }{} - confirmQs := []*survey.Question{ - { - Name: "confirmation", - Prompt: &survey.Select{ - Message: "What's next?", - Options: options, - }, - }, - } - - //nolint:staticcheck // SA1019: prompt.SurveyAsk is deprecated: use Prompter - err := prompt.SurveyAsk(confirmQs, &confirmAnswers) + result, err := p.Select("What's next?", "", options) if err != nil { return -1, fmt.Errorf("could not prompt: %w", err) } - switch options[confirmAnswers.Confirmation] { + switch options[result] { case submitLabel: return SubmitAction, nil case submitDraftLabel: @@ -87,11 +79,11 @@ func confirmSubmission(allowPreview, allowMetadata, allowDraft, isDraft bool) (A case cancelLabel: return CancelAction, nil default: - return -1, fmt.Errorf("invalid index: %d", confirmAnswers.Confirmation) + return -1, fmt.Errorf("invalid index: %d", result) } } -func BodySurvey(state *IssueMetadataState, templateContent, editorCommand string) error { +func BodySurvey(p Prompt, state *IssueMetadataState, templateContent string) error { if templateContent != "" { if state.Body != "" { // prevent excessive newlines between default body and template @@ -101,63 +93,32 @@ func BodySurvey(state *IssueMetadataState, templateContent, editorCommand string state.Body += templateContent } - preBody := state.Body - - // TODO should just be an AskOne but ran into problems with the stubber - qs := []*survey.Question{ - { - Name: "Body", - Prompt: &surveyext.GhEditor{ - BlankAllowed: true, - EditorCommand: editorCommand, - Editor: &survey.Editor{ - Message: "Body", - FileName: "*.md", - Default: state.Body, - HideDefault: true, - AppendDefault: true, - }, - }, - }, - } - - //nolint:staticcheck // SA1019: prompt.SurveyAsk is deprecated: use Prompter - err := prompt.SurveyAsk(qs, state) + result, err := p.MarkdownEditor("Body", state.Body, true) if err != nil { return err } - if preBody != state.Body { + if state.Body != result { state.MarkDirty() } + state.Body = result + return nil } -func TitleSurvey(state *IssueMetadataState) error { - preTitle := state.Title - - // TODO should just be an AskOne but ran into problems with the stubber - qs := []*survey.Question{ - { - Name: "Title", - Prompt: &survey.Input{ - Message: "Title", - Default: state.Title, - }, - }, - } - - //nolint:staticcheck // SA1019: prompt.SurveyAsk is deprecated: use Prompter - err := prompt.SurveyAsk(qs, state) +func TitleSurvey(p Prompt, state *IssueMetadataState) error { + result, err := p.Input("Title", state.Title) if err != nil { return err } - if preTitle != state.Title { + if result != state.Title { state.MarkDirty() } + state.Title = result + return nil } diff --git a/pkg/cmd/pr/shared/templates.go b/pkg/cmd/pr/shared/templates.go index 7aab57978..998952246 100644 --- a/pkg/cmd/pr/shared/templates.go +++ b/pkg/cmd/pr/shared/templates.go @@ -6,13 +6,11 @@ import ( "net/http" "time" - "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/git" fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/githubtemplate" - "github.com/cli/cli/v2/pkg/prompt" "github.com/shurcooL/githubv4" ) @@ -112,6 +110,10 @@ type Template interface { Body() []byte } +type iprompter interface { + Select(string, string, []string) (int, error) +} + type templateManager struct { repo ghrepo.Interface rootDir string @@ -119,6 +121,7 @@ type templateManager struct { isPR bool httpClient *http.Client detector fd.Detector + prompter iprompter templates []Template legacyTemplate Template @@ -127,7 +130,7 @@ type templateManager struct { fetchError error } -func NewTemplateManager(httpClient *http.Client, repo ghrepo.Interface, dir string, allowFS bool, isPR bool) *templateManager { +func NewTemplateManager(httpClient *http.Client, repo ghrepo.Interface, p iprompter, dir string, allowFS bool, isPR bool) *templateManager { cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) return &templateManager{ repo: repo, @@ -135,6 +138,7 @@ func NewTemplateManager(httpClient *http.Client, repo ghrepo.Interface, dir stri allowFS: allowFS, isPR: isPR, httpClient: httpClient, + prompter: p, detector: fd.NewDetector(cachedClient, repo.RepoHost()), } } @@ -184,12 +188,7 @@ func (m *templateManager) Choose() (Template, error) { blankOption = "Open a blank pull request" } - var selectedOption int - //nolint:staticcheck // SA1019: prompt.SurveyAskOne is deprecated: use Prompter - err := prompt.SurveyAskOne(&survey.Select{ - Message: "Choose a template", - Options: append(names, blankOption), - }, &selectedOption) + selectedOption, err := m.prompter.Select("Choose a template", "", append(names, blankOption)) if err != nil { return nil, fmt.Errorf("could not prompt: %w", err) } diff --git a/pkg/cmd/pr/shared/templates_test.go b/pkg/cmd/pr/shared/templates_test.go index cce39ad65..a962bb187 100644 --- a/pkg/cmd/pr/shared/templates_test.go +++ b/pkg/cmd/pr/shared/templates_test.go @@ -8,8 +8,8 @@ import ( fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/httpmock" - "github.com/cli/cli/v2/pkg/prompt" "github.com/stretchr/testify/assert" ) @@ -32,6 +32,15 @@ func TestTemplateManager_hasAPI(t *testing.T) { ] }}}`)) + pm := &prompter.PrompterMock{} + pm.SelectFunc = func(p, _ string, opts []string) (int, error) { + if p == "Choose a template" { + return prompter.IndexFor(opts, "Feature request") + } else { + return -1, prompter.NoSuchPromptErr(p) + } + } + m := templateManager{ repo: ghrepo.NewWithHost("OWNER", "REPO", "example.com"), rootDir: rootDir, @@ -39,6 +48,7 @@ func TestTemplateManager_hasAPI(t *testing.T) { isPR: false, httpClient: httpClient, detector: &fd.EnabledDetectorMock{}, + prompter: pm, } hasTemplates, err := m.HasTemplates() @@ -47,12 +57,6 @@ func TestTemplateManager_hasAPI(t *testing.T) { assert.Equal(t, "LEGACY", string(m.LegacyBody())) - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("Choose a template"). - AssertOptions([]string{"Bug report", "Feature request", "Open a blank issue"}). - AnswerWith("Feature request") - tpl, err := m.Choose() assert.NoError(t, err) @@ -79,6 +83,14 @@ func TestTemplateManager_hasAPI_PullRequest(t *testing.T) { ] }}}`)) + pm := &prompter.PrompterMock{} + pm.SelectFunc = func(p, _ string, opts []string) (int, error) { + if p == "Choose a template" { + return prompter.IndexFor(opts, "bug_pr.md") + } else { + return -1, prompter.NoSuchPromptErr(p) + } + } m := templateManager{ repo: ghrepo.NewWithHost("OWNER", "REPO", "example.com"), rootDir: rootDir, @@ -86,6 +98,7 @@ func TestTemplateManager_hasAPI_PullRequest(t *testing.T) { isPR: true, httpClient: httpClient, detector: &fd.EnabledDetectorMock{}, + prompter: pm, } hasTemplates, err := m.HasTemplates() @@ -94,12 +107,6 @@ func TestTemplateManager_hasAPI_PullRequest(t *testing.T) { assert.Equal(t, "LEGACY", string(m.LegacyBody())) - //nolint:staticcheck // SA1019: prompt.NewAskStubber is deprecated: use PrompterMock - as := prompt.NewAskStubber(t) - as.StubPrompt("Choose a template"). - AssertOptions([]string{"bug_pr.md", "feature_pr.md", "Open a blank pull request"}). - AnswerWith("bug_pr.md") - tpl, err := m.Choose() assert.NoError(t, err)