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 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 000000000..65d675154 Binary files /dev/null and b/git/fixtures/simple.git/index differ diff --git a/git/fixtures/simple.git/logs/HEAD b/git/fixtures/simple.git/logs/HEAD new file mode 100644 index 000000000..216887f9e --- /dev/null +++ b/git/fixtures/simple.git/logs/HEAD @@ -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/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 000000000..adf64119a Binary files /dev/null and b/git/fixtures/simple.git/objects/4b/825dc642cb6eb9a060e54bf8d69288fbee4904 differ diff --git a/git/fixtures/simple.git/objects/6f/1a2405cace1633d89a79c74c65f22fe78f9659 b/git/fixtures/simple.git/objects/6f/1a2405cace1633d89a79c74c65f22fe78f9659 new file mode 100644 index 000000000..8f2fded03 Binary files /dev/null and b/git/fixtures/simple.git/objects/6f/1a2405cace1633d89a79c74c65f22fe78f9659 differ 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.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/git/git_test.go b/git/git_test.go index 899ca3b49..685b4d1c0 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -1,12 +1,53 @@ 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) + } +} + +/* + 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 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) { 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) + } + }) + } +} 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") }) } diff --git a/pkg/cmd/gist/delete/delete.go b/pkg/cmd/gist/delete/delete.go index 54f745030..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: cmdutil.MinimumArgs(1, "cannot delete: gist argument required"), + Args: cmdutil.ExactArgs(1, "cannot delete: gist argument required"), RunE: func(c *cobra.Command, args []string) error { opts.Selector = args[0] if runF != nil { diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 7cba14800..27df4a104 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -5,7 +5,9 @@ import ( "encoding/json" "errors" "fmt" + "io/ioutil" "net/http" + "path/filepath" "sort" "strings" @@ -28,8 +30,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 { @@ -49,7 +52,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] @@ -60,7 +63,9 @@ 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 new file to the gist") + cmd.Flags().StringVarP(&opts.EditFilename, "filename", "f", "", "Select a file to edit") return cmd } @@ -85,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 } @@ -97,10 +105,20 @@ func editRun(opts *EditOptions) error { return fmt.Errorf("You do not own this gist.") } + if opts.AddFilename != "" { + files, err := getFilesToAdd(opts.AddFilename) + if err != nil { + return err + } + + gist.Files = files + return updateGist(apiClient, ghinstance.OverridableDefault(), gist) + } + filesToUpdate := map[string]string{} for { - filename := opts.Filename + filename := opts.EditFilename candidates := []string{} for filename := range gist.Files { candidates = append(candidates, filename) @@ -222,3 +240,22 @@ func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error return nil } + +func getFilesToAdd(file string) (map[string]*shared.GistFile, error) { + content, err := ioutil.ReadFile(file) + if err != nil { + return nil, fmt.Errorf("failed to read file %s: %w", file, err) + } + + if len(content) == 0 { + return nil, errors.New("file contents cannot be empty") + } + + 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 b9b711a38..73a53e523 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" @@ -15,8 +16,26 @@ import ( "github.com/cli/cli/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func Test_getFilesToAdd(t *testing.T) { + 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{ + filename: { + Filename: filename, + Content: "hello", + }, + }, gf) +} + func TestNewCmdEdit(t *testing.T) { tests := []struct { name string @@ -34,8 +53,16 @@ func TestNewCmdEdit(t *testing.T) { name: "filename", cli: "123 --filename cool.md", wants: EditOptions{ - Selector: "123", - Filename: "cool.md", + Selector: "123", + EditFilename: "cool.md", + }, + }, + { + name: "add", + cli: "123 --add cool.md", + wants: EditOptions{ + Selector: "123", + AddFilename: "cool.md", }, }, } @@ -60,13 +87,18 @@ 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) }) } } 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 @@ -74,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", @@ -163,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{ @@ -208,8 +239,28 @@ 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", + 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: fileToAdd, + }, }, } @@ -246,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 @@ -260,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) @@ -278,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 } 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/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..7319b55b4 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") { @@ -70,6 +72,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 +109,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 +139,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 +210,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..ae1cc72c8 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, }, @@ -65,6 +74,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 +277,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 +330,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/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() 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 || diff --git a/pkg/cmd/repo/list/fixtures/repoList.json b/pkg/cmd/repo/list/fixtures/repoList.json new file mode 100644 index 000000000..18bb5eff8 --- /dev/null +++ b/pkg/cmd/repo/list/fixtures/repoList.json @@ -0,0 +1,40 @@ +{ + "data": { + "repositoryOwner": { + "login": "octocat", + "repositories": { + "totalCount": 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/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 new file mode 100644 index 000000000..7e2ccecd2 --- /dev/null +++ b/pkg/cmd/repo/list/http.go @@ -0,0 +1,235 @@ +package list + +import ( + "context" + "fmt" + "net/http" + "reflect" + "strings" + "time" + + "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 { + Owner string + Repositories []Repository + TotalCount int + FromSearch bool +} + +type FilterOptions struct { + 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 != "" || filter.Archived || filter.NonArchived { + return searchRepos(client, hostname, limit, owner, filter) + } + + perPage := limit + if perPage > 100 { + perPage = 100 + } + + variables := map[string]interface{}{ + "perPage": githubv4.Int(perPage), + "endCursor": (*githubv4.String)(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) + } + + 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), + }, + }) + + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) + listResult := RepositoryList{} +pagination: + for { + result := reflect.New(query) + err := gql.QueryNamed(context.Background(), "RepositoryList", result.Interface(), variables) + if err != nil { + return nil, err + } + + 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 !owner.Repositories.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(owner.Repositories.PageInfo.EndCursor) + } + + 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{FromSearch: true} +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") + } + + 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 new file mode 100644 index 000000000..0544a750b --- /dev/null +++ b/pkg/cmd/repo/list/http_test.go @@ -0,0 +1,162 @@ +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, true, res.FromSearch) + 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\"", + }, + { + 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) { + 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 new file mode 100644 index 000000000..0b0b5fd5e --- /dev/null +++ b/pkg/cmd/repo/list/list.go @@ -0,0 +1,169 @@ +package list + +import ( + "fmt" + "net/http" + "time" + + "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 ListOptions struct { + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + + Limit int + Owner string + + Visibility string + Fork bool + Source bool + Language string + Archived bool + NonArchived 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 ( + flagPublic bool + flagPrivate bool + ) + + cmd := &cobra.Command{ + Use: "list []", + Args: cobra.MaximumNArgs(1), + 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)} + } + + if flagPrivate && flagPublic { + return &cmdutil.FlagError{Err: fmt.Errorf("specify only one of `--public` or `--private`")} + } + 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" + } else if flagPublic { + opts.Visibility = "public" + } + + 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(&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 +} + +func listRun(opts *ListOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + + filter := FilterOptions{ + 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) + if err != nil { + 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() + + for _, repo := range listResult.Repositories { + info := repo.Info() + infoColor := cs.Gray + if repo.IsPrivate { + 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, t), nil, cs.Gray) + } else { + tp.AddField(t.Format(time.RFC3339), nil, nil) + } + tp.EndRow() + } + + if opts.IO.IsStdoutTTY() { + 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) + } + + return tp.Render() +} + +func listHeader(owner string, matchCount, totalMatchCount int, hasFilters bool) string { + if totalMatchCount == 0 { + if hasFilters { + return "No results match your search" + } else if owner != "" { + return "There are no repositories in @" + owner + } + return "No results" + } + + 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 new file mode 100644 index 000000000..59552beda --- /dev/null +++ b/pkg/cmd/repo/list/list_test.go @@ -0,0 +1,359 @@ +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" + "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, + Language: "", + Archived: false, + NonArchived: false, + }, + }, + { + name: "with owner", + cli: "monalisa", + wants: ListOptions{ + 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: "", + Archived: false, + NonArchived: false, + }, + }, + { + name: "only public", + cli: "--public", + wants: ListOptions{ + 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: "", + Archived: false, + NonArchived: false, + }, + }, + { + name: "only forks", + cli: "--fork", + wants: ListOptions{ + 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: "", + Archived: false, + NonArchived: false, + }, + }, + { + name: "with language", + cli: "-l go", + wants: ListOptions{ + 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, + }, + }, + { + 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: "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 { + 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) + assert.Equal(t, tt.wants.Archived, gotOpts.Archived) + assert.Equal(t, tt.wants.NonArchived, gotOpts.NonArchived) + }) + } +} + +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 RepositoryList\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, fork 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 RepositoryList\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 public 8h + octocat/cli GitHub CLI public, 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 RepositoryList\b`), + httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { + assert.Equal(t, "PRIVATE", params["privacy"]) + assert.Equal(t, float64(2), params["perPage"]) + }), + ) + + 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()) +} 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)) diff --git a/pkg/cmdutil/args.go b/pkg/cmdutil/args.go index 65f3ade51..ee9c5e350 100644 --- a/pkg/cmdutil/args.go +++ b/pkg/cmdutil/args.go @@ -21,6 +21,21 @@ func MinimumArgs(n int, msg string) cobra.PositionalArgs { } } +func ExactArgs(n int, msg string) cobra.PositionalArgs { + + 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(msg)} + } + + return nil + } +} + func NoArgsQuoteReminder(cmd *cobra.Command, args []string) error { if len(args) < 1 { return nil