From d46f42a752e7348b57db708b88e6ea6d78bf08d9 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Fri, 12 Dec 2025 12:07:47 -0700 Subject: [PATCH] Refactor MultiSelectWithSearch to use result struct Refactored the MultiSelectWithSearch function and related interfaces to use a MultiSelectSearchResult struct instead of multiple return values. This change improves clarity and extensibility of the search function signature, and updates all usages, mocks, and tests accordingly. --- internal/prompter/accessible_prompter_test.go | 81 +++++++++++++------ internal/prompter/prompter.go | 33 +++++--- internal/prompter/prompter_mock.go | 14 ++-- internal/prompter/test.go | 2 +- pkg/cmd/pr/edit/edit.go | 19 ++++- pkg/cmd/pr/shared/editable.go | 5 +- pkg/cmd/pr/shared/survey.go | 3 +- pkg/cmd/preview/prompter/prompter.go | 16 +++- 8 files changed, 122 insertions(+), 51 deletions(-) diff --git a/internal/prompter/accessible_prompter_test.go b/internal/prompter/accessible_prompter_test.go index f205d042d..20ce5d844 100644 --- a/internal/prompter/accessible_prompter_test.go +++ b/internal/prompter/accessible_prompter_test.go @@ -228,26 +228,36 @@ func TestAccessiblePrompter(t *testing.T) { console := newTestVirtualTerminal(t) p := newTestAccessiblePrompter(t, console) persistentOptions := []string{"persistent-option-1"} - searchFunc := func(input string) ([]string, []string, int, error) { - var searchResultKeys []string - var searchResultLabels []string + searchFunc := func(input string) prompter.MultiSelectSearchResult { + var searchResultKeys []string + var searchResultLabels []string - // Initial search with no input - if input == "" { - moreResults := 2 - searchResultKeys = []string{"initial-result-1", "initial-result-2"} - searchResultLabels = []string{"Initial Result Label 1", "Initial Result Label 2"} - return searchResultKeys, searchResultLabels, moreResults, nil + // Initial search with no input + if input == "" { + moreResults := 2 + searchResultKeys = []string{"initial-result-1", "initial-result-2"} + searchResultLabels = []string{"Initial Result Label 1", "Initial Result Label 2"} + return prompter.MultiSelectSearchResult{ + Keys: searchResultKeys, + Labels: searchResultLabels, + MoreResults: moreResults, + Err: nil, } - - // Subsequent search with input - moreResults := 0 - searchResultKeys = []string{"search-result-1", "search-result-2"} - searchResultLabels = []string{"Search Result Label 1", "Search Result Label 2"} - return searchResultKeys, searchResultLabels, moreResults, nil } - go func() { + // Subsequent search with input + moreResults := 0 + searchResultKeys = []string{"search-result-1", "search-result-2"} + searchResultLabels = []string{"Search Result Label 1", "Search Result Label 2"} + return prompter.MultiSelectSearchResult{ + Keys: searchResultKeys, + Labels: searchResultLabels, + MoreResults: moreResults, + Err: nil, + } + } + + go func() { // Wait for prompt to appear _, err := console.ExpectString("Select an option \r\n") require.NoError(t, err) @@ -291,16 +301,26 @@ func TestAccessiblePrompter(t *testing.T) { initialSearchResultKeys := []string{"initial-result-1"} initialSearchResultLabels := []string{"Initial Result Label 1"} defaultOptions := initialSearchResultKeys - searchFunc := func(input string) ([]string, []string, int, error) { + searchFunc := func(input string) prompter.MultiSelectSearchResult { // Initial search with no input if input == "" { moreResults := 2 - return initialSearchResultKeys, initialSearchResultLabels, moreResults, nil + return prompter.MultiSelectSearchResult{ + Keys: initialSearchResultKeys, + Labels: initialSearchResultLabels, + MoreResults: moreResults, + Err: nil, + } } // No search selected, so this should fail the test. t.FailNow() - return nil, nil, 0, nil + return prompter.MultiSelectSearchResult{ + Keys: nil, + Labels: nil, + MoreResults: 0, + Err: nil, + } } go func() { @@ -325,21 +345,36 @@ func TestAccessiblePrompter(t *testing.T) { moreResultKeys := []string{"more-result-1"} moreResultLabels := []string{"More Result Label 1"} - searchFunc := func(input string) ([]string, []string, int, error) { + searchFunc := func(input string) prompter.MultiSelectSearchResult { // Initial search with no input if input == "" { moreResults := 2 - return initialSearchResultKeys, initialSearchResultLabels, moreResults, nil + return prompter.MultiSelectSearchResult{ + Keys: initialSearchResultKeys, + Labels: initialSearchResultLabels, + MoreResults: moreResults, + Err: nil, + } } // Subsequent search with input "more" if input == "more" { - return moreResultKeys, moreResultLabels, 0, nil + return prompter.MultiSelectSearchResult{ + Keys: moreResultKeys, + Labels: moreResultLabels, + MoreResults: 0, + Err: nil, + } } // No other searches expected t.FailNow() - return nil, nil, 0, nil + return prompter.MultiSelectSearchResult{ + Keys: nil, + Labels: nil, + MoreResults: 0, + Err: nil, + } } go func() { diff --git a/internal/prompter/prompter.go b/internal/prompter/prompter.go index dddb49035..5fef325d5 100644 --- a/internal/prompter/prompter.go +++ b/internal/prompter/prompter.go @@ -29,7 +29,7 @@ type Prompter interface { // not their indices, since the list of options is dynamic. // The searchFunc args and return values are: func(query) (map[keys]labels, moreResultsCount, searchError) // Where the selected keys are eventually returned by MultiSelectWithSearch and the labels are what is shown to the user in the prompt. - MultiSelectWithSearch(prompt, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) ([]string, []string, int, error)) ([]string, error) + MultiSelectWithSearch(prompt, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) MultiSelectSearchResult) ([]string, error) // Input prompts the user to enter a string value. Input(prompt string, defaultValue string) (string, error) // Password prompts the user to enter a password. @@ -329,7 +329,7 @@ func (p *accessiblePrompter) MarkdownEditor(prompt, defaultValue string, blankAl return text, nil } -func (p *accessiblePrompter) MultiSelectWithSearch(prompt, searchPrompt string, defaultValues, persistentValues []string, searchFunc func(string) ([]string, []string, int, error)) ([]string, error) { +func (p *accessiblePrompter) MultiSelectWithSearch(prompt, searchPrompt string, defaultValues, persistentValues []string, searchFunc func(string) MultiSelectSearchResult) ([]string, error) { return multiSelectWithSearch(p, prompt, searchPrompt, defaultValues, persistentValues, searchFunc) } @@ -349,11 +349,18 @@ func (p *surveyPrompter) MultiSelect(prompt string, defaultValues, options []str return p.prompter.MultiSelect(prompt, defaultValues, options) } -func (p *surveyPrompter) MultiSelectWithSearch(prompt string, searchPrompt string, defaultValues, persistentValues []string, searchFunc func(string) ([]string, []string, int, error)) ([]string, error) { +func (p *surveyPrompter) MultiSelectWithSearch(prompt string, searchPrompt string, defaultValues, persistentValues []string, searchFunc func(string) MultiSelectSearchResult) ([]string, error) { return multiSelectWithSearch(p, prompt, searchPrompt, defaultValues, persistentValues, searchFunc) } -func multiSelectWithSearch(p Prompter, prompt, searchPrompt string, defaultValues, persistentValues []string, searchFunc func(string) ([]string, []string, int, error)) ([]string, error) { +type MultiSelectSearchResult struct { + Keys []string + Labels []string + MoreResults int + Err error +} + +func multiSelectWithSearch(p Prompter, prompt, searchPrompt string, defaultValues, persistentValues []string, searchFunc func(string) MultiSelectSearchResult) ([]string, error) { selectedOptions := defaultValues // The optionKeyLabels map is used to uniquely identify optionKeyLabels @@ -363,10 +370,13 @@ func multiSelectWithSearch(p Prompter, prompt, searchPrompt string, defaultValue optionKeyLabels[k] = k } - searchResultKeys, searchResultLabels, moreResults, err := searchFunc("") - if err != nil { - return nil, fmt.Errorf("failed to search: %w", err) + searchResult := searchFunc("") + if searchResult.Err != nil { + return nil, fmt.Errorf("failed to search: %w", searchResult.Err) } + searchResultKeys := searchResult.Keys + searchResultLabels := searchResult.Labels + moreResults := searchResult.MoreResults for i, k := range searchResultKeys { optionKeyLabels[k] = searchResultLabels[i] @@ -474,10 +484,13 @@ func multiSelectWithSearch(p Prompter, prompt, searchPrompt string, defaultValue return nil, err } - searchResultKeys, searchResultLabels, moreResults, err = searchFunc(query) - if err != nil { - return nil, err + searchResult := searchFunc(query) + if searchResult.Err != nil { + return nil, searchResult.Err } + searchResultKeys = searchResult.Keys + searchResultLabels = searchResult.Labels + moreResults = searchResult.MoreResults for i, k := range searchResultKeys { optionKeyLabels[k] = searchResultLabels[i] diff --git a/internal/prompter/prompter_mock.go b/internal/prompter/prompter_mock.go index 4543004a0..fd6492df8 100644 --- a/internal/prompter/prompter_mock.go +++ b/internal/prompter/prompter_mock.go @@ -38,7 +38,7 @@ var _ Prompter = &PrompterMock{} // MultiSelectFunc: func(prompt string, defaults []string, options []string) ([]int, error) { // panic("mock out the MultiSelect method") // }, -// MultiSelectWithSearchFunc: func(prompt string, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) ([]string, []string, int, error)) ([]string, error) { +// MultiSelectWithSearchFunc: func(prompt string, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) MultiSelectSearchResult) ([]string, error) { // panic("mock out the MultiSelectWithSearch method") // }, // PasswordFunc: func(prompt string) (string, error) { @@ -76,7 +76,7 @@ type PrompterMock struct { MultiSelectFunc func(prompt string, defaults []string, options []string) ([]int, error) // MultiSelectWithSearchFunc mocks the MultiSelectWithSearch method. - MultiSelectWithSearchFunc func(prompt string, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) ([]string, []string, int, error)) ([]string, error) + MultiSelectWithSearchFunc func(prompt string, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) MultiSelectSearchResult) ([]string, error) // PasswordFunc mocks the Password method. PasswordFunc func(prompt string) (string, error) @@ -140,7 +140,7 @@ type PrompterMock struct { // PersistentOptions is the persistentOptions argument value. PersistentOptions []string // SearchFunc is the searchFunc argument value. - SearchFunc func(string) ([]string, []string, int, error) + SearchFunc func(string) MultiSelectSearchResult } // Password holds details about calls to the Password method. Password []struct { @@ -408,7 +408,7 @@ func (mock *PrompterMock) MultiSelectCalls() []struct { } // MultiSelectWithSearch calls MultiSelectWithSearchFunc. -func (mock *PrompterMock) MultiSelectWithSearch(prompt string, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) ([]string, []string, int, error)) ([]string, error) { +func (mock *PrompterMock) MultiSelectWithSearch(prompt string, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) MultiSelectSearchResult) ([]string, error) { if mock.MultiSelectWithSearchFunc == nil { panic("PrompterMock.MultiSelectWithSearchFunc: method is nil but Prompter.MultiSelectWithSearch was just called") } @@ -417,7 +417,7 @@ func (mock *PrompterMock) MultiSelectWithSearch(prompt string, searchPrompt stri SearchPrompt string Defaults []string PersistentOptions []string - SearchFunc func(string) ([]string, []string, int, error) + SearchFunc func(string) MultiSelectSearchResult }{ Prompt: prompt, SearchPrompt: searchPrompt, @@ -440,14 +440,14 @@ func (mock *PrompterMock) MultiSelectWithSearchCalls() []struct { SearchPrompt string Defaults []string PersistentOptions []string - SearchFunc func(string) ([]string, []string, int, error) + SearchFunc func(string) MultiSelectSearchResult } { var calls []struct { Prompt string SearchPrompt string Defaults []string PersistentOptions []string - SearchFunc func(string) ([]string, []string, int, error) + SearchFunc func(string) MultiSelectSearchResult } mock.lockMultiSelectWithSearch.RLock() calls = mock.calls.MultiSelectWithSearch diff --git a/internal/prompter/test.go b/internal/prompter/test.go index adaa0db6d..8d3f64700 100644 --- a/internal/prompter/test.go +++ b/internal/prompter/test.go @@ -99,7 +99,7 @@ func (m *MockPrompter) MarkdownEditor(prompt, defaultValue string, blankAllowed return s.fn(prompt, defaultValue, blankAllowed) } -func (m *MockPrompter) MultiSelectWithSearch(prompt, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) ([]string, []string, int, error)) ([]string, error) { +func (m *MockPrompter) MultiSelectWithSearch(prompt, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) MultiSelectSearchResult) ([]string, error) { var s multiSelectWithSearchStub if len(m.multiSelectWithSearchStubs) == 0 { return nil, NoSuchPromptErr(prompt) diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index b327500fa..ff683af98 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -12,6 +12,7 @@ import ( fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" shared "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" @@ -335,15 +336,20 @@ func editRun(opts *EditOptions) error { return nil } -func assigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, editable *shared.Editable, assignableID string) func(string) ([]string, []string, int, error) { - searchFunc := func(input string) ([]string, []string, int, error) { +func assigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, editable *shared.Editable, assignableID string) func(string) prompter.MultiSelectSearchResult { + searchFunc := func(input string) prompter.MultiSelectSearchResult { actors, err := api.SuggestedAssignableActors( apiClient, repo, assignableID, input) if err != nil { - return nil, nil, 0, err + return prompter.MultiSelectSearchResult{ + Keys: nil, + Labels: nil, + MoreResults: 0, + Err: err, + } } var logins []string @@ -366,7 +372,12 @@ func assigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, editable * // so that updating the PR later can resolve the actor ID. editable.Metadata.AssignableActors = append(editable.Metadata.AssignableActors, a) } - return logins, displayNames, 0, nil + return prompter.MultiSelectSearchResult{ + Keys: logins, + Labels: displayNames, + MoreResults: 0, + Err: nil, + } } return searchFunc } diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index c69e5a0b5..7cfe49d17 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -6,6 +6,7 @@ import ( "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/internal/prompter" "github.com/cli/cli/v2/pkg/set" ) @@ -16,7 +17,7 @@ type Editable struct { Reviewers EditableSlice ReviewerSearchFunc func(string) ([]string, []string, error) Assignees EditableAssignees - AssigneeSearchFunc func(string) ([]string, []string, int, error) + AssigneeSearchFunc func(string) prompter.MultiSelectSearchResult Labels EditableSlice Projects EditableProjects Milestone EditableString @@ -279,7 +280,7 @@ type EditPrompter interface { Input(string, string) (string, error) MarkdownEditor(string, string, bool) (string, error) MultiSelect(string, []string, []string) ([]int, error) - MultiSelectWithSearch(prompt, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) ([]string, []string, int, error)) ([]string, error) + MultiSelectWithSearch(prompt, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) prompter.MultiSelectSearchResult) ([]string, error) Confirm(string, bool) (bool, error) } diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 5197d6ace..9ba600ae4 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -9,6 +9,7 @@ import ( "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/internal/prompter" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/cli/cli/v2/pkg/surveyext" @@ -40,7 +41,7 @@ type Prompt interface { MarkdownEditor(prompt string, defaultValue string, blankAllowed bool) (string, error) Confirm(prompt string, defaultValue bool) (bool, error) MultiSelect(prompt string, defaults []string, options []string) ([]int, error) - MultiSelectWithSearch(prompt, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) ([]string, []string, int, error)) ([]string, error) + MultiSelectWithSearch(prompt, searchPrompt string, defaults []string, persistentOptions []string, searchFunc func(string) prompter.MultiSelectSearchResult) ([]string, error) } func ConfirmIssueSubmission(p Prompt, allowPreview bool, allowMetadata bool) (Action, error) { diff --git a/pkg/cmd/preview/prompter/prompter.go b/pkg/cmd/preview/prompter/prompter.go index 35ff901d4..93dbbe611 100644 --- a/pkg/cmd/preview/prompter/prompter.go +++ b/pkg/cmd/preview/prompter/prompter.go @@ -157,7 +157,7 @@ func runMultiSelect(p prompter.Prompter, io *iostreams.IOStreams) error { func runMultiSelectWithSearch(p prompter.Prompter, io *iostreams.IOStreams) error { fmt.Fprintln(io.Out, "Demonstrating Multi Select With Search") persistentOptions := []string{"persistent-option-1"} - searchFunc := func(input string) ([]string, []string, int, error) { + searchFunc := func(input string) prompter.MultiSelectSearchResult { var searchResultKeys []string var searchResultLabels []string @@ -165,7 +165,12 @@ func runMultiSelectWithSearch(p prompter.Prompter, io *iostreams.IOStreams) erro moreResults := 2 // Indicate that there are more results available searchResultKeys = []string{"initial-result-1", "initial-result-2"} searchResultLabels = []string{"Initial Result Label 1", "Initial Result Label 2"} - return searchResultKeys, searchResultLabels, moreResults, nil + return prompter.MultiSelectSearchResult{ + Keys: searchResultKeys, + Labels: searchResultLabels, + MoreResults: moreResults, + Err: nil, + } } // In a real implementation, this function would perform a search based on the input. @@ -173,7 +178,12 @@ func runMultiSelectWithSearch(p prompter.Prompter, io *iostreams.IOStreams) erro moreResults := 0 searchResultKeys = []string{"search-result-1", "search-result-2"} searchResultLabels = []string{"Search Result Label 1", "Search Result Label 2"} - return searchResultKeys, searchResultLabels, moreResults, nil + return prompter.MultiSelectSearchResult{ + Keys: searchResultKeys, + Labels: searchResultLabels, + MoreResults: moreResults, + Err: nil, + } } selections, err := p.MultiSelectWithSearch("Select an option", "Search for an option", []string{}, persistentOptions, searchFunc)