From 9a149d7694b7ce767dcd84f9dfa91e0c89b4d0ca Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Thu, 11 Feb 2021 19:43:08 -0300 Subject: [PATCH 01/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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/38] 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 9bf1668b3f83997f6fdecf1410bf0f586c3c11f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 19 Feb 2021 15:37:11 +0100 Subject: [PATCH 13/38] Fix `auth git-credential` when the token comes from environment When a token such as GH_TOKEN is set through environment variables and `~/.config/gh/hosts.yml` is non-existent, the `auth git-credential get` command used to fail due to missing username. Since GitHub username isn't at all required for token authentication, use the `x-access-token` faux username instead of trying to obtain one from a config file. --- pkg/cmd/auth/gitcredential/helper.go | 16 ++++++++--- pkg/cmd/auth/gitcredential/helper_test.go | 34 +++++++++++++++++++++-- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/auth/gitcredential/helper.go b/pkg/cmd/auth/gitcredential/helper.go index c62cef3a8..4f80afbe7 100644 --- a/pkg/cmd/auth/gitcredential/helper.go +++ b/pkg/cmd/auth/gitcredential/helper.go @@ -11,8 +11,10 @@ import ( "github.com/spf13/cobra" ) +const tokenUser = "x-access-token" + type config interface { - Get(string, string) (string, error) + GetWithSource(string, string) (string, string, error) } type CredentialOptions struct { @@ -98,13 +100,19 @@ func helperRun(opts *CredentialOptions) error { return err } - gotUser, _ := cfg.Get(wants["host"], "user") - gotToken, _ := cfg.Get(wants["host"], "oauth_token") + var gotUser string + gotToken, source, _ := cfg.GetWithSource(wants["host"], "oauth_token") + if strings.HasSuffix(source, "_TOKEN") { + gotUser = tokenUser + } else { + gotUser, _, _ = cfg.GetWithSource(wants["host"], "user") + } + if gotUser == "" || gotToken == "" { return cmdutil.SilentError } - if wants["username"] != "" && !strings.EqualFold(wants["username"], gotUser) { + if wants["username"] != "" && gotUser != tokenUser && !strings.EqualFold(wants["username"], gotUser) { return cmdutil.SilentError } diff --git a/pkg/cmd/auth/gitcredential/helper_test.go b/pkg/cmd/auth/gitcredential/helper_test.go index 336d4ef34..c8f449526 100644 --- a/pkg/cmd/auth/gitcredential/helper_test.go +++ b/pkg/cmd/auth/gitcredential/helper_test.go @@ -10,8 +10,8 @@ import ( type tinyConfig map[string]string -func (c tinyConfig) Get(host, key string) (string, error) { - return c[fmt.Sprintf("%s:%s", host, key)], nil +func (c tinyConfig) GetWithSource(host, key string) (string, string, error) { + return c[fmt.Sprintf("%s:%s", host, key)], c["_source"], nil } func Test_helperRun(t *testing.T) { @@ -29,6 +29,7 @@ func Test_helperRun(t *testing.T) { Operation: "get", Config: func() (config, error) { return tinyConfig{ + "_source": "/Users/monalisa/.config/gh/hosts.yml", "example.com:user": "monalisa", "example.com:oauth_token": "OTOKEN", }, nil @@ -53,6 +54,7 @@ func Test_helperRun(t *testing.T) { Operation: "get", Config: func() (config, error) { return tinyConfig{ + "_source": "/Users/monalisa/.config/gh/hosts.yml", "example.com:user": "monalisa", "example.com:oauth_token": "OTOKEN", }, nil @@ -78,6 +80,7 @@ func Test_helperRun(t *testing.T) { Operation: "get", Config: func() (config, error) { return tinyConfig{ + "_source": "/Users/monalisa/.config/gh/hosts.yml", "example.com:user": "monalisa", "example.com:oauth_token": "OTOKEN", }, nil @@ -101,6 +104,7 @@ func Test_helperRun(t *testing.T) { Operation: "get", Config: func() (config, error) { return tinyConfig{ + "_source": "/Users/monalisa/.config/gh/hosts.yml", "example.com:user": "monalisa", }, nil }, @@ -119,6 +123,7 @@ func Test_helperRun(t *testing.T) { Operation: "get", Config: func() (config, error) { return tinyConfig{ + "_source": "/Users/monalisa/.config/gh/hosts.yml", "example.com:user": "monalisa", "example.com:oauth_token": "OTOKEN", }, nil @@ -133,6 +138,31 @@ func Test_helperRun(t *testing.T) { wantStdout: "", wantStderr: "", }, + { + name: "token from env", + opts: CredentialOptions{ + Operation: "get", + Config: func() (config, error) { + return tinyConfig{ + "_source": "GITHUB_ENTERPRISE_TOKEN", + "example.com:oauth_token": "OTOKEN", + }, nil + }, + }, + input: heredoc.Doc(` + protocol=https + host=example.com + username=hubot + `), + wantErr: false, + wantStdout: heredoc.Doc(` + protocol=https + host=example.com + username=x-access-token + password=OTOKEN + `), + wantStderr: "", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 2284ef43d079c1c5ae280848e098f04e6ee1ddf3 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Fri, 19 Feb 2021 17:34:17 -0300 Subject: [PATCH 14/38] 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 f807795491e90954e26873e39d2f90ec1f07bf33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 23 Feb 2021 10:19:11 +0100 Subject: [PATCH 15/38] Fix pasting Personal Access Token to `auth login` for GHE --- pkg/cmd/auth/shared/oauth_scopes.go | 2 +- pkg/cmd/auth/shared/oauth_scopes_test.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/auth/shared/oauth_scopes.go b/pkg/cmd/auth/shared/oauth_scopes.go index c99b5543d..20dbf9a83 100644 --- a/pkg/cmd/auth/shared/oauth_scopes.go +++ b/pkg/cmd/auth/shared/oauth_scopes.go @@ -40,7 +40,7 @@ func HasMinimumScopes(httpClient httpClient, hostname, authToken string) error { return err } - req.Header.Set("Autorization", "token "+authToken) + req.Header.Set("Authorization", "token "+authToken) res, err := httpClient.Do(req) if err != nil { diff --git a/pkg/cmd/auth/shared/oauth_scopes_test.go b/pkg/cmd/auth/shared/oauth_scopes_test.go index 3b9766a95..0f6bd9f32 100644 --- a/pkg/cmd/auth/shared/oauth_scopes_test.go +++ b/pkg/cmd/auth/shared/oauth_scopes_test.go @@ -52,7 +52,9 @@ func Test_HasMinimumScopes(t *testing.T) { fakehttp := &httpmock.Registry{} defer fakehttp.Verify(t) + var gotAuthorization string fakehttp.Register(httpmock.REST("GET", ""), func(req *http.Request) (*http.Response, error) { + gotAuthorization = req.Header.Get("authorization") return &http.Response{ Request: req, StatusCode: 200, @@ -70,6 +72,7 @@ func Test_HasMinimumScopes(t *testing.T) { } else { assert.NoError(t, err) } + assert.Equal(t, gotAuthorization, "token ATOKEN") }) } From cfddda8829cd205b88a1e854b6fca76d6acc34b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 23 Feb 2021 10:52:29 +0100 Subject: [PATCH 16/38] Indicate `workflow` scope is GHE 3.0+ only during `auth login` --- pkg/cmd/auth/shared/login_flow.go | 10 +++-- pkg/cmd/auth/shared/login_flow_test.go | 52 ++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/auth/shared/login_flow.go b/pkg/cmd/auth/shared/login_flow.go index fbff9882e..6f75a784f 100644 --- a/pkg/cmd/auth/shared/login_flow.go +++ b/pkg/cmd/auth/shared/login_flow.go @@ -9,6 +9,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/internal/authflow" + "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/prompt" ) @@ -125,7 +126,7 @@ func Login(opts *LoginOptions) error { fmt.Fprint(opts.IO.ErrOut, heredoc.Docf(` Tip: you can generate a Personal Access Token here https://%s/settings/tokens The minimum required scopes are %s. - `, hostname, scopesSentence(minimumScopes))) + `, hostname, scopesSentence(minimumScopes, ghinstance.IsEnterprise(hostname)))) err := prompt.SurveyAskOne(&survey.Password{ Message: "Paste your authentication token:", @@ -193,11 +194,14 @@ func Login(opts *LoginOptions) error { return nil } -func scopesSentence(scopes []string) string { +func scopesSentence(scopes []string, isEnterprise bool) string { quoted := make([]string, len(scopes)) for i, s := range scopes { quoted[i] = fmt.Sprintf("'%s'", s) + if s == "workflow" && isEnterprise { + // remove when GHE 2.x reaches EOL + quoted[i] += " (GHE 3.0+)" + } } - // TODO: insert an "and" before the last element return strings.Join(quoted, ", ") } diff --git a/pkg/cmd/auth/shared/login_flow_test.go b/pkg/cmd/auth/shared/login_flow_test.go index c37d89b46..521616dfa 100644 --- a/pkg/cmd/auth/shared/login_flow_test.go +++ b/pkg/cmd/auth/shared/login_flow_test.go @@ -101,3 +101,55 @@ func TestLogin_ssh(t *testing.T) { assert.Equal(t, "ATOKEN", cfg["example.com:oauth_token"]) assert.Equal(t, "ssh", cfg["example.com:git_protocol"]) } + +func Test_scopesSentence(t *testing.T) { + type args struct { + scopes []string + isEnterprise bool + } + tests := []struct { + name string + args args + want string + }{ + { + name: "basic scopes", + args: args{ + scopes: []string{"repo", "read:org"}, + isEnterprise: false, + }, + want: "'repo', 'read:org'", + }, + { + name: "empty", + args: args{ + scopes: []string(nil), + isEnterprise: false, + }, + want: "", + }, + { + name: "workflow scope for dotcom", + args: args{ + scopes: []string{"repo", "workflow"}, + isEnterprise: false, + }, + want: "'repo', 'workflow'", + }, + { + name: "workflow scope for GHE", + args: args{ + scopes: []string{"repo", "workflow"}, + isEnterprise: true, + }, + want: "'repo', 'workflow' (GHE 3.0+)", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := scopesSentence(tt.args.scopes, tt.args.isEnterprise); got != tt.want { + t.Errorf("scopesSentence() = %q, want %q", got, tt.want) + } + }) + } +} From 27aea42d8a43951b880145fcbbd42b262ed6af08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 23 Feb 2021 12:10:56 +0100 Subject: [PATCH 17/38] Avoid upgrade notice for recent release if gh is under Homebrew prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before, when gh detected there was a new release in the `cli/cli` repo, it would show this notice: A new release of gh is available: {V1} → {V2} Additionally, when the release was more than 24h old, we would show this to Homebrew users: To upgrade, run: brew update && brew upgrade gh Ref. feb4acc2c00ce42d5ed67252ae1ec66addeb786d This change makes it so that the original notice "A new release of gh is available" is NOT shown to Homebrew users unless the release is older than 24h. We effectively hide the fact that any release happened until we're sure that the version bump has made it to `homebrew-core`. --- cmd/gh/main.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index a0eba09a5..e20452808 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -163,12 +163,19 @@ func main() { newRelease := <-updateMessageChan if newRelease != nil { - ghExe, _ := os.Executable() + isHomebrew := false + if ghExe, err := os.Executable(); err == nil { + isHomebrew = isUnderHomebrew(ghExe) + } + if isHomebrew && isRecentRelease(newRelease.PublishedAt) { + // do not notify Homebrew users before the version bump had a chance to get merged into homebrew-core + return + } fmt.Fprintf(stderr, "\n\n%s %s → %s\n", ansi.Color("A new release of gh is available:", "yellow"), ansi.Color(buildVersion, "cyan"), ansi.Color(newRelease.Version, "cyan")) - if suggestBrewUpgrade(newRelease, ghExe) { + if isHomebrew { fmt.Fprintf(stderr, "To upgrade, run: %s\n", "brew update && brew upgrade gh") } fmt.Fprintf(stderr, "%s\n\n", @@ -265,13 +272,12 @@ func apiVerboseLog() api.ClientOption { return api.VerboseLog(colorable.NewColorable(os.Stderr), logTraffic, colorize) } -// Suggest to `brew upgrade gh` only if gh was found under homebrew prefix and when the release was -// published over 24h ago, allowing homebrew-core ample time to merge the formula bump. -func suggestBrewUpgrade(rel *update.ReleaseInfo, ghBinary string) bool { - if rel.PublishedAt.IsZero() || time.Since(rel.PublishedAt) < time.Duration(time.Hour*24) { - return false - } +func isRecentRelease(publishedAt time.Time) bool { + return !publishedAt.IsZero() && time.Since(publishedAt) < time.Hour*24 +} +// Check whether the gh binary was found under the Homebrew prefix +func isUnderHomebrew(ghBinary string) bool { brewExe, err := safeexec.LookPath("brew") if err != nil { return false From cbf8a0d9641869de104ad1c94493e07addcd45c9 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Tue, 23 Feb 2021 20:12:26 +0530 Subject: [PATCH 18/38] Accept only one argument when deleting a gist --- pkg/cmd/gist/delete/delete.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/delete/delete.go b/pkg/cmd/gist/delete/delete.go index 54f745030..add7e3335 100644 --- a/pkg/cmd/gist/delete/delete.go +++ b/pkg/cmd/gist/delete/delete.go @@ -29,7 +29,7 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co cmd := &cobra.Command{ Use: "delete { | }", Short: "Delete a gist", - Args: cmdutil.MinimumArgs(1, "cannot delete: gist argument required"), + Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { opts.Selector = args[0] if runF != nil { From 732e919a835882700262b0a448c1150faa24d1bf Mon Sep 17 00:00:00 2001 From: boonhong Date: Tue, 23 Feb 2021 22:51:10 +0800 Subject: [PATCH 19/38] Add `pr edit --base` to change the base branch of a PR --- pkg/cmd/pr/edit/edit.go | 8 ++++++++ pkg/cmd/pr/edit/edit_test.go | 22 ++++++++++++++++++++++ pkg/cmd/pr/shared/editable.go | 2 ++ 3 files changed, 32 insertions(+) diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 6c1592b4c..3fc1c8f10 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -70,6 +70,9 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman if flags.Changed("body") { opts.Editable.Body.Edited = true } + if flags.Changed("base") { + opts.Editable.Base.Edited = true + } if flags.Changed("add-reviewer") || flags.Changed("remove-reviewer") { opts.Editable.Reviewers.Edited = true } @@ -104,6 +107,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman cmd.Flags().StringVarP(&opts.Editable.Title.Value, "title", "t", "", "Set the new title.") cmd.Flags().StringVarP(&opts.Editable.Body.Value, "body", "b", "", "Set the new body.") + cmd.Flags().StringVarP(&opts.Editable.Base.Value, "base", "B", "", "Change the base `branch` for this pull request") cmd.Flags().StringSliceVar(&opts.Editable.Reviewers.Add, "add-reviewer", nil, "Add reviewers by their `login`.") cmd.Flags().StringSliceVar(&opts.Editable.Reviewers.Remove, "remove-reviewer", nil, "Remove reviewers by their `login`.") cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Add, "add-assignee", nil, "Add assigned users by their `login`. Use \"@me\" to assign yourself.") @@ -133,6 +137,7 @@ func editRun(opts *EditOptions) error { editable.Reviewers.Allowed = true editable.Title.Default = pr.Title editable.Body.Default = pr.Body + editable.Base.Default = pr.BaseRefName editable.Reviewers.Default = pr.ReviewRequests.Logins() editable.Assignees.Default = pr.Assignees.Logins() editable.Labels.Default = pr.Labels.Names() @@ -203,6 +208,9 @@ func updatePullRequest(client *api.Client, repo ghrepo.Interface, id string, edi return err } params.MilestoneID = ghId(milestoneId) + if editable.Base.Edited { + params.BaseRefName = ghString(&editable.Base.Value) + } err = api.UpdatePullRequest(client, repo, params) if err != nil { return err diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 35d8278dc..da328ba5b 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -65,6 +65,20 @@ func TestNewCmdEdit(t *testing.T) { }, wantsErr: false, }, + { + name: "base flag", + input: "23 --base base-branch-name", + output: EditOptions{ + SelectorArg: "23", + Editable: shared.Editable{ + Base: shared.EditableString{ + Value: "base-branch-name", + Edited: true, + }, + }, + }, + wantsErr: false, + }, { name: "add-reviewer flag", input: "23 --add-reviewer monalisa,owner/core", @@ -254,6 +268,10 @@ func Test_editRun(t *testing.T) { Value: "new body", Edited: true, }, + Base: shared.EditableString{ + Value: "base-branch-name", + Edited: true, + }, Reviewers: shared.EditableSlice{ Add: []string{"OWNER/core", "OWNER/external", "monalisa", "hubot"}, Remove: []string{"dependabot"}, @@ -303,6 +321,10 @@ func Test_editRun(t *testing.T) { Value: "new body", Edited: true, }, + Base: shared.EditableString{ + Value: "base-branch-name", + Edited: true, + }, Assignees: shared.EditableSlice{ Add: []string{"monalisa", "hubot"}, Remove: []string{"octocat"}, diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 16faca4de..abcfc9b43 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -14,6 +14,7 @@ import ( type Editable struct { Title EditableString Body EditableString + Base EditableString Reviewers EditableSlice Assignees EditableSlice Labels EditableSlice @@ -42,6 +43,7 @@ type EditableSlice struct { func (e Editable) Dirty() bool { return e.Title.Edited || e.Body.Edited || + e.Base.Edited || e.Reviewers.Edited || e.Assignees.Edited || e.Labels.Edited || From 9d062ed8fcdd5845e55ebfe364a41c3524a09fc3 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Tue, 23 Feb 2021 09:17:35 -0800 Subject: [PATCH 20/38] 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 21/38] 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 22/38] 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 23/38] 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 24/38] 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 56ead91702e61575622233713ccf247af4644bcb Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 24 Feb 2021 15:49:40 +0530 Subject: [PATCH 25/38] Add helper function to validate exact args in cmdutil --- pkg/cmd/gist/edit/edit.go | 2 +- pkg/cmd/gist/view/view.go | 2 +- pkg/cmd/pr/checkout/checkout.go | 2 +- pkg/cmdutil/args.go | 18 ++++++++++++++++++ 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 7cba14800..a8fa4d683 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -49,7 +49,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "edit { | }", Short: "Edit one of your gists", - Args: cmdutil.MinimumArgs(1, "cannot edit: gist argument required"), + Args: cmdutil.ExactArgs(1, "cannot edit: gist argument required"), RunE: func(c *cobra.Command, args []string) error { opts.Selector = args[0] diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index 70bb0a238..e1e9408cf 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -35,7 +35,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "view { | }", Short: "View a gist", - Args: cmdutil.MinimumArgs(1, "cannot view: gist argument required"), + Args: cmdutil.ExactArgs(1, "cannot view: gist argument required"), RunE: func(cmd *cobra.Command, args []string) error { opts.Selector = args[0] diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index d2cd84304..f7f73bb28 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -46,7 +46,7 @@ func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobr cmd := &cobra.Command{ Use: "checkout { | | }", Short: "Check out a pull request in git", - Args: cmdutil.MinimumArgs(1, "argument required"), + Args: cmdutil.ExactArgs(1, "argument required"), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo diff --git a/pkg/cmdutil/args.go b/pkg/cmdutil/args.go index 65f3ade51..d58757c20 100644 --- a/pkg/cmdutil/args.go +++ b/pkg/cmdutil/args.go @@ -21,6 +21,24 @@ func MinimumArgs(n int, msg string) cobra.PositionalArgs { } } +func ExactArgs(n int, msg string) cobra.PositionalArgs { + if msg == "" { + return cobra.MinimumNArgs(1) + } + + return func(cmd *cobra.Command, args []string) error { + if len(args) > n { + return &FlagError{Err: errors.New("too many arguments")} + } + + if len(args) < n { + return &FlagError{Err: errors.New("not enough arguments")} + } + + return nil + } +} + func NoArgsQuoteReminder(cmd *cobra.Command, args []string) error { if len(args) < 1 { return nil From 61eb7eeeab3f346ff7a4c61a79d5b0341cc71cff Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 24 Feb 2021 15:53:07 +0530 Subject: [PATCH 26/38] Add msg in gist delete --- pkg/cmd/gist/delete/delete.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/delete/delete.go b/pkg/cmd/gist/delete/delete.go index add7e3335..674c33ad6 100644 --- a/pkg/cmd/gist/delete/delete.go +++ b/pkg/cmd/gist/delete/delete.go @@ -29,7 +29,7 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co cmd := &cobra.Command{ Use: "delete { | }", Short: "Delete a gist", - Args: cobra.ExactArgs(1), + Args: cmdutil.ExactArgs(1, "cannot delete: gist argument required"), RunE: func(c *cobra.Command, args []string) error { opts.Selector = args[0] if runF != nil { From 66d4307bce0d0189c1cb4650b432852cde39f328 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Wed, 24 Feb 2021 18:05:11 +0530 Subject: [PATCH 27/38] return msg instead of too many arguments --- pkg/cmdutil/args.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/cmdutil/args.go b/pkg/cmdutil/args.go index d58757c20..ee9c5e350 100644 --- a/pkg/cmdutil/args.go +++ b/pkg/cmdutil/args.go @@ -22,9 +22,6 @@ func MinimumArgs(n int, msg string) cobra.PositionalArgs { } func ExactArgs(n int, msg string) cobra.PositionalArgs { - if msg == "" { - return cobra.MinimumNArgs(1) - } return func(cmd *cobra.Command, args []string) error { if len(args) > n { @@ -32,7 +29,7 @@ func ExactArgs(n int, msg string) cobra.PositionalArgs { } if len(args) < n { - return &FlagError{Err: errors.New("not enough arguments")} + return &FlagError{Err: errors.New(msg)} } return nil 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 28/38] 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 29/38] 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 30/38] 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 31/38] :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 32/38] 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 33/38] 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 34/38] 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 35/38] 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 36/38] 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 37/38] 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 38/38] 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) }) } }