From 88887c8d5556770517a6d4480bcb08ec3a6d6b97 Mon Sep 17 00:00:00 2001 From: Josh Kraft Date: Mon, 8 May 2023 17:02:53 -0600 Subject: [PATCH] incorporate code review feedback --- pkg/cmd/search/code/code.go | 73 +++++++++++++++----------------- pkg/cmd/search/code/code_test.go | 29 ++++++------- pkg/search/query.go | 1 - pkg/search/result.go | 37 +++++++++++++--- pkg/search/result_test.go | 5 ++- pkg/search/searcher.go | 2 +- pkg/search/searcher_test.go | 7 ++- 7 files changed, 87 insertions(+), 67 deletions(-) diff --git a/pkg/cmd/search/code/code.go b/pkg/cmd/search/code/code.go index 7dad2b673..cde1703e4 100644 --- a/pkg/cmd/search/code/code.go +++ b/pkg/cmd/search/code/code.go @@ -29,7 +29,7 @@ func NewCmdCode(f *cmdutil.Factory, runF func(*CodeOptions) error) *cobra.Comman opts := &CodeOptions{ Browser: f.Browser, IO: f.IOStreams, - Query: search.Query{Kind: search.KindCode, TextMatch: true}, + Query: search.Query{Kind: search.KindCode}, } cmd := &cobra.Command{ @@ -59,9 +59,6 @@ func NewCmdCode(f *cmdutil.Factory, runF func(*CodeOptions) error) *cobra.Comman # search code matching "cli" in repositories owned by microsoft organization $ gh search code cli --owner=microsoft - # search code matching "html" in repositories owned by octocat - $ gh search code html --owner=octocat - # search code matching "panic" in cli repository $ gh search code panic --repo cli/cli @@ -99,8 +96,8 @@ func NewCmdCode(f *cmdutil.Factory, runF func(*CodeOptions) error) *cobra.Comman cmd.Flags().StringVar(&opts.Query.Qualifiers.Extension, "extension", "", "Filter on file extension") cmd.Flags().StringVar(&opts.Query.Qualifiers.Filename, "filename", "", "Filter on filename") cmdutil.StringSliceEnumFlag(cmd, &opts.Query.Qualifiers.In, "match", "", nil, []string{"file", "path"}, "Restrict search to file contents or file path") - cmd.Flags().StringVarP(&opts.Query.Qualifiers.Language, "language", "l", "", "Filter results by language") - cmd.Flags().StringSliceVar(&opts.Query.Qualifiers.Repo, "repo", nil, "Filter on repository") + cmd.Flags().StringVarP(&opts.Query.Qualifiers.Language, "language", "", "", "Filter results by language") + cmd.Flags().StringSliceVarP(&opts.Query.Qualifiers.Repo, "repo", "R", nil, "Filter on repository") cmd.Flags().StringVar(&opts.Query.Qualifiers.Size, "size", "", "Filter on size range, in kilobytes") cmd.Flags().StringSliceVar(&opts.Query.Qualifiers.User, "owner", nil, "Filter on owner") @@ -131,7 +128,7 @@ func codeRun(opts *CodeOptions) error { fmt.Fprintf(io.ErrOut, "failed to start pager: %v\n", err) } if opts.Exporter != nil { - return opts.Exporter.Write(io, results) + return opts.Exporter.Write(io, results.Items) } return displayResults(io, results, opts.Query.Limit) @@ -141,33 +138,28 @@ func displayResults(io *iostreams.IOStreams, results search.CodeResult, limit in cs := io.ColorScheme() tp := tableprinter.New(io) displayed := 0 +outer: for _, code := range results.Items { - if displayed == limit { - break - } if io.IsStdoutTTY() { header := fmt.Sprintf("%s %s", cs.GreenBold(code.Repo.FullName), cs.GreenBold(code.Path)) tp.AddField(header) tp.EndRow() } - row := 0 - for _, textMatch := range code.TextMatches { - out, shouldPrint := buildOutput(textMatch, cs) - for i, line := range strings.Split(out, "\n") { - if !shouldPrint[i] { - continue - } - if displayed == limit { - break - } + i := 1 + for _, match := range code.TextMatches { + fragments := formatMatch(cs, match) + for _, fragment := range fragments { if io.IsStdoutTTY() { - tp.AddField(fmt.Sprintf("%s: %s", cs.CyanBold(strconv.Itoa(row+1)), line)) + tp.AddField(fmt.Sprintf("%s: %s", strconv.Itoa(i), fragment)) } else { - tp.AddField(fmt.Sprintf("%s %s: %s", code.Repo.FullName, code.Path, line)) + tp.AddField(fmt.Sprintf("%s %s: %s", code.Repo.FullName, code.Path, fragment)) } tp.EndRow() - row++ + i++ displayed++ + if displayed == limit { + break outer + } } } } @@ -178,27 +170,32 @@ func displayResults(io *iostreams.IOStreams, results search.CodeResult, limit in return tp.Render() } -func buildOutput(tm search.TextMatch, cs *iostreams.ColorScheme) (string, map[int]bool) { - shouldHighlight := getHighlightIndices(tm.Matches) - linesToPrint := make(map[int]bool) - line := 0 - var out strings.Builder +func formatMatch(cs *iostreams.ColorScheme, tm search.TextMatch) []string { + shouldHighlight := matchHighlightMask(tm.Fragment, tm.Matches) + var lines []string + var found bool + var b strings.Builder for i, c := range tm.Fragment { - if shouldHighlight[i] { - out.WriteString(cs.CyanBold(string(c))) - linesToPrint[line] = true - } else { - out.WriteRune(c) - } if c == '\n' { - line++ + if found { + lines = append(lines, b.String()) + } + found = false + b.Reset() + continue + } + if shouldHighlight[i] { + b.WriteString(cs.CyanBold(string(c))) + found = true + } else { + b.WriteRune(c) } } - return out.String(), linesToPrint + return lines } -func getHighlightIndices(matches []search.Match) map[int]bool { - m := make(map[int]bool) +func matchHighlightMask(fragment string, matches []search.Match) []bool { + m := make([]bool, len(fragment)) for _, match := range matches { start := match.Indices[0] stop := match.Indices[1] diff --git a/pkg/cmd/search/code/code_test.go b/pkg/cmd/search/code/code_test.go index f48cb4bfc..8770ffbbc 100644 --- a/pkg/cmd/search/code/code_test.go +++ b/pkg/cmd/search/code/code_test.go @@ -32,10 +32,9 @@ func TestNewCmdCode(t *testing.T) { input: "react lifecycle", output: CodeOptions{ Query: search.Query{ - Keywords: []string{"react", "lifecycle"}, - Kind: "code", - Limit: 30, - TextMatch: true, + Keywords: []string{"react", "lifecycle"}, + Kind: "code", + Limit: 30, }, }, }, @@ -44,10 +43,9 @@ func TestNewCmdCode(t *testing.T) { input: "--web", output: CodeOptions{ Query: search.Query{ - Keywords: []string{}, - Kind: "code", - Limit: 30, - TextMatch: true, + Keywords: []string{}, + Kind: "code", + Limit: 30, }, WebMode: true, }, @@ -57,10 +55,9 @@ func TestNewCmdCode(t *testing.T) { input: "--limit 10", output: CodeOptions{ Query: search.Query{ - Keywords: []string{}, - Kind: "code", - Limit: 10, - TextMatch: true, + Keywords: []string{}, + Kind: "code", + Limit: 10, }, }, }, @@ -83,10 +80,9 @@ func TestNewCmdCode(t *testing.T) { `, output: CodeOptions{ Query: search.Query{ - Keywords: []string{}, - Kind: "code", - Limit: 30, - TextMatch: true, + Keywords: []string{}, + Kind: "code", + Limit: 30, Qualifiers: search.Qualifiers{ Extension: "extension", Filename: "filename", @@ -141,7 +137,6 @@ func TestCodeRun(t *testing.T) { Language: "go", Repo: []string{"cli/cli"}, }, - TextMatch: true, } tests := []struct { errMsg string diff --git a/pkg/search/query.go b/pkg/search/query.go index 5e9232bea..0181a2240 100644 --- a/pkg/search/query.go +++ b/pkg/search/query.go @@ -23,7 +23,6 @@ type Query struct { Page int Qualifiers Qualifiers Sort string - TextMatch bool } type Qualifiers struct { diff --git a/pkg/search/result.go b/pkg/search/result.go index beac37e4f..cfd78ccf1 100644 --- a/pkg/search/result.go +++ b/pkg/search/result.go @@ -15,6 +15,14 @@ var CodeFields = []string{ "url", } +var TextMatchFields = []string{ + "fragment", + "matches", + "url", + "type", + "property", +} + var CommitFields = []string{ "author", "commit", @@ -115,11 +123,11 @@ type Code struct { } type TextMatch struct { - Fragment string `json:"fragment"` - Matches []Match `json:"matches"` - ObjectURL string `json:"object_url"` - ObjectType string `json:"object_type"` - Property string `json:"property"` + Fragment string `json:"fragment"` + Matches []Match `json:"matches"` + URL string `json:"object_url"` + Type string `json:"object_type"` + Property string `json:"property"` } type Match struct { @@ -274,6 +282,25 @@ func (code Code) ExportData(fields []string) map[string]interface{} { switch f { case "repository": data[f] = code.Repo.ExportData(RepositoryFields) + case "textMatches": + matches := make([]interface{}, 0, len(code.TextMatches)) + for _, match := range code.TextMatches { + matches = append(matches, match.ExportData(TextMatchFields)) + } + data[f] = matches + default: + sf := fieldByName(v, f) + data[f] = sf.Interface() + } + } + return data +} + +func (textMatch TextMatch) ExportData(fields []string) map[string]interface{} { + v := reflect.ValueOf(textMatch) + data := map[string]interface{}{} + for _, f := range fields { + switch f { default: sf := fieldByName(v, f) data[f] = sf.Interface() diff --git a/pkg/search/result_test.go b/pkg/search/result_test.go index 0381d1856..930b80351 100644 --- a/pkg/search/result_test.go +++ b/pkg/search/result_test.go @@ -39,10 +39,13 @@ func TestCodeExportData(t *testing.T) { }, }, }, + Property: "property", + Type: "type", + URL: "url", }, }, }, - output: `{"name":"name","path":"path","textMatches":[{"fragment":"fragment","matches":[{"indices":[0,1],"text":"fr"}],"object_url":"","object_type":"","property":""}]}`, + output: `{"name":"name","path":"path","textMatches":[{"fragment":"fragment","matches":[{"indices":[0,1],"text":"fr"}],"property":"property","type":"type","url":"url"}]}`, }, } for _, tt := range tests { diff --git a/pkg/search/searcher.go b/pkg/search/searcher.go index 889a69487..4168dc7f3 100644 --- a/pkg/search/searcher.go +++ b/pkg/search/searcher.go @@ -173,7 +173,7 @@ func (s searcher) search(query Query, result interface{}) (*http.Response, error } req.Header.Set("Content-Type", "application/json; charset=utf-8") req.Header.Set("Accept", "application/vnd.github.v3+json") - if query.TextMatch { + if query.Kind == KindCode { req.Header.Set("Accept", "application/vnd.github.text-match+json") } diff --git a/pkg/search/searcher_test.go b/pkg/search/searcher_test.go index b6a8ae873..e0c41cfca 100644 --- a/pkg/search/searcher_test.go +++ b/pkg/search/searcher_test.go @@ -12,10 +12,9 @@ import ( func TestSearcherCode(t *testing.T) { query := Query{ - Keywords: []string{"keyword"}, - Kind: "code", - Limit: 30, - TextMatch: true, + Keywords: []string{"keyword"}, + Kind: "code", + Limit: 30, Qualifiers: Qualifiers{ Language: "go", },