From 8c6d4c9c52604c9a84d7ae94b9c8442a5b47ea9e Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Tue, 8 Oct 2024 02:12:03 -0700 Subject: [PATCH 01/13] Add `gist search` command Resolves #9704 --- pkg/cmd/gist/gist.go | 2 + pkg/cmd/gist/list/list.go | 49 +--- pkg/cmd/gist/search/search.go | 135 +++++++++++ pkg/cmd/gist/search/search_test.go | 361 +++++++++++++++++++++++++++++ pkg/cmd/gist/shared/shared.go | 64 ++++- 5 files changed, 557 insertions(+), 54 deletions(-) create mode 100644 pkg/cmd/gist/search/search.go create mode 100644 pkg/cmd/gist/search/search_test.go diff --git a/pkg/cmd/gist/gist.go b/pkg/cmd/gist/gist.go index 1f7fc9ea0..bf562bf74 100644 --- a/pkg/cmd/gist/gist.go +++ b/pkg/cmd/gist/gist.go @@ -8,6 +8,7 @@ import ( gistEditCmd "github.com/cli/cli/v2/pkg/cmd/gist/edit" gistListCmd "github.com/cli/cli/v2/pkg/cmd/gist/list" gistRenameCmd "github.com/cli/cli/v2/pkg/cmd/gist/rename" + gistSearchCmd "github.com/cli/cli/v2/pkg/cmd/gist/search" gistViewCmd "github.com/cli/cli/v2/pkg/cmd/gist/view" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" @@ -35,6 +36,7 @@ func NewCmdGist(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(gistEditCmd.NewCmdEdit(f, nil)) cmd.AddCommand(gistDeleteCmd.NewCmdDelete(f, nil)) cmd.AddCommand(gistRenameCmd.NewCmdRename(f, nil)) + cmd.AddCommand(gistSearchCmd.NewCmdSearch(f, nil)) return cmd } diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 5d0e47150..6fb70a3d0 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -1,14 +1,9 @@ package list import ( - "fmt" "net/http" - "strings" - "time" "github.com/cli/cli/v2/internal/gh" - "github.com/cli/cli/v2/internal/tableprinter" - "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/cmd/gist/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" @@ -78,7 +73,7 @@ func listRun(opts *ListOptions) error { host, _ := cfg.Authentication().DefaultHost() - gists, err := shared.ListGists(client, host, opts.Limit, opts.Visibility) + gists, err := shared.ListGists(client, host, opts.Limit, opts.Visibility, false) if err != nil { return err } @@ -87,45 +82,5 @@ func listRun(opts *ListOptions) error { return cmdutil.NewNoResultsError("no gists found") } - if err := opts.IO.StartPager(); err == nil { - defer opts.IO.StopPager() - } else { - fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) - } - - cs := opts.IO.ColorScheme() - tp := tableprinter.New(opts.IO, tableprinter.WithHeader("ID", "DESCRIPTION", "FILES", "VISIBILITY", "UPDATED")) - - for _, gist := range gists { - fileCount := len(gist.Files) - - visibility := "public" - visColor := cs.Green - if !gist.Public { - visibility = "secret" - visColor = cs.Red - } - - description := gist.Description - if description == "" { - for filename := range gist.Files { - if !strings.HasPrefix(filename, "gistfile") { - description = filename - break - } - } - } - - tp.AddField(gist.ID) - tp.AddField( - text.RemoveExcessiveWhitespace(description), - tableprinter.WithColor(cs.Bold), - ) - tp.AddField(text.Pluralize(fileCount, "file")) - tp.AddField(visibility, tableprinter.WithColor(visColor)) - tp.AddTimeField(time.Now(), gist.UpdatedAt, cs.Gray) - tp.EndRow() - } - - return tp.Render() + return shared.PrintGists(opts.IO, gists) } diff --git a/pkg/cmd/gist/search/search.go b/pkg/cmd/gist/search/search.go new file mode 100644 index 000000000..948a79483 --- /dev/null +++ b/pkg/cmd/gist/search/search.go @@ -0,0 +1,135 @@ +package search + +import ( + "net/http" + "regexp" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/gh" + "github.com/cli/cli/v2/pkg/cmd/gist/shared" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +type SearchOptions struct { + IO *iostreams.IOStreams + Config func() (gh.Config, error) + HttpClient func() (*http.Client, error) + + Pattern *regexp.Regexp + Filename bool + Code bool + Visibility string // all, secret, public + Limit int +} + +func NewCmdSearch(f *cmdutil.Factory, runF func(*SearchOptions) error) *cobra.Command { + opts := &SearchOptions{ + IO: f.IOStreams, + Config: f.Config, + HttpClient: f.HttpClient, + } + + var flagPublic bool + var flagSecret bool + + cmd := &cobra.Command{ + Use: "search ", + Short: "Search your gists", + Long: heredoc.Docf(` + Search your gists' for the given 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. + `, "`"), + Example: heredoc.Doc(` + # search public gists' descriptions for "octo" + $ gh gist search --public octo + + # search all gists' descriptions, file names, and code for "foo|bar" + $ gh gist search --filename --code "foo|bar" + `), + Args: cmdutil.ExactArgs(1, "no search pattern passed"), + RunE: func(cmd *cobra.Command, args []string) error { + var err error + opts.Pattern, err = regexp.CompilePOSIX(args[0]) + if err != nil { + return err + } + + // Replicate precedence of existing `gist` commands instead of mutually exclusive arguments. + opts.Visibility = "all" + if flagSecret { + opts.Visibility = "secret" + } else if flagPublic { + opts.Visibility = "public" + } + + if runF != nil { + return runF(opts) + } + return searchRun(opts) + }, + } + + cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 10, "Maximum number of gists to fetch") + cmd.Flags().BoolVar(&flagPublic, "public", false, "Show only public gists") + cmd.Flags().BoolVar(&flagSecret, "secret", false, "Show only secret gists") + cmd.Flags().BoolVar(&opts.Filename, "filename", false, "Include file names in search") + cmd.Flags().BoolVar(&opts.Code, "code", false, "Include code in search") + + return cmd +} + +func searchRun(opts *SearchOptions) error { + client, err := opts.HttpClient() + if err != nil { + return err + } + + cfg, err := opts.Config() + if err != nil { + return err + } + + 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 + } + + if opts.Pattern.MatchString(gist.Description) { + gists = append(gists, gist) + continue + } + + if opts.Filename || opts.Code { + for _, file := range gist.Files { + if opts.Filename && opts.Pattern.MatchString(file.Filename) { + gists = append(gists, gist) + continue + } + + if opts.Code && opts.Pattern.MatchString(file.Content) { + gists = append(gists, gist) + continue + } + } + } + } + + if len(gists) == 0 { + return cmdutil.NewNoResultsError("no gists found") + } + + return shared.PrintGists(opts.IO, gists) +} diff --git a/pkg/cmd/gist/search/search_test.go b/pkg/cmd/gist/search/search_test.go new file mode 100644 index 000000000..ef1bff1f5 --- /dev/null +++ b/pkg/cmd/gist/search/search_test.go @@ -0,0 +1,361 @@ +package search + +import ( + "bytes" + "fmt" + "net/http" + "regexp" + "testing" + "time" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/gh" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/httpmock" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdSearch(t *testing.T) { + tests := []struct { + name string + cli string + wants SearchOptions + }{ + { + name: "pattern only", + cli: "octo", + wants: SearchOptions{ + Pattern: regexp.MustCompilePOSIX("octo"), + Limit: 10, + Visibility: "all", + }, + }, + { + name: "public", + cli: "--public octo", + wants: SearchOptions{ + Pattern: regexp.MustCompilePOSIX("octo"), + Limit: 10, + Visibility: "public", + }, + }, + { + name: "secret", + cli: `"foo|bar" --secret`, + wants: SearchOptions{ + Pattern: regexp.MustCompilePOSIX("foo|bar"), + Limit: 10, + Visibility: "secret", + }, + }, + { + name: "secret with explicit false value", + cli: "--secret=false octo", + wants: SearchOptions{ + Pattern: regexp.MustCompilePOSIX("octo"), + Limit: 10, + Visibility: "all", + }, + }, + { + name: "public and secret", + cli: "--secret --public octo", + wants: SearchOptions{ + Pattern: regexp.MustCompilePOSIX("octo"), + Limit: 10, + Visibility: "secret", + }, + }, + { + name: "limit", + cli: "--limit 30 octo", + wants: SearchOptions{ + Pattern: regexp.MustCompilePOSIX("octo"), + Limit: 30, + Visibility: "all", + }, + }, + { + name: "all arguments", + cli: "--public --secret --filename --code --limit 30 octo", + wants: SearchOptions{ + Pattern: regexp.MustCompilePOSIX("octo"), + Filename: true, + Code: true, + Limit: 30, + Visibility: "secret", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &cmdutil.Factory{} + + argv, err := shlex.Split(tt.cli) + assert.NoError(t, err) + + var gotOpts *SearchOptions + cmd := NewCmdSearch(f, func(opts *SearchOptions) error { + gotOpts = opts + return nil + }) + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + assert.NoError(t, err) + + assert.Equal(t, tt.wants.Pattern.String(), gotOpts.Pattern.String()) + assert.Equal(t, tt.wants.Filename, gotOpts.Filename) + assert.Equal(t, tt.wants.Code, gotOpts.Code) + assert.Equal(t, tt.wants.Visibility, gotOpts.Visibility) + assert.Equal(t, tt.wants.Limit, gotOpts.Limit) + }) + } +} + +func Test_searchRun(t *testing.T) { + const query = `query GistList\b` + sixHours, _ := time.ParseDuration("6h") + sixHoursAgo := time.Now().Add(-sixHours) + + tests := []struct { + name string + opts *SearchOptions + wantErr bool + wantOut string + stubs func(*httpmock.Registry) + nontty bool + }{ + { + name: "no gists", + opts: &SearchOptions{}, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(`{ "data": { "viewer": { + "gists": { "nodes": [] } + } } }`)) + }, + wantErr: true, + }, + { + name: "description only", + opts: &SearchOptions{ + Pattern: regexp.MustCompilePOSIX("foo"), + }, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "1234567890", + "files": [{ "name": "cool.txt" }], + "description": "foo example", + "updatedAt": "%[1]v", + "isPublic": true + }, + { + "name": "4567890123", + "files": [{ "name": "gistfile0.txt" }], + "description": "bar example", + "updatedAt": "%[1]v", + "isPublic": true + } + ] } } } }`, + sixHoursAgo.Format(time.RFC3339), + )), + ) + }, + wantOut: heredoc.Doc(` + ID DESCRIPTION FILES VISIBILITY UPDATED + 1234567890 foo example 1 file public about 6 hours ago + `), + }, + { + name: "no results with public filter", + opts: &SearchOptions{ + Pattern: regexp.MustCompilePOSIX("foo"), + Visibility: "public", + }, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "4567890123", + "files": [{ "name": "gistfile0.txt" }], + "description": "bar example", + "updatedAt": "%[1]v", + "isPublic": true + } + ] } } } }`, + sixHoursAgo.Format(time.RFC3339), + )), + ) + }, + wantErr: true, + }, + { + name: "description only", + opts: &SearchOptions{ + Pattern: regexp.MustCompilePOSIX("foo"), + }, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "1234567890", + "files": [{ "name": "cool.txt" }], + "description": "foo example", + "updatedAt": "%[1]v", + "isPublic": true + }, + { + "name": "4567890123", + "files": [{ "name": "gistfile0.txt" }], + "description": "bar example", + "updatedAt": "%[1]v", + "isPublic": true + } + ] } } } }`, + sixHoursAgo.Format(time.RFC3339), + )), + ) + }, + wantOut: heredoc.Doc(` + ID DESCRIPTION FILES VISIBILITY UPDATED + 1234567890 foo example 1 file public about 6 hours ago + `), + }, + { + name: "include filenames", + opts: &SearchOptions{ + Pattern: regexp.MustCompilePOSIX("gistfile7"), + Filename: true, + }, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "2345678901", + "files": [ + { "name": "gistfile0.txt" }, + { "name": "gistfile1.txt" } + ], + "description": "foo example", + "updatedAt": "%[1]v", + "isPublic": true + }, + { + "name": "3456789012", + "files": [ + { "name": "gistfile0.txt" }, + { "name": "gistfile1.txt" }, + { "name": "gistfile2.txt" }, + { "name": "gistfile3.txt" }, + { "name": "gistfile4.txt" }, + { "name": "gistfile5.txt" }, + { "name": "gistfile6.txt" }, + { "name": "gistfile7.txt" }, + { "name": "gistfile9.txt" }, + { "name": "gistfile10.txt" }, + { "name": "gistfile11.txt" } + ], + "description": "bar example", + "updatedAt": "%[1]v", + "isPublic": false + } + ] } } } }`, + sixHoursAgo.Format(time.RFC3339), + )), + ) + }, + wantOut: heredoc.Doc(` + ID DESCRIPTION FILES VISIBILITY UPDATED + 3456789012 bar example 11 files secret about 6 hours ago + `), + }, + { + name: "include code", + opts: &SearchOptions{ + Pattern: regexp.MustCompilePOSIX("octo"), + Code: true, + }, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "2345678901", + "files": [ + { + "name": "gistfile0.txt", + "text": "octo" + } + ], + "description": "foo example", + "updatedAt": "%[1]v", + "isPublic": true + } + ] } } } }`, + sixHoursAgo.Format(time.RFC3339), + )), + ) + }, + wantOut: heredoc.Doc(` + ID DESCRIPTION FILES VISIBILITY UPDATED + 2345678901 foo example 1 file public about 6 hours ago + `), + }, + } + + for _, tt := range tests { + reg := &httpmock.Registry{} + tt.stubs(reg) + + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + + tt.opts.Config = func() (gh.Config, error) { + return config.NewBlankConfig(), nil + } + + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(!tt.nontty) + tt.opts.IO = ios + + if tt.opts.Limit == 0 { + tt.opts.Limit = 10 + } + + if tt.opts.Visibility == "" { + tt.opts.Visibility = "all" + } + t.Run(tt.name, func(t *testing.T) { + err := searchRun(tt.opts) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + + assert.Equal(t, tt.wantOut, stdout.String()) + reg.Verify(t) + }) + } +} diff --git a/pkg/cmd/gist/shared/shared.go b/pkg/cmd/gist/shared/shared.go index 66ba6fe0a..497a8e218 100644 --- a/pkg/cmd/gist/shared/shared.go +++ b/pkg/cmd/gist/shared/shared.go @@ -11,6 +11,7 @@ import ( "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/prompter" + "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/iostreams" "github.com/gabriel-vasile/mimetype" @@ -74,7 +75,9 @@ func GistIDFromURL(gistURL string) (string, error) { return "", fmt.Errorf("Invalid gist URL %s", u) } -func ListGists(client *http.Client, hostname string, limit int, visibility string) ([]Gist, error) { +const MaxPerPage = 100 + +func ListGists(client *http.Client, hostname string, limit int, visibility string, includeText bool) ([]Gist, error) { type response struct { Viewer struct { Gists struct { @@ -82,6 +85,7 @@ func ListGists(client *http.Client, hostname string, limit int, visibility strin Description string Files []struct { Name string + Text string `graphql:"text @include(if: $includeText)"` } IsPublic bool Name string @@ -96,14 +100,15 @@ func ListGists(client *http.Client, hostname string, limit int, visibility strin } perPage := limit - if perPage > 100 { - perPage = 100 + if perPage > MaxPerPage { + perPage = MaxPerPage } variables := map[string]interface{}{ - "per_page": githubv4.Int(perPage), - "endCursor": (*githubv4.String)(nil), - "visibility": githubv4.GistPrivacy(strings.ToUpper(visibility)), + "per_page": githubv4.Int(perPage), + "endCursor": (*githubv4.String)(nil), + "visibility": githubv4.GistPrivacy(strings.ToUpper(visibility)), + "includeText": githubv4.Boolean(includeText), } gql := api.NewClientFromHTTP(client) @@ -122,6 +127,7 @@ pagination: for _, file := range gist.Files { files[file.Name] = &GistFile{ Filename: file.Name, + Content: file.Text, } } @@ -177,7 +183,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") + gists, err := ListGists(client, host, 10, "all", false) if err != nil { return "", err } @@ -220,3 +226,47 @@ func PromptGists(prompter prompter.Prompter, client *http.Client, host string, c return gistIDs[result], nil } + +func PrintGists(io *iostreams.IOStreams, gists []Gist) error { + if err := io.StartPager(); err == nil { + defer io.StopPager() + } else { + fmt.Fprintf(io.ErrOut, "failed to start pager: %v\n", err) + } + + cs := io.ColorScheme() + tp := tableprinter.New(io, tableprinter.WithHeader("ID", "DESCRIPTION", "FILES", "VISIBILITY", "UPDATED")) + + for _, gist := range gists { + fileCount := len(gist.Files) + + visibility := "public" + visColor := cs.Green + if !gist.Public { + visibility = "secret" + visColor = cs.Red + } + + description := gist.Description + if description == "" { + for filename := range gist.Files { + if !strings.HasPrefix(filename, "gistfile") { + description = filename + break + } + } + } + + tp.AddField(gist.ID) + tp.AddField( + text.RemoveExcessiveWhitespace(description), + tableprinter.WithColor(cs.Bold), + ) + tp.AddField(text.Pluralize(fileCount, "file")) + tp.AddField(visibility, tableprinter.WithColor(visColor)) + tp.AddTimeField(time.Now(), gist.UpdatedAt, cs.Gray) + tp.EndRow() + } + + return tp.Render() +} From 2fb5687777747b21859084ba77068dddf2a8c6d2 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Tue, 8 Oct 2024 10:53:54 -0700 Subject: [PATCH 02/13] 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 6fb70a3d0..43c4473b5 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 948a79483..b1c04a2c7 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 ef1bff1f5..d6f98c1a3 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 497a8e218..a616ca9b6 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 } From 34203d7e6cc95fdb782556a6a9bdc915fa3d902f Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Wed, 9 Oct 2024 00:10:17 -0700 Subject: [PATCH 03/13] Refactor filtering into existing `gist list` Resolves feedback in issue #9704 --- pkg/cmd/gist/gist.go | 2 - pkg/cmd/gist/list/list.go | 65 ++++- pkg/cmd/gist/list/list_test.go | 84 ++++++- pkg/cmd/gist/search/search.go | 126 ---------- pkg/cmd/gist/search/search_test.go | 371 ----------------------------- pkg/cmd/gist/shared/shared.go | 80 +++---- 6 files changed, 169 insertions(+), 559 deletions(-) delete mode 100644 pkg/cmd/gist/search/search.go delete mode 100644 pkg/cmd/gist/search/search_test.go diff --git a/pkg/cmd/gist/gist.go b/pkg/cmd/gist/gist.go index bf562bf74..1f7fc9ea0 100644 --- a/pkg/cmd/gist/gist.go +++ b/pkg/cmd/gist/gist.go @@ -8,7 +8,6 @@ import ( gistEditCmd "github.com/cli/cli/v2/pkg/cmd/gist/edit" gistListCmd "github.com/cli/cli/v2/pkg/cmd/gist/list" gistRenameCmd "github.com/cli/cli/v2/pkg/cmd/gist/rename" - gistSearchCmd "github.com/cli/cli/v2/pkg/cmd/gist/search" gistViewCmd "github.com/cli/cli/v2/pkg/cmd/gist/view" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" @@ -36,7 +35,6 @@ func NewCmdGist(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(gistEditCmd.NewCmdEdit(f, nil)) cmd.AddCommand(gistDeleteCmd.NewCmdDelete(f, nil)) cmd.AddCommand(gistRenameCmd.NewCmdRename(f, nil)) - cmd.AddCommand(gistSearchCmd.NewCmdSearch(f, nil)) return cmd } diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 43c4473b5..215fedcce 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -1,9 +1,15 @@ package list import ( + "fmt" "net/http" + "regexp" + "strings" + "time" "github.com/cli/cli/v2/internal/gh" + "github.com/cli/cli/v2/internal/tableprinter" + "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/cmd/gist/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" @@ -15,8 +21,10 @@ type ListOptions struct { Config func() (gh.Config, error) HttpClient func() (*http.Client, error) - Limit int - Visibility string // all, secret, public + Limit int + Filter *regexp.Regexp + IncludeContent bool + Visibility string // all, secret, public } func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { @@ -28,6 +36,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman var flagPublic bool var flagSecret bool + var flagFilter string cmd := &cobra.Command{ Use: "list", @@ -39,6 +48,12 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman return cmdutil.FlagErrorf("invalid limit: %v", opts.Limit) } + if filter, err := regexp.CompilePOSIX(flagFilter); err != nil { + return err + } else { + opts.Filter = filter + } + opts.Visibility = "all" if flagSecret { opts.Visibility = "secret" @@ -56,6 +71,8 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 10, "Maximum number of gists to fetch") cmd.Flags().BoolVar(&flagPublic, "public", false, "Show only public gists") cmd.Flags().BoolVar(&flagSecret, "secret", false, "Show only secret gists") + cmd.Flags().StringVar(&flagFilter, "filter", "", "Filter gists using a regular expression") + cmd.Flags().BoolVar(&opts.IncludeContent, "include-content", false, "Include gists' file content when filtering") return cmd } @@ -73,7 +90,7 @@ func listRun(opts *ListOptions) error { host, _ := cfg.Authentication().DefaultHost() - gists, err := shared.ListGists(client, host, opts.Limit, opts.Visibility, false, nil) + gists, err := shared.ListGists(client, host, opts.Limit, opts.Filter, opts.IncludeContent, opts.Visibility) if err != nil { return err } @@ -82,5 +99,45 @@ func listRun(opts *ListOptions) error { return cmdutil.NewNoResultsError("no gists found") } - return shared.PrintGists(opts.IO, gists) + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) + } + + cs := opts.IO.ColorScheme() + tp := tableprinter.New(opts.IO, tableprinter.WithHeader("ID", "DESCRIPTION", "FILES", "VISIBILITY", "UPDATED")) + + for _, gist := range gists { + fileCount := len(gist.Files) + + visibility := "public" + visColor := cs.Green + if !gist.Public { + visibility = "secret" + visColor = cs.Red + } + + description := gist.Description + if description == "" { + for filename := range gist.Files { + if !strings.HasPrefix(filename, "gistfile") { + description = filename + break + } + } + } + + tp.AddField(gist.ID) + tp.AddField( + text.RemoveExcessiveWhitespace(description), + tableprinter.WithColor(cs.Bold), + ) + tp.AddField(text.Pluralize(fileCount, "file")) + tp.AddField(visibility, tableprinter.WithColor(visColor)) + tp.AddTimeField(time.Now(), gist.UpdatedAt, cs.Gray) + tp.EndRow() + } + + return tp.Render() } diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index a15bae6aa..80e3178ab 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "net/http" + "regexp" "testing" "time" @@ -19,9 +20,10 @@ import ( func TestNewCmdList(t *testing.T) { tests := []struct { - name string - cli string - wants ListOptions + name string + cli string + wants ListOptions + wantsErr bool }{ { name: "no arguments", @@ -70,6 +72,26 @@ func TestNewCmdList(t *testing.T) { Visibility: "all", }, }, + { + name: "invalid limit", + cli: "--limit 0", + wantsErr: true, + }, + { + name: "filter and include-content", + cli: "--filter octo --include-content", + wants: ListOptions{ + Limit: 10, + Filter: regexp.MustCompilePOSIX("octo"), + IncludeContent: true, + Visibility: "all", + }, + }, + { + name: "invalid filter", + cli: "--filter octo(", + wantsErr: true, + }, } for _, tt := range tests { @@ -90,6 +112,10 @@ func TestNewCmdList(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.Visibility, gotOpts.Visibility) @@ -358,6 +384,58 @@ func Test_listRun(t *testing.T) { `), nontty: true, }, + { + name: "with content filter", + opts: &ListOptions{ + Filter: regexp.MustCompile("octo"), + IncludeContent: true, + Visibility: "all", + }, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "1234", + "files": [ + { "name": "main.txt", "text": "foo" } + ], + "description": "octo match in the description", + "updatedAt": "%[1]v", + "isPublic": true + }, + { + "name": "2345", + "files": [ + { "name": "main.txt", "text": "foo" }, + { "name": "octo.txt", "text": "bar" } + ], + "description": "match in the file name", + "updatedAt": "%[1]v", + "isPublic": false + }, + { + "name": "3456", + "files": [ + { "name": "main.txt", "text": "octo in the text" } + ], + "description": "match in the file text", + "updatedAt": "%[1]v", + "isPublic": true + } + ] } } } }`, + sixHoursAgo.Format(time.RFC3339), + )), + ) + }, + wantOut: heredoc.Doc(` + ID DESCRIPTION FILES VISIBILITY UPDATED + 1234 octo match in the description 1 file public about 6 hours ago + 2345 match in the file name 2 files secret about 6 hours ago + 3456 match in the file text 1 file public about 6 hours ago + `), + }, } for _, tt := range tests { diff --git a/pkg/cmd/gist/search/search.go b/pkg/cmd/gist/search/search.go deleted file mode 100644 index b1c04a2c7..000000000 --- a/pkg/cmd/gist/search/search.go +++ /dev/null @@ -1,126 +0,0 @@ -package search - -import ( - "net/http" - "regexp" - - "github.com/MakeNowJust/heredoc" - "github.com/cli/cli/v2/internal/gh" - "github.com/cli/cli/v2/pkg/cmd/gist/shared" - "github.com/cli/cli/v2/pkg/cmdutil" - "github.com/cli/cli/v2/pkg/iostreams" - "github.com/spf13/cobra" -) - -type SearchOptions struct { - IO *iostreams.IOStreams - Config func() (gh.Config, error) - HttpClient func() (*http.Client, error) - - Pattern *regexp.Regexp - Filename bool - Code bool - Visibility string // all, secret, public - Limit int -} - -func NewCmdSearch(f *cmdutil.Factory, runF func(*SearchOptions) error) *cobra.Command { - opts := &SearchOptions{ - IO: f.IOStreams, - Config: f.Config, - HttpClient: f.HttpClient, - } - - var flagPublic bool - var flagSecret bool - - cmd := &cobra.Command{ - Use: "search ", - Short: "Search your gists", - Long: heredoc.Docf(` - 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. - `, "`"), - Example: heredoc.Doc(` - # search public gists' descriptions for "octo" - $ gh gist search --public octo - - # search all gists' descriptions, file names, and code for "foo|bar" - $ gh gist search --filename --code "foo|bar" - `), - Args: cmdutil.ExactArgs(1, "no search pattern passed"), - RunE: func(cmd *cobra.Command, args []string) error { - var err error - opts.Pattern, err = regexp.CompilePOSIX(args[0]) - if err != nil { - return err - } - - // Replicate precedence of existing `gist` commands instead of mutually exclusive arguments. - opts.Visibility = "all" - if flagSecret { - opts.Visibility = "secret" - } else if flagPublic { - opts.Visibility = "public" - } - - if runF != nil { - return runF(opts) - } - return searchRun(opts) - }, - } - - cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 10, "Maximum number of gists to fetch") - cmd.Flags().BoolVar(&flagPublic, "public", false, "Show only public gists") - cmd.Flags().BoolVar(&flagSecret, "secret", false, "Show only secret gists") - cmd.Flags().BoolVar(&opts.Filename, "filename", false, "Include file names in search") - cmd.Flags().BoolVar(&opts.Code, "code", false, "Include code in search") - - return cmd -} - -func searchRun(opts *SearchOptions) error { - client, err := opts.HttpClient() - if err != nil { - return err - } - - cfg, err := opts.Config() - if err != nil { - return err - } - - host, _ := cfg.Authentication().DefaultHost() - - gists, err := shared.ListGists(client, host, opts.Limit, opts.Visibility, opts.Code, func(gist *shared.Gist) bool { - if opts.Pattern.MatchString(gist.Description) { - return true - } - - if opts.Filename || opts.Code { - for _, file := range gist.Files { - if opts.Filename && opts.Pattern.MatchString(file.Filename) { - return true - } - - if opts.Code && opts.Pattern.MatchString(file.Content) { - return true - } - } - } - - return false - }) - if err != nil { - return err - } - - if len(gists) == 0 { - return cmdutil.NewNoResultsError("no gists found") - } - - return shared.PrintGists(opts.IO, gists) -} diff --git a/pkg/cmd/gist/search/search_test.go b/pkg/cmd/gist/search/search_test.go deleted file mode 100644 index d6f98c1a3..000000000 --- a/pkg/cmd/gist/search/search_test.go +++ /dev/null @@ -1,371 +0,0 @@ -package search - -import ( - "bytes" - "fmt" - "net/http" - "regexp" - "testing" - "time" - - "github.com/MakeNowJust/heredoc" - "github.com/cli/cli/v2/internal/config" - "github.com/cli/cli/v2/internal/gh" - "github.com/cli/cli/v2/pkg/cmdutil" - "github.com/cli/cli/v2/pkg/httpmock" - "github.com/cli/cli/v2/pkg/iostreams" - "github.com/google/shlex" - "github.com/stretchr/testify/assert" -) - -func TestNewCmdSearch(t *testing.T) { - tests := []struct { - name string - cli string - wants SearchOptions - wantsErr bool - }{ - { - name: "pattern only", - cli: "octo", - wants: SearchOptions{ - Pattern: regexp.MustCompilePOSIX("octo"), - Limit: 10, - Visibility: "all", - }, - }, - { - name: "public", - cli: "--public octo", - wants: SearchOptions{ - Pattern: regexp.MustCompilePOSIX("octo"), - Limit: 10, - Visibility: "public", - }, - }, - { - name: "secret", - cli: `"foo|bar" --secret`, - wants: SearchOptions{ - Pattern: regexp.MustCompilePOSIX("foo|bar"), - Limit: 10, - Visibility: "secret", - }, - }, - { - name: "secret with explicit false value", - cli: "--secret=false octo", - wants: SearchOptions{ - Pattern: regexp.MustCompilePOSIX("octo"), - Limit: 10, - Visibility: "all", - }, - }, - { - name: "public and secret", - cli: "--secret --public octo", - wants: SearchOptions{ - Pattern: regexp.MustCompilePOSIX("octo"), - Limit: 10, - Visibility: "secret", - }, - }, - { - name: "limit", - cli: "--limit 30 octo", - wants: SearchOptions{ - Pattern: regexp.MustCompilePOSIX("octo"), - Limit: 30, - Visibility: "all", - }, - }, - { - name: "all arguments", - cli: "--public --secret --filename --code --limit 30 octo", - wants: SearchOptions{ - Pattern: regexp.MustCompilePOSIX("octo"), - Filename: true, - Code: true, - Limit: 30, - Visibility: "secret", - }, - }, - { - name: "invalid regexp pattern", - cli: `invalid(\\`, - wantsErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - f := &cmdutil.Factory{} - - argv, err := shlex.Split(tt.cli) - assert.NoError(t, err) - - var gotOpts *SearchOptions - cmd := NewCmdSearch(f, func(opts *SearchOptions) error { - gotOpts = opts - return nil - }) - cmd.SetArgs(argv) - cmd.SetIn(&bytes.Buffer{}) - cmd.SetOut(&bytes.Buffer{}) - 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()) - assert.Equal(t, tt.wants.Filename, gotOpts.Filename) - assert.Equal(t, tt.wants.Code, gotOpts.Code) - assert.Equal(t, tt.wants.Visibility, gotOpts.Visibility) - assert.Equal(t, tt.wants.Limit, gotOpts.Limit) - }) - } -} - -func Test_searchRun(t *testing.T) { - const query = `query GistList\b` - sixHours, _ := time.ParseDuration("6h") - sixHoursAgo := time.Now().Add(-sixHours) - - tests := []struct { - name string - opts *SearchOptions - wantErr bool - wantOut string - stubs func(*httpmock.Registry) - nontty bool - }{ - { - name: "no gists", - opts: &SearchOptions{}, - stubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(query), - httpmock.StringResponse(`{ "data": { "viewer": { - "gists": { "nodes": [] } - } } }`)) - }, - wantErr: true, - }, - { - name: "description only", - opts: &SearchOptions{ - Pattern: regexp.MustCompilePOSIX("foo"), - }, - stubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(query), - httpmock.StringResponse(fmt.Sprintf( - `{ "data": { "viewer": { "gists": { "nodes": [ - { - "name": "1234567890", - "files": [{ "name": "cool.txt" }], - "description": "foo example", - "updatedAt": "%[1]v", - "isPublic": true - }, - { - "name": "4567890123", - "files": [{ "name": "gistfile0.txt" }], - "description": "bar example", - "updatedAt": "%[1]v", - "isPublic": true - } - ] } } } }`, - sixHoursAgo.Format(time.RFC3339), - )), - ) - }, - wantOut: heredoc.Doc(` - ID DESCRIPTION FILES VISIBILITY UPDATED - 1234567890 foo example 1 file public about 6 hours ago - `), - }, - { - name: "no results with public filter", - opts: &SearchOptions{ - Pattern: regexp.MustCompilePOSIX("foo"), - Visibility: "public", - }, - stubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(query), - httpmock.StringResponse(fmt.Sprintf( - `{ "data": { "viewer": { "gists": { "nodes": [ - { - "name": "4567890123", - "files": [{ "name": "gistfile0.txt" }], - "description": "bar example", - "updatedAt": "%[1]v", - "isPublic": true - } - ] } } } }`, - sixHoursAgo.Format(time.RFC3339), - )), - ) - }, - wantErr: true, - }, - { - name: "description only", - opts: &SearchOptions{ - Pattern: regexp.MustCompilePOSIX("foo"), - }, - stubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(query), - httpmock.StringResponse(fmt.Sprintf( - `{ "data": { "viewer": { "gists": { "nodes": [ - { - "name": "1234567890", - "files": [{ "name": "cool.txt" }], - "description": "foo example", - "updatedAt": "%[1]v", - "isPublic": true - }, - { - "name": "4567890123", - "files": [{ "name": "gistfile0.txt" }], - "description": "bar example", - "updatedAt": "%[1]v", - "isPublic": true - } - ] } } } }`, - sixHoursAgo.Format(time.RFC3339), - )), - ) - }, - wantOut: heredoc.Doc(` - ID DESCRIPTION FILES VISIBILITY UPDATED - 1234567890 foo example 1 file public about 6 hours ago - `), - }, - { - name: "include filenames", - opts: &SearchOptions{ - Pattern: regexp.MustCompilePOSIX("gistfile7"), - Filename: true, - }, - stubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(query), - httpmock.StringResponse(fmt.Sprintf( - `{ "data": { "viewer": { "gists": { "nodes": [ - { - "name": "2345678901", - "files": [ - { "name": "gistfile0.txt" }, - { "name": "gistfile1.txt" } - ], - "description": "foo example", - "updatedAt": "%[1]v", - "isPublic": true - }, - { - "name": "3456789012", - "files": [ - { "name": "gistfile0.txt" }, - { "name": "gistfile1.txt" }, - { "name": "gistfile2.txt" }, - { "name": "gistfile3.txt" }, - { "name": "gistfile4.txt" }, - { "name": "gistfile5.txt" }, - { "name": "gistfile6.txt" }, - { "name": "gistfile7.txt" }, - { "name": "gistfile9.txt" }, - { "name": "gistfile10.txt" }, - { "name": "gistfile11.txt" } - ], - "description": "bar example", - "updatedAt": "%[1]v", - "isPublic": false - } - ] } } } }`, - sixHoursAgo.Format(time.RFC3339), - )), - ) - }, - wantOut: heredoc.Doc(` - ID DESCRIPTION FILES VISIBILITY UPDATED - 3456789012 bar example 11 files secret about 6 hours ago - `), - }, - { - name: "include code", - opts: &SearchOptions{ - Pattern: regexp.MustCompilePOSIX("octo"), - Code: true, - }, - stubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(query), - httpmock.StringResponse(fmt.Sprintf( - `{ "data": { "viewer": { "gists": { "nodes": [ - { - "name": "2345678901", - "files": [ - { - "name": "gistfile0.txt", - "text": "octo" - } - ], - "description": "foo example", - "updatedAt": "%[1]v", - "isPublic": true - } - ] } } } }`, - sixHoursAgo.Format(time.RFC3339), - )), - ) - }, - wantOut: heredoc.Doc(` - ID DESCRIPTION FILES VISIBILITY UPDATED - 2345678901 foo example 1 file public about 6 hours ago - `), - }, - } - - for _, tt := range tests { - reg := &httpmock.Registry{} - tt.stubs(reg) - - tt.opts.HttpClient = func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil - } - - tt.opts.Config = func() (gh.Config, error) { - return config.NewBlankConfig(), nil - } - - ios, _, stdout, _ := iostreams.Test() - ios.SetStdoutTTY(!tt.nontty) - tt.opts.IO = ios - - if tt.opts.Limit == 0 { - tt.opts.Limit = 10 - } - - if tt.opts.Visibility == "" { - tt.opts.Visibility = "all" - } - t.Run(tt.name, func(t *testing.T) { - err := searchRun(tt.opts) - if tt.wantErr { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - - assert.Equal(t, tt.wantOut, stdout.String()) - reg.Verify(t) - }) - } -} diff --git a/pkg/cmd/gist/shared/shared.go b/pkg/cmd/gist/shared/shared.go index a616ca9b6..ee6dcf1e3 100644 --- a/pkg/cmd/gist/shared/shared.go +++ b/pkg/cmd/gist/shared/shared.go @@ -5,13 +5,13 @@ import ( "fmt" "net/http" "net/url" + "regexp" "sort" "strings" "time" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/prompter" - "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/iostreams" "github.com/gabriel-vasile/mimetype" @@ -77,7 +77,7 @@ func GistIDFromURL(gistURL string) (string, error) { const maxPerPage = 100 -func ListGists(client *http.Client, hostname string, limit int, visibility string, includeText bool, filter func(*Gist) bool) ([]Gist, error) { +func ListGists(client *http.Client, hostname string, limit int, filter *regexp.Regexp, includeContent bool, visibility string) ([]Gist, error) { type response struct { Viewer struct { Gists struct { @@ -85,7 +85,7 @@ func ListGists(client *http.Client, hostname string, limit int, visibility strin Description string Files []struct { Name string - Text string `graphql:"text @include(if: $includeText)"` + Text string `graphql:"text @include(if: $includeContent)"` } IsPublic bool Name string @@ -105,10 +105,28 @@ func ListGists(client *http.Client, hostname string, limit int, visibility strin } variables := map[string]interface{}{ - "per_page": githubv4.Int(perPage), - "endCursor": (*githubv4.String)(nil), - "visibility": githubv4.GistPrivacy(strings.ToUpper(visibility)), - "includeText": githubv4.Boolean(includeText), + "per_page": githubv4.Int(perPage), + "endCursor": (*githubv4.String)(nil), + "visibility": githubv4.GistPrivacy(strings.ToUpper(visibility)), + "includeContent": githubv4.Boolean(includeContent), + } + + filterFunc := func(gist *Gist) bool { + if filter.MatchString(gist.Description) { + return true + } + + for _, file := range gist.Files { + if filter.MatchString(file.Filename) { + return true + } + + if includeContent && filter.MatchString(file.Content) { + return true + } + } + + return false } gql := api.NewClientFromHTTP(client) @@ -139,7 +157,7 @@ pagination: Public: gist.IsPublic, } - if filter == nil || filter(&gist) { + if filter == nil || filterFunc(&gist) { gists = append(gists, gist) } @@ -185,7 +203,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, nil) + gists, err := ListGists(client, host, 10, nil, false, "all") if err != nil { return "", err } @@ -228,47 +246,3 @@ func PromptGists(prompter prompter.Prompter, client *http.Client, host string, c return gistIDs[result], nil } - -func PrintGists(io *iostreams.IOStreams, gists []Gist) error { - if err := io.StartPager(); err == nil { - defer io.StopPager() - } else { - fmt.Fprintf(io.ErrOut, "failed to start pager: %v\n", err) - } - - cs := io.ColorScheme() - tp := tableprinter.New(io, tableprinter.WithHeader("ID", "DESCRIPTION", "FILES", "VISIBILITY", "UPDATED")) - - for _, gist := range gists { - fileCount := len(gist.Files) - - visibility := "public" - visColor := cs.Green - if !gist.Public { - visibility = "secret" - visColor = cs.Red - } - - description := gist.Description - if description == "" { - for filename := range gist.Files { - if !strings.HasPrefix(filename, "gistfile") { - description = filename - break - } - } - } - - tp.AddField(gist.ID) - tp.AddField( - text.RemoveExcessiveWhitespace(description), - tableprinter.WithColor(cs.Bold), - ) - tp.AddField(text.Pluralize(fileCount, "file")) - tp.AddField(visibility, tableprinter.WithColor(visColor)) - tp.AddTimeField(time.Now(), gist.UpdatedAt, cs.Gray) - tp.EndRow() - } - - return tp.Render() -} From 73af9f8bd98494d212d9a671ee00af74135376bd Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Wed, 9 Oct 2024 08:56:13 -0700 Subject: [PATCH 04/13] Improve help docs Also changes `--filter` argument to "expression" to coincide with some other commands' help text e.g., `--jq`. --- pkg/cmd/gist/list/list.go | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 215fedcce..87db0fc5c 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/internal/text" @@ -39,8 +40,24 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman var flagFilter string cmd := &cobra.Command{ - Use: "list", - Short: "List your gists", + Use: "list", + Short: "List your gists", + Long: heredoc.Docf(` + List gists from your user account. + + You can use a regular expression to filter the description, file names, + or even the content of files in the gist. See https://pkg.go.dev/regexp/syntax + for the regular expression syntax you can pass to %[1]s--filter%[1]s. Pass + %[1]s--include-content%[1]s to also search the content of files noting that + this will take longer since all files' content is fetched. + `, "`"), + Example: heredoc.Doc(` + # list all secret gists from your user account + $ gh gist list --secret + + # find all gists from your user account mentioning "octo" anywhere + $ gh gist list --filter octo --include-content + `), Aliases: []string{"ls"}, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { @@ -71,7 +88,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 10, "Maximum number of gists to fetch") cmd.Flags().BoolVar(&flagPublic, "public", false, "Show only public gists") cmd.Flags().BoolVar(&flagSecret, "secret", false, "Show only secret gists") - cmd.Flags().StringVar(&flagFilter, "filter", "", "Filter gists using a regular expression") + cmd.Flags().StringVar(&flagFilter, "filter", "", "Filter gists using a regular `expression`") cmd.Flags().BoolVar(&opts.IncludeContent, "include-content", false, "Include gists' file content when filtering") return cmd From b56353dc0222bffaa224cc40f64da39d43dcbd8b Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Thu, 10 Oct 2024 11:12:24 -0700 Subject: [PATCH 05/13] Disallow use of --include-content without --filter --- pkg/cmd/gist/list/list.go | 6 ++++++ pkg/cmd/gist/list/list_test.go | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 87db0fc5c..6e228b51c 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -65,6 +65,12 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman return cmdutil.FlagErrorf("invalid limit: %v", opts.Limit) } + if flagFilter == "" { + if opts.IncludeContent { + return cmdutil.FlagErrorf("cannot use --include-content without --filter") + } + } + if filter, err := regexp.CompilePOSIX(flagFilter); err != nil { return err } else { diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index 80e3178ab..0b88caf83 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -92,6 +92,11 @@ func TestNewCmdList(t *testing.T) { cli: "--filter octo(", wantsErr: true, }, + { + name: "include content without filter", + cli: "--include-content", + wantsErr: true, + }, } for _, tt := range tests { From 1615f2c1e1c9cb3547807becb07bd961e8eb3764 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Fri, 11 Oct 2024 10:28:57 -0700 Subject: [PATCH 06/13] Simplify description --- pkg/cmd/gist/list/list.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 6e228b51c..40e422c73 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -46,10 +46,12 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman List gists from your user account. You can use a regular expression to filter the description, file names, - or even the content of files in the gist. See https://pkg.go.dev/regexp/syntax - for the regular expression syntax you can pass to %[1]s--filter%[1]s. Pass - %[1]s--include-content%[1]s to also search the content of files noting that - this will take longer since all files' content is fetched. + or even the content of files in the gist using %[1]s--filter%[1]s. + + Use %[1]s--include-content%[1]s to include content of files, noting that + this will be slower and increase the rate limit used. + + For supported regular expression syntax, see https://pkg.go.dev/regexp/syntax `, "`"), Example: heredoc.Doc(` # list all secret gists from your user account From 4eaeda580b6f33292f3dddc0f767e4fdf803acf5 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Fri, 11 Oct 2024 13:20:43 -0700 Subject: [PATCH 07/13] Show progress when filtering Filtering can take a while, so show progress. This also uncovered a bug that I wasn't checking if a filter was actually specified, resulting in a non-nil `opts.Filter`. --- pkg/cmd/gist/list/list.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 40e422c73..ae138d6a2 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -71,12 +71,12 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman if opts.IncludeContent { return cmdutil.FlagErrorf("cannot use --include-content without --filter") } - } - - if filter, err := regexp.CompilePOSIX(flagFilter); err != nil { - return err } else { - opts.Filter = filter + if filter, err := regexp.CompilePOSIX(flagFilter); err != nil { + return err + } else { + opts.Filter = filter + } } opts.Visibility = "all" @@ -115,7 +115,13 @@ func listRun(opts *ListOptions) error { host, _ := cfg.Authentication().DefaultHost() + // Filtering can take a while so start the progress indicator. StopProgressIndicator will no-op if not running. + if opts.Filter != nil { + opts.IO.StartProgressIndicator() + } gists, err := shared.ListGists(client, host, opts.Limit, opts.Filter, opts.IncludeContent, opts.Visibility) + opts.IO.StopProgressIndicator() + if err != nil { return err } From 70b14215ec1d0dccce17109ea8b680dc0caec1e2 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Fri, 11 Oct 2024 00:52:00 -0700 Subject: [PATCH 08/13] Print filtered gists similar to code search --- pkg/cmd/gist/list/list.go | 85 ++++++++++++++++++++++++++++++++++++- pkg/cmd/search/code/code.go | 16 +++---- pkg/iostreams/color.go | 30 +++++++++++++ 3 files changed, 120 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index ae138d6a2..c58ed06ea 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -136,8 +136,16 @@ func listRun(opts *ListOptions) error { fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) } - cs := opts.IO.ColorScheme() - tp := tableprinter.New(opts.IO, tableprinter.WithHeader("ID", "DESCRIPTION", "FILES", "VISIBILITY", "UPDATED")) + if opts.IO.ColorEnabled() && opts.Filter != nil { + return printHighlights(opts.IO, gists, opts.Filter) + } + + return printTable(opts.IO, gists) +} + +func printTable(io *iostreams.IOStreams, gists []shared.Gist) error { + cs := io.ColorScheme() + tp := tableprinter.New(io, tableprinter.WithHeader("ID", "DESCRIPTION", "FILES", "VISIBILITY", "UPDATED")) for _, gist := range gists { fileCount := len(gist.Files) @@ -172,3 +180,76 @@ func listRun(opts *ListOptions) error { return tp.Render() } + +func printHighlights(io *iostreams.IOStreams, gists []shared.Gist, filter *regexp.Regexp) error { + const tab string = " " + cs := io.ColorScheme() + + out := &strings.Builder{} + var filename, description string + var err error + text := func(s string) string { + return s + } + for _, gist := range gists { + for _, file := range gist.Files { + found := false + out.Reset() + + if filename, err = highlightMatch(file.Filename, filter, &found, cs.Green, cs.Highlight); err != nil { + return err + } + fmt.Fprintln(out, cs.Blue(gist.ID), filename) + + if gist.Description != "" { + if description, err = highlightMatch(gist.Description, filter, &found, cs.Bold, cs.Highlight); err != nil { + return err + } + fmt.Fprintln(out, tab, description) + } + + if file.Content != "" { + for _, line := range strings.Split(file.Content, "\n") { + if filter.MatchString(line) { + if line, err = highlightMatch(line, filter, &found, text, cs.Highlight); err != nil { + return err + } + fmt.Fprintln(out, tab, tab, line) + } + } + } + + if found { + fmt.Fprintln(io.Out, out.String()) + } + } + } + + return nil +} + +func highlightMatch(s string, filter *regexp.Regexp, found *bool, color, highlight func(string) string) (string, error) { + matches := filter.FindAllStringIndex(s, -1) + if matches == nil { + return color(s), nil + } + + out := strings.Builder{} + if _, err := out.WriteString(color(s[:matches[0][0]])); err != nil { + return "", err + } + + for _, match := range matches { + if _, err := out.WriteString(highlight(s[match[0]:match[1]])); err != nil { + return "", err + } + if _, err := out.WriteString(color(s[match[1]:])); err != nil { + return "", nil + } + } + + if found != nil { + *found = *found || true + } + return out.String(), nil +} diff --git a/pkg/cmd/search/code/code.go b/pkg/cmd/search/code/code.go index 0c318be74..63633cc53 100644 --- a/pkg/cmd/search/code/code.go +++ b/pkg/cmd/search/code/code.go @@ -143,7 +143,7 @@ func displayResults(io *iostreams.IOStreams, results search.CodeResult) error { } fmt.Fprintf(io.Out, "%s %s\n", cs.Blue(code.Repository.FullName), cs.GreenBold(code.Path)) for _, match := range code.TextMatches { - lines := formatMatch(match.Fragment, match.Matches, io.ColorEnabled()) + lines := formatMatch(match.Fragment, match.Matches, io) for _, line := range lines { fmt.Fprintf(io.Out, "\t%s\n", strings.TrimSpace(line)) } @@ -153,7 +153,7 @@ func displayResults(io *iostreams.IOStreams, results search.CodeResult) error { } for _, code := range results.Items { for _, match := range code.TextMatches { - lines := formatMatch(match.Fragment, match.Matches, io.ColorEnabled()) + lines := formatMatch(match.Fragment, match.Matches, io) for _, line := range lines { fmt.Fprintf(io.Out, "%s:%s: %s\n", cs.Blue(code.Repository.FullName), cs.GreenBold(code.Path), strings.TrimSpace(line)) } @@ -162,7 +162,9 @@ func displayResults(io *iostreams.IOStreams, results search.CodeResult) error { return nil } -func formatMatch(t string, matches []search.Match, colorize bool) []string { +func formatMatch(t string, matches []search.Match, io *iostreams.IOStreams) []string { + cs := io.ColorScheme() + startIndices := map[int]struct{}{} endIndices := map[int]struct{}{} for _, m := range matches { @@ -186,14 +188,10 @@ func formatMatch(t string, matches []search.Match, colorize bool) []string { continue } if _, ok := startIndices[i]; ok { - if colorize { - b.WriteString("\x1b[30;43m") // black text on yellow background - } + b.WriteString(cs.HighlightStart()) // black text on yellow background found = true } else if _, ok := endIndices[i]; ok { - if colorize { - b.WriteString("\x1b[m") // color reset - } + b.WriteString(cs.Reset()) // color reset } b.WriteRune(c) } diff --git a/pkg/iostreams/color.go b/pkg/iostreams/color.go index 804b9a275..c8d48168f 100644 --- a/pkg/iostreams/color.go +++ b/pkg/iostreams/color.go @@ -8,6 +8,10 @@ import ( "github.com/mgutz/ansi" ) +const ( + highlightStyle = "black:yellow" +) + var ( magenta = ansi.ColorFunc("magenta") cyan = ansi.ColorFunc("cyan") @@ -20,6 +24,8 @@ var ( bold = ansi.ColorFunc("default+b") cyanBold = ansi.ColorFunc("cyan+b") greenBold = ansi.ColorFunc("green+b") + highlightStart = ansi.ColorCode(highlightStyle) + highlight = ansi.ColorFunc(highlightStyle) gray256 = func(t string) string { return fmt.Sprintf("\x1b[%d;5;%dm%s\x1b[m", 38, 242, t) @@ -176,6 +182,30 @@ func (c *ColorScheme) FailureIconWithColor(colo func(string) string) string { return colo("X") } +func (c *ColorScheme) HighlightStart() string { + if !c.enabled { + return "" + } + + return highlightStart +} + +func (c *ColorScheme) Highlight(t string) string { + if !c.enabled { + return t + } + + return highlight(t) +} + +func (c *ColorScheme) Reset() string { + if !c.enabled { + return "" + } + + return ansi.Reset +} + func (c *ColorScheme) ColorFromString(s string) func(string) string { s = strings.ToLower(s) var fn func(string) string From 86f045ef0e0e0b4d21296747aad08b48b9676df9 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Fri, 11 Oct 2024 10:40:54 -0700 Subject: [PATCH 09/13] Split all newlines, and output no-color to non-TTY --- pkg/cmd/gist/list/list.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index c58ed06ea..e5d55d8c1 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -136,7 +136,7 @@ func listRun(opts *ListOptions) error { fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) } - if opts.IO.ColorEnabled() && opts.Filter != nil { + if opts.Filter != nil { return printHighlights(opts.IO, gists, opts.Filter) } @@ -188,9 +188,13 @@ func printHighlights(io *iostreams.IOStreams, gists []shared.Gist, filter *regex out := &strings.Builder{} var filename, description string var err error + split := func(r rune) bool { + return r == '\n' || r == '\r' + } text := func(s string) string { return s } + for _, gist := range gists { for _, file := range gist.Files { found := false @@ -209,7 +213,7 @@ func printHighlights(io *iostreams.IOStreams, gists []shared.Gist, filter *regex } if file.Content != "" { - for _, line := range strings.Split(file.Content, "\n") { + for _, line := range strings.FieldsFunc(file.Content, split) { if filter.MatchString(line) { if line, err = highlightMatch(line, filter, &found, text, cs.Highlight); err != nil { return err @@ -248,8 +252,6 @@ func highlightMatch(s string, filter *regexp.Regexp, found *bool, color, highlig } } - if found != nil { - *found = *found || true - } + *found = *found || true return out.String(), nil } From bddadef5740558caf40ebd0487c3de4a0b508f4b Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Sat, 12 Oct 2024 02:03:47 -0700 Subject: [PATCH 10/13] Highlight matches in table and content When `--filter` is passed, matches will be highlighted in the existing table. If file names match, the "n file(s)" cell will be highlighted. When `--include-content` is additionally passed, the file name, description, and/or content will be printed like `search code` with matches highlighted. --- pkg/cmd/gist/list/list.go | 70 +++++++++---- pkg/cmd/gist/list/list_test.go | 185 +++++++++++++++++++++++++++++++-- 2 files changed, 227 insertions(+), 28 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index e5d55d8c1..2975a55df 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -49,7 +49,8 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman or even the content of files in the gist using %[1]s--filter%[1]s. Use %[1]s--include-content%[1]s to include content of files, noting that - this will be slower and increase the rate limit used. + this will be slower and increase the rate limit used. Instead of printing a table, + code will be printed with highlights. For supported regular expression syntax, see https://pkg.go.dev/regexp/syntax `, "`"), @@ -136,17 +137,39 @@ func listRun(opts *ListOptions) error { fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) } - if opts.Filter != nil { - return printHighlights(opts.IO, gists, opts.Filter) + if opts.Filter != nil && opts.IncludeContent { + return printContent(opts.IO, gists, opts.Filter) } - return printTable(opts.IO, gists) + return printTable(opts.IO, gists, opts.Filter) } -func printTable(io *iostreams.IOStreams, gists []shared.Gist) error { +func printTable(io *iostreams.IOStreams, gists []shared.Gist, filter *regexp.Regexp) error { cs := io.ColorScheme() tp := tableprinter.New(io, tableprinter.WithHeader("ID", "DESCRIPTION", "FILES", "VISIBILITY", "UPDATED")) + // Highlight filter matches in the description when printing the table. + highlightDescription := func(s string) string { + if filter != nil { + if str, err := highlightMatch(s, filter, nil, cs.Bold, cs.Highlight); err == nil { + return str + } + } + return cs.Bold(s) + } + + // Highlight the files column when any file name matches the filter. + highlightFilesFunc := func(gist *shared.Gist) func(string) string { + if filter != nil { + for _, file := range gist.Files { + if filter.MatchString(file.Filename) { + return cs.Highlight + } + } + } + return normal + } + for _, gist := range gists { fileCount := len(gist.Files) @@ -170,9 +193,12 @@ func printTable(io *iostreams.IOStreams, gists []shared.Gist) error { tp.AddField(gist.ID) tp.AddField( text.RemoveExcessiveWhitespace(description), - tableprinter.WithColor(cs.Bold), + tableprinter.WithColor(highlightDescription), + ) + tp.AddField( + text.Pluralize(fileCount, "file"), + tableprinter.WithColor(highlightFilesFunc(&gist)), ) - tp.AddField(text.Pluralize(fileCount, "file")) tp.AddField(visibility, tableprinter.WithColor(visColor)) tp.AddTimeField(time.Now(), gist.UpdatedAt, cs.Gray) tp.EndRow() @@ -181,7 +207,7 @@ func printTable(io *iostreams.IOStreams, gists []shared.Gist) error { return tp.Render() } -func printHighlights(io *iostreams.IOStreams, gists []shared.Gist, filter *regexp.Regexp) error { +func printContent(io *iostreams.IOStreams, gists []shared.Gist, filter *regexp.Regexp) error { const tab string = " " cs := io.ColorScheme() @@ -191,39 +217,36 @@ func printHighlights(io *iostreams.IOStreams, gists []shared.Gist, filter *regex split := func(r rune) bool { return r == '\n' || r == '\r' } - text := func(s string) string { - return s - } for _, gist := range gists { for _, file := range gist.Files { - found := false + matched := false out.Reset() - if filename, err = highlightMatch(file.Filename, filter, &found, cs.Green, cs.Highlight); err != nil { + if filename, err = highlightMatch(file.Filename, filter, &matched, cs.Green, cs.Highlight); err != nil { return err } fmt.Fprintln(out, cs.Blue(gist.ID), filename) if gist.Description != "" { - if description, err = highlightMatch(gist.Description, filter, &found, cs.Bold, cs.Highlight); err != nil { + if description, err = highlightMatch(gist.Description, filter, &matched, cs.Bold, cs.Highlight); err != nil { return err } - fmt.Fprintln(out, tab, description) + fmt.Fprintf(out, "%s%s\n", tab, description) } if file.Content != "" { for _, line := range strings.FieldsFunc(file.Content, split) { if filter.MatchString(line) { - if line, err = highlightMatch(line, filter, &found, text, cs.Highlight); err != nil { + if line, err = highlightMatch(line, filter, &matched, normal, cs.Highlight); err != nil { return err } - fmt.Fprintln(out, tab, tab, line) + fmt.Fprintf(out, "%[1]s%[1]s%[2]s\n", tab, line) } } } - if found { + if matched { fmt.Fprintln(io.Out, out.String()) } } @@ -232,7 +255,7 @@ func printHighlights(io *iostreams.IOStreams, gists []shared.Gist, filter *regex return nil } -func highlightMatch(s string, filter *regexp.Regexp, found *bool, color, highlight func(string) string) (string, error) { +func highlightMatch(s string, filter *regexp.Regexp, matched *bool, color, highlight func(string) string) (string, error) { matches := filter.FindAllStringIndex(s, -1) if matches == nil { return color(s), nil @@ -252,6 +275,13 @@ func highlightMatch(s string, filter *regexp.Regexp, found *bool, color, highlig } } - *found = *found || true + if matched != nil { + *matched = *matched || true + } + return out.String(), nil } + +func normal(s string) string { + return s +} diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index 0b88caf83..b20c0efeb 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -141,6 +141,7 @@ func Test_listRun(t *testing.T) { wantErr bool wantOut string stubs func(*httpmock.Registry) + color bool nontty bool }{ { @@ -390,12 +391,62 @@ func Test_listRun(t *testing.T) { nontty: true, }, { - name: "with content filter", + name: "filtered", opts: &ListOptions{ - Filter: regexp.MustCompile("octo"), - IncludeContent: true, - Visibility: "all", + Filter: regexp.MustCompile("octo"), + Visibility: "all", }, + nontty: true, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "1234", + "files": [ + { "name": "main.txt", "text": "foo" } + ], + "description": "octo match in the description", + "updatedAt": "%[1]v", + "isPublic": true + }, + { + "name": "2345", + "files": [ + { "name": "main.txt", "text": "foo" }, + { "name": "octo.txt", "text": "bar" } + ], + "description": "match in the file name", + "updatedAt": "%[1]v", + "isPublic": false + }, + { + "name": "3456", + "files": [ + { "name": "main.txt", "text": "octo in the text" } + ], + "description": "match in the file text", + "updatedAt": "%[1]v", + "isPublic": true + } + ] } } } }`, + absTime.Format(time.RFC3339), + )), + ) + }, + wantOut: heredoc.Docf(` + 1234%[1]socto match in the description%[1]s1 file%[1]spublic%[1]s2020-07-30T15:24:28Z + 2345%[1]smatch in the file name%[1]s2 files%[1]ssecret%[1]s2020-07-30T15:24:28Z + `, "\t"), + }, + { + name: "filtered (tty)", + opts: &ListOptions{ + Filter: regexp.MustCompile("octo"), + Visibility: "all", + }, + color: true, stubs: func(reg *httpmock.Registry) { reg.Register( httpmock.GraphQL(query), @@ -434,13 +485,130 @@ func Test_listRun(t *testing.T) { )), ) }, + wantOut: heredoc.Docf(` + %[1]s[0;2;4;37mID %[1]s[0m %[1]s[0;2;4;37mDESCRIPTION %[1]s[0m %[1]s[0;2;4;37mFILES %[1]s[0m %[1]s[0;2;4;37mVISIBILITY%[1]s[0m %[1]s[0;2;4;37mUPDATED %[1]s[0m + 1234 %[1]s[0;30;43mocto%[1]s[0m%[1]s[0;1;39m match in the description%[1]s[0m 1 file %[1]s[0;32mpublic %[1]s[0m %[1]s[38;5;242mabout 6 hours ago%[1]s[m + 2345 %[1]s[0;1;39mmatch in the file name %[1]s[0m %[1]s[0;30;43m2 files%[1]s[0m %[1]s[0;31msecret %[1]s[0m %[1]s[38;5;242mabout 6 hours ago%[1]s[m + `, "\x1b"), + }, + { + name: "filtered with content", + opts: &ListOptions{ + Filter: regexp.MustCompile("octo"), + IncludeContent: true, + Visibility: "all", + }, + nontty: true, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "1234", + "files": [ + { "name": "main.txt", "text": "foo" } + ], + "description": "octo match in the description", + "updatedAt": "%[1]v", + "isPublic": true + }, + { + "name": "2345", + "files": [ + { "name": "main.txt", "text": "foo" }, + { "name": "octo.txt", "text": "bar" } + ], + "description": "match in the file name", + "updatedAt": "%[1]v", + "isPublic": false + }, + { + "name": "3456", + "files": [ + { "name": "main.txt", "text": "octo in the text" } + ], + "description": "match in the file text", + "updatedAt": "%[1]v", + "isPublic": true + } + ] } } } }`, + absTime.Format(time.RFC3339), + )), + ) + }, wantOut: heredoc.Doc(` - ID DESCRIPTION FILES VISIBILITY UPDATED - 1234 octo match in the description 1 file public about 6 hours ago - 2345 match in the file name 2 files secret about 6 hours ago - 3456 match in the file text 1 file public about 6 hours ago + 1234 main.txt + octo match in the description + + 2345 octo.txt + match in the file name + + 3456 main.txt + match in the file text + octo in the text + `), }, + { + name: "filtered with content (tty)", + opts: &ListOptions{ + Filter: regexp.MustCompile("octo"), + IncludeContent: true, + Visibility: "all", + }, + color: true, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "1234", + "files": [ + { "name": "main.txt", "text": "foo" } + ], + "description": "octo match in the description", + "updatedAt": "%[1]v", + "isPublic": true + }, + { + "name": "2345", + "files": [ + { "name": "main.txt", "text": "foo" }, + { "name": "octo.txt", "text": "bar" } + ], + "description": "match in the file name", + "updatedAt": "%[1]v", + "isPublic": false + }, + { + "name": "3456", + "files": [ + { "name": "main.txt", "text": "octo in the text" } + ], + "description": "match in the file text", + "updatedAt": "%[1]v", + "isPublic": true + } + ] } } } }`, + sixHoursAgo.Format(time.RFC3339), + )), + ) + }, + wantOut: heredoc.Docf(` + %[1]s[0;34m1234%[1]s[0m %[1]s[0;32mmain.txt%[1]s[0m + %[1]s[0;30;43mocto%[1]s[0m%[1]s[0;1;39m match in the description%[1]s[0m + + %[1]s[0;34m2345%[1]s[0m %[1]s[0;30;43mocto%[1]s[0m%[1]s[0;32m.txt%[1]s[0m + %[1]s[0;1;39mmatch in the file name%[1]s[0m + + %[1]s[0;34m3456%[1]s[0m %[1]s[0;32mmain.txt%[1]s[0m + %[1]s[0;1;39mmatch in the file text%[1]s[0m + %[1]s[0;30;43mocto%[1]s[0m in the text + + `, "\x1b"), + }, } for _, tt := range tests { @@ -456,6 +624,7 @@ func Test_listRun(t *testing.T) { } ios, _, stdout, _ := iostreams.Test() + ios.SetColorEnabled(tt.color) ios.SetStdoutTTY(!tt.nontty) tt.opts.IO = ios From e9d8092ffc632186791b703944fa0e9e043378c0 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Sat, 12 Oct 2024 11:04:31 -0700 Subject: [PATCH 11/13] Don't append remaining text if more matches --- pkg/cmd/gist/list/list.go | 12 ++++++-- pkg/cmd/gist/list/list_test.go | 56 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 2975a55df..bb5b7089c 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -262,15 +262,23 @@ func highlightMatch(s string, filter *regexp.Regexp, matched *bool, color, highl } out := strings.Builder{} + + // Color up to the first match. If an empty string, no ANSI color sequence is added. if _, err := out.WriteString(color(s[:matches[0][0]])); err != nil { return "", err } - for _, match := range matches { + // Highlight each match, then color the remaining text which, if an empty string, no ANSI color sequence is added. + for i, match := range matches { if _, err := out.WriteString(highlight(s[match[0]:match[1]])); err != nil { return "", err } - if _, err := out.WriteString(color(s[match[1]:])); err != nil { + + text := s[match[1]:] + if i+1 < len(matches) { + text = s[match[1]:matches[i+1][0]] + } + if _, err := out.WriteString(color(text)); err != nil { return "", nil } } diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index b20c0efeb..e6cb35d53 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -648,3 +648,59 @@ func Test_listRun(t *testing.T) { }) } } + +func Test_highlightMatch(t *testing.T) { + regex := regexp.MustCompilePOSIX(`[Oo]cto`) + tests := []struct { + name string + input string + color bool + want string + }{ + { + name: "single match", + input: "Octo", + want: "Octo", + }, + { + name: "single match (color)", + input: "Octo", + color: true, + want: "\x1b[0;30;43mOcto\x1b[0m", + }, + { + name: "single match with extra", + input: "Hello, Octocat!", + want: "Hello, Octocat!", + }, + { + name: "single match with extra (color)", + input: "Hello, Octocat!", + color: true, + want: "\x1b[0;34mHello, \x1b[0m\x1b[0;30;43mOcto\x1b[0m\x1b[0;34mcat!\x1b[0m", + }, + { + name: "multiple matches", + input: "Octocat/octo", + want: "Octocat/octo", + }, + { + name: "multiple matches (color)", + input: "Octocat/octo", + color: true, + want: "\x1b[0;30;43mOcto\x1b[0m\x1b[0;34mcat/\x1b[0m\x1b[0;30;43mocto\x1b[0m", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cs := iostreams.NewColorScheme(tt.color, false, false) + + matched := false + got, err := highlightMatch(tt.input, regex, &matched, cs.Blue, cs.Highlight) + assert.NoError(t, err) + assert.True(t, matched) + assert.Equal(t, tt.want, got) + }) + } +} From 9e00f1e4f227c5a68998cfdcd1ea19e56e0918bc Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Mon, 14 Oct 2024 15:08:23 -0700 Subject: [PATCH 12/13] Resolve PR feedback --- pkg/cmd/gist/list/list.go | 2 +- pkg/cmd/search/code/code.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index bb5b7089c..b1e76e52e 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -52,7 +52,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman this will be slower and increase the rate limit used. Instead of printing a table, code will be printed with highlights. - For supported regular expression syntax, see https://pkg.go.dev/regexp/syntax + For supported regular expression syntax, see `, "`"), Example: heredoc.Doc(` # list all secret gists from your user account diff --git a/pkg/cmd/search/code/code.go b/pkg/cmd/search/code/code.go index 63633cc53..761d2602c 100644 --- a/pkg/cmd/search/code/code.go +++ b/pkg/cmd/search/code/code.go @@ -188,10 +188,10 @@ func formatMatch(t string, matches []search.Match, io *iostreams.IOStreams) []st continue } if _, ok := startIndices[i]; ok { - b.WriteString(cs.HighlightStart()) // black text on yellow background + b.WriteString(cs.HighlightStart()) found = true } else if _, ok := endIndices[i]; ok { - b.WriteString(cs.Reset()) // color reset + b.WriteString(cs.Reset()) } b.WriteRune(c) } From 446f66070a38b9718357255f8abd66c6acf57c7d Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Tue, 15 Oct 2024 14:39:21 -0700 Subject: [PATCH 13/13] Add filtered content output to docs --- pkg/cmd/gist/list/list.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index b1e76e52e..00a401be4 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -48,11 +48,17 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman You can use a regular expression to filter the description, file names, or even the content of files in the gist using %[1]s--filter%[1]s. + For supported regular expression syntax, see . + Use %[1]s--include-content%[1]s to include content of files, noting that this will be slower and increase the rate limit used. Instead of printing a table, - code will be printed with highlights. + code will be printed with highlights similar to %[1]sgh search code%[1]s: - For supported regular expression syntax, see + {{gist ID}} {{file name}} + {{description}} + {{matching lines from content}} + + No highlights or other color is printed when output is redirected. `, "`"), Example: heredoc.Doc(` # list all secret gists from your user account @@ -207,6 +213,14 @@ func printTable(io *iostreams.IOStreams, gists []shared.Gist, filter *regexp.Reg return tp.Render() } +// printContent prints a gist with optional description and content similar to `gh search code` +// including highlighted matches in the form: +// +// {{gist ID}} {{file name}} +// {{description, if any}} +// {{content lines with matches, if any}} +// +// If printing to a non-TTY stream the format will be the same but without highlights. func printContent(io *iostreams.IOStreams, gists []shared.Gist, filter *regexp.Regexp) error { const tab string = " " cs := io.ColorScheme()