From 9a149d7694b7ce767dcd84f9dfa91e0c89b4d0ca Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Thu, 11 Feb 2021 19:43:08 -0300 Subject: [PATCH 01/29] Add `repo list` command --- pkg/cmd/repo/list/http.go | 176 ++++++++++++++++++++++++++++++ pkg/cmd/repo/list/list.go | 224 ++++++++++++++++++++++++++++++++++++++ pkg/cmd/repo/repo.go | 2 + 3 files changed, 402 insertions(+) create mode 100644 pkg/cmd/repo/list/http.go create mode 100644 pkg/cmd/repo/list/list.go diff --git a/pkg/cmd/repo/list/http.go b/pkg/cmd/repo/list/http.go new file mode 100644 index 000000000..3409990c8 --- /dev/null +++ b/pkg/cmd/repo/list/http.go @@ -0,0 +1,176 @@ +package list + +import ( + "fmt" + "strings" + + "github.com/cli/cli/api" + "github.com/shurcooL/githubv4" +) + +type RepositoryList struct { + Repositories []Repository + TotalCount int +} + +func listRepos(client *api.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { + type reposBlock struct { + TotalCount int + RepositoryCount int + Nodes []Repository + PageInfo struct { + HasNextPage bool + EndCursor string + } + } + + type response struct { + RepositoryOwner struct { + Repositories reposBlock + } + Search reposBlock + } + + fragment := ` + fragment repo on Repository { + nameWithOwner + description + isFork + isPrivate + isArchived + updatedAt + }` + + // If `--archived` wasn't specified, use `repositoryOwner.repositores` + query := fragment + ` + query RepoList($owner: String!, $per_page: Int!, $endCursor: String, $fork: Boolean, $privacy: RepositoryPrivacy) { + repositoryOwner(login: $owner) { + repositories( + first: $per_page, + after: $endCursor, + privacy: $privacy, + isFork: $fork, + ownerAffiliations: OWNER, + orderBy: { field: UPDATED_AT, direction: DESC }) { + totalCount + nodes { + ...repo + } + pageInfo { + hasNextPage + endCursor + } + } + } + }` + + perPage := limit + if perPage > 100 { + perPage = 100 + } + + variables := map[string]interface{}{ + "per_page": githubv4.Int(perPage), + "endCursor": (*githubv4.String)(nil), + } + + hasArchivedFilter := filter.Archived + + if hasArchivedFilter { + // If `--archived` was specified, use the `search` API rather than + // `repositoryOwner.repositories` + query = fragment + ` + query RepoList($per_page: Int!, $endCursor: String, $query: String!) { + search(first: $per_page, after:$endCursor, type: REPOSITORY, query: $query) { + repositoryCount + nodes { + ... on Repository { + ...repo + } + } + pageInfo { + hasNextPage + endCursor + } + } + }` + + search := []string{fmt.Sprintf("user:%s archived:true fork:true sort:updated-desc", owner)} + + switch filter.Visibility { + case "private": + search = append(search, "is:private") + case "public": + search = append(search, "is:public") + default: + search = append(search, "is:all") + } + + variables["query"] = strings.Join(search, " ") + } else { + variables["owner"] = githubv4.String(owner) + + if filter.Visibility != "" { + variables["privacy"] = githubv4.RepositoryPrivacy(strings.ToUpper(filter.Visibility)) + } else { + variables["privacy"] = (*githubv4.RepositoryPrivacy)(nil) + } + + if filter.Fork { + variables["fork"] = githubv4.Boolean(true) + } else if filter.Source { + variables["fork"] = githubv4.Boolean(false) + } else { + variables["fork"] = (*githubv4.Boolean)(nil) + } + } + + var repos []Repository + var totalCount int + + var result response + +pagination: + for { + err := client.GraphQL(hostname, query, variables, &result) + if err != nil { + return nil, err + } + + if hasArchivedFilter { + repos = append(repos, result.Search.Nodes...) + } else { + repos = append(repos, result.RepositoryOwner.Repositories.Nodes...) + } + + if len(repos) >= limit { + if len(repos) > limit { + repos = repos[:limit] + } + break pagination + } + + if !result.RepositoryOwner.Repositories.PageInfo.HasNextPage { + if !result.Search.PageInfo.HasNextPage { + break + } + } + + variables["endCursor"] = githubv4.String(result.RepositoryOwner.Repositories.PageInfo.EndCursor) + if hasArchivedFilter { + variables["endCursor"] = githubv4.String(result.Search.PageInfo.EndCursor) + } + } + + totalCount = result.RepositoryOwner.Repositories.TotalCount + if hasArchivedFilter { + totalCount = result.Search.RepositoryCount + } + + listResult := &RepositoryList{ + Repositories: repos, + TotalCount: totalCount, + } + + return listResult, nil +} diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go new file mode 100644 index 000000000..b066764b3 --- /dev/null +++ b/pkg/cmd/repo/list/list.go @@ -0,0 +1,224 @@ +package list + +import ( + "fmt" + "net/http" + "strings" + "time" + "unicode" + + "github.com/cli/cli/api" + "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/text" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type FilterOptions struct { + Visibility string // private, public + Fork bool + Source bool + Archived bool +} + +type ListOptions struct { + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + + Limit int + Owner string + + Visibility string + Fork bool + Source bool + Archived bool +} + +func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { + opts := ListOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + } + + var ( + flagPublic bool + flagPrivate bool + flagSource bool + flagFork bool + flagArchived bool + ) + + cmd := &cobra.Command{ + Use: "list", + Args: cobra.MaximumNArgs(1), + Short: "List repositories from a user or organization", + RunE: func(c *cobra.Command, args []string) error { + if opts.Limit < 1 { + return &cmdutil.FlagError{Err: fmt.Errorf("invalid limit: %v", opts.Limit)} + } + + if flagPrivate && flagPublic { + return &cmdutil.FlagError{Err: fmt.Errorf("specify only one of `--public` or `--private`")} + } + if flagSource && flagFork { + return &cmdutil.FlagError{Err: fmt.Errorf("specify only one of `--source` or `--fork`")} + } + + if flagPrivate { + opts.Visibility = "private" + } else if flagPublic { + opts.Visibility = "public" + } + + opts.Archived = flagArchived + opts.Fork = flagFork + opts.Source = flagSource + + if len(args) > 0 { + opts.Owner = args[0] + } + + if runF != nil { + return runF(&opts) + } + return listRun(&opts) + }, + } + + cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum number of repositories to list") + cmd.Flags().BoolVar(&flagPrivate, "private", false, "Show only private repositories") + cmd.Flags().BoolVar(&flagPublic, "public", false, "Show only public repositories") + cmd.Flags().BoolVar(&flagSource, "source", false, "Show only source repositories") + cmd.Flags().BoolVar(&flagArchived, "archived", false, "Show only archived repositories") + cmd.Flags().BoolVar(&flagFork, "fork", false, "Show only forks") + + return cmd +} + +func listRun(opts *ListOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + + apiClient := api.NewClientFromHTTP(httpClient) + + isTerminal := opts.IO.IsStdoutTTY() + + owner := opts.Owner + if owner == "" { + owner, err = api.CurrentLoginName(apiClient, ghinstance.OverridableDefault()) + if err != nil { + return err + } + } + + filter := FilterOptions{ + Visibility: opts.Visibility, + Fork: opts.Fork, + Source: opts.Source, + Archived: opts.Archived, + } + + listResult, err := listRepos(apiClient, ghinstance.OverridableDefault(), opts.Limit, owner, filter) + if err != nil { + return err + } + + cs := opts.IO.ColorScheme() + + tp := utils.NewTablePrinter(opts.IO) + + notArchived := (filter.Fork || filter.Source) && !filter.Archived + + matchCount := len(listResult.Repositories) + now := time.Now() + + for _, repo := range listResult.Repositories { + if notArchived && repo.IsArchived { + matchCount-- + listResult.TotalCount-- + continue + } + + nameWithOwner := repo.NameWithOwner + + info := repo.Info() + infoColor := cs.Gray + visibility := "Public" + + if repo.IsPrivate { + infoColor = cs.Yellow + visibility = "Private" + } + + description := repo.Description + updatedAt := repo.UpdatedAt.Format(time.RFC3339) + + if tp.IsTTY() { + tp.AddField(nameWithOwner, nil, cs.Bold) + tp.AddField(info, nil, infoColor) + tp.AddField(text.ReplaceExcessiveWhitespace(description), nil, nil) + tp.AddField(utils.FuzzyAgoAbbr(now, repo.UpdatedAt), nil, nil) + } else { + tp.AddField(nameWithOwner, nil, nil) + tp.AddField(visibility, nil, nil) + tp.AddField(text.ReplaceExcessiveWhitespace(description), nil, nil) + tp.AddField(updatedAt, nil, nil) + } + tp.EndRow() + } + + if isTerminal { + hasFilters := filter.Visibility != "" || filter.Fork || filter.Source || filter.Archived + title := listHeader(owner, matchCount, listResult.TotalCount, hasFilters) + fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) + } + + return tp.Render() +} + +func listHeader(owner string, matchCount, totalMatchCount int, hasFilters bool) string { + if totalMatchCount == 0 { + if hasFilters { + return "No results match your search" + } + return "There are no repositories in @" + owner + } + + return fmt.Sprintf("Showing %d of %d repositories in @%s", matchCount, totalMatchCount, owner) +} + +type Repository struct { + NameWithOwner string + Description string + IsFork bool + IsPrivate bool + IsArchived bool + UpdatedAt time.Time +} + +func (r Repository) Info() string { + var info string + + if r.IsPrivate { + info = "private" + } + if r.IsFork { + info += " fork" + } + if r.IsArchived { + info += " archived" + } + + if info != "" { + info = strings.TrimPrefix(info, " ") + infoRunes := []rune(info) + infoRunes[0] = unicode.ToUpper(infoRunes[0]) + info = fmt.Sprintf("(%s)", string(infoRunes)) + } + + return info +} diff --git a/pkg/cmd/repo/repo.go b/pkg/cmd/repo/repo.go index 02e6c368b..4fee2b19c 100644 --- a/pkg/cmd/repo/repo.go +++ b/pkg/cmd/repo/repo.go @@ -7,6 +7,7 @@ import ( creditsCmd "github.com/cli/cli/pkg/cmd/repo/credits" repoForkCmd "github.com/cli/cli/pkg/cmd/repo/fork" gardenCmd "github.com/cli/cli/pkg/cmd/repo/garden" + repoListCmd "github.com/cli/cli/pkg/cmd/repo/list" repoViewCmd "github.com/cli/cli/pkg/cmd/repo/view" "github.com/cli/cli/pkg/cmdutil" "github.com/spf13/cobra" @@ -36,6 +37,7 @@ func NewCmdRepo(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(repoForkCmd.NewCmdFork(f, nil)) cmd.AddCommand(repoCloneCmd.NewCmdClone(f, nil)) cmd.AddCommand(repoCreateCmd.NewCmdCreate(f, nil)) + cmd.AddCommand(repoListCmd.NewCmdList(f, nil)) cmd.AddCommand(creditsCmd.NewCmdRepoCredits(f, nil)) cmd.AddCommand(gardenCmd.NewCmdGarden(f, nil)) From 05e45e38637e2e385885dab28814e449d4d2d668 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 17 Feb 2021 11:27:57 +0530 Subject: [PATCH 02/29] Feature of adding new files to an existing Github gist --- pkg/cmd/gist/edit/edit.go | 177 +++++++++++++++++++-------------- pkg/cmd/gist/edit/edit_test.go | 26 ++++- 2 files changed, 129 insertions(+), 74 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 7cba14800..0d460643b 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -5,10 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "net/http" - "sort" - "strings" - "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/api" "github.com/cli/cli/internal/config" @@ -19,6 +15,9 @@ import ( "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/pkg/surveyext" "github.com/spf13/cobra" + "net/http" + "sort" + "strings" ) type EditOptions struct { @@ -28,8 +27,9 @@ type EditOptions struct { Edit func(string, string, string, *iostreams.IOStreams) (string, error) - Selector string - Filename string + Selector string + EditFilename string + AddFilename string } func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command { @@ -48,7 +48,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "edit { | }", - Short: "Edit one of your gists", + Short: "Edit one of your gists or add new ones to it", Args: cmdutil.MinimumArgs(1, "cannot edit: gist argument required"), RunE: func(c *cobra.Command, args []string) error { opts.Selector = args[0] @@ -60,7 +60,8 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman return editRun(&opts) }, } - cmd.Flags().StringVarP(&opts.Filename, "filename", "f", "", "Select a file to edit") + cmd.Flags().StringVarP(&opts.AddFilename, "add", "a", "", "Add a file") + cmd.Flags().StringVarP(&opts.EditFilename, "filename", "f", "", "Select a file to edit") return cmd } @@ -99,89 +100,121 @@ func editRun(opts *EditOptions) error { filesToUpdate := map[string]string{} - for { - filename := opts.Filename - candidates := []string{} - for filename := range gist.Files { - candidates = append(candidates, filename) - } - - sort.Strings(candidates) - - if filename == "" { - if len(candidates) == 1 { - filename = candidates[0] - } else { - if !opts.IO.CanPrompt() { - return errors.New("unsure what file to edit; either specify --filename or run interactively") - } - err = prompt.SurveyAskOne(&survey.Select{ - Message: "Edit which file?", - Options: candidates, - }, &filename) - - if err != nil { - return fmt.Errorf("could not prompt: %w", err) - } - } - } - - if _, ok := gist.Files[filename]; !ok { - return fmt.Errorf("gist has no file %q", filename) - } + addFilename := opts.AddFilename + if addFilename != "" { + //Add files to an existing gist editorCommand, err := cmdutil.DetermineEditor(opts.Config) if err != nil { return err } - text, err := opts.Edit(editorCommand, filename, gist.Files[filename].Content, opts.IO) + text, err := opts.Edit(editorCommand, addFilename, "", opts.IO) if err != nil { return err } - if text != gist.Files[filename].Content { - gistFile := gist.Files[filename] - gistFile.Content = text // so it appears if they re-edit - filesToUpdate[filename] = text + if text == "" { + return fmt.Errorf("Contents can't be empty") } - if !opts.IO.CanPrompt() { - break - } - - if len(candidates) == 1 { - break - } - - choice := "" - - err = prompt.SurveyAskOne(&survey.Select{ - Message: "What next?", - Options: []string{ - "Edit another file", - "Submit", - "Cancel", + filesToAdd := map[string]*shared.GistFile{ + addFilename: { + Filename: addFilename, + Content: text, }, - }, &choice) + } + gist.Files = filesToAdd + err = updateGist(apiClient, ghinstance.OverridableDefault(), gist) if err != nil { - return fmt.Errorf("could not prompt: %w", err) + return err } + } else { + for { + filename := opts.EditFilename + candidates := []string{} + for filename := range gist.Files { + candidates = append(candidates, filename) + } - stop := false + sort.Strings(candidates) - switch choice { - case "Edit another file": - continue - case "Submit": - stop = true - case "Cancel": - return cmdutil.SilentError - } + if filename == "" { + if len(candidates) == 1 { + filename = candidates[0] + } else { + if !opts.IO.CanPrompt() { + return errors.New("unsure what file to edit; either specify --filename or run interactively") + } + err = prompt.SurveyAskOne(&survey.Select{ + Message: "Edit which file?", + Options: candidates, + }, &filename) - if stop { - break + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } + } + } + + if _, ok := gist.Files[filename]; !ok { + return fmt.Errorf("gist has no file %q", filename) + } + + editorCommand, err := cmdutil.DetermineEditor(opts.Config) + if err != nil { + return err + } + text, err := opts.Edit(editorCommand, filename, gist.Files[filename].Content, opts.IO) + + if err != nil { + return err + } + + if text != gist.Files[filename].Content { + gistFile := gist.Files[filename] + gistFile.Content = text // so it appears if they re-edit + filesToUpdate[filename] = text + } + + if !opts.IO.CanPrompt() { + break + } + + if len(candidates) == 1 { + break + } + + choice := "" + + err = prompt.SurveyAskOne(&survey.Select{ + Message: "What next?", + Options: []string{ + "Edit another file", + "Submit", + "Cancel", + }, + }, &choice) + + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } + + stop := false + + switch choice { + case "Edit another file": + continue + case "Submit": + stop = true + case "Cancel": + return cmdutil.SilentError + } + + if stop { + break + } } } diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index b9b711a38..59156dec6 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -35,7 +35,15 @@ func TestNewCmdEdit(t *testing.T) { cli: "123 --filename cool.md", wants: EditOptions{ Selector: "123", - Filename: "cool.md", + EditFilename: "cool.md", + }, + }, + { + name: "add", + cli: "123 --add cool.md", + wants: EditOptions{ + Selector: "123", + AddFilename: "cool.md", }, }, } @@ -60,7 +68,8 @@ func TestNewCmdEdit(t *testing.T) { _, err = cmd.ExecuteC() assert.NoError(t, err) - assert.Equal(t, tt.wants.Filename, gotOpts.Filename) + assert.Equal(t, tt.wants.EditFilename, gotOpts.EditFilename) + assert.Equal(t, tt.wants.AddFilename, gotOpts.AddFilename) assert.Equal(t, tt.wants.Selector, gotOpts.Selector) }) } @@ -211,6 +220,19 @@ func Test_editRun(t *testing.T) { wantErr: true, wantStderr: "You do not own this gist.", }, + { + name: "add file to existing gist", + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "foo.txt": { + Filename: "foo.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + }, + }, } for _, tt := range tests { From 037343c5c2bb7994f01f6480cb881a602ab426db Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 17 Feb 2021 19:20:43 +0530 Subject: [PATCH 03/29] Add existing files in the current wd to gist --- pkg/cmd/gist/edit/edit.go | 55 +++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 0d460643b..ed36e6914 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -15,7 +15,10 @@ import ( "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/pkg/surveyext" "github.com/spf13/cobra" + "io/ioutil" "net/http" + "os" + "path/filepath" "sort" "strings" ) @@ -104,27 +107,63 @@ func editRun(opts *EditOptions) error { if addFilename != "" { //Add files to an existing gist - editorCommand, err := cmdutil.DetermineEditor(opts.Config) + wd, err := os.Getwd() if err != nil { return err } - text, err := opts.Edit(editorCommand, addFilename, "", opts.IO) + m, err := filepath.Glob(wd + "/[^.]*.*") if err != nil { return err } - if text == "" { - return fmt.Errorf("Contents can't be empty") + filesToAdd := map[string]*shared.GistFile{} + fileExists := false + + for _, f := range m { + if addFilename == filepath.Base(f) { + fileExists = true + break + } } - filesToAdd := map[string]*shared.GistFile{ - addFilename: { + if fileExists { + content, err := ioutil.ReadFile(addFilename) + if err != nil { + return fmt.Errorf("failed to read file %s: %w", addFilename, err) + } + + if string(content) == "" { + return fmt.Errorf("Contents can't be empty") + } + + filesToAdd[addFilename] = &shared.GistFile{ + Filename: addFilename, + Content: string(content), + } + gist.Files = filesToAdd + } else { + editorCommand, err := cmdutil.DetermineEditor(opts.Config) + if err != nil { + return err + } + + text, err := opts.Edit(editorCommand, addFilename, "", opts.IO) + if err != nil { + return err + } + + if text == "" { + return fmt.Errorf("Contents can't be empty") + } + + filesToAdd[addFilename] = &shared.GistFile{ Filename: addFilename, Content: text, - }, + } + + gist.Files = filesToAdd } - gist.Files = filesToAdd err = updateGist(apiClient, ghinstance.OverridableDefault(), gist) if err != nil { From 298388745879a2978573a8822440bcc646358683 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 17 Feb 2021 20:39:43 +0530 Subject: [PATCH 04/29] Add files by absolute path to gist --- pkg/cmd/gist/edit/edit.go | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index ed36e6914..988b7c141 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -104,9 +104,21 @@ func editRun(opts *EditOptions) error { filesToUpdate := map[string]string{} addFilename := opts.AddFilename + fileExists := false if addFilename != "" { //Add files to an existing gist + if fi, err := os.Stat(addFilename); err != nil { + return err + } else { + switch mode := fi.Mode(); { + case mode.IsDir(): + return fmt.Errorf("found directory %s", addFilename) + case mode.IsRegular(): + fileExists = true + } + } + wd, err := os.Getwd() if err != nil { return err @@ -118,11 +130,18 @@ func editRun(opts *EditOptions) error { } filesToAdd := map[string]*shared.GistFile{} - fileExists := false for _, f := range m { if addFilename == filepath.Base(f) { - fileExists = true + choice := false + err := prompt.Confirm("File found in this directory. Proceed?", &choice) + if err != nil { + return err + } + + if choice { + fileExists = true + } break } } @@ -137,8 +156,10 @@ func editRun(opts *EditOptions) error { return fmt.Errorf("Contents can't be empty") } - filesToAdd[addFilename] = &shared.GistFile{ - Filename: addFilename, + filename := filepath.Base(addFilename) + + filesToAdd[filename] = &shared.GistFile{ + Filename: filename, Content: string(content), } gist.Files = filesToAdd From bff8b3007a85aed6206beb079257b5d44fb367fc Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Thu, 18 Feb 2021 18:45:43 +0530 Subject: [PATCH 05/29] Add test cases and improve errors with color schemes --- pkg/cmd/gist/edit/edit.go | 130 ++++++++++++++++----------- pkg/cmd/gist/edit/edit_test.go | 160 +++++++++++++++++++++++++++++++-- 2 files changed, 233 insertions(+), 57 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 988b7c141..b430f0d97 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -5,6 +5,13 @@ import ( "encoding/json" "errors" "fmt" + "io/ioutil" + "net/http" + "os" + "path/filepath" + "sort" + "strings" + "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/api" "github.com/cli/cli/internal/config" @@ -15,12 +22,6 @@ import ( "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/pkg/surveyext" "github.com/spf13/cobra" - "io/ioutil" - "net/http" - "os" - "path/filepath" - "sort" - "strings" ) type EditOptions struct { @@ -28,8 +29,8 @@ type EditOptions struct { HttpClient func() (*http.Client, error) Config func() (config.Config, error) - Edit func(string, string, string, *iostreams.IOStreams) (string, error) - + Edit func(string, string, string, *iostreams.IOStreams) (string, error) + Add func(string, string, *iostreams.IOStreams) (string, error) Selector string EditFilename string AddFilename string @@ -47,6 +48,13 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman defaultContent, io.In, io.Out, io.ErrOut, nil) }, + Add: func(editorCmd, filename string, io *iostreams.IOStreams) (string, error) { + return surveyext.Edit( + editorCmd, + "*."+filename, + "", + io.In, io.Out, io.ErrOut, nil) + }, } cmd := &cobra.Command{ @@ -104,60 +112,34 @@ func editRun(opts *EditOptions) error { filesToUpdate := map[string]string{} addFilename := opts.AddFilename - fileExists := false + cs := opts.IO.ColorScheme() if addFilename != "" { - //Add files to an existing gist - if fi, err := os.Stat(addFilename); err != nil { - return err - } else { - switch mode := fi.Mode(); { - case mode.IsDir(): - return fmt.Errorf("found directory %s", addFilename) - case mode.IsRegular(): - fileExists = true - } - } - - wd, err := os.Getwd() + //Add files to an existing gist. Aborts when file is already present + // in the directory but the user chooses not to proceed + filenamePath, fileExists, userAbort, err := processFiles(addFilename) if err != nil { - return err - } - - m, err := filepath.Glob(wd + "/[^.]*.*") - if err != nil { - return err + return fmt.Errorf("%s %s", cs.Red("!"), err) } filesToAdd := map[string]*shared.GistFile{} - for _, f := range m { - if addFilename == filepath.Base(f) { - choice := false - err := prompt.Confirm("File found in this directory. Proceed?", &choice) - if err != nil { - return err - } - - if choice { - fileExists = true - } - break - } + if userAbort { + return nil } + filename := filepath.Base(filenamePath) + if fileExists { - content, err := ioutil.ReadFile(addFilename) + content, err := ioutil.ReadFile(filenamePath) if err != nil { - return fmt.Errorf("failed to read file %s: %w", addFilename, err) + return fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), addFilename, err) } if string(content) == "" { - return fmt.Errorf("Contents can't be empty") + return fmt.Errorf("%s Contents can't be empty", cs.FailureIcon()) } - filename := filepath.Base(addFilename) - filesToAdd[filename] = &shared.GistFile{ Filename: filename, Content: string(content), @@ -169,17 +151,17 @@ func editRun(opts *EditOptions) error { return err } - text, err := opts.Edit(editorCommand, addFilename, "", opts.IO) + text, err := opts.Add(editorCommand, filename, opts.IO) if err != nil { return err } if text == "" { - return fmt.Errorf("Contents can't be empty") + return fmt.Errorf("%s Contents can't be empty", cs.Red("!")) } - filesToAdd[addFilename] = &shared.GistFile{ - Filename: addFilename, + filesToAdd[filename] = &shared.GistFile{ + Filename: filename, Content: text, } @@ -190,6 +172,11 @@ func editRun(opts *EditOptions) error { if err != nil { return err } + + completionMessage := filename + " added to gist" + + fmt.Fprintf(opts.IO.Out, "%s %s\n", cs.SuccessIconWithColor(cs.Green), completionMessage) + } else { for { filename := opts.EditFilename @@ -315,3 +302,46 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error return nil } + +func processFiles(filename string) (string, bool, bool, error) { + fileExists := false + + if fi, err := os.Stat(filename); err != nil { + } else { + switch mode := fi.Mode(); { + case mode.IsDir(): + return "", false, false, fmt.Errorf("found directory %s" , filename) + case mode.IsRegular(): + fileExists = true + } + } + + wd, err := os.Getwd() + if err != nil { + return "", false, false, err + } + + m, err := filepath.Glob(wd + "/[^.]*.*") + if err != nil { + return "", false, false, err + } + + for _, f := range m { + if filename == filepath.Base(f) { + choice := false + err := prompt.Confirm("File found in this directory. Proceed?", &choice) + if err != nil { + return "", false, false, err + } + + if choice { + fileExists = true + } else { + return "", true, true, nil + } + break + } + } + + return filename, fileExists, false, nil +} diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 59156dec6..6f0df8040 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -17,6 +17,32 @@ import ( "github.com/stretchr/testify/assert" ) +const ( + fixtureFile = "../fixture.txt" + nonExistentFile = "../file.txt" +) + +func Test_processFiles(t *testing.T) { + filePath, fileExists, userAbort, err := processFiles(fixtureFile) + if err != nil { + t.Fatalf("unexpected error processing files: %s", err) + } + + assert.Equal(t, "../fixture.txt", filePath) + assert.Equal(t, true, fileExists) + assert.Equal(t, false, userAbort) + + filePath, fileExists, userAbort, err = processFiles(nonExistentFile) + if err != nil { + t.Fatalf("unexpected error processing files: %s", err) + } + + assert.Equal(t, "../file.txt", filePath) + assert.Equal(t, false, fileExists) + assert.Equal(t, false, userAbort) + +} + func TestNewCmdEdit(t *testing.T) { tests := []struct { name string @@ -34,15 +60,15 @@ func TestNewCmdEdit(t *testing.T) { name: "filename", cli: "123 --filename cool.md", wants: EditOptions{ - Selector: "123", + Selector: "123", EditFilename: "cool.md", }, }, { name: "add", - cli: "123 --add cool.md", + cli: "123 --add cool.md", wants: EditOptions{ - Selector: "123", + Selector: "123", AddFilename: "cool.md", }, }, @@ -225,10 +251,126 @@ func Test_editRun(t *testing.T) { gist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ - "foo.txt": { - Filename: "foo.txt", - Content: "bwhiizzzbwhuiiizzzz", - Type: "text/plain", + "sample.txt": { + Filename: "sample.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + Owner: &shared.GistOwner{Login: "octocat"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + opts: &EditOptions{ + AddFilename: "foo.txt", + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "foo.txt": map[string]interface{}{ + "content": "new content to existing gist", + "filename": "foo.txt", + }, + }, + }, + }, + { + name: "add file to existing gist with absolute path", + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "sample.txt": { + Filename: "sample.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + Owner: &shared.GistOwner{Login: "octocat"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + opts: &EditOptions{ + AddFilename: "/Users/octocat/foo.txt", + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "foo.txt": map[string]interface{}{ + "content": "new content to existing gist", + "filename": "foo.txt", + }, + }, + }, + }, + { + name: "add file to existing gist with relative path", + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "sample.txt": { + Filename: "sample.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + Owner: &shared.GistOwner{Login: "octocat"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + opts: &EditOptions{ + AddFilename: "../foo.txt", + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "foo.txt": map[string]interface{}{ + "content": "new content to existing gist", + "filename": "foo.txt", + }, + }, + }, + }, + { + name: "add file to existing gist in same directory", + + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "sample.txt": { + Filename: "sample.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + Owner: &shared.GistOwner{Login: "octocat"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + opts: &EditOptions{ + AddFilename: "foo.txt", + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "foo.txt": map[string]interface{}{ + "content": "new content to existing gist", + "filename": "foo.txt", }, }, }, @@ -265,6 +407,10 @@ func Test_editRun(t *testing.T) { return "new file content", nil } + tt.opts.Add = func(_, _ string, _ *iostreams.IOStreams) (string, error) { + return "new content to existing gist", nil + } + tt.opts.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } From 9a4fd0d706219af9fee861cc831d0d43400b43ec Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Thu, 18 Feb 2021 19:17:42 +0530 Subject: [PATCH 06/29] Remove unwanted prompt for user. Unwanted test as well --- pkg/cmd/gist/edit/edit.go | 31 +++++++------------------- pkg/cmd/gist/edit/edit_test.go | 40 ++-------------------------------- 2 files changed, 10 insertions(+), 61 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index b430f0d97..453006049 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -115,19 +115,14 @@ func editRun(opts *EditOptions) error { cs := opts.IO.ColorScheme() if addFilename != "" { - //Add files to an existing gist. Aborts when file is already present - // in the directory but the user chooses not to proceed - filenamePath, fileExists, userAbort, err := processFiles(addFilename) + //Add files to an existing gist. + filenamePath, fileExists, err := processFiles(addFilename) if err != nil { return fmt.Errorf("%s %s", cs.Red("!"), err) } filesToAdd := map[string]*shared.GistFile{} - if userAbort { - return nil - } - filename := filepath.Base(filenamePath) if fileExists { @@ -303,14 +298,14 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error return nil } -func processFiles(filename string) (string, bool, bool, error) { +func processFiles(filename string) (string, bool, error) { fileExists := false if fi, err := os.Stat(filename); err != nil { } else { switch mode := fi.Mode(); { case mode.IsDir(): - return "", false, false, fmt.Errorf("found directory %s" , filename) + return "", false, fmt.Errorf("found directory %s" , filename) case mode.IsRegular(): fileExists = true } @@ -318,30 +313,20 @@ func processFiles(filename string) (string, bool, bool, error) { wd, err := os.Getwd() if err != nil { - return "", false, false, err + return "", false, err } m, err := filepath.Glob(wd + "/[^.]*.*") if err != nil { - return "", false, false, err + return "", false, err } for _, f := range m { if filename == filepath.Base(f) { - choice := false - err := prompt.Confirm("File found in this directory. Proceed?", &choice) - if err != nil { - return "", false, false, err - } - - if choice { - fileExists = true - } else { - return "", true, true, nil - } + fileExists = true break } } - return filename, fileExists, false, nil + return filename, fileExists, nil } diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 6f0df8040..fac698abf 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -23,24 +23,21 @@ const ( ) func Test_processFiles(t *testing.T) { - filePath, fileExists, userAbort, err := processFiles(fixtureFile) + filePath, fileExists, err := processFiles(fixtureFile) if err != nil { t.Fatalf("unexpected error processing files: %s", err) } assert.Equal(t, "../fixture.txt", filePath) assert.Equal(t, true, fileExists) - assert.Equal(t, false, userAbort) - filePath, fileExists, userAbort, err = processFiles(nonExistentFile) + filePath, fileExists, err = processFiles(nonExistentFile) if err != nil { t.Fatalf("unexpected error processing files: %s", err) } assert.Equal(t, "../file.txt", filePath) assert.Equal(t, false, fileExists) - assert.Equal(t, false, userAbort) - } func TestNewCmdEdit(t *testing.T) { @@ -342,39 +339,6 @@ func Test_editRun(t *testing.T) { }, }, }, - { - name: "add file to existing gist in same directory", - - gist: &shared.Gist{ - ID: "1234", - Files: map[string]*shared.GistFile{ - "sample.txt": { - Filename: "sample.txt", - Content: "bwhiizzzbwhuiiizzzz", - Type: "text/plain", - }, - }, - Owner: &shared.GistOwner{Login: "octocat"}, - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register(httpmock.REST("POST", "gists/1234"), - httpmock.StatusStringResponse(201, "{}")) - }, - opts: &EditOptions{ - AddFilename: "foo.txt", - }, - wantParams: map[string]interface{}{ - "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, - "files": map[string]interface{}{ - "foo.txt": map[string]interface{}{ - "content": "new content to existing gist", - "filename": "foo.txt", - }, - }, - }, - }, } for _, tt := range tests { From a4a194011ff072f1c5127ae3714363b1c6cdbd9c Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Thu, 18 Feb 2021 19:18:49 +0530 Subject: [PATCH 07/29] gofmt --- pkg/cmd/gist/edit/edit_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index fac698abf..3fa90db7f 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -18,7 +18,7 @@ import ( ) const ( - fixtureFile = "../fixture.txt" + fixtureFile = "../fixture.txt" nonExistentFile = "../file.txt" ) @@ -261,7 +261,7 @@ func Test_editRun(t *testing.T) { httpmock.StatusStringResponse(201, "{}")) }, opts: &EditOptions{ - AddFilename: "foo.txt", + AddFilename: "foo.txt", }, wantParams: map[string]interface{}{ "description": "", @@ -293,7 +293,7 @@ func Test_editRun(t *testing.T) { httpmock.StatusStringResponse(201, "{}")) }, opts: &EditOptions{ - AddFilename: "/Users/octocat/foo.txt", + AddFilename: "/Users/octocat/foo.txt", }, wantParams: map[string]interface{}{ "description": "", @@ -325,7 +325,7 @@ func Test_editRun(t *testing.T) { httpmock.StatusStringResponse(201, "{}")) }, opts: &EditOptions{ - AddFilename: "../foo.txt", + AddFilename: "../foo.txt", }, wantParams: map[string]interface{}{ "description": "", From 882bd1adb1fadc1c99e67d56f71d9204b19e25cc Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Thu, 18 Feb 2021 22:39:56 +0530 Subject: [PATCH 08/29] add go lint to pass checks --- pkg/cmd/gist/edit/edit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 453006049..2d06ee554 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -305,7 +305,7 @@ func processFiles(filename string) (string, bool, error) { } else { switch mode := fi.Mode(); { case mode.IsDir(): - return "", false, fmt.Errorf("found directory %s" , filename) + return "", false, fmt.Errorf("found directory %s", filename) case mode.IsRegular(): fileExists = true } From b7c2865d0f021f581ce5d14b18d3e81dd711af30 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Thu, 18 Feb 2021 17:34:00 -0300 Subject: [PATCH 09/29] Remove archived filter from repo list --- pkg/cmd/repo/list/http.go | 116 ++++++++++---------------------------- pkg/cmd/repo/list/list.go | 18 ++---- 2 files changed, 35 insertions(+), 99 deletions(-) diff --git a/pkg/cmd/repo/list/http.go b/pkg/cmd/repo/list/http.go index 3409990c8..4669e72a3 100644 --- a/pkg/cmd/repo/list/http.go +++ b/pkg/cmd/repo/list/http.go @@ -1,7 +1,6 @@ package list import ( - "fmt" "strings" "github.com/cli/cli/api" @@ -14,35 +13,21 @@ type RepositoryList struct { } func listRepos(client *api.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { - type reposBlock struct { - TotalCount int - RepositoryCount int - Nodes []Repository - PageInfo struct { - HasNextPage bool - EndCursor string - } - } - type response struct { RepositoryOwner struct { - Repositories reposBlock + Repositories struct { + TotalCount int + RepositoryCount int + Nodes []Repository + PageInfo struct { + HasNextPage bool + EndCursor string + } + } } - Search reposBlock } - fragment := ` - fragment repo on Repository { - nameWithOwner - description - isFork - isPrivate - isArchived - updatedAt - }` - - // If `--archived` wasn't specified, use `repositoryOwner.repositores` - query := fragment + ` + query := ` query RepoList($owner: String!, $per_page: Int!, $endCursor: String, $fork: Boolean, $privacy: RepositoryPrivacy) { repositoryOwner(login: $owner) { repositories( @@ -54,7 +39,12 @@ func listRepos(client *api.Client, hostname string, limit int, owner string, fil orderBy: { field: UPDATED_AT, direction: DESC }) { totalCount nodes { - ...repo + nameWithOwner + description + isFork + isPrivate + isArchived + updatedAt } pageInfo { hasNextPage @@ -70,59 +60,23 @@ func listRepos(client *api.Client, hostname string, limit int, owner string, fil } variables := map[string]interface{}{ + "owner": githubv4.String(owner), "per_page": githubv4.Int(perPage), "endCursor": (*githubv4.String)(nil), } - hasArchivedFilter := filter.Archived - - if hasArchivedFilter { - // If `--archived` was specified, use the `search` API rather than - // `repositoryOwner.repositories` - query = fragment + ` - query RepoList($per_page: Int!, $endCursor: String, $query: String!) { - search(first: $per_page, after:$endCursor, type: REPOSITORY, query: $query) { - repositoryCount - nodes { - ... on Repository { - ...repo - } - } - pageInfo { - hasNextPage - endCursor - } - } - }` - - search := []string{fmt.Sprintf("user:%s archived:true fork:true sort:updated-desc", owner)} - - switch filter.Visibility { - case "private": - search = append(search, "is:private") - case "public": - search = append(search, "is:public") - default: - search = append(search, "is:all") - } - - variables["query"] = strings.Join(search, " ") + if filter.Visibility != "" { + variables["privacy"] = githubv4.RepositoryPrivacy(strings.ToUpper(filter.Visibility)) } else { - variables["owner"] = githubv4.String(owner) + variables["privacy"] = (*githubv4.RepositoryPrivacy)(nil) + } - if filter.Visibility != "" { - variables["privacy"] = githubv4.RepositoryPrivacy(strings.ToUpper(filter.Visibility)) - } else { - variables["privacy"] = (*githubv4.RepositoryPrivacy)(nil) - } - - if filter.Fork { - variables["fork"] = githubv4.Boolean(true) - } else if filter.Source { - variables["fork"] = githubv4.Boolean(false) - } else { - variables["fork"] = (*githubv4.Boolean)(nil) - } + if filter.Fork { + variables["fork"] = githubv4.Boolean(true) + } else if filter.Source { + variables["fork"] = githubv4.Boolean(false) + } else { + variables["fork"] = (*githubv4.Boolean)(nil) } var repos []Repository @@ -137,11 +91,7 @@ pagination: return nil, err } - if hasArchivedFilter { - repos = append(repos, result.Search.Nodes...) - } else { - repos = append(repos, result.RepositoryOwner.Repositories.Nodes...) - } + repos = append(repos, result.RepositoryOwner.Repositories.Nodes...) if len(repos) >= limit { if len(repos) > limit { @@ -151,21 +101,13 @@ pagination: } if !result.RepositoryOwner.Repositories.PageInfo.HasNextPage { - if !result.Search.PageInfo.HasNextPage { - break - } + break } variables["endCursor"] = githubv4.String(result.RepositoryOwner.Repositories.PageInfo.EndCursor) - if hasArchivedFilter { - variables["endCursor"] = githubv4.String(result.Search.PageInfo.EndCursor) - } } totalCount = result.RepositoryOwner.Repositories.TotalCount - if hasArchivedFilter { - totalCount = result.Search.RepositoryCount - } listResult := &RepositoryList{ Repositories: repos, diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go index b066764b3..55a713962 100644 --- a/pkg/cmd/repo/list/list.go +++ b/pkg/cmd/repo/list/list.go @@ -20,7 +20,6 @@ type FilterOptions struct { Visibility string // private, public Fork bool Source bool - Archived bool } type ListOptions struct { @@ -33,7 +32,6 @@ type ListOptions struct { Visibility string Fork bool Source bool - Archived bool } func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { @@ -43,11 +41,10 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman } var ( - flagPublic bool - flagPrivate bool - flagSource bool - flagFork bool - flagArchived bool + flagPublic bool + flagPrivate bool + flagSource bool + flagFork bool ) cmd := &cobra.Command{ @@ -72,7 +69,6 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman opts.Visibility = "public" } - opts.Archived = flagArchived opts.Fork = flagFork opts.Source = flagSource @@ -91,7 +87,6 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().BoolVar(&flagPrivate, "private", false, "Show only private repositories") cmd.Flags().BoolVar(&flagPublic, "public", false, "Show only public repositories") cmd.Flags().BoolVar(&flagSource, "source", false, "Show only source repositories") - cmd.Flags().BoolVar(&flagArchived, "archived", false, "Show only archived repositories") cmd.Flags().BoolVar(&flagFork, "fork", false, "Show only forks") return cmd @@ -119,7 +114,6 @@ func listRun(opts *ListOptions) error { Visibility: opts.Visibility, Fork: opts.Fork, Source: opts.Source, - Archived: opts.Archived, } listResult, err := listRepos(apiClient, ghinstance.OverridableDefault(), opts.Limit, owner, filter) @@ -131,7 +125,7 @@ func listRun(opts *ListOptions) error { tp := utils.NewTablePrinter(opts.IO) - notArchived := (filter.Fork || filter.Source) && !filter.Archived + notArchived := filter.Fork || filter.Source matchCount := len(listResult.Repositories) now := time.Now() @@ -172,7 +166,7 @@ func listRun(opts *ListOptions) error { } if isTerminal { - hasFilters := filter.Visibility != "" || filter.Fork || filter.Source || filter.Archived + hasFilters := filter.Visibility != "" || filter.Fork || filter.Source title := listHeader(owner, matchCount, listResult.TotalCount, hasFilters) fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) } From cad875a05fa006bf1ff8cee0f62e6d99f1d666b6 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Thu, 18 Feb 2021 19:02:59 -0300 Subject: [PATCH 10/29] repo list: render repo tags into the 3rd column instead of the 2nd --- pkg/cmd/repo/list/list.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go index 55a713962..06c79daa2 100644 --- a/pkg/cmd/repo/list/list.go +++ b/pkg/cmd/repo/list/list.go @@ -5,7 +5,6 @@ import ( "net/http" "strings" "time" - "unicode" "github.com/cli/cli/api" "github.com/cli/cli/internal/ghinstance" @@ -153,13 +152,13 @@ func listRun(opts *ListOptions) error { if tp.IsTTY() { tp.AddField(nameWithOwner, nil, cs.Bold) - tp.AddField(info, nil, infoColor) tp.AddField(text.ReplaceExcessiveWhitespace(description), nil, nil) + tp.AddField(info, nil, infoColor) tp.AddField(utils.FuzzyAgoAbbr(now, repo.UpdatedAt), nil, nil) } else { tp.AddField(nameWithOwner, nil, nil) - tp.AddField(visibility, nil, nil) tp.AddField(text.ReplaceExcessiveWhitespace(description), nil, nil) + tp.AddField(visibility, nil, nil) tp.AddField(updatedAt, nil, nil) } tp.EndRow() @@ -196,22 +195,21 @@ type Repository struct { func (r Repository) Info() string { var info string + var tags []string if r.IsPrivate { - info = "private" + tags = append(tags, "private") } if r.IsFork { - info += " fork" + tags = append(tags, "fork") } if r.IsArchived { - info += " archived" + tags = append(tags, "archived") } - if info != "" { - info = strings.TrimPrefix(info, " ") - infoRunes := []rune(info) - infoRunes[0] = unicode.ToUpper(infoRunes[0]) - info = fmt.Sprintf("(%s)", string(infoRunes)) + if len(tags) > 0 { + tags[0] = strings.Title(tags[0]) + info = strings.Join(tags, ", ") } return info From 4ed10140ab191fbf0465560a51ce6be04a46e402 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Fri, 19 Feb 2021 12:02:17 +0530 Subject: [PATCH 11/29] Resolved PR review comments and test cases --- pkg/cmd/gist/edit/edit.go | 276 ++++++++++++++++----------------- pkg/cmd/gist/edit/edit_test.go | 27 +++- 2 files changed, 158 insertions(+), 145 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 2d06ee554..c3465242a 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -59,7 +59,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "edit { | }", - Short: "Edit one of your gists or add new ones to it", + Short: "Edit or add files in a gist", Args: cmdutil.MinimumArgs(1, "cannot edit: gist argument required"), RunE: func(c *cobra.Command, args []string) error { opts.Selector = args[0] @@ -116,147 +116,109 @@ func editRun(opts *EditOptions) error { if addFilename != "" { //Add files to an existing gist. - filenamePath, fileExists, err := processFiles(addFilename) + files, err := getFilesToAdd(addFilename, opts) if err != nil { - return fmt.Errorf("%s %s", cs.Red("!"), err) + return err } - filesToAdd := map[string]*shared.GistFile{} - - filename := filepath.Base(filenamePath) - - if fileExists { - content, err := ioutil.ReadFile(filenamePath) - if err != nil { - return fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), addFilename, err) - } - - if string(content) == "" { - return fmt.Errorf("%s Contents can't be empty", cs.FailureIcon()) - } - - filesToAdd[filename] = &shared.GistFile{ - Filename: filename, - Content: string(content), - } - gist.Files = filesToAdd - } else { - editorCommand, err := cmdutil.DetermineEditor(opts.Config) - if err != nil { - return err - } - - text, err := opts.Add(editorCommand, filename, opts.IO) - if err != nil { - return err - } - - if text == "" { - return fmt.Errorf("%s Contents can't be empty", cs.Red("!")) - } - - filesToAdd[filename] = &shared.GistFile{ - Filename: filename, - Content: text, - } - - gist.Files = filesToAdd - } + fmt.Printf("%v", files) + gist.Files = files err = updateGist(apiClient, ghinstance.OverridableDefault(), gist) if err != nil { return err } - completionMessage := filename + " added to gist" + completionMessage := filepath.Base(addFilename) + " added to gist" fmt.Fprintf(opts.IO.Out, "%s %s\n", cs.SuccessIconWithColor(cs.Green), completionMessage) - } else { - for { - filename := opts.EditFilename - candidates := []string{} - for filename := range gist.Files { - candidates = append(candidates, filename) - } + return nil + } - sort.Strings(candidates) + for { + filename := opts.EditFilename + candidates := []string{} + for filename := range gist.Files { + candidates = append(candidates, filename) + } - if filename == "" { - if len(candidates) == 1 { - filename = candidates[0] - } else { - if !opts.IO.CanPrompt() { - return errors.New("unsure what file to edit; either specify --filename or run interactively") - } - err = prompt.SurveyAskOne(&survey.Select{ - Message: "Edit which file?", - Options: candidates, - }, &filename) + sort.Strings(candidates) - if err != nil { - return fmt.Errorf("could not prompt: %w", err) - } + if filename == "" { + if len(candidates) == 1 { + filename = candidates[0] + } else { + if !opts.IO.CanPrompt() { + return errors.New("unsure what file to edit; either specify --filename or run interactively") + } + err = prompt.SurveyAskOne(&survey.Select{ + Message: "Edit which file?", + Options: candidates, + }, &filename) + + if err != nil { + return fmt.Errorf("could not prompt: %w", err) } } + } - if _, ok := gist.Files[filename]; !ok { - return fmt.Errorf("gist has no file %q", filename) - } + if _, ok := gist.Files[filename]; !ok { + return fmt.Errorf("gist has no file %q", filename) + } - editorCommand, err := cmdutil.DetermineEditor(opts.Config) - if err != nil { - return err - } - text, err := opts.Edit(editorCommand, filename, gist.Files[filename].Content, opts.IO) + editorCommand, err := cmdutil.DetermineEditor(opts.Config) + if err != nil { + return err + } + text, err := opts.Edit(editorCommand, filename, gist.Files[filename].Content, opts.IO) - if err != nil { - return err - } + if err != nil { + return err + } - if text != gist.Files[filename].Content { - gistFile := gist.Files[filename] - gistFile.Content = text // so it appears if they re-edit - filesToUpdate[filename] = text - } + if text != gist.Files[filename].Content { + gistFile := gist.Files[filename] + gistFile.Content = text // so it appears if they re-edit + filesToUpdate[filename] = text + } - if !opts.IO.CanPrompt() { - break - } + if !opts.IO.CanPrompt() { + break + } - if len(candidates) == 1 { - break - } + if len(candidates) == 1 { + break + } - choice := "" + choice := "" - err = prompt.SurveyAskOne(&survey.Select{ - Message: "What next?", - Options: []string{ - "Edit another file", - "Submit", - "Cancel", - }, - }, &choice) + err = prompt.SurveyAskOne(&survey.Select{ + Message: "What next?", + Options: []string{ + "Edit another file", + "Submit", + "Cancel", + }, + }, &choice) - if err != nil { - return fmt.Errorf("could not prompt: %w", err) - } + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } - stop := false + stop := false - switch choice { - case "Edit another file": - continue - case "Submit": - stop = true - case "Cancel": - return cmdutil.SilentError - } + switch choice { + case "Edit another file": + continue + case "Submit": + stop = true + case "Cancel": + return cmdutil.SilentError + } - if stop { - break - } + if stop { + break } } @@ -298,35 +260,73 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error return nil } -func processFiles(filename string) (string, bool, error) { - fileExists := false +func getFilesToAdd(file string, opts *EditOptions) (map[string]*shared.GistFile, error) { + cs := opts.IO.ColorScheme() - if fi, err := os.Stat(filename); err != nil { + fileExists, err := fileExists(file) + if err != nil { + return nil, fmt.Errorf("%s %s", cs.Red("!"), err) + } + + filesToAdd := map[string]*shared.GistFile{} + + filename := filepath.Base(file) + + if fileExists { + content, err := ioutil.ReadFile(file) + if err != nil { + return nil, fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), file, err) + } + + if string(content) == "" { + return nil, fmt.Errorf("%s Contents can't be empty", cs.FailureIcon()) + } + + filesToAdd[filename] = &shared.GistFile{ + Filename: filename, + Content: string(content), + } + return filesToAdd, nil } else { - switch mode := fi.Mode(); { - case mode.IsDir(): - return "", false, fmt.Errorf("found directory %s", filename) - case mode.IsRegular(): - fileExists = true + editorCommand, err := cmdutil.DetermineEditor(opts.Config) + if err != nil { + return nil, err } - } - wd, err := os.Getwd() - if err != nil { - return "", false, err - } - - m, err := filepath.Glob(wd + "/[^.]*.*") - if err != nil { - return "", false, err - } - - for _, f := range m { - if filename == filepath.Base(f) { - fileExists = true - break + text, err := opts.Add(editorCommand, filename, opts.IO) + if err != nil { + return nil, err } - } - return filename, fileExists, nil + if text == "" { + return nil, fmt.Errorf("%s Contents can't be empty", cs.Red("!")) + } + + filesToAdd[filename] = &shared.GistFile{ + Filename: filename, + Content: text, + } + + return filesToAdd, nil + } +} + +func fileExists(filename string) (bool, error) { + + fi, err := os.Stat(filename) + + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + } + + switch mode := fi.Mode(); { + case mode.IsDir(): + return false, fmt.Errorf("found directory %s", filename) + case mode.IsRegular(): + return true, nil + } + + return false, nil } diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 3fa90db7f..c6ea04126 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "io/ioutil" "net/http" + "path/filepath" "testing" "github.com/cli/cli/internal/config" @@ -22,22 +23,34 @@ const ( nonExistentFile = "../file.txt" ) -func Test_processFiles(t *testing.T) { - filePath, fileExists, err := processFiles(fixtureFile) +func Test_fileExists(t *testing.T) { + fixtureFileExists, err := fileExists(fixtureFile) if err != nil { t.Fatalf("unexpected error processing files: %s", err) } - assert.Equal(t, "../fixture.txt", filePath) - assert.Equal(t, true, fileExists) + assert.Equal(t, true, fixtureFileExists) - filePath, fileExists, err = processFiles(nonExistentFile) + neFileExists, err := fileExists(nonExistentFile) + assert.Equal(t, nil, err) + assert.Equal(t, false, neFileExists) +} + +func Test_getFilesToAdd(t *testing.T) { + gf, err := getFilesToAdd(fixtureFile, &EditOptions{ + AddFilename: fixtureFile, + IO: &iostreams.IOStreams{}, + }) if err != nil { t.Fatalf("unexpected error processing files: %s", err) } - assert.Equal(t, "../file.txt", filePath) - assert.Equal(t, false, fileExists) + assert.Equal(t, map[string]*shared.GistFile{ + filepath.Base(fixtureFile): { + Filename: filepath.Base(fixtureFile), + Content: "{}", + }, + }, gf) } func TestNewCmdEdit(t *testing.T) { From faffc4de95e31e8e59951956ad967f82688ab419 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Fri, 19 Feb 2021 12:05:20 +0530 Subject: [PATCH 12/29] Add go fmt to pass ci/cd --- pkg/cmd/gist/edit/edit_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index c6ea04126..854e4be05 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -38,8 +38,8 @@ func Test_fileExists(t *testing.T) { func Test_getFilesToAdd(t *testing.T) { gf, err := getFilesToAdd(fixtureFile, &EditOptions{ - AddFilename: fixtureFile, - IO: &iostreams.IOStreams{}, + AddFilename: fixtureFile, + IO: &iostreams.IOStreams{}, }) if err != nil { t.Fatalf("unexpected error processing files: %s", err) From 2284ef43d079c1c5ae280848e098f04e6ee1ddf3 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Fri, 19 Feb 2021 17:34:17 -0300 Subject: [PATCH 13/29] repo list: add tests --- pkg/cmd/repo/list/fixtures/repoList.json | 39 +++++ pkg/cmd/repo/list/list.go | 5 +- pkg/cmd/repo/list/list_test.go | 192 +++++++++++++++++++++++ 3 files changed, 235 insertions(+), 1 deletion(-) create mode 100644 pkg/cmd/repo/list/fixtures/repoList.json create mode 100644 pkg/cmd/repo/list/list_test.go diff --git a/pkg/cmd/repo/list/fixtures/repoList.json b/pkg/cmd/repo/list/fixtures/repoList.json new file mode 100644 index 000000000..ca0faa85c --- /dev/null +++ b/pkg/cmd/repo/list/fixtures/repoList.json @@ -0,0 +1,39 @@ +{ + "data": { + "repositoryOwner": { + "repositories": { + "totalCount": 3, + "nodes": [ + { + "nameWithOwner": "octocat/hello-world", + "description": "My first repository", + "isFork": false, + "isPrivate": false, + "isArchived": false, + "updatedAt": "2021-02-19T06:34:58Z" + }, + { + "nameWithOwner": "octocat/cli", + "description": "GitHub CLI", + "isFork": true, + "isPrivate": false, + "isArchived": false, + "updatedAt": "2021-02-19T06:06:06Z" + }, + { + "nameWithOwner": "octocat/testing", + "description": null, + "isFork": false, + "isPrivate": true, + "isArchived": false, + "updatedAt": "2021-02-11T22:32:05Z" + } + ], + "pageInfo": { + "hasNextPage": false, + "endCursor": "" + } + } + } + } +} diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go index 06c79daa2..631a50bf3 100644 --- a/pkg/cmd/repo/list/list.go +++ b/pkg/cmd/repo/list/list.go @@ -31,12 +31,15 @@ type ListOptions struct { Visibility string Fork bool Source bool + + Now func() time.Time } func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { opts := ListOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + Now: time.Now, } var ( @@ -127,7 +130,7 @@ func listRun(opts *ListOptions) error { notArchived := filter.Fork || filter.Source matchCount := len(listResult.Repositories) - now := time.Now() + now := opts.Now() for _, repo := range listResult.Repositories { if notArchived && repo.IsArchived { diff --git a/pkg/cmd/repo/list/list_test.go b/pkg/cmd/repo/list/list_test.go new file mode 100644 index 000000000..7ffe6de17 --- /dev/null +++ b/pkg/cmd/repo/list/list_test.go @@ -0,0 +1,192 @@ +package list + +import ( + "bytes" + "io/ioutil" + "net/http" + "testing" + "time" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + } + + cmd := NewCmdList(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func TestRepoList_nontty(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(false) + io.SetStdinTTY(false) + io.SetStderrTTY(false) + + httpReg := &httpmock.Registry{} + defer httpReg.Verify(t) + + httpReg.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`), + ) + httpReg.Register( + httpmock.GraphQL(`query RepoList\b`), + httpmock.FileResponse("./fixtures/repoList.json"), + ) + + opts := ListOptions{ + IO: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: httpReg}, nil + }, + Now: func() time.Time { + t, _ := time.Parse(time.RFC822, "19 Feb 21 15:00 UTC") + return t + }, + Limit: 30, + } + + err := listRun(&opts) + assert.NoError(t, err) + + assert.Equal(t, "", stderr.String()) + + assert.Equal(t, heredoc.Doc(` + octocat/hello-world My first repository Public 2021-02-19T06:34:58Z + octocat/cli GitHub CLI Public 2021-02-19T06:06:06Z + octocat/testing Private 2021-02-11T22:32:05Z + `), stdout.String()) +} + +func TestRepoList_tty(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(true) + io.SetStdinTTY(true) + io.SetStderrTTY(true) + + httpReg := &httpmock.Registry{} + defer httpReg.Verify(t) + + httpReg.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`), + ) + httpReg.Register( + httpmock.GraphQL(`query RepoList\b`), + httpmock.FileResponse("./fixtures/repoList.json"), + ) + + opts := ListOptions{ + IO: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: httpReg}, nil + }, + Now: func() time.Time { + t, _ := time.Parse(time.RFC822, "19 Feb 21 15:00 UTC") + return t + }, + Limit: 30, + } + + err := listRun(&opts) + assert.NoError(t, err) + + assert.Equal(t, "", stderr.String()) + + assert.Equal(t, heredoc.Doc(` + + Showing 3 of 3 repositories in @octocat + + octocat/hello-world My first repository 8h + octocat/cli GitHub CLI Fork 8h + octocat/testing Private 7d + `), stdout.String()) +} + +func TestRepoList_filtering(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`), + ) + http.Register( + httpmock.GraphQL(`query RepoList\b`), + httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { + assert.Equal(t, "PRIVATE", params["privacy"]) + assert.Equal(t, float64(2), params["per_page"]) + }), + ) + + output, err := runCommand(http, true, `--private --limit 2 `) + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, "", output.Stderr()) + assert.Equal(t, "\nNo results match your search\n\n", output.String()) +} + +func TestRepoList_withInvalidFlagCombinations(t *testing.T) { + tests := []struct { + name string + cli string + wantStderr string + }{ + { + name: "invalid limit", + cli: "--limit 0", + wantStderr: "invalid limit: 0", + }, + { + name: "both private and public", + cli: "--private --public", + wantStderr: "specify only one of `--public` or `--private`", + }, + { + name: "both source and fork", + cli: "--source --fork", + wantStderr: "specify only one of `--source` or `--fork`", + }, + } + + for _, tt := range tests { + httpReg := &httpmock.Registry{} + + _, err := runCommand(httpReg, true, tt.cli) + assert.EqualError(t, err, tt.wantStderr) + } +} From 9d062ed8fcdd5845e55ebfe364a41c3524a09fc3 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Tue, 23 Feb 2021 09:17:35 -0800 Subject: [PATCH 14/29] Normalize pr command arguments --- pkg/cmd/pr/close/close.go | 4 ++-- pkg/cmd/pr/comment/comment.go | 1 + pkg/cmd/pr/comment/comment_test.go | 6 ++++++ pkg/cmd/pr/edit/edit.go | 8 +++++--- pkg/cmd/pr/edit/edit_test.go | 13 +++++++++++-- pkg/cmd/pr/reopen/reopen.go | 4 ++-- 6 files changed, 27 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/pr/close/close.go b/pkg/cmd/pr/close/close.go index f4cc442c2..20db4960c 100644 --- a/pkg/cmd/pr/close/close.go +++ b/pkg/cmd/pr/close/close.go @@ -35,9 +35,9 @@ func NewCmdClose(f *cmdutil.Factory, runF func(*CloseOptions) error) *cobra.Comm } cmd := &cobra.Command{ - Use: "close { | | }", + Use: "close [ | | ]", Short: "Close a pull request", - Args: cobra.ExactArgs(1), + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo diff --git a/pkg/cmd/pr/comment/comment.go b/pkg/cmd/pr/comment/comment.go index 27847b4fa..8e7c98cb1 100644 --- a/pkg/cmd/pr/comment/comment.go +++ b/pkg/cmd/pr/comment/comment.go @@ -30,6 +30,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*shared.CommentableOptions) err Example: heredoc.Doc(` $ gh pr comment 22 --body "This looks great, lets get it deployed." `), + Args: cobra.MaximumNArgs(1), PreRunE: func(cmd *cobra.Command, args []string) error { if repoOverride, _ := cmd.Flags().GetString("repo"); repoOverride != "" && len(args) == 0 { return &cmdutil.FlagError{Err: errors.New("argument required when using the --repo flag")} diff --git a/pkg/cmd/pr/comment/comment_test.go b/pkg/cmd/pr/comment/comment_test.go index 52256d431..94c58a720 100644 --- a/pkg/cmd/pr/comment/comment_test.go +++ b/pkg/cmd/pr/comment/comment_test.go @@ -32,6 +32,12 @@ func TestNewCmdComment(t *testing.T) { }, wantsErr: false, }, + { + name: "two arguments", + input: "1 2", + output: shared.CommentableOptions{}, + wantsErr: true, + }, { name: "pr number", input: "1", diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 6c1592b4c..46a64068a 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -46,7 +46,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman } cmd := &cobra.Command{ - Use: "edit { | }", + Use: "edit [ | | ]", Short: "Edit a pull request", Example: heredoc.Doc(` $ gh pr edit 23 --title "I found a bug" --body "Nothing works" @@ -56,12 +56,14 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman $ gh pr edit 23 --add-project "Roadmap" --remove-project v1,v2 $ gh pr edit 23 --milestone "Version 1" `), - Args: cobra.ExactArgs(1), + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo - opts.SelectorArg = args[0] + if len(args) > 0 { + opts.SelectorArg = args[0] + } flags := cmd.Flags() if flags.Changed("title") { diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 35d8278dc..a8f5cf4b2 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -23,8 +23,17 @@ func TestNewCmdEdit(t *testing.T) { wantsErr bool }{ { - name: "no argument", - input: "", + name: "no argument", + input: "", + output: EditOptions{ + SelectorArg: "", + Interactive: true, + }, + wantsErr: false, + }, + { + name: "two arguments", + input: "1 2", output: EditOptions{}, wantsErr: true, }, diff --git a/pkg/cmd/pr/reopen/reopen.go b/pkg/cmd/pr/reopen/reopen.go index 135e341fa..8283e87ef 100644 --- a/pkg/cmd/pr/reopen/reopen.go +++ b/pkg/cmd/pr/reopen/reopen.go @@ -30,9 +30,9 @@ func NewCmdReopen(f *cmdutil.Factory, runF func(*ReopenOptions) error) *cobra.Co } cmd := &cobra.Command{ - Use: "reopen { | | }", + Use: "reopen [ | | ]", Short: "Reopen a pull request", - Args: cobra.ExactArgs(1), + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo From 34da59777bd5245e2958290f4d1ab21867933362 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Tue, 23 Feb 2021 13:24:48 -0800 Subject: [PATCH 15/29] Revert close and reopen changes --- pkg/cmd/pr/close/close.go | 4 ++-- pkg/cmd/pr/reopen/reopen.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/pr/close/close.go b/pkg/cmd/pr/close/close.go index 20db4960c..f4cc442c2 100644 --- a/pkg/cmd/pr/close/close.go +++ b/pkg/cmd/pr/close/close.go @@ -35,9 +35,9 @@ func NewCmdClose(f *cmdutil.Factory, runF func(*CloseOptions) error) *cobra.Comm } cmd := &cobra.Command{ - Use: "close [ | | ]", + Use: "close { | | }", Short: "Close a pull request", - Args: cobra.MaximumNArgs(1), + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo diff --git a/pkg/cmd/pr/reopen/reopen.go b/pkg/cmd/pr/reopen/reopen.go index 8283e87ef..135e341fa 100644 --- a/pkg/cmd/pr/reopen/reopen.go +++ b/pkg/cmd/pr/reopen/reopen.go @@ -30,9 +30,9 @@ func NewCmdReopen(f *cmdutil.Factory, runF func(*ReopenOptions) error) *cobra.Co } cmd := &cobra.Command{ - Use: "reopen [ | | ]", + Use: "reopen { | | }", Short: "Reopen a pull request", - Args: cobra.MaximumNArgs(1), + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo From a6fa14866be9e521b82ea0ea09cdb6653319b855 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 24 Feb 2021 09:44:11 +0530 Subject: [PATCH 16/29] updating tests WIP --- pkg/cmd/gist/edit/edit.go | 69 +++++----------------------------- pkg/cmd/gist/edit/edit_test.go | 46 ----------------------- 2 files changed, 9 insertions(+), 106 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index c3465242a..65f91732f 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -7,7 +7,6 @@ import ( "fmt" "io/ioutil" "net/http" - "os" "path/filepath" "sort" "strings" @@ -121,8 +120,6 @@ func editRun(opts *EditOptions) error { return err } - fmt.Printf("%v", files) - gist.Files = files err = updateGist(apiClient, ghinstance.OverridableDefault(), gist) if err != nil { @@ -263,70 +260,22 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error func getFilesToAdd(file string, opts *EditOptions) (map[string]*shared.GistFile, error) { cs := opts.IO.ColorScheme() - fileExists, err := fileExists(file) - if err != nil { - return nil, fmt.Errorf("%s %s", cs.Red("!"), err) - } - filesToAdd := map[string]*shared.GistFile{} filename := filepath.Base(file) - if fileExists { - content, err := ioutil.ReadFile(file) - if err != nil { - return nil, fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), file, err) - } - - if string(content) == "" { - return nil, fmt.Errorf("%s Contents can't be empty", cs.FailureIcon()) - } - - filesToAdd[filename] = &shared.GistFile{ - Filename: filename, - Content: string(content), - } - return filesToAdd, nil - } else { - editorCommand, err := cmdutil.DetermineEditor(opts.Config) - if err != nil { - return nil, err - } - - text, err := opts.Add(editorCommand, filename, opts.IO) - if err != nil { - return nil, err - } - - if text == "" { - return nil, fmt.Errorf("%s Contents can't be empty", cs.Red("!")) - } - - filesToAdd[filename] = &shared.GistFile{ - Filename: filename, - Content: text, - } - - return filesToAdd, nil - } -} - -func fileExists(filename string) (bool, error) { - - fi, err := os.Stat(filename) - + content, err := ioutil.ReadFile(file) if err != nil { - if os.IsNotExist(err) { - return false, nil - } + return nil, fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), file, err) } - switch mode := fi.Mode(); { - case mode.IsDir(): - return false, fmt.Errorf("found directory %s", filename) - case mode.IsRegular(): - return true, nil + if string(content) == "" { + return nil, fmt.Errorf("%s Contents can't be empty", cs.FailureIcon()) } - return false, nil + filesToAdd[filename] = &shared.GistFile{ + Filename: filename, + Content: string(content), + } + return filesToAdd, nil } diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 854e4be05..c1b411a47 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -20,22 +20,8 @@ import ( const ( fixtureFile = "../fixture.txt" - nonExistentFile = "../file.txt" ) -func Test_fileExists(t *testing.T) { - fixtureFileExists, err := fileExists(fixtureFile) - if err != nil { - t.Fatalf("unexpected error processing files: %s", err) - } - - assert.Equal(t, true, fixtureFileExists) - - neFileExists, err := fileExists(nonExistentFile) - assert.Equal(t, nil, err) - assert.Equal(t, false, neFileExists) -} - func Test_getFilesToAdd(t *testing.T) { gf, err := getFilesToAdd(fixtureFile, &EditOptions{ AddFilename: fixtureFile, @@ -256,38 +242,6 @@ func Test_editRun(t *testing.T) { wantErr: true, wantStderr: "You do not own this gist.", }, - { - name: "add file to existing gist", - gist: &shared.Gist{ - ID: "1234", - Files: map[string]*shared.GistFile{ - "sample.txt": { - Filename: "sample.txt", - Content: "bwhiizzzbwhuiiizzzz", - Type: "text/plain", - }, - }, - Owner: &shared.GistOwner{Login: "octocat"}, - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register(httpmock.REST("POST", "gists/1234"), - httpmock.StatusStringResponse(201, "{}")) - }, - opts: &EditOptions{ - AddFilename: "foo.txt", - }, - wantParams: map[string]interface{}{ - "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, - "files": map[string]interface{}{ - "foo.txt": map[string]interface{}{ - "content": "new content to existing gist", - "filename": "foo.txt", - }, - }, - }, - }, { name: "add file to existing gist with absolute path", gist: &shared.Gist{ From d4e14beb57880f493438849b97d6ca92090333fb Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 24 Feb 2021 10:14:31 +0530 Subject: [PATCH 17/29] remove unwanted tests and unwanted functionality --- pkg/cmd/gist/edit/edit.go | 8 ----- pkg/cmd/gist/edit/edit_test.go | 53 ++-------------------------------- 2 files changed, 3 insertions(+), 58 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 65f91732f..3bea3757b 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -29,7 +29,6 @@ type EditOptions struct { Config func() (config.Config, error) Edit func(string, string, string, *iostreams.IOStreams) (string, error) - Add func(string, string, *iostreams.IOStreams) (string, error) Selector string EditFilename string AddFilename string @@ -47,13 +46,6 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman defaultContent, io.In, io.Out, io.ErrOut, nil) }, - Add: func(editorCmd, filename string, io *iostreams.IOStreams) (string, error) { - return surveyext.Edit( - editorCmd, - "*."+filename, - "", - io.In, io.Out, io.ErrOut, nil) - }, } cmd := &cobra.Command{ diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index c1b411a47..1b5ac25b4 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -19,7 +19,7 @@ import ( ) const ( - fixtureFile = "../fixture.txt" + fixtureFile = "../fixture.txt" ) func Test_getFilesToAdd(t *testing.T) { @@ -243,7 +243,7 @@ func Test_editRun(t *testing.T) { wantStderr: "You do not own this gist.", }, { - name: "add file to existing gist with absolute path", + name: "add file to existing gist", gist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ @@ -260,50 +260,7 @@ func Test_editRun(t *testing.T) { httpmock.StatusStringResponse(201, "{}")) }, opts: &EditOptions{ - AddFilename: "/Users/octocat/foo.txt", - }, - wantParams: map[string]interface{}{ - "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, - "files": map[string]interface{}{ - "foo.txt": map[string]interface{}{ - "content": "new content to existing gist", - "filename": "foo.txt", - }, - }, - }, - }, - { - name: "add file to existing gist with relative path", - gist: &shared.Gist{ - ID: "1234", - Files: map[string]*shared.GistFile{ - "sample.txt": { - Filename: "sample.txt", - Content: "bwhiizzzbwhuiiizzzz", - Type: "text/plain", - }, - }, - Owner: &shared.GistOwner{Login: "octocat"}, - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register(httpmock.REST("POST", "gists/1234"), - httpmock.StatusStringResponse(201, "{}")) - }, - opts: &EditOptions{ - AddFilename: "../foo.txt", - }, - wantParams: map[string]interface{}{ - "description": "", - "updated_at": "0001-01-01T00:00:00Z", - "public": false, - "files": map[string]interface{}{ - "foo.txt": map[string]interface{}{ - "content": "new content to existing gist", - "filename": "foo.txt", - }, - }, + AddFilename: "../fixture.txt", }, }, } @@ -338,10 +295,6 @@ func Test_editRun(t *testing.T) { return "new file content", nil } - tt.opts.Add = func(_, _ string, _ *iostreams.IOStreams) (string, error) { - return "new content to existing gist", nil - } - tt.opts.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } From 3e5d5a23c083848b510a7ad6c0b6056140031680 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 24 Feb 2021 10:22:56 +0530 Subject: [PATCH 18/29] add fixturefile const in tests --- pkg/cmd/gist/edit/edit_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 1b5ac25b4..abbda4acd 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -260,7 +260,7 @@ func Test_editRun(t *testing.T) { httpmock.StatusStringResponse(201, "{}")) }, opts: &EditOptions{ - AddFilename: "../fixture.txt", + AddFilename: fixtureFile, }, }, } From 0f85304e3e144b06ee142da461e43d0f0443ed4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 24 Feb 2021 14:37:29 +0100 Subject: [PATCH 19/29] Avoid crash in `pr merge` when verifying whether a PR had diverged A PR is not guaranteed to have commits, it seems, so add a guard against assuming that there is always a head commit. --- git/git.go | 41 +++------ pkg/cmd/pr/merge/merge.go | 5 +- pkg/cmd/pr/merge/merge_test.go | 157 +++++++++++++++++++++++++++------ 3 files changed, 148 insertions(+), 55 deletions(-) diff --git a/git/git.go b/git/git.go index 684b00c10..abff866d6 100644 --- a/git/git.go +++ b/git/git.go @@ -180,43 +180,30 @@ func Commits(baseRef, headRef string) ([]*Commit, error) { return commits, nil } +func lookupCommit(sha, format string) ([]byte, error) { + logCmd, err := GitCommand("-c", "log.ShowSignature=false", "show", "-s", "--pretty=format:"+format, sha) + if err != nil { + return nil, err + } + return run.PrepareCmd(logCmd).Output() +} + func LastCommit() (*Commit, error) { - logCmd, err := GitCommand("-c", "log.ShowSignature=false", "log", "--pretty=format:%H,%s", "-1") + output, err := lookupCommit("HEAD", "%H,%s") if err != nil { return nil, err } - output, err := run.PrepareCmd(logCmd).Output() - if err != nil { - return nil, err - } - - lines := outputLines(output) - if len(lines) != 1 { - return nil, ErrNotOnAnyBranch - } - - split := strings.SplitN(lines[0], ",", 2) - if len(split) != 2 { - return nil, ErrNotOnAnyBranch - } - + idx := bytes.IndexByte(output, ',') return &Commit{ - Sha: split[0], - Title: split[1], + Sha: string(output[0:idx]), + Title: strings.TrimSpace(string(output[idx+1:])), }, nil } func CommitBody(sha string) (string, error) { - showCmd, err := GitCommand("-c", "log.ShowSignature=false", "show", "-s", "--pretty=format:%b", sha) - if err != nil { - return "", err - } - output, err := run.PrepareCmd(showCmd).Output() - if err != nil { - return "", err - } - return string(output), nil + output, err := lookupCommit(sha, "%b") + return string(output), err } // Push publishes a git ref to a remote and sets up upstream configuration diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index 67498b726..c979a7f91 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -157,9 +157,8 @@ func mergeRun(opts *MergeOptions) error { return nil } - if opts.SelectorArg == "" { - localBranchLastCommit, err := git.LastCommit() - if err == nil { + if opts.SelectorArg == "" && len(pr.Commits.Nodes) > 0 { + if localBranchLastCommit, err := git.LastCommit(); err == nil { if localBranchLastCommit.Sha != pr.Commits.Nodes[0].Commit.Oid { fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) has diverged from local branch\n", cs.Yellow("!"), pr.Number, pr.Title) diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index dd4ff80cc..538f75056 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/git" @@ -336,7 +337,7 @@ func TestPrMerge_deleteBranch(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register("git -c log.ShowSignature=false log --pretty=format:%H,%s -1", 0, "") + cs.Register(`git .+ show .+ HEAD`, 1, "") cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") cs.Register(`git checkout master`, 0, "") cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") @@ -347,8 +348,11 @@ func TestPrMerge_deleteBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #10 (Blueberries are a good fruit) + ✓ Deleted branch blueberries and switched to branch master + `), output.Stderr()) } func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { @@ -380,8 +384,11 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #10 (Blueberries are a good fruit) + ✓ Deleted branch blueberries + `), output.Stderr()) } func TestPrMerge_noPrNumberGiven(t *testing.T) { @@ -402,7 +409,7 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register("git -c log.ShowSignature=false log --pretty=format:%H,%s -1", 0, "") + cs.Register(`git .+ show .+ HEAD`, 1, "") cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") output, err := runCommand(http, "blueberries", true, "pr merge --merge") @@ -410,20 +417,33 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) { t.Fatalf("error running command `pr merge`: %v", err) } - r := regexp.MustCompile(`Merged pull request #10 \(Blueberries are a good fruit\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #10 (Blueberries are a good fruit) + `), output.Stderr()) } -func Test_divergingPullRequestWarning(t *testing.T) { +func Test_nonDivergingPullRequest(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) http.Register( httpmock.GraphQL(`query PullRequestForBranch\b`), - // FIXME: references fixture from another package - httpmock.FileResponse("../view/fixtures/prViewPreviewWithMetadataByBranch.json")) + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes": [{ + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"}, + "id": "PR_10", + "title": "Blueberries are a good fruit", + "number": 10, + "commits": { + "nodes": [{ + "commit": { + "oid": "COMMITSHA1" + } + }], + "totalCount": 1 + } + }] } } } }`)) http.Register( httpmock.GraphQL(`mutation PullRequestMerge\b`), httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { @@ -435,7 +455,7 @@ func Test_divergingPullRequestWarning(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register("git -c log.ShowSignature=false log --pretty=format:%H,%s -1", 0, "deadbeef,title") + cs.Register(`git .+ show .+ HEAD`, 0, "COMMITSHA1,title") cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") output, err := runCommand(http, "blueberries", true, "pr merge --merge") @@ -443,11 +463,95 @@ func Test_divergingPullRequestWarning(t *testing.T) { t.Fatalf("error running command `pr merge`: %v", err) } - r := regexp.MustCompile(`. Pull request #10 \(Blueberries are a good fruit\) has diverged from local branch\n. Merged pull request #10 \(Blueberries are a good fruit\)\n`) + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #10 (Blueberries are a good fruit) + `), output.Stderr()) +} - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) +func Test_divergingPullRequestWarning(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes": [{ + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"}, + "id": "PR_10", + "title": "Blueberries are a good fruit", + "number": 10, + "commits": { + "nodes": [{ + "commit": { + "oid": "COMMITSHA1" + } + }], + "totalCount": 1 + } + }] } } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "PR_10", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + assert.NotContains(t, input, "commitHeadline") + })) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git .+ show .+ HEAD`, 0, "COMMITSHA2,title") + cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") + + output, err := runCommand(http, "blueberries", true, "pr merge --merge") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) } + + assert.Equal(t, heredoc.Doc(` + ! Pull request #10 (Blueberries are a good fruit) has diverged from local branch + ✓ Merged pull request #10 (Blueberries are a good fruit) + `), output.Stderr()) +} + +func Test_pullRequestWithoutCommits(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes": [{ + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"}, + "id": "PR_10", + "title": "Blueberries are a good fruit", + "number": 10, + "commits": { + "nodes": [], + "totalCount": 0 + } + }] } } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "PR_10", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + assert.NotContains(t, input, "commitHeadline") + })) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") + + output, err := runCommand(http, "blueberries", true, "pr merge --merge") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #10 (Blueberries are a good fruit) + `), output.Stderr()) } func TestPrMerge_rebase(t *testing.T) { @@ -517,8 +621,10 @@ func TestPrMerge_squash(t *testing.T) { t.Fatalf("error running command `pr merge`: %v", err) } - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Squashed and merged pull request #3") + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Squashed and merged pull request #3 (The title of the PR) + `), output.Stderr()) } func TestPrMerge_alreadyMerged(t *testing.T) { @@ -614,7 +720,6 @@ func TestPRMerge_interactive(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register("git -c log.ShowSignature=false log --pretty=format:%H,%s -1", 0, "") cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") as, surveyTeardown := prompt.InitAskStubber() @@ -643,6 +748,7 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { "headRefName": "blueberries", "headRepositoryOwner": {"login": "OWNER"}, "id": "THE-ID", + "title": "It was the best of times", "number": 3 }] } } } }`)) http.Register( @@ -667,7 +773,6 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register("git -c log.ShowSignature=false log --pretty=format:%H,%s -1", 0, "") cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") cs.Register(`git checkout master`, 0, "") cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") @@ -684,8 +789,11 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } - //nolint:staticcheck // prefer exact matchers over ExpectLines - test.ExpectLines(t, output.Stderr(), "Merged pull request #3", "Deleted branch blueberries and switched to branch master") + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #3 (It was the best of times) + ✓ Deleted branch blueberries and switched to branch master + `), output.Stderr()) } func TestPRMerge_interactiveSquashEditCommitMsg(t *testing.T) { @@ -777,7 +885,6 @@ func TestPRMerge_interactiveCancelled(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register("git -c log.ShowSignature=false log --pretty=format:%H,%s -1", 0, "") cs.Register(`git config --get-regexp.+branch\\\.blueberries\\\.`, 0, "") as, surveyTeardown := prompt.InitAskStubber() From d97e8fe172d272d1718109234c7a4f24def7f871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 24 Feb 2021 15:05:56 +0100 Subject: [PATCH 20/29] Add live tests for some methods in the `git` package We relied too much on stubs for these methods. These new tests actually invoke `git` commands in the context of a test repository. --- git/fixtures/.gitignore | 1 + git/fixtures/simple.git/HEAD | 1 + git/fixtures/simple.git/config | 9 +++++ git/fixtures/simple.git/index | Bin 0 -> 65 bytes git/fixtures/simple.git/logs/HEAD | 2 + git/fixtures/simple.git/logs/refs/heads/main | 2 + .../4b/825dc642cb6eb9a060e54bf8d69288fbee4904 | Bin 0 -> 15 bytes .../6f/1a2405cace1633d89a79c74c65f22fe78f9659 | Bin 0 -> 191 bytes .../d1/e0abfb7d158ed544a202a6958c62d4fc22e12f | 3 ++ git/fixtures/simple.git/refs/heads/main | 1 + git/git_test.go | 35 ++++++++++++++++++ 11 files changed, 54 insertions(+) create mode 100644 git/fixtures/.gitignore create mode 100644 git/fixtures/simple.git/HEAD create mode 100644 git/fixtures/simple.git/config create mode 100644 git/fixtures/simple.git/index create mode 100644 git/fixtures/simple.git/logs/HEAD create mode 100644 git/fixtures/simple.git/logs/refs/heads/main create mode 100644 git/fixtures/simple.git/objects/4b/825dc642cb6eb9a060e54bf8d69288fbee4904 create mode 100644 git/fixtures/simple.git/objects/6f/1a2405cace1633d89a79c74c65f22fe78f9659 create mode 100644 git/fixtures/simple.git/objects/d1/e0abfb7d158ed544a202a6958c62d4fc22e12f create mode 100644 git/fixtures/simple.git/refs/heads/main diff --git a/git/fixtures/.gitignore b/git/fixtures/.gitignore new file mode 100644 index 000000000..abae30d02 --- /dev/null +++ b/git/fixtures/.gitignore @@ -0,0 +1 @@ +*.git/COMMIT_EDITMSG diff --git a/git/fixtures/simple.git/HEAD b/git/fixtures/simple.git/HEAD new file mode 100644 index 000000000..b870d8262 --- /dev/null +++ b/git/fixtures/simple.git/HEAD @@ -0,0 +1 @@ +ref: refs/heads/main diff --git a/git/fixtures/simple.git/config b/git/fixtures/simple.git/config new file mode 100644 index 000000000..f0858dd73 --- /dev/null +++ b/git/fixtures/simple.git/config @@ -0,0 +1,9 @@ +[core] + repositoryformatversion = 0 + filemode = true + ;bare = true + ignorecase = true + precomposeunicode = true +[user] + name = Mona the Cat + email = monalisa@github.com diff --git a/git/fixtures/simple.git/index b/git/fixtures/simple.git/index new file mode 100644 index 0000000000000000000000000000000000000000..65d675154f23ffb2d0196e017d44a5e7017550f5 GIT binary patch literal 65 zcmZ?q402{*U|<4bhL9jvS0E+HV4z^Y<=qr}%;|LA&IJiiy? 1614174263 +0100 commit (initial): Initial commit +d1e0abfb7d158ed544a202a6958c62d4fc22e12f 6f1a2405cace1633d89a79c74c65f22fe78f9659 Mona the Cat 1614174275 +0100 commit: Second commit diff --git a/git/fixtures/simple.git/logs/refs/heads/main b/git/fixtures/simple.git/logs/refs/heads/main new file mode 100644 index 000000000..216887f9e --- /dev/null +++ b/git/fixtures/simple.git/logs/refs/heads/main @@ -0,0 +1,2 @@ +0000000000000000000000000000000000000000 d1e0abfb7d158ed544a202a6958c62d4fc22e12f Mona the Cat 1614174263 +0100 commit (initial): Initial commit +d1e0abfb7d158ed544a202a6958c62d4fc22e12f 6f1a2405cace1633d89a79c74c65f22fe78f9659 Mona the Cat 1614174275 +0100 commit: Second commit diff --git a/git/fixtures/simple.git/objects/4b/825dc642cb6eb9a060e54bf8d69288fbee4904 b/git/fixtures/simple.git/objects/4b/825dc642cb6eb9a060e54bf8d69288fbee4904 new file mode 100644 index 0000000000000000000000000000000000000000..adf64119a33d7621aeeaa505d30adb58afaa5559 GIT binary patch literal 15 Wcmb zVTQ@QwM_vh`=W;kP>SeF4um-cNi*AE#Z#)Wgc)P3NrYxg=7$g26^awfsivtoAEkIA zMvEL~A9KJ$H6x0{YWSgRKj7MT23-ZdSmC`5x^E|cE}O28^p<=302ds&iE#38vCdjE t-0@N6e{FM<-1h>1E5>}kHaL|J-S!2v!y@`TwDRCyhaSOcegWSFR-)K;Tv-4B literal 0 HcmV?d00001 diff --git a/git/fixtures/simple.git/objects/d1/e0abfb7d158ed544a202a6958c62d4fc22e12f b/git/fixtures/simple.git/objects/d1/e0abfb7d158ed544a202a6958c62d4fc22e12f new file mode 100644 index 000000000..ec3ada617 --- /dev/null +++ b/git/fixtures/simple.git/objects/d1/e0abfb7d158ed544a202a6958c62d4fc22e12f @@ -0,0 +1,3 @@ +xNK +0tS ILc +"+@M뢷7"^@ bZxFhb轴;lKr3<33Kc#-"k8Z.2d=^*)ES&iqɏiϋP jAy 3*H/ \ No newline at end of file diff --git a/git/fixtures/simple.git/refs/heads/main b/git/fixtures/simple.git/refs/heads/main new file mode 100644 index 000000000..8316cdaf5 --- /dev/null +++ b/git/fixtures/simple.git/refs/heads/main @@ -0,0 +1 @@ +6f1a2405cace1633d89a79c74c65f22fe78f9659 diff --git a/git/git_test.go b/git/git_test.go index 899ca3b49..1cd816fa3 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -1,12 +1,47 @@ package git import ( + "os" "reflect" "testing" "github.com/cli/cli/internal/run" ) +func setGitDir(t *testing.T, dir string) { + // TODO: also set XDG_CONFIG_HOME, GIT_CONFIG_NOSYSTEM + old_GIT_DIR := os.Getenv("GIT_DIR") + os.Setenv("GIT_DIR", dir) + t.Cleanup(func() { + os.Setenv("GIT_DIR", old_GIT_DIR) + }) +} + +func TestLastCommit(t *testing.T) { + setGitDir(t, "./fixtures/simple.git") + c, err := LastCommit() + if err != nil { + t.Fatalf("LastCommit error: %v", err) + } + if c.Sha != "6f1a2405cace1633d89a79c74c65f22fe78f9659" { + t.Errorf("expected sha %q, got %q", "6f1a2405cace1633d89a79c74c65f22fe78f9659", c.Sha) + } + if c.Title != "Second commit" { + t.Errorf("expected title %q, got %q", "Second commit", c.Title) + } +} + +func TestCommitBody(t *testing.T) { + setGitDir(t, "./fixtures/simple.git") + body, err := CommitBody("6f1a2405cace1633d89a79c74c65f22fe78f9659") + if err != nil { + t.Fatalf("CommitBody error: %v", err) + } + if body != "I'm starting to get the hang of things\n" { + t.Errorf("expected %q, got %q", "I'm starting to get the hang of things\n", body) + } +} + func Test_UncommittedChangeCount(t *testing.T) { type c struct { Label string From 492f45422ecd796416acaaaaac2e5a37ae43dbc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 26 Feb 2021 13:07:38 +0100 Subject: [PATCH 21/29] Add a note about the style of git tests --- git/git_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/git/git_test.go b/git/git_test.go index 1cd816fa3..685b4d1c0 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -42,6 +42,12 @@ func TestCommitBody(t *testing.T) { } } +/* + NOTE: below this are stubbed git tests, i.e. those that do not actually invoke `git`. If possible, utilize + `setGitDir()` to allow new tests to interact with `git`. For write operations, you can use `t.TempDir()` to + host a temporary git repository that is safe to be changed. +*/ + func Test_UncommittedChangeCount(t *testing.T) { type c struct { Label string From 406d7eee456f8c91da5a7ba66c2d9e8ab30c2213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Sat, 27 Feb 2021 12:03:29 +0100 Subject: [PATCH 22/29] :nail_care: cleanup `gist edit -a` feature --- pkg/cmd/gist/edit/edit.go | 40 +++++++++++++++------------------- pkg/cmd/gist/edit/edit_test.go | 31 +++++++++++++------------- 2 files changed, 33 insertions(+), 38 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 5d8146f8b..238f9c64d 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -28,7 +28,8 @@ type EditOptions struct { HttpClient func() (*http.Client, error) Config func() (config.Config, error) - Edit func(string, string, string, *iostreams.IOStreams) (string, error) + Edit func(string, string, string, *iostreams.IOStreams) (string, error) + Selector string EditFilename string AddFilename string @@ -62,7 +63,8 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman return editRun(&opts) }, } - cmd.Flags().StringVarP(&opts.AddFilename, "add", "a", "", "Add a file") + + cmd.Flags().StringVarP(&opts.AddFilename, "add", "a", "", "Add a new file to the gist") cmd.Flags().StringVarP(&opts.EditFilename, "filename", "f", "", "Select a file to edit") return cmd @@ -106,8 +108,7 @@ func editRun(opts *EditOptions) error { cs := opts.IO.ColorScheme() if addFilename != "" { - //Add files to an existing gist. - files, err := getFilesToAdd(addFilename, opts) + files, err := getFilesToAdd(addFilename) if err != nil { return err } @@ -118,10 +119,7 @@ func editRun(opts *EditOptions) error { return err } - completionMessage := filepath.Base(addFilename) + " added to gist" - - fmt.Fprintf(opts.IO.Out, "%s %s\n", cs.SuccessIconWithColor(cs.Green), completionMessage) - + fmt.Fprintf(opts.IO.Out, "%s %s added to gist\n", cs.SuccessIconWithColor(cs.Green), filepath.Base(addFilename)) return nil } @@ -249,25 +247,21 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error return nil } -func getFilesToAdd(file string, opts *EditOptions) (map[string]*shared.GistFile, error) { - cs := opts.IO.ColorScheme() - - filesToAdd := map[string]*shared.GistFile{} - - filename := filepath.Base(file) - +func getFilesToAdd(file string) (map[string]*shared.GistFile, error) { content, err := ioutil.ReadFile(file) if err != nil { - return nil, fmt.Errorf("%s failed to read file %s: %w", cs.FailureIcon(), file, err) + return nil, fmt.Errorf("failed to read file %s: %w", file, err) } - if string(content) == "" { - return nil, fmt.Errorf("%s Contents can't be empty", cs.FailureIcon()) + if len(content) == 0 { + return nil, errors.New("file contents cannot be empty") } - filesToAdd[filename] = &shared.GistFile{ - Filename: filename, - Content: string(content), - } - return filesToAdd, nil + filename := filepath.Base(file) + return map[string]*shared.GistFile{ + filename: { + Filename: filename, + Content: string(content), + }, + }, nil } diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index abbda4acd..e96c1fc28 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -16,25 +16,22 @@ import ( "github.com/cli/cli/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" -) - -const ( - fixtureFile = "../fixture.txt" + "github.com/stretchr/testify/require" ) func Test_getFilesToAdd(t *testing.T) { - gf, err := getFilesToAdd(fixtureFile, &EditOptions{ - AddFilename: fixtureFile, - IO: &iostreams.IOStreams{}, - }) - if err != nil { - t.Fatalf("unexpected error processing files: %s", err) - } + fileToAdd := filepath.Join(t.TempDir(), "gist-test.txt") + err := ioutil.WriteFile(fileToAdd, []byte("hello"), 0600) + require.NoError(t, err) + gf, err := getFilesToAdd(fileToAdd) + require.NoError(t, err) + + filename := filepath.Base(fileToAdd) assert.Equal(t, map[string]*shared.GistFile{ - filepath.Base(fixtureFile): { - Filename: filepath.Base(fixtureFile), - Content: "{}", + filename: { + Filename: filename, + Content: "hello", }, }, gf) } @@ -98,6 +95,10 @@ func TestNewCmdEdit(t *testing.T) { } func Test_editRun(t *testing.T) { + fileToAdd := filepath.Join(t.TempDir(), "gist-test.txt") + err := ioutil.WriteFile(fileToAdd, []byte("hello"), 0600) + require.NoError(t, err) + tests := []struct { name string opts *EditOptions @@ -260,7 +261,7 @@ func Test_editRun(t *testing.T) { httpmock.StatusStringResponse(201, "{}")) }, opts: &EditOptions{ - AddFilename: fixtureFile, + AddFilename: fileToAdd, }, }, } From 8f96e406ac450bdaeda7129bf75ad2341701774b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Sat, 27 Feb 2021 12:23:18 +0100 Subject: [PATCH 23/29] Improve error handling and avoid writing confirmation to stdout Right now the `gist edit` command doesn't write anything to stdout, so let's keep it that way until we want to intentionally provide some feedback in the terminal. --- pkg/cmd/gist/edit/edit.go | 22 ++++++++-------------- pkg/cmd/gist/edit/edit_test.go | 22 ++++++++++------------ pkg/cmd/gist/shared/shared.go | 7 +++++++ 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 238f9c64d..27df4a104 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -90,6 +90,9 @@ func editRun(opts *EditOptions) error { gist, err := shared.GetGist(client, ghinstance.OverridableDefault(), gistID) if err != nil { + if errors.Is(err, shared.NotFoundErr) { + return fmt.Errorf("gist not found: %s", gistID) + } return err } @@ -102,27 +105,18 @@ func editRun(opts *EditOptions) error { return fmt.Errorf("You do not own this gist.") } - filesToUpdate := map[string]string{} - - addFilename := opts.AddFilename - cs := opts.IO.ColorScheme() - - if addFilename != "" { - files, err := getFilesToAdd(addFilename) + if opts.AddFilename != "" { + files, err := getFilesToAdd(opts.AddFilename) if err != nil { return err } gist.Files = files - err = updateGist(apiClient, ghinstance.OverridableDefault(), gist) - if err != nil { - return err - } - - fmt.Fprintf(opts.IO.Out, "%s %s added to gist\n", cs.SuccessIconWithColor(cs.Green), filepath.Base(addFilename)) - return nil + return updateGist(apiClient, ghinstance.OverridableDefault(), gist) } + filesToUpdate := map[string]string{} + for { filename := opts.EditFilename candidates := []string{} diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index e96c1fc28..73a53e523 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -106,13 +106,12 @@ func Test_editRun(t *testing.T) { httpStubs func(*httpmock.Registry) askStubs func(*prompt.AskStubber) nontty bool - wantErr bool - wantStderr string + wantErr string wantParams map[string]interface{} }{ { name: "no such gist", - wantErr: true, + wantErr: "gist not found: 1234", }, { name: "one file", @@ -195,7 +194,7 @@ func Test_editRun(t *testing.T) { as.StubOne("unix.md") as.StubOne("Cancel") }, - wantErr: true, + wantErr: "SilentError", gist: &shared.Gist{ ID: "1234", Files: map[string]*shared.GistFile{ @@ -240,8 +239,7 @@ func Test_editRun(t *testing.T) { }, Owner: &shared.GistOwner{Login: "octocat2"}, }, - wantErr: true, - wantStderr: "You do not own this gist.", + wantErr: "You do not own this gist.", }, { name: "add file to existing gist", @@ -299,7 +297,7 @@ func Test_editRun(t *testing.T) { tt.opts.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } - io, _, _, _ := iostreams.Test() + io, _, stdout, stderr := iostreams.Test() io.SetStdoutTTY(!tt.nontty) io.SetStdinTTY(!tt.nontty) tt.opts.IO = io @@ -313,11 +311,8 @@ func Test_editRun(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := editRun(tt.opts) reg.Verify(t) - if tt.wantErr { - assert.Error(t, err) - if tt.wantStderr != "" { - assert.EqualError(t, err, tt.wantStderr) - } + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) return } assert.NoError(t, err) @@ -331,6 +326,9 @@ func Test_editRun(t *testing.T) { } assert.Equal(t, tt.wantParams, reqBody) } + + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) }) } } diff --git a/pkg/cmd/gist/shared/shared.go b/pkg/cmd/gist/shared/shared.go index 04e63ce86..04fe2c33b 100644 --- a/pkg/cmd/gist/shared/shared.go +++ b/pkg/cmd/gist/shared/shared.go @@ -1,6 +1,7 @@ package shared import ( + "errors" "fmt" "net/http" "net/url" @@ -31,6 +32,8 @@ type Gist struct { Owner *GistOwner `json:"owner,omitempty"` } +var NotFoundErr = errors.New("not found") + func GetGist(client *http.Client, hostname, gistID string) (*Gist, error) { gist := Gist{} path := fmt.Sprintf("gists/%s", gistID) @@ -38,6 +41,10 @@ func GetGist(client *http.Client, hostname, gistID string) (*Gist, error) { apiClient := api.NewClientFromHTTP(client) err := apiClient.REST(hostname, "GET", path, nil, &gist) if err != nil { + var httpErr api.HTTPError + if errors.As(err, &httpErr) && httpErr.StatusCode == 404 { + return nil, NotFoundErr + } return nil, err } From 4da02614ed6bc7ff22307723bda6d15c87ff324e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Sat, 27 Feb 2021 13:17:59 +0100 Subject: [PATCH 24/29] Switch `repo list` to query via `graphql` package Also order results by PUSHED_AT instead of UPDATED_AT to match the web interface. --- pkg/cmd/repo/list/fixtures/repoList.json | 6 +- pkg/cmd/repo/list/http.go | 108 +++++++++++------------ pkg/cmd/repo/list/list.go | 83 +++-------------- pkg/cmd/repo/list/list_test.go | 20 ++--- 4 files changed, 75 insertions(+), 142 deletions(-) diff --git a/pkg/cmd/repo/list/fixtures/repoList.json b/pkg/cmd/repo/list/fixtures/repoList.json index ca0faa85c..8ee348f57 100644 --- a/pkg/cmd/repo/list/fixtures/repoList.json +++ b/pkg/cmd/repo/list/fixtures/repoList.json @@ -10,7 +10,7 @@ "isFork": false, "isPrivate": false, "isArchived": false, - "updatedAt": "2021-02-19T06:34:58Z" + "pushedAt": "2021-02-19T06:34:58Z" }, { "nameWithOwner": "octocat/cli", @@ -18,7 +18,7 @@ "isFork": true, "isPrivate": false, "isArchived": false, - "updatedAt": "2021-02-19T06:06:06Z" + "pushedAt": "2021-02-19T06:06:06Z" }, { "nameWithOwner": "octocat/testing", @@ -26,7 +26,7 @@ "isFork": false, "isPrivate": true, "isArchived": false, - "updatedAt": "2021-02-11T22:32:05Z" + "pushedAt": "2021-02-11T22:32:05Z" } ], "pageInfo": { diff --git a/pkg/cmd/repo/list/http.go b/pkg/cmd/repo/list/http.go index 4669e72a3..8413b479f 100644 --- a/pkg/cmd/repo/list/http.go +++ b/pkg/cmd/repo/list/http.go @@ -1,59 +1,62 @@ package list import ( + "context" + "net/http" "strings" + "time" - "github.com/cli/cli/api" + "github.com/cli/cli/internal/ghinstance" "github.com/shurcooL/githubv4" + "github.com/shurcooL/graphql" ) +type Repository struct { + NameWithOwner string + Description string + IsFork bool + IsPrivate bool + IsArchived bool + PushedAt time.Time +} + +func (r Repository) Info() string { + var tags []string + + if r.IsPrivate { + tags = append(tags, "private") + } else { + tags = append(tags, "public") + } + if r.IsFork { + tags = append(tags, "fork") + } + if r.IsArchived { + tags = append(tags, "archived") + } + + return strings.Join(tags, ", ") +} + type RepositoryList struct { Repositories []Repository TotalCount int } -func listRepos(client *api.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { - type response struct { +func listRepos(client *http.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { + type query struct { RepositoryOwner struct { Repositories struct { - TotalCount int - RepositoryCount int - Nodes []Repository - PageInfo struct { + Nodes []Repository + TotalCount int + PageInfo struct { HasNextPage bool EndCursor string } - } - } + } `graphql:"repositories(first: $perPage, after: $endCursor, privacy: $privacy, isFork: $fork, ownerAffiliations: OWNER, orderBy: { field: PUSHED_AT, direction: DESC })"` + } `graphql:"repositoryOwner(login: $owner)"` } - query := ` - query RepoList($owner: String!, $per_page: Int!, $endCursor: String, $fork: Boolean, $privacy: RepositoryPrivacy) { - repositoryOwner(login: $owner) { - repositories( - first: $per_page, - after: $endCursor, - privacy: $privacy, - isFork: $fork, - ownerAffiliations: OWNER, - orderBy: { field: UPDATED_AT, direction: DESC }) { - totalCount - nodes { - nameWithOwner - description - isFork - isPrivate - isArchived - updatedAt - } - pageInfo { - hasNextPage - endCursor - } - } - } - }` - perPage := limit if perPage > 100 { perPage = 100 @@ -61,7 +64,7 @@ func listRepos(client *api.Client, hostname string, limit int, owner string, fil variables := map[string]interface{}{ "owner": githubv4.String(owner), - "per_page": githubv4.Int(perPage), + "perPage": githubv4.Int(perPage), "endCursor": (*githubv4.String)(nil), } @@ -79,40 +82,29 @@ func listRepos(client *api.Client, hostname string, limit int, owner string, fil variables["fork"] = (*githubv4.Boolean)(nil) } - var repos []Repository - var totalCount int - - var result response - + listResult := RepositoryList{} pagination: for { - err := client.GraphQL(hostname, query, variables, &result) + var result query + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) + err := gql.QueryNamed(context.Background(), "RepositoryList", &result, variables) if err != nil { return nil, err } - repos = append(repos, result.RepositoryOwner.Repositories.Nodes...) - - if len(repos) >= limit { - if len(repos) > limit { - repos = repos[:limit] + listResult.TotalCount = result.RepositoryOwner.Repositories.TotalCount + for _, repo := range result.RepositoryOwner.Repositories.Nodes { + listResult.Repositories = append(listResult.Repositories, repo) + if len(listResult.Repositories) >= limit { + break pagination } - break pagination } if !result.RepositoryOwner.Repositories.PageInfo.HasNextPage { break } - variables["endCursor"] = githubv4.String(result.RepositoryOwner.Repositories.PageInfo.EndCursor) } - totalCount = result.RepositoryOwner.Repositories.TotalCount - - listResult := &RepositoryList{ - Repositories: repos, - TotalCount: totalCount, - } - - return listResult, nil + return &listResult, nil } diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go index 631a50bf3..4fd74f827 100644 --- a/pkg/cmd/repo/list/list.go +++ b/pkg/cmd/repo/list/list.go @@ -3,7 +3,6 @@ package list import ( "fmt" "net/http" - "strings" "time" "github.com/cli/cli/api" @@ -45,14 +44,12 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman var ( flagPublic bool flagPrivate bool - flagSource bool - flagFork bool ) cmd := &cobra.Command{ - Use: "list", + Use: "list []", Args: cobra.MaximumNArgs(1), - Short: "List repositories from a user or organization", + Short: "List repositories owned by user or organization", RunE: func(c *cobra.Command, args []string) error { if opts.Limit < 1 { return &cmdutil.FlagError{Err: fmt.Errorf("invalid limit: %v", opts.Limit)} @@ -61,7 +58,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman if flagPrivate && flagPublic { return &cmdutil.FlagError{Err: fmt.Errorf("specify only one of `--public` or `--private`")} } - if flagSource && flagFork { + if opts.Source && opts.Fork { return &cmdutil.FlagError{Err: fmt.Errorf("specify only one of `--source` or `--fork`")} } @@ -71,9 +68,6 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman opts.Visibility = "public" } - opts.Fork = flagFork - opts.Source = flagSource - if len(args) > 0 { opts.Owner = args[0] } @@ -88,8 +82,8 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum number of repositories to list") cmd.Flags().BoolVar(&flagPrivate, "private", false, "Show only private repositories") cmd.Flags().BoolVar(&flagPublic, "public", false, "Show only public repositories") - cmd.Flags().BoolVar(&flagSource, "source", false, "Show only source repositories") - cmd.Flags().BoolVar(&flagFork, "fork", false, "Show only forks") + cmd.Flags().BoolVar(&opts.Source, "source", false, "Show only non-forks") + cmd.Flags().BoolVar(&opts.Fork, "fork", false, "Show only forks") return cmd } @@ -118,58 +112,36 @@ func listRun(opts *ListOptions) error { Source: opts.Source, } - listResult, err := listRepos(apiClient, ghinstance.OverridableDefault(), opts.Limit, owner, filter) + listResult, err := listRepos(httpClient, ghinstance.OverridableDefault(), opts.Limit, owner, filter) if err != nil { return err } cs := opts.IO.ColorScheme() - tp := utils.NewTablePrinter(opts.IO) - - notArchived := filter.Fork || filter.Source - - matchCount := len(listResult.Repositories) now := opts.Now() for _, repo := range listResult.Repositories { - if notArchived && repo.IsArchived { - matchCount-- - listResult.TotalCount-- - continue - } - - nameWithOwner := repo.NameWithOwner - info := repo.Info() infoColor := cs.Gray - visibility := "Public" - if repo.IsPrivate { infoColor = cs.Yellow - visibility = "Private" } - description := repo.Description - updatedAt := repo.UpdatedAt.Format(time.RFC3339) - + tp.AddField(repo.NameWithOwner, nil, cs.Bold) + tp.AddField(text.ReplaceExcessiveWhitespace(repo.Description), nil, nil) + tp.AddField(info, nil, infoColor) if tp.IsTTY() { - tp.AddField(nameWithOwner, nil, cs.Bold) - tp.AddField(text.ReplaceExcessiveWhitespace(description), nil, nil) - tp.AddField(info, nil, infoColor) - tp.AddField(utils.FuzzyAgoAbbr(now, repo.UpdatedAt), nil, nil) + tp.AddField(utils.FuzzyAgoAbbr(now, repo.PushedAt), nil, cs.Gray) } else { - tp.AddField(nameWithOwner, nil, nil) - tp.AddField(text.ReplaceExcessiveWhitespace(description), nil, nil) - tp.AddField(visibility, nil, nil) - tp.AddField(updatedAt, nil, nil) + tp.AddField(repo.PushedAt.Format(time.RFC3339), nil, nil) } tp.EndRow() } if isTerminal { hasFilters := filter.Visibility != "" || filter.Fork || filter.Source - title := listHeader(owner, matchCount, listResult.TotalCount, hasFilters) + title := listHeader(owner, len(listResult.Repositories), listResult.TotalCount, hasFilters) fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) } @@ -186,34 +158,3 @@ func listHeader(owner string, matchCount, totalMatchCount int, hasFilters bool) return fmt.Sprintf("Showing %d of %d repositories in @%s", matchCount, totalMatchCount, owner) } - -type Repository struct { - NameWithOwner string - Description string - IsFork bool - IsPrivate bool - IsArchived bool - UpdatedAt time.Time -} - -func (r Repository) Info() string { - var info string - var tags []string - - if r.IsPrivate { - tags = append(tags, "private") - } - if r.IsFork { - tags = append(tags, "fork") - } - if r.IsArchived { - tags = append(tags, "archived") - } - - if len(tags) > 0 { - tags[0] = strings.Title(tags[0]) - info = strings.Join(tags, ", ") - } - - return info -} diff --git a/pkg/cmd/repo/list/list_test.go b/pkg/cmd/repo/list/list_test.go index 7ffe6de17..d6bc37afe 100644 --- a/pkg/cmd/repo/list/list_test.go +++ b/pkg/cmd/repo/list/list_test.go @@ -62,7 +62,7 @@ func TestRepoList_nontty(t *testing.T) { httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`), ) httpReg.Register( - httpmock.GraphQL(`query RepoList\b`), + httpmock.GraphQL(`query RepositoryList\b`), httpmock.FileResponse("./fixtures/repoList.json"), ) @@ -84,9 +84,9 @@ func TestRepoList_nontty(t *testing.T) { assert.Equal(t, "", stderr.String()) assert.Equal(t, heredoc.Doc(` - octocat/hello-world My first repository Public 2021-02-19T06:34:58Z - octocat/cli GitHub CLI Public 2021-02-19T06:06:06Z - octocat/testing Private 2021-02-11T22:32:05Z + octocat/hello-world My first repository public 2021-02-19T06:34:58Z + octocat/cli GitHub CLI public, fork 2021-02-19T06:06:06Z + octocat/testing private 2021-02-11T22:32:05Z `), stdout.String()) } @@ -104,7 +104,7 @@ func TestRepoList_tty(t *testing.T) { httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`), ) httpReg.Register( - httpmock.GraphQL(`query RepoList\b`), + httpmock.GraphQL(`query RepositoryList\b`), httpmock.FileResponse("./fixtures/repoList.json"), ) @@ -129,9 +129,9 @@ func TestRepoList_tty(t *testing.T) { Showing 3 of 3 repositories in @octocat - octocat/hello-world My first repository 8h - octocat/cli GitHub CLI Fork 8h - octocat/testing Private 7d + octocat/hello-world My first repository public 8h + octocat/cli GitHub CLI public, fork 8h + octocat/testing private 7d `), stdout.String()) } @@ -144,10 +144,10 @@ func TestRepoList_filtering(t *testing.T) { httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`), ) http.Register( - httpmock.GraphQL(`query RepoList\b`), + httpmock.GraphQL(`query RepositoryList\b`), httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { assert.Equal(t, "PRIVATE", params["privacy"]) - assert.Equal(t, float64(2), params["per_page"]) + assert.Equal(t, float64(2), params["perPage"]) }), ) From 1fa763f51472075d26a67f0fc89e2696bd867280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Sat, 27 Feb 2021 14:21:26 +0100 Subject: [PATCH 25/29] Avoid having to first query for username in `repo list` Dynamically construct the GraphQL query by using the `viewer` connection if the owner isn't set and the `repositoryOwner(login:"...")` connection if the owner was set. --- pkg/cmd/repo/list/fixtures/repoList.json | 1 + pkg/cmd/repo/list/http.go | 58 ++++++++++++++++-------- pkg/cmd/repo/list/list.go | 19 ++------ pkg/cmd/repo/list/list_test.go | 12 ----- 4 files changed, 42 insertions(+), 48 deletions(-) diff --git a/pkg/cmd/repo/list/fixtures/repoList.json b/pkg/cmd/repo/list/fixtures/repoList.json index 8ee348f57..18bb5eff8 100644 --- a/pkg/cmd/repo/list/fixtures/repoList.json +++ b/pkg/cmd/repo/list/fixtures/repoList.json @@ -1,6 +1,7 @@ { "data": { "repositoryOwner": { + "login": "octocat", "repositories": { "totalCount": 3, "nodes": [ diff --git a/pkg/cmd/repo/list/http.go b/pkg/cmd/repo/list/http.go index 8413b479f..97bf584fb 100644 --- a/pkg/cmd/repo/list/http.go +++ b/pkg/cmd/repo/list/http.go @@ -3,6 +3,7 @@ package list import ( "context" "net/http" + "reflect" "strings" "time" @@ -39,31 +40,18 @@ func (r Repository) Info() string { } type RepositoryList struct { + Owner string Repositories []Repository TotalCount int } func listRepos(client *http.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { - type query struct { - RepositoryOwner struct { - Repositories struct { - Nodes []Repository - TotalCount int - PageInfo struct { - HasNextPage bool - EndCursor string - } - } `graphql:"repositories(first: $perPage, after: $endCursor, privacy: $privacy, isFork: $fork, ownerAffiliations: OWNER, orderBy: { field: PUSHED_AT, direction: DESC })"` - } `graphql:"repositoryOwner(login: $owner)"` - } - perPage := limit if perPage > 100 { perPage = 100 } variables := map[string]interface{}{ - "owner": githubv4.String(owner), "perPage": githubv4.Int(perPage), "endCursor": (*githubv4.String)(nil), } @@ -82,28 +70,58 @@ func listRepos(client *http.Client, hostname string, limit int, owner string, fi variables["fork"] = (*githubv4.Boolean)(nil) } + var ownerConnection string + if owner == "" { + ownerConnection = `graphql:"repositoryOwner: viewer"` + } else { + ownerConnection = `graphql:"repositoryOwner(login: $owner)"` + variables["owner"] = githubv4.String(owner) + } + + type repositoryOwner struct { + Login string + Repositories struct { + Nodes []Repository + TotalCount int + PageInfo struct { + HasNextPage bool + EndCursor string + } + } `graphql:"repositories(first: $perPage, after: $endCursor, privacy: $privacy, isFork: $fork, ownerAffiliations: OWNER, orderBy: { field: PUSHED_AT, direction: DESC })"` + } + query := reflect.StructOf([]reflect.StructField{ + { + Name: "RepositoryOwner", + Type: reflect.TypeOf(repositoryOwner{}), + Tag: reflect.StructTag(ownerConnection), + }, + }) + listResult := RepositoryList{} pagination: for { - var result query + result := reflect.New(query) gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) - err := gql.QueryNamed(context.Background(), "RepositoryList", &result, variables) + err := gql.QueryNamed(context.Background(), "RepositoryList", result.Interface(), variables) if err != nil { return nil, err } - listResult.TotalCount = result.RepositoryOwner.Repositories.TotalCount - for _, repo := range result.RepositoryOwner.Repositories.Nodes { + owner := result.Elem().FieldByName("RepositoryOwner").Interface().(repositoryOwner) + listResult.TotalCount = owner.Repositories.TotalCount + listResult.Owner = owner.Login + + for _, repo := range owner.Repositories.Nodes { listResult.Repositories = append(listResult.Repositories, repo) if len(listResult.Repositories) >= limit { break pagination } } - if !result.RepositoryOwner.Repositories.PageInfo.HasNextPage { + if !owner.Repositories.PageInfo.HasNextPage { break } - variables["endCursor"] = githubv4.String(result.RepositoryOwner.Repositories.PageInfo.EndCursor) + variables["endCursor"] = githubv4.String(owner.Repositories.PageInfo.EndCursor) } return &listResult, nil diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go index 4fd74f827..62e16a596 100644 --- a/pkg/cmd/repo/list/list.go +++ b/pkg/cmd/repo/list/list.go @@ -5,7 +5,6 @@ import ( "net/http" "time" - "github.com/cli/cli/api" "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" @@ -94,25 +93,13 @@ func listRun(opts *ListOptions) error { return err } - apiClient := api.NewClientFromHTTP(httpClient) - - isTerminal := opts.IO.IsStdoutTTY() - - owner := opts.Owner - if owner == "" { - owner, err = api.CurrentLoginName(apiClient, ghinstance.OverridableDefault()) - if err != nil { - return err - } - } - filter := FilterOptions{ Visibility: opts.Visibility, Fork: opts.Fork, Source: opts.Source, } - listResult, err := listRepos(httpClient, ghinstance.OverridableDefault(), opts.Limit, owner, filter) + listResult, err := listRepos(httpClient, ghinstance.OverridableDefault(), opts.Limit, opts.Owner, filter) if err != nil { return err } @@ -139,9 +126,9 @@ func listRun(opts *ListOptions) error { tp.EndRow() } - if isTerminal { + if opts.IO.IsStdoutTTY() { hasFilters := filter.Visibility != "" || filter.Fork || filter.Source - title := listHeader(owner, len(listResult.Repositories), listResult.TotalCount, hasFilters) + title := listHeader(listResult.Owner, len(listResult.Repositories), listResult.TotalCount, hasFilters) fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) } diff --git a/pkg/cmd/repo/list/list_test.go b/pkg/cmd/repo/list/list_test.go index d6bc37afe..fb55cb11c 100644 --- a/pkg/cmd/repo/list/list_test.go +++ b/pkg/cmd/repo/list/list_test.go @@ -57,10 +57,6 @@ func TestRepoList_nontty(t *testing.T) { httpReg := &httpmock.Registry{} defer httpReg.Verify(t) - httpReg.Register( - httpmock.GraphQL(`query UserCurrent\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`), - ) httpReg.Register( httpmock.GraphQL(`query RepositoryList\b`), httpmock.FileResponse("./fixtures/repoList.json"), @@ -99,10 +95,6 @@ func TestRepoList_tty(t *testing.T) { httpReg := &httpmock.Registry{} defer httpReg.Verify(t) - httpReg.Register( - httpmock.GraphQL(`query UserCurrent\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`), - ) httpReg.Register( httpmock.GraphQL(`query RepositoryList\b`), httpmock.FileResponse("./fixtures/repoList.json"), @@ -139,10 +131,6 @@ func TestRepoList_filtering(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) - http.Register( - httpmock.GraphQL(`query UserCurrent\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`), - ) http.Register( httpmock.GraphQL(`query RepositoryList\b`), httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { From 2bdffc85e21b7e7161164d8570bc2b102a2f2d9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Sat, 27 Feb 2021 14:39:06 +0100 Subject: [PATCH 26/29] Isolate flag processing tests in `repo list` --- pkg/cmd/repo/list/list_test.go | 166 +++++++++++++++++++++++++++------ 1 file changed, 135 insertions(+), 31 deletions(-) diff --git a/pkg/cmd/repo/list/list_test.go b/pkg/cmd/repo/list/list_test.go index fb55cb11c..7c7cbf938 100644 --- a/pkg/cmd/repo/list/list_test.go +++ b/pkg/cmd/repo/list/list_test.go @@ -14,8 +14,143 @@ import ( "github.com/cli/cli/test" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func TestNewCmdList(t *testing.T) { + tests := []struct { + name string + cli string + wants ListOptions + wantsErr string + }{ + { + name: "no arguments", + cli: "", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "", + Fork: false, + Source: false, + }, + }, + { + name: "with owner", + cli: "monalisa", + wants: ListOptions{ + Limit: 30, + Owner: "monalisa", + Visibility: "", + Fork: false, + Source: false, + }, + }, + { + name: "with limit", + cli: "-L 101", + wants: ListOptions{ + Limit: 101, + Owner: "", + Visibility: "", + Fork: false, + Source: false, + }, + }, + { + name: "only public", + cli: "--public", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "public", + Fork: false, + Source: false, + }, + }, + { + name: "only private", + cli: "--private", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "private", + Fork: false, + Source: false, + }, + }, + { + name: "only forks", + cli: "--fork", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "", + Fork: true, + Source: false, + }, + }, + { + name: "only sources", + cli: "--source", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "", + Fork: false, + Source: true, + }, + }, + { + name: "no public and private", + cli: "--public --private", + wantsErr: "specify only one of `--public` or `--private`", + }, + { + name: "no forks with sources", + cli: "--fork --source", + wantsErr: "specify only one of `--source` or `--fork`", + }, + { + name: "too many arguments", + cli: "monalisa hubot", + wantsErr: "accepts at most 1 arg(s), received 2", + }, + } + + 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 *ListOptions + cmd := NewCmdList(f, func(opts *ListOptions) 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.EqualError(t, err, tt.wantsErr) + return + } + require.NoError(t, err) + + assert.Equal(t, tt.wants.Limit, gotOpts.Limit) + assert.Equal(t, tt.wants.Owner, gotOpts.Owner) + assert.Equal(t, tt.wants.Visibility, gotOpts.Visibility) + assert.Equal(t, tt.wants.Fork, gotOpts.Fork) + assert.Equal(t, tt.wants.Source, gotOpts.Source) + }) + } +} + func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { io, _, stdout, stderr := iostreams.Test() io.SetStdoutTTY(isTTY) @@ -147,34 +282,3 @@ func TestRepoList_filtering(t *testing.T) { assert.Equal(t, "", output.Stderr()) assert.Equal(t, "\nNo results match your search\n\n", output.String()) } - -func TestRepoList_withInvalidFlagCombinations(t *testing.T) { - tests := []struct { - name string - cli string - wantStderr string - }{ - { - name: "invalid limit", - cli: "--limit 0", - wantStderr: "invalid limit: 0", - }, - { - name: "both private and public", - cli: "--private --public", - wantStderr: "specify only one of `--public` or `--private`", - }, - { - name: "both source and fork", - cli: "--source --fork", - wantStderr: "specify only one of `--source` or `--fork`", - }, - } - - for _, tt := range tests { - httpReg := &httpmock.Registry{} - - _, err := runCommand(httpReg, true, tt.cli) - assert.EqualError(t, err, tt.wantStderr) - } -} From f75144dd1f1d45dfa7b6ec2adf2942f5b06313e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Sat, 27 Feb 2021 15:05:11 +0100 Subject: [PATCH 27/29] Enable pager for `repo list` output --- pkg/cmd/repo/list/list.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go index 62e16a596..158b12ce8 100644 --- a/pkg/cmd/repo/list/list.go +++ b/pkg/cmd/repo/list/list.go @@ -104,6 +104,11 @@ func listRun(opts *ListOptions) error { return err } + if err := opts.IO.StartPager(); err != nil { + fmt.Fprintf(opts.IO.ErrOut, "error starting pager: %v\n", err) + } + defer opts.IO.StopPager() + cs := opts.IO.ColorScheme() tp := utils.NewTablePrinter(opts.IO) now := opts.Now() From 5da8301d5d6490c2082e228398e884db41e52bad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Sat, 27 Feb 2021 16:51:45 +0100 Subject: [PATCH 28/29] Enable filtering `repo list` by coding language --- pkg/cmd/repo/list/fixtures/repoSearch.json | 37 ++++++ pkg/cmd/repo/list/http.go | 100 ++++++++++++++- pkg/cmd/repo/list/http_test.go | 141 +++++++++++++++++++++ pkg/cmd/repo/list/list.go | 21 +-- pkg/cmd/repo/list/list_test.go | 19 +++ 5 files changed, 308 insertions(+), 10 deletions(-) create mode 100644 pkg/cmd/repo/list/fixtures/repoSearch.json create mode 100644 pkg/cmd/repo/list/http_test.go diff --git a/pkg/cmd/repo/list/fixtures/repoSearch.json b/pkg/cmd/repo/list/fixtures/repoSearch.json new file mode 100644 index 000000000..eecae8ac4 --- /dev/null +++ b/pkg/cmd/repo/list/fixtures/repoSearch.json @@ -0,0 +1,37 @@ +{ + "data": { + "search": { + "repositoryCount": 3, + "nodes": [ + { + "nameWithOwner": "octocat/hello-world", + "description": "My first repository", + "isFork": false, + "isPrivate": false, + "isArchived": false, + "pushedAt": "2021-02-19T06:34:58Z" + }, + { + "nameWithOwner": "octocat/cli", + "description": "GitHub CLI", + "isFork": true, + "isPrivate": false, + "isArchived": false, + "pushedAt": "2021-02-19T06:06:06Z" + }, + { + "nameWithOwner": "octocat/testing", + "description": null, + "isFork": false, + "isPrivate": true, + "isArchived": false, + "pushedAt": "2021-02-11T22:32:05Z" + } + ], + "pageInfo": { + "hasNextPage": false, + "endCursor": "" + } + } + } +} diff --git a/pkg/cmd/repo/list/http.go b/pkg/cmd/repo/list/http.go index 97bf584fb..7aefeaf9d 100644 --- a/pkg/cmd/repo/list/http.go +++ b/pkg/cmd/repo/list/http.go @@ -2,6 +2,7 @@ package list import ( "context" + "fmt" "net/http" "reflect" "strings" @@ -45,7 +46,18 @@ type RepositoryList struct { TotalCount int } +type FilterOptions struct { + Visibility string // private, public + Fork bool + Source bool + Language string +} + func listRepos(client *http.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { + if filter.Language != "" { + return searchRepos(client, hostname, limit, owner, filter) + } + perPage := limit if perPage > 100 { perPage = 100 @@ -97,11 +109,11 @@ func listRepos(client *http.Client, hostname string, limit int, owner string, fi }, }) + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) listResult := RepositoryList{} pagination: for { result := reflect.New(query) - gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) err := gql.QueryNamed(context.Background(), "RepositoryList", result.Interface(), variables) if err != nil { return nil, err @@ -126,3 +138,89 @@ pagination: return &listResult, nil } + +func searchRepos(client *http.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { + type query struct { + Search struct { + RepositoryCount int + Nodes []struct { + Repository Repository `graphql:"...on Repository"` + } + PageInfo struct { + HasNextPage bool + EndCursor string + } + } `graphql:"search(type: REPOSITORY, query: $query, first: $perPage, after: $endCursor)"` + } + + perPage := limit + if perPage > 100 { + perPage = 100 + } + + variables := map[string]interface{}{ + "query": githubv4.String(searchQuery(owner, filter)), + "perPage": githubv4.Int(perPage), + "endCursor": (*githubv4.String)(nil), + } + + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) + listResult := RepositoryList{} +pagination: + for { + var result query + err := gql.QueryNamed(context.Background(), "RepositoryListSearch", &result, variables) + if err != nil { + return nil, err + } + + listResult.TotalCount = result.Search.RepositoryCount + for _, node := range result.Search.Nodes { + if listResult.Owner == "" { + idx := strings.IndexRune(node.Repository.NameWithOwner, '/') + listResult.Owner = node.Repository.NameWithOwner[:idx] + } + listResult.Repositories = append(listResult.Repositories, node.Repository) + if len(listResult.Repositories) >= limit { + break pagination + } + } + + if !result.Search.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(result.Search.PageInfo.EndCursor) + } + + return &listResult, nil +} + +func searchQuery(owner string, filter FilterOptions) string { + queryParts := []string{"sort:updated-desc"} + if owner == "" { + queryParts = append(queryParts, "user:@me") + } else { + queryParts = append(queryParts, "user:"+owner) + } + + if filter.Fork { + queryParts = append(queryParts, "fork:only") + } else if filter.Source { + queryParts = append(queryParts, "fork:false") + } else { + queryParts = append(queryParts, "fork:true") + } + + if filter.Language != "" { + queryParts = append(queryParts, fmt.Sprintf("language:%q", filter.Language)) + } + + switch filter.Visibility { + case "public": + queryParts = append(queryParts, "is:public") + case "private": + queryParts = append(queryParts, "is:private") + } + + return strings.Join(queryParts, " ") +} diff --git a/pkg/cmd/repo/list/http_test.go b/pkg/cmd/repo/list/http_test.go new file mode 100644 index 000000000..8c6b8d5f4 --- /dev/null +++ b/pkg/cmd/repo/list/http_test.go @@ -0,0 +1,141 @@ +package list + +import ( + "encoding/json" + "io/ioutil" + "net/http" + "os" + "testing" + + "github.com/cli/cli/pkg/httpmock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_listReposWithLanguage(t *testing.T) { + reg := httpmock.Registry{} + defer reg.Verify(t) + + var searchData struct { + Query string + Variables map[string]interface{} + } + reg.Register( + httpmock.GraphQL(`query RepositoryListSearch\b`), + func(req *http.Request) (*http.Response, error) { + jsonData, err := ioutil.ReadAll(req.Body) + if err != nil { + return nil, err + } + err = json.Unmarshal(jsonData, &searchData) + if err != nil { + return nil, err + } + + respBody, err := os.Open("./fixtures/repoSearch.json") + if err != nil { + return nil, err + } + + return &http.Response{ + StatusCode: 200, + Request: req, + Body: respBody, + }, nil + }, + ) + + client := http.Client{Transport: ®} + res, err := listRepos(&client, "github.com", 10, "", FilterOptions{ + Language: "go", + }) + require.NoError(t, err) + + assert.Equal(t, 3, res.TotalCount) + assert.Equal(t, "octocat", res.Owner) + assert.Equal(t, "octocat/hello-world", res.Repositories[0].NameWithOwner) + + assert.Equal(t, float64(10), searchData.Variables["perPage"]) + assert.Equal(t, `sort:updated-desc user:@me fork:true language:"go"`, searchData.Variables["query"]) +} + +func Test_searchQuery(t *testing.T) { + type args struct { + owner string + filter FilterOptions + } + tests := []struct { + name string + args args + want string + }{ + { + name: "blank", + want: "sort:updated-desc user:@me fork:true", + }, + { + name: "in org", + args: args{ + owner: "cli", + }, + want: "sort:updated-desc user:cli fork:true", + }, + { + name: "only public", + args: args{ + owner: "", + filter: FilterOptions{ + Visibility: "public", + }, + }, + want: "sort:updated-desc user:@me fork:true is:public", + }, + { + name: "only private", + args: args{ + owner: "", + filter: FilterOptions{ + Visibility: "private", + }, + }, + want: "sort:updated-desc user:@me fork:true is:private", + }, + { + name: "only forks", + args: args{ + owner: "", + filter: FilterOptions{ + Fork: true, + }, + }, + want: "sort:updated-desc user:@me fork:only", + }, + { + name: "no forks", + args: args{ + owner: "", + filter: FilterOptions{ + Source: true, + }, + }, + want: "sort:updated-desc user:@me fork:false", + }, + { + name: "with language", + args: args{ + owner: "", + filter: FilterOptions{ + Language: "ruby", + }, + }, + want: "sort:updated-desc user:@me fork:true language:\"ruby\"", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := searchQuery(tt.args.owner, tt.args.filter); got != tt.want { + t.Errorf("searchQuery() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go index 158b12ce8..39b330a04 100644 --- a/pkg/cmd/repo/list/list.go +++ b/pkg/cmd/repo/list/list.go @@ -13,12 +13,6 @@ import ( "github.com/spf13/cobra" ) -type FilterOptions struct { - Visibility string // private, public - Fork bool - Source bool -} - type ListOptions struct { HttpClient func() (*http.Client, error) IO *iostreams.IOStreams @@ -29,6 +23,7 @@ type ListOptions struct { Visibility string Fork bool Source bool + Language string Now func() time.Time } @@ -83,6 +78,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().BoolVar(&flagPublic, "public", false, "Show only public repositories") cmd.Flags().BoolVar(&opts.Source, "source", false, "Show only non-forks") cmd.Flags().BoolVar(&opts.Fork, "fork", false, "Show only forks") + cmd.Flags().StringVarP(&opts.Language, "language", "l", "", "Filter by primary coding language") return cmd } @@ -97,6 +93,7 @@ func listRun(opts *ListOptions) error { Visibility: opts.Visibility, Fork: opts.Fork, Source: opts.Source, + Language: opts.Language, } listResult, err := listRepos(httpClient, ghinstance.OverridableDefault(), opts.Limit, opts.Owner, filter) @@ -132,7 +129,7 @@ func listRun(opts *ListOptions) error { } if opts.IO.IsStdoutTTY() { - hasFilters := filter.Visibility != "" || filter.Fork || filter.Source + hasFilters := filter.Visibility != "" || filter.Fork || filter.Source || filter.Language != "" title := listHeader(listResult.Owner, len(listResult.Repositories), listResult.TotalCount, hasFilters) fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) } @@ -144,9 +141,15 @@ func listHeader(owner string, matchCount, totalMatchCount int, hasFilters bool) if totalMatchCount == 0 { if hasFilters { return "No results match your search" + } else if owner != "" { + return "There are no repositories in @" + owner } - return "There are no repositories in @" + owner + return "No results" } - return fmt.Sprintf("Showing %d of %d repositories in @%s", matchCount, totalMatchCount, owner) + var matchStr string + if hasFilters { + matchStr = " that match your search" + } + return fmt.Sprintf("Showing %d of %d repositories in @%s%s", matchCount, totalMatchCount, owner, matchStr) } diff --git a/pkg/cmd/repo/list/list_test.go b/pkg/cmd/repo/list/list_test.go index 7c7cbf938..86ccdf420 100644 --- a/pkg/cmd/repo/list/list_test.go +++ b/pkg/cmd/repo/list/list_test.go @@ -33,6 +33,7 @@ func TestNewCmdList(t *testing.T) { Visibility: "", Fork: false, Source: false, + Language: "", }, }, { @@ -44,6 +45,7 @@ func TestNewCmdList(t *testing.T) { Visibility: "", Fork: false, Source: false, + Language: "", }, }, { @@ -55,6 +57,7 @@ func TestNewCmdList(t *testing.T) { Visibility: "", Fork: false, Source: false, + Language: "", }, }, { @@ -66,6 +69,7 @@ func TestNewCmdList(t *testing.T) { Visibility: "public", Fork: false, Source: false, + Language: "", }, }, { @@ -77,6 +81,7 @@ func TestNewCmdList(t *testing.T) { Visibility: "private", Fork: false, Source: false, + Language: "", }, }, { @@ -88,6 +93,7 @@ func TestNewCmdList(t *testing.T) { Visibility: "", Fork: true, Source: false, + Language: "", }, }, { @@ -99,6 +105,19 @@ func TestNewCmdList(t *testing.T) { Visibility: "", Fork: false, Source: true, + Language: "", + }, + }, + { + name: "with language", + cli: "-l go", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "", + Fork: false, + Source: false, + Language: "go", }, }, { From e27a77fc99fd7e76c93fc45bdf9ad0901a372dec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Sat, 27 Feb 2021 17:20:06 +0100 Subject: [PATCH 29/29] Add ability to filter by archived in `repo list` Like `--language`, archived filters also use the Search API. --- pkg/cmd/repo/list/http.go | 21 +++-- pkg/cmd/repo/list/http_test.go | 21 +++++ pkg/cmd/repo/list/list.go | 34 +++++--- pkg/cmd/repo/list/list_test.go | 152 ++++++++++++++++++++++----------- 4 files changed, 164 insertions(+), 64 deletions(-) diff --git a/pkg/cmd/repo/list/http.go b/pkg/cmd/repo/list/http.go index 7aefeaf9d..7e2ccecd2 100644 --- a/pkg/cmd/repo/list/http.go +++ b/pkg/cmd/repo/list/http.go @@ -44,17 +44,20 @@ type RepositoryList struct { Owner string Repositories []Repository TotalCount int + FromSearch bool } type FilterOptions struct { - Visibility string // private, public - Fork bool - Source bool - Language string + Visibility string // private, public + Fork bool + Source bool + Language string + Archived bool + NonArchived bool } func listRepos(client *http.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { - if filter.Language != "" { + if filter.Language != "" || filter.Archived || filter.NonArchived { return searchRepos(client, hostname, limit, owner, filter) } @@ -165,7 +168,7 @@ func searchRepos(client *http.Client, hostname string, limit int, owner string, } gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) - listResult := RepositoryList{} + listResult := RepositoryList{FromSearch: true} pagination: for { var result query @@ -222,5 +225,11 @@ func searchQuery(owner string, filter FilterOptions) string { queryParts = append(queryParts, "is:private") } + if filter.Archived { + queryParts = append(queryParts, "archived:true") + } else if filter.NonArchived { + queryParts = append(queryParts, "archived:false") + } + return strings.Join(queryParts, " ") } diff --git a/pkg/cmd/repo/list/http_test.go b/pkg/cmd/repo/list/http_test.go index 8c6b8d5f4..0544a750b 100644 --- a/pkg/cmd/repo/list/http_test.go +++ b/pkg/cmd/repo/list/http_test.go @@ -52,6 +52,7 @@ func Test_listReposWithLanguage(t *testing.T) { require.NoError(t, err) assert.Equal(t, 3, res.TotalCount) + assert.Equal(t, true, res.FromSearch) assert.Equal(t, "octocat", res.Owner) assert.Equal(t, "octocat/hello-world", res.Repositories[0].NameWithOwner) @@ -130,6 +131,26 @@ func Test_searchQuery(t *testing.T) { }, want: "sort:updated-desc user:@me fork:true language:\"ruby\"", }, + { + name: "only archived", + args: args{ + owner: "", + filter: FilterOptions{ + Archived: true, + }, + }, + want: "sort:updated-desc user:@me fork:true archived:true", + }, + { + name: "only non-archived", + args: args{ + owner: "", + filter: FilterOptions{ + NonArchived: true, + }, + }, + want: "sort:updated-desc user:@me fork:true archived:false", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go index 39b330a04..0b0b5fd5e 100644 --- a/pkg/cmd/repo/list/list.go +++ b/pkg/cmd/repo/list/list.go @@ -20,10 +20,12 @@ type ListOptions struct { Limit int Owner string - Visibility string - Fork bool - Source bool - Language string + Visibility string + Fork bool + Source bool + Language string + Archived bool + NonArchived bool Now func() time.Time } @@ -55,6 +57,9 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman if opts.Source && opts.Fork { return &cmdutil.FlagError{Err: fmt.Errorf("specify only one of `--source` or `--fork`")} } + if opts.Archived && opts.NonArchived { + return &cmdutil.FlagError{Err: fmt.Errorf("specify only one of `--archived` or `--no-archived`")} + } if flagPrivate { opts.Visibility = "private" @@ -79,6 +84,8 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().BoolVar(&opts.Source, "source", false, "Show only non-forks") cmd.Flags().BoolVar(&opts.Fork, "fork", false, "Show only forks") cmd.Flags().StringVarP(&opts.Language, "language", "l", "", "Filter by primary coding language") + cmd.Flags().BoolVar(&opts.Archived, "archived", false, "Show only archived repositories") + cmd.Flags().BoolVar(&opts.NonArchived, "no-archived", false, "Omit archived repositories") return cmd } @@ -90,10 +97,12 @@ func listRun(opts *ListOptions) error { } filter := FilterOptions{ - Visibility: opts.Visibility, - Fork: opts.Fork, - Source: opts.Source, - Language: opts.Language, + Visibility: opts.Visibility, + Fork: opts.Fork, + Source: opts.Source, + Language: opts.Language, + Archived: opts.Archived, + NonArchived: opts.NonArchived, } listResult, err := listRepos(httpClient, ghinstance.OverridableDefault(), opts.Limit, opts.Owner, filter) @@ -117,13 +126,18 @@ func listRun(opts *ListOptions) error { infoColor = cs.Yellow } + t := repo.PushedAt + // if listResult.FromSearch { + // t = repo.UpdatedAt + // } + tp.AddField(repo.NameWithOwner, nil, cs.Bold) tp.AddField(text.ReplaceExcessiveWhitespace(repo.Description), nil, nil) tp.AddField(info, nil, infoColor) if tp.IsTTY() { - tp.AddField(utils.FuzzyAgoAbbr(now, repo.PushedAt), nil, cs.Gray) + tp.AddField(utils.FuzzyAgoAbbr(now, t), nil, cs.Gray) } else { - tp.AddField(repo.PushedAt.Format(time.RFC3339), nil, nil) + tp.AddField(t.Format(time.RFC3339), nil, nil) } tp.EndRow() } diff --git a/pkg/cmd/repo/list/list_test.go b/pkg/cmd/repo/list/list_test.go index 86ccdf420..59552beda 100644 --- a/pkg/cmd/repo/list/list_test.go +++ b/pkg/cmd/repo/list/list_test.go @@ -28,96 +28,140 @@ func TestNewCmdList(t *testing.T) { name: "no arguments", cli: "", wants: ListOptions{ - Limit: 30, - Owner: "", - Visibility: "", - Fork: false, - Source: false, - Language: "", + Limit: 30, + Owner: "", + Visibility: "", + Fork: false, + Source: false, + Language: "", + Archived: false, + NonArchived: false, }, }, { name: "with owner", cli: "monalisa", wants: ListOptions{ - Limit: 30, - Owner: "monalisa", - Visibility: "", - Fork: false, - Source: false, - Language: "", + Limit: 30, + Owner: "monalisa", + Visibility: "", + Fork: false, + Source: false, + Language: "", + Archived: false, + NonArchived: false, }, }, { name: "with limit", cli: "-L 101", wants: ListOptions{ - Limit: 101, - Owner: "", - Visibility: "", - Fork: false, - Source: false, - Language: "", + Limit: 101, + Owner: "", + Visibility: "", + Fork: false, + Source: false, + Language: "", + Archived: false, + NonArchived: false, }, }, { name: "only public", cli: "--public", wants: ListOptions{ - Limit: 30, - Owner: "", - Visibility: "public", - Fork: false, - Source: false, - Language: "", + Limit: 30, + Owner: "", + Visibility: "public", + Fork: false, + Source: false, + Language: "", + Archived: false, + NonArchived: false, }, }, { name: "only private", cli: "--private", wants: ListOptions{ - Limit: 30, - Owner: "", - Visibility: "private", - Fork: false, - Source: false, - Language: "", + Limit: 30, + Owner: "", + Visibility: "private", + Fork: false, + Source: false, + Language: "", + Archived: false, + NonArchived: false, }, }, { name: "only forks", cli: "--fork", wants: ListOptions{ - Limit: 30, - Owner: "", - Visibility: "", - Fork: true, - Source: false, - Language: "", + Limit: 30, + Owner: "", + Visibility: "", + Fork: true, + Source: false, + Language: "", + Archived: false, + NonArchived: false, }, }, { name: "only sources", cli: "--source", wants: ListOptions{ - Limit: 30, - Owner: "", - Visibility: "", - Fork: false, - Source: true, - Language: "", + Limit: 30, + Owner: "", + Visibility: "", + Fork: false, + Source: true, + Language: "", + Archived: false, + NonArchived: false, }, }, { name: "with language", cli: "-l go", wants: ListOptions{ - Limit: 30, - Owner: "", - Visibility: "", - Fork: false, - Source: false, - Language: "go", + Limit: 30, + Owner: "", + Visibility: "", + Fork: false, + Source: false, + Language: "go", + Archived: false, + NonArchived: false, + }, + }, + { + name: "only archived", + cli: "--archived", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "", + Fork: false, + Source: false, + Language: "", + Archived: true, + NonArchived: false, + }, + }, + { + name: "only non-archived", + cli: "--no-archived", + wants: ListOptions{ + Limit: 30, + Owner: "", + Visibility: "", + Fork: false, + Source: false, + Language: "", + Archived: false, + NonArchived: true, }, }, { @@ -130,11 +174,21 @@ func TestNewCmdList(t *testing.T) { cli: "--fork --source", wantsErr: "specify only one of `--source` or `--fork`", }, + { + name: "conflicting archived", + cli: "--archived --no-archived", + wantsErr: "specify only one of `--archived` or `--no-archived`", + }, { name: "too many arguments", cli: "monalisa hubot", wantsErr: "accepts at most 1 arg(s), received 2", }, + { + name: "invalid limit", + cli: "-L 0", + wantsErr: "invalid limit: 0", + }, } for _, tt := range tests { @@ -166,6 +220,8 @@ func TestNewCmdList(t *testing.T) { assert.Equal(t, tt.wants.Visibility, gotOpts.Visibility) assert.Equal(t, tt.wants.Fork, gotOpts.Fork) assert.Equal(t, tt.wants.Source, gotOpts.Source) + assert.Equal(t, tt.wants.Archived, gotOpts.Archived) + assert.Equal(t, tt.wants.NonArchived, gotOpts.NonArchived) }) } }