Skip to content

Commit

Permalink
Improve performance
Browse files Browse the repository at this point in the history
Filter as we get results instead of getting them all. This allows us to more easily terminate pagination when opts.Limit is reached.
  • Loading branch information
heaths committed Oct 9, 2024
1 parent c9e1db9 commit 383d175
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 38 deletions.
2 changes: 1 addition & 1 deletion pkg/cmd/gist/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
29 changes: 10 additions & 19 deletions pkg/cmd/gist/search/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func NewCmdSearch(f *cmdutil.Factory, runF func(*SearchOptions) error) *cobra.Co
Use: "search <pattern>",
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.
Expand Down Expand Up @@ -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 {
Expand Down
16 changes: 13 additions & 3 deletions pkg/cmd/gist/search/search_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -89,6 +90,11 @@ func TestNewCmdSearch(t *testing.T) {
Visibility: "secret",
},
},
{
name: "invalid regexp pattern",
cli: `invalid(\\`,
wantsErr: true,
},
}

for _, tt := range tests {
Expand All @@ -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())
Expand Down
32 changes: 17 additions & 15 deletions pkg/cmd/gist/shared/shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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{}{
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 383d175

Please sign in to comment.