From 383d175118933debe5a65a66b007910903723f93 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Tue, 8 Oct 2024 10:53:54 -0700 Subject: [PATCH] Improve performance Filter as we get results instead of getting them all. This allows us to more easily terminate pagination when opts.Limit is reached. --- pkg/cmd/gist/list/list.go | 2 +- pkg/cmd/gist/search/search.go | 29 ++++++++++----------------- pkg/cmd/gist/search/search_test.go | 16 ++++++++++++--- pkg/cmd/gist/shared/shared.go | 32 ++++++++++++++++-------------- 4 files changed, 41 insertions(+), 38 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 6fb70a3d071..43c4473b505 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -73,7 +73,7 @@ func listRun(opts *ListOptions) error { host, _ := cfg.Authentication().DefaultHost() - gists, err := shared.ListGists(client, host, opts.Limit, opts.Visibility, false) + gists, err := shared.ListGists(client, host, opts.Limit, opts.Visibility, false, nil) if err != nil { return err } diff --git a/pkg/cmd/gist/search/search.go b/pkg/cmd/gist/search/search.go index 948a7948304..b1c04a2c7ac 100644 --- a/pkg/cmd/gist/search/search.go +++ b/pkg/cmd/gist/search/search.go @@ -38,7 +38,7 @@ func NewCmdSearch(f *cmdutil.Factory, runF func(*SearchOptions) error) *cobra.Co Use: "search ", Short: "Search your gists", Long: heredoc.Docf(` - Search your gists' for the given POSIX regular expression. + Search your gists' for a case-sensitive POSIX regular expression. By default, all gists' descriptions are searched. Pass %[1]s--filename%[1]s to search file names, or %[1]s--code%[1]s to search content as well. @@ -95,36 +95,27 @@ func searchRun(opts *SearchOptions) error { host, _ := cfg.Authentication().DefaultHost() - // Query as many as possible. Limit will apply to search results. - allGists, err := shared.ListGists(client, host, shared.MaxPerPage, opts.Visibility, opts.Code) - if err != nil { - return err - } - - gists := make([]shared.Gist, 0, opts.Limit) - for _, gist := range allGists { - if len(gists) == opts.Limit { - break - } - + gists, err := shared.ListGists(client, host, opts.Limit, opts.Visibility, opts.Code, func(gist *shared.Gist) bool { if opts.Pattern.MatchString(gist.Description) { - gists = append(gists, gist) - continue + return true } if opts.Filename || opts.Code { for _, file := range gist.Files { if opts.Filename && opts.Pattern.MatchString(file.Filename) { - gists = append(gists, gist) - continue + return true } if opts.Code && opts.Pattern.MatchString(file.Content) { - gists = append(gists, gist) - continue + return true } } } + + return false + }) + if err != nil { + return err } if len(gists) == 0 { diff --git a/pkg/cmd/gist/search/search_test.go b/pkg/cmd/gist/search/search_test.go index ef1bff1f540..d6f98c1a3b2 100644 --- a/pkg/cmd/gist/search/search_test.go +++ b/pkg/cmd/gist/search/search_test.go @@ -20,9 +20,10 @@ import ( func TestNewCmdSearch(t *testing.T) { tests := []struct { - name string - cli string - wants SearchOptions + name string + cli string + wants SearchOptions + wantsErr bool }{ { name: "pattern only", @@ -89,6 +90,11 @@ func TestNewCmdSearch(t *testing.T) { Visibility: "secret", }, }, + { + name: "invalid regexp pattern", + cli: `invalid(\\`, + wantsErr: true, + }, } for _, tt := range tests { @@ -109,6 +115,10 @@ func TestNewCmdSearch(t *testing.T) { cmd.SetErr(&bytes.Buffer{}) _, err = cmd.ExecuteC() + if tt.wantsErr { + assert.Error(t, err) + return + } assert.NoError(t, err) assert.Equal(t, tt.wants.Pattern.String(), gotOpts.Pattern.String()) diff --git a/pkg/cmd/gist/shared/shared.go b/pkg/cmd/gist/shared/shared.go index 497a8e218a8..a616ca9b6f7 100644 --- a/pkg/cmd/gist/shared/shared.go +++ b/pkg/cmd/gist/shared/shared.go @@ -75,9 +75,9 @@ func GistIDFromURL(gistURL string) (string, error) { return "", fmt.Errorf("Invalid gist URL %s", u) } -const MaxPerPage = 100 +const maxPerPage = 100 -func ListGists(client *http.Client, hostname string, limit int, visibility string, includeText bool) ([]Gist, error) { +func ListGists(client *http.Client, hostname string, limit int, visibility string, includeText bool, filter func(*Gist) bool) ([]Gist, error) { type response struct { Viewer struct { Gists struct { @@ -100,8 +100,8 @@ func ListGists(client *http.Client, hostname string, limit int, visibility strin } perPage := limit - if perPage > MaxPerPage { - perPage = MaxPerPage + if perPage > maxPerPage { + perPage = maxPerPage } variables := map[string]interface{}{ @@ -131,16 +131,18 @@ pagination: } } - gists = append( - gists, - Gist{ - ID: gist.Name, - Description: gist.Description, - Files: files, - UpdatedAt: gist.UpdatedAt, - Public: gist.IsPublic, - }, - ) + gist := Gist{ + ID: gist.Name, + Description: gist.Description, + Files: files, + UpdatedAt: gist.UpdatedAt, + Public: gist.IsPublic, + } + + if filter == nil || filter(&gist) { + gists = append(gists, gist) + } + if len(gists) == limit { break pagination } @@ -183,7 +185,7 @@ func IsBinaryContents(contents []byte) bool { } func PromptGists(prompter prompter.Prompter, client *http.Client, host string, cs *iostreams.ColorScheme) (gistID string, err error) { - gists, err := ListGists(client, host, 10, "all", false) + gists, err := ListGists(client, host, 10, "all", false, nil) if err != nil { return "", err }