From 6b49e21295c992f5159384cda4c3dd930bb56d19 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Tue, 27 Apr 2021 16:23:11 -0300 Subject: [PATCH 01/20] Improve issue status detection --- api/queries_issue.go | 2 -- pkg/cmd/issue/close/close.go | 2 +- pkg/cmd/issue/close/close_test.go | 2 +- pkg/cmd/issue/reopen/reopen.go | 2 +- pkg/cmd/issue/reopen/reopen_test.go | 4 ++-- pkg/cmd/issue/transfer/transfer_test.go | 2 +- 6 files changed, 6 insertions(+), 8 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 7804a7613..99c334182 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -26,7 +26,6 @@ type Issue struct { Title string URL string State string - Closed bool Body string CreatedAt time.Time UpdatedAt time.Time @@ -237,7 +236,6 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e id title state - closed body author { login diff --git a/pkg/cmd/issue/close/close.go b/pkg/cmd/issue/close/close.go index 0a5355c21..a90f99e13 100644 --- a/pkg/cmd/issue/close/close.go +++ b/pkg/cmd/issue/close/close.go @@ -65,7 +65,7 @@ func closeRun(opts *CloseOptions) error { return err } - if issue.Closed { + if issue.State == "CLOSED" { fmt.Fprintf(opts.IO.ErrOut, "%s Issue #%d (%s) is already closed\n", cs.Yellow("!"), issue.Number, issue.Title) return nil } diff --git a/pkg/cmd/issue/close/close_test.go b/pkg/cmd/issue/close/close_test.go index 5a2054309..35c2fe10f 100644 --- a/pkg/cmd/issue/close/close_test.go +++ b/pkg/cmd/issue/close/close_test.go @@ -96,7 +96,7 @@ func TestIssueClose_alreadyClosed(t *testing.T) { httpmock.StringResponse(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "number": 13, "title": "The title of the issue", "closed": true} + "issue": { "number": 13, "title": "The title of the issue", "state": "CLOSED"} } } }`), ) diff --git a/pkg/cmd/issue/reopen/reopen.go b/pkg/cmd/issue/reopen/reopen.go index b0bb784f3..8c57a157c 100644 --- a/pkg/cmd/issue/reopen/reopen.go +++ b/pkg/cmd/issue/reopen/reopen.go @@ -65,7 +65,7 @@ func reopenRun(opts *ReopenOptions) error { return err } - if !issue.Closed { + if issue.State == "OPEN" { fmt.Fprintf(opts.IO.ErrOut, "%s Issue #%d (%s) is already open\n", cs.Yellow("!"), issue.Number, issue.Title) return nil } diff --git a/pkg/cmd/issue/reopen/reopen_test.go b/pkg/cmd/issue/reopen/reopen_test.go index bce4bc882..e2604baaa 100644 --- a/pkg/cmd/issue/reopen/reopen_test.go +++ b/pkg/cmd/issue/reopen/reopen_test.go @@ -64,7 +64,7 @@ func TestIssueReopen(t *testing.T) { httpmock.StringResponse(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "id": "THE-ID", "number": 2, "closed": true, "title": "The title of the issue"} + "issue": { "id": "THE-ID", "number": 2, "state": "CLOSED", "title": "The title of the issue"} } } }`), ) http.Register( @@ -96,7 +96,7 @@ func TestIssueReopen_alreadyOpen(t *testing.T) { httpmock.StringResponse(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "number": 2, "closed": false, "title": "The title of the issue"} + "issue": { "number": 2, "state": "OPEN", "title": "The title of the issue"} } } }`), ) diff --git a/pkg/cmd/issue/transfer/transfer_test.go b/pkg/cmd/issue/transfer/transfer_test.go index 51b2e1066..44b9ff112 100644 --- a/pkg/cmd/issue/transfer/transfer_test.go +++ b/pkg/cmd/issue/transfer/transfer_test.go @@ -118,7 +118,7 @@ func Test_transferRunSuccessfulIssueTransfer(t *testing.T) { httpmock.StringResponse(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "id": "THE-ID", "number": 1234, "closed": true, "title": "The title of the issue"} + "issue": { "id": "THE-ID", "number": 1234, "title": "The title of the issue"} } } }`)) http.Register( From 59b4d5cb7c0a3696843cf0b06db07efd8bb86e5f Mon Sep 17 00:00:00 2001 From: Robin Neatherway Date: Thu, 29 Apr 2021 16:31:31 +0100 Subject: [PATCH 02/20] Support standard path variable replacement syntax Add support for the following synonyms: {owner} for :owner {repo} for :repo {branch} for :branch --- pkg/cmd/api/api.go | 24 ++++++++-------- pkg/cmd/api/api_test.go | 61 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index da208b08a..e4cd8c041 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -71,7 +71,7 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command The endpoint argument should either be a path of a GitHub API v3 endpoint, or "graphql" to access the GitHub API v4. - Placeholder values ":owner", ":repo", and ":branch" in the endpoint argument will + Placeholder values "{owner}", "{repo}", and "{branch}" in the endpoint argument will get replaced with values from the repository of the current directory. The default HTTP request method is "GET" normally and "POST" if any parameters @@ -87,7 +87,7 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command - literal values "true", "false", "null", and integer numbers get converted to appropriate JSON types; - - placeholder values ":owner", ":repo", and ":branch" get populated with values + - placeholder values "{owner}", "{repo}", and "{branch}" get populated with values from the repository of the current directory; - if the value starts with "@", the rest of the value is interpreted as a filename to read the value from. Pass "-" to read from standard input. @@ -106,10 +106,10 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command `, "`"), Example: heredoc.Doc(` # list releases in the current repository - $ gh api repos/:owner/:repo/releases + $ gh api repos/{owner}/{repo}/releases # post an issue comment - $ gh api repos/:owner/:repo/issues/123/comments -f body='Hi from CLI' + $ gh api repos/{owner}/{repo}/issues/123/comments -f body='Hi from CLI' # add parameters to a GET request $ gh api -X GET search/issues -f q='repo:cli/cli is:open remote' @@ -121,14 +121,14 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command $ gh api --preview baptiste,nebula ... # print only specific fields from the response - $ gh api repos/:owner/:repo/issues --jq '.[].title' + $ gh api repos/{owner}/{repo}/issues --jq '.[].title' # use a template for the output - $ gh api repos/:owner/:repo/issues --template \ + $ gh api repos/{owner}/{repo}/issues --template \ '{{range .}}{{.title}} ({{.labels | pluck "name" | join ", " | color "yellow"}}){{"\n"}}{{end}}' # list releases with GraphQL - $ gh api graphql -F owner=':owner' -F name=':repo' -f query=' + $ gh api graphql -F owner='{owner}' -F name='{repo}' -f query=' query($name: String!, $owner: String!) { repository(owner: $owner, name: $name) { releases(last: 3) { @@ -397,9 +397,9 @@ func processResponse(resp *http.Response, opts *ApiOptions, headersOutputStream return } -var placeholderRE = regexp.MustCompile(`\:(owner|repo|branch)\b`) +var placeholderRE = regexp.MustCompile(`(\:(owner|repo|branch)\b|\{(owner|repo|branch)\})`) -// fillPlaceholders populates `:owner` and `:repo` placeholders with values from the current repository +// fillPlaceholders populates `{owner}` and `{repo}` placeholders with values from the current repository func fillPlaceholders(value string, opts *ApiOptions) (string, error) { if !placeholderRE.MatchString(value) { return value, nil @@ -412,11 +412,11 @@ func fillPlaceholders(value string, opts *ApiOptions) (string, error) { filled := placeholderRE.ReplaceAllStringFunc(value, func(m string) string { switch m { - case ":owner": + case ":owner", "{owner}": return baseRepo.RepoOwner() - case ":repo": + case ":repo", "{repo}": return baseRepo.RepoName() - case ":branch": + case ":branch", "{branch}": branch, e := opts.Branch() if e != nil { err = e diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 6bab83f09..a14f04292 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -870,7 +870,7 @@ func Test_magicFieldValue(t *testing.T) { wantErr: false, }, { - name: "placeholder", + name: "placeholder colon", args: args{ v: ":owner", opts: &ApiOptions{ @@ -883,6 +883,20 @@ func Test_magicFieldValue(t *testing.T) { want: "hubot", wantErr: false, }, + { + name: "placeholder braces", + args: args{ + v: "{owner}", + opts: &ApiOptions{ + IO: io, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("hubot", "robot-uprising"), nil + }, + }, + }, + want: "hubot", + wantErr: false, + }, { name: "file", args: args{ @@ -1008,6 +1022,51 @@ func Test_fillPlaceholders(t *testing.T) { want: "repos/:owner/:repo/branches/:branch", wantErr: true, }, + { + name: "has substitutes (braces)", + args: args{ + value: "repos/{owner}/{repo}/releases", + opts: &ApiOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("hubot", "robot-uprising"), nil + }, + }, + }, + want: "repos/hubot/robot-uprising/releases", + wantErr: false, + }, + { + name: "has branch placeholder (braces)", + args: args{ + value: "repos/cli/cli/branches/{branch}/protection/required_status_checks", + opts: &ApiOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("cli", "cli"), nil + }, + Branch: func() (string, error) { + return "trunk", nil + }, + }, + }, + want: "repos/cli/cli/branches/trunk/protection/required_status_checks", + wantErr: false, + }, + { + name: "has branch placeholder and git is in detached head (braces)", + args: args{ + value: "repos/{owner}/{repo}/branches/{branch}", + opts: &ApiOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("cli", "cli"), nil + }, + Branch: func() (string, error) { + return "", git.ErrNotOnAnyBranch + }, + }, + }, + want: "repos/{owner}/{repo}/branches/{branch}", + wantErr: true, + }, { name: "no greedy substitutes", args: args{ From 6a57dcfd7dde07908676646dcea3021ad14c5a78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 30 Apr 2021 14:19:26 +0200 Subject: [PATCH 03/20] :nail_care: cleanup placeholder implementation --- pkg/cmd/api/api.go | 62 ++++++++++++++++++++------------------- pkg/cmd/api/api_test.go | 65 ++++++++++++++++++++++++++++------------- 2 files changed, 76 insertions(+), 51 deletions(-) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index e4cd8c041..27e26f004 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -72,7 +72,9 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command "graphql" to access the GitHub API v4. Placeholder values "{owner}", "{repo}", and "{branch}" in the endpoint argument will - get replaced with values from the repository of the current directory. + get replaced with values from the repository of the current directory. Note that in + some shells, for example PowerShell, you may need to enclose any value that contains + "{...}" in quotes to prevent the shell from applying special meaning to curly braces. The default HTTP request method is "GET" normally and "POST" if any parameters were added. Override the method with %[1]s--method%[1]s. @@ -397,41 +399,41 @@ func processResponse(resp *http.Response, opts *ApiOptions, headersOutputStream return } -var placeholderRE = regexp.MustCompile(`(\:(owner|repo|branch)\b|\{(owner|repo|branch)\})`) +var placeholderRE = regexp.MustCompile(`(\:(owner|repo|branch)\b|\{[a-z]+\})`) -// fillPlaceholders populates `{owner}` and `{repo}` placeholders with values from the current repository +// fillPlaceholders replaces placeholders with values from the current repository func fillPlaceholders(value string, opts *ApiOptions) (string, error) { - if !placeholderRE.MatchString(value) { - return value, nil - } + var err error + return placeholderRE.ReplaceAllStringFunc(value, func(m string) string { + var name string + if m[0] == ':' { + name = m[1:] + } else { + name = m[1 : len(m)-1] + } - baseRepo, err := opts.BaseRepo() - if err != nil { - return value, err - } - - filled := placeholderRE.ReplaceAllStringFunc(value, func(m string) string { - switch m { - case ":owner", "{owner}": - return baseRepo.RepoOwner() - case ":repo", "{repo}": - return baseRepo.RepoName() - case ":branch", "{branch}": - branch, e := opts.Branch() - if e != nil { + switch name { + case "owner": + if baseRepo, e := opts.BaseRepo(); e == nil { + return baseRepo.RepoOwner() + } else { + err = e + } + case "repo": + if baseRepo, e := opts.BaseRepo(); e == nil { + return baseRepo.RepoName() + } else { + err = e + } + case "branch": + if branch, e := opts.Branch(); e == nil { + return branch + } else { err = e } - return branch - default: - panic(fmt.Sprintf("invalid placeholder: %q", m)) } - }) - - if err != nil { - return value, err - } - - return filled, nil + return m + }), err } func printHeaders(w io.Writer, headers http.Header, colorize bool) { diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index a14f04292..acaff19da 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -978,7 +978,7 @@ func Test_fillPlaceholders(t *testing.T) { wantErr: false, }, { - name: "has substitutes", + name: "has substitutes (colon)", args: args{ value: "repos/:owner/:repo/releases", opts: &ApiOptions{ @@ -991,39 +991,37 @@ func Test_fillPlaceholders(t *testing.T) { wantErr: false, }, { - name: "has branch placeholder", + name: "has branch placeholder (colon)", args: args{ - value: "repos/cli/cli/branches/:branch/protection/required_status_checks", + value: "repos/owner/repo/branches/:branch/protection/required_status_checks", opts: &ApiOptions{ - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("cli", "cli"), nil - }, + BaseRepo: nil, Branch: func() (string, error) { return "trunk", nil }, }, }, - want: "repos/cli/cli/branches/trunk/protection/required_status_checks", + want: "repos/owner/repo/branches/trunk/protection/required_status_checks", wantErr: false, }, { - name: "has branch placeholder and git is in detached head", + name: "has branch placeholder and git is in detached head (colon)", args: args{ value: "repos/:owner/:repo/branches/:branch", opts: &ApiOptions{ BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("cli", "cli"), nil + return ghrepo.New("hubot", "robot-uprising"), nil }, Branch: func() (string, error) { return "", git.ErrNotOnAnyBranch }, }, }, - want: "repos/:owner/:repo/branches/:branch", + want: "repos/hubot/robot-uprising/branches/:branch", wantErr: true, }, { - name: "has substitutes (braces)", + name: "has substitutes", args: args{ value: "repos/{owner}/{repo}/releases", opts: &ApiOptions{ @@ -1036,39 +1034,53 @@ func Test_fillPlaceholders(t *testing.T) { wantErr: false, }, { - name: "has branch placeholder (braces)", + name: "has branch placeholder", args: args{ - value: "repos/cli/cli/branches/{branch}/protection/required_status_checks", + value: "repos/owner/repo/branches/{branch}/protection/required_status_checks", opts: &ApiOptions{ - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("cli", "cli"), nil - }, + BaseRepo: nil, Branch: func() (string, error) { return "trunk", nil }, }, }, - want: "repos/cli/cli/branches/trunk/protection/required_status_checks", + want: "repos/owner/repo/branches/trunk/protection/required_status_checks", wantErr: false, }, { - name: "has branch placeholder and git is in detached head (braces)", + name: "has branch placeholder and git is in detached head", args: args{ value: "repos/{owner}/{repo}/branches/{branch}", opts: &ApiOptions{ BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("cli", "cli"), nil + return ghrepo.New("hubot", "robot-uprising"), nil }, Branch: func() (string, error) { return "", git.ErrNotOnAnyBranch }, }, }, - want: "repos/{owner}/{repo}/branches/{branch}", + want: "repos/hubot/robot-uprising/branches/{branch}", wantErr: true, }, { - name: "no greedy substitutes", + name: "surfaces errors in earlier placeholders", + args: args{ + value: "{branch}-{owner}", + opts: &ApiOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("hubot", "robot-uprising"), nil + }, + Branch: func() (string, error) { + return "", git.ErrNotOnAnyBranch + }, + }, + }, + want: "{branch}-hubot", + wantErr: true, + }, + { + name: "no greedy substitutes (colon)", args: args{ value: ":ownership/:repository", opts: &ApiOptions{ @@ -1078,6 +1090,17 @@ func Test_fillPlaceholders(t *testing.T) { want: ":ownership/:repository", wantErr: false, }, + { + name: "non-placeholders are left intact", + args: args{ + value: "{}{ownership}/{repository}", + opts: &ApiOptions{ + BaseRepo: nil, + }, + }, + want: "{}{ownership}/{repository}", + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 796d2e24ef06266a343244dc6f78ef5a39f94f93 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Fri, 30 Apr 2021 17:32:16 -0300 Subject: [PATCH 04/20] Bring back `Closed` field --- api/queries_issue.go | 1 + 1 file changed, 1 insertion(+) diff --git a/api/queries_issue.go b/api/queries_issue.go index 99c334182..2dfab5742 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -26,6 +26,7 @@ type Issue struct { Title string URL string State string + Closed bool Body string CreatedAt time.Time UpdatedAt time.Time From 2f94adabb2ddbda4cfbb717019714dca6f0a3fa1 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez <6853656+cristiand391@users.noreply.github.com> Date: Fri, 7 May 2021 10:21:26 +0000 Subject: [PATCH 05/20] Use `T.TempDir` for temporary dirs in tests (#3580) --- internal/docs/man_test.go | 3 +-- internal/docs/markdown_test.go | 3 +-- pkg/cmd/api/api_test.go | 20 +++++++++++--------- pkg/cmd/issue/create/create_test.go | 4 ++-- pkg/cmd/pr/create/create_test.go | 4 ++-- pkg/cmd/pr/shared/preserve_test.go | 17 ++++------------- pkg/cmd/workflow/run/run_test.go | 7 +++---- pkg/githubtemplate/github_template_test.go | 10 ++++------ 8 files changed, 28 insertions(+), 40 deletions(-) diff --git a/internal/docs/man_test.go b/internal/docs/man_test.go index 58591e19e..d72ea7214 100644 --- a/internal/docs/man_test.go +++ b/internal/docs/man_test.go @@ -175,11 +175,10 @@ func assertNextLineEquals(scanner *bufio.Scanner, expectedLine string) error { } func BenchmarkGenManToFile(b *testing.B) { - file, err := ioutil.TempFile("", "") + file, err := ioutil.TempFile(b.TempDir(), "") if err != nil { b.Fatal(err) } - defer os.Remove(file.Name()) defer file.Close() b.ResetTimer() diff --git a/internal/docs/markdown_test.go b/internal/docs/markdown_test.go index 27be95efa..497a0384a 100644 --- a/internal/docs/markdown_test.go +++ b/internal/docs/markdown_test.go @@ -83,11 +83,10 @@ func TestGenMdTree(t *testing.T) { } func BenchmarkGenMarkdownToFile(b *testing.B) { - file, err := ioutil.TempFile("", "") + file, err := ioutil.TempFile(b.TempDir(), "") if err != nil { b.Fatal(err) } - defer os.Remove(file.Name()) defer file.Close() b.ResetTimer() diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index acaff19da..0d7290835 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -693,6 +693,9 @@ func Test_apiRun_inputFile(t *testing.T) { contentLength: 10, }, } + + tempDir := t.TempDir() + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { io, stdin, _, _ := iostreams.Test() @@ -702,13 +705,12 @@ func Test_apiRun_inputFile(t *testing.T) { if tt.inputFile == "-" { _, _ = stdin.Write(tt.inputContents) } else { - f, err := ioutil.TempFile("", tt.inputFile) + f, err := ioutil.TempFile(tempDir, tt.inputFile) if err != nil { t.Fatal(err) } _, _ = f.Write(tt.inputContents) - f.Close() - t.Cleanup(func() { os.Remove(f.Name()) }) + defer f.Close() inputFile = f.Name() } @@ -825,13 +827,13 @@ func Test_parseFields(t *testing.T) { } func Test_magicFieldValue(t *testing.T) { - f, err := ioutil.TempFile("", "gh-test") + f, err := ioutil.TempFile(t.TempDir(), "gh-test") if err != nil { t.Fatal(err) } + defer f.Close() + fmt.Fprint(f, "file contents") - f.Close() - t.Cleanup(func() { os.Remove(f.Name()) }) io, _, _, _ := iostreams.Test() @@ -932,13 +934,13 @@ func Test_magicFieldValue(t *testing.T) { } func Test_openUserFile(t *testing.T) { - f, err := ioutil.TempFile("", "gh-test") + f, err := ioutil.TempFile(t.TempDir(), "gh-test") if err != nil { t.Fatal(err) } + defer f.Close() + fmt.Fprint(f, "file contents") - f.Close() - t.Cleanup(func() { os.Remove(f.Name()) }) file, length, err := openUserFile(f.Name(), nil) if err != nil { diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index eece13daf..3e69f386c 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -6,7 +6,6 @@ import ( "fmt" "io/ioutil" "net/http" - "os" "path/filepath" "strings" "testing" @@ -422,8 +421,9 @@ func TestIssueCreate_recover(t *testing.T) { }, }) - tmpfile, err := ioutil.TempFile(os.TempDir(), "testrecover*") + tmpfile, err := ioutil.TempFile(t.TempDir(), "testrecover*") assert.NoError(t, err) + defer tmpfile.Close() state := prShared.IssueMetadataState{ Title: "recovered title", diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 8480462ed..0633e3dad 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -6,7 +6,6 @@ import ( "fmt" "io/ioutil" "net/http" - "os" "path/filepath" "testing" @@ -309,8 +308,9 @@ func TestPRCreate_recover(t *testing.T) { }, }) - tmpfile, err := ioutil.TempFile(os.TempDir(), "testrecover*") + tmpfile, err := ioutil.TempFile(t.TempDir(), "testrecover*") assert.NoError(t, err) + defer tmpfile.Close() state := prShared.IssueMetadataState{ Title: "recovered title", diff --git a/pkg/cmd/pr/shared/preserve_test.go b/pkg/cmd/pr/shared/preserve_test.go index 892973f26..6b6e348d0 100644 --- a/pkg/cmd/pr/shared/preserve_test.go +++ b/pkg/cmd/pr/shared/preserve_test.go @@ -4,7 +4,6 @@ import ( "encoding/json" "errors" "io/ioutil" - "os" "testing" "github.com/cli/cli/pkg/iostreams" @@ -70,6 +69,8 @@ func Test_PreserveInput(t *testing.T) { }, } + tempDir := t.TempDir() + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if tt.state == nil { @@ -78,9 +79,9 @@ func Test_PreserveInput(t *testing.T) { io, _, _, errOut := iostreams.Test() - tf, tferr := tmpfile() + tf, tferr := ioutil.TempFile(tempDir, "testfile*") assert.NoError(t, tferr) - defer os.Remove(tf.Name()) + defer tf.Close() io.TempFileOverride = tf @@ -111,13 +112,3 @@ func Test_PreserveInput(t *testing.T) { }) } } - -func tmpfile() (*os.File, error) { - dir := os.TempDir() - tmpfile, err := ioutil.TempFile(dir, "testfile*") - if err != nil { - return nil, err - } - - return tmpfile, nil -} diff --git a/pkg/cmd/workflow/run/run_test.go b/pkg/cmd/workflow/run/run_test.go index ee5195071..49693faef 100644 --- a/pkg/cmd/workflow/run/run_test.go +++ b/pkg/cmd/workflow/run/run_test.go @@ -7,7 +7,6 @@ import ( "fmt" "io/ioutil" "net/http" - "os" "testing" "github.com/cli/cli/api" @@ -149,13 +148,13 @@ func TestNewCmdRun(t *testing.T) { } func Test_magicFieldValue(t *testing.T) { - f, err := ioutil.TempFile("", "gh-test") + f, err := ioutil.TempFile(t.TempDir(), "gh-test") if err != nil { t.Fatal(err) } + defer f.Close() + fmt.Fprint(f, "file contents") - f.Close() - t.Cleanup(func() { os.Remove(f.Name()) }) io, _, _, _ := iostreams.Test() diff --git a/pkg/githubtemplate/github_template_test.go b/pkg/githubtemplate/github_template_test.go index c9f42f552..b92d4523c 100644 --- a/pkg/githubtemplate/github_template_test.go +++ b/pkg/githubtemplate/github_template_test.go @@ -261,12 +261,11 @@ func TestFindLegacy(t *testing.T) { } func TestExtractName(t *testing.T) { - tmpfile, err := ioutil.TempFile("", "gh-cli") + tmpfile, err := ioutil.TempFile(t.TempDir(), "gh-cli") if err != nil { t.Fatal(err) } - tmpfile.Close() - defer os.Remove(tmpfile.Name()) + defer tmpfile.Close() type args struct { filePath string @@ -322,12 +321,11 @@ about: This is how you report bugs } func TestExtractContents(t *testing.T) { - tmpfile, err := ioutil.TempFile("", "gh-cli") + tmpfile, err := ioutil.TempFile(t.TempDir(), "gh-cli") if err != nil { t.Fatal(err) } - tmpfile.Close() - defer os.Remove(tmpfile.Name()) + defer tmpfile.Close() type args struct { filePath string From cc94dc762d14eeb54e13cb184a9c55c2234a3b20 Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Tue, 4 May 2021 18:50:27 +0530 Subject: [PATCH 06/20] shift gist validation to server rather than client --- pkg/cmd/gist/create/create.go | 15 ++++++ pkg/cmd/gist/create/create_test.go | 80 +++++++++++++++++++++++++----- pkg/cmd/gist/empty.txt | 0 3 files changed, 83 insertions(+), 12 deletions(-) create mode 100644 pkg/cmd/gist/empty.txt diff --git a/pkg/cmd/gist/create/create.go b/pkg/cmd/gist/create/create.go index 645638bca..291162ad4 100644 --- a/pkg/cmd/gist/create/create.go +++ b/pkg/cmd/gist/create/create.go @@ -153,6 +153,12 @@ func createRun(opts *CreateOptions) error { if httpError.OAuthScopes != "" && !strings.Contains(httpError.OAuthScopes, "gist") { return fmt.Errorf("This command requires the 'gist' OAuth scope.\nPlease re-authenticate by doing `gh config set -h github.com oauth_token ''` and running the command again.") } + if httpError.StatusCode == http.StatusUnprocessableEntity { + if detectEmptyFiles(files) { + fmt.Fprintf(errOut, "%s Failed to create gist: %s", cs.FailureIcon(), "a gist file cannot be blank") + return cmdutil.SilentError + } + } } return fmt.Errorf("%s Failed to create gist: %w", cs.Red("X"), err) } @@ -266,3 +272,12 @@ func createGist(client *http.Client, hostname, description string, public bool, return &result, nil } + +func detectEmptyFiles(files map[string]*shared.GistFile) bool { + for _, file := range files { + if strings.TrimSpace(file.Content) == "" { + return true + } + } + return false +} diff --git a/pkg/cmd/gist/create/create_test.go b/pkg/cmd/gist/create/create_test.go index 0c2d0f39f..d4069cfdf 100644 --- a/pkg/cmd/gist/create/create_test.go +++ b/pkg/cmd/gist/create/create_test.go @@ -20,6 +20,7 @@ import ( const ( fixtureFile = "../fixture.txt" + emptyFile = "../empty.txt" ) func Test_processFiles(t *testing.T) { @@ -165,14 +166,15 @@ func TestNewCmdCreate(t *testing.T) { func Test_createRun(t *testing.T) { tests := []struct { - name string - opts *CreateOptions - stdin string - wantOut string - wantStderr string - wantParams map[string]interface{} - wantErr bool - wantBrowse string + name string + opts *CreateOptions + stdin string + wantOut string + wantStderr string + wantParams map[string]interface{} + wantErr bool + wantBrowse string + responseStatus int }{ { name: "public", @@ -193,6 +195,7 @@ func Test_createRun(t *testing.T) { }, }, }, + responseStatus: http.StatusOK, }, { name: "with description", @@ -213,6 +216,7 @@ func Test_createRun(t *testing.T) { }, }, }, + responseStatus: http.StatusOK, }, { name: "multiple files", @@ -236,6 +240,25 @@ func Test_createRun(t *testing.T) { }, }, }, + responseStatus: http.StatusOK, + }, + { + name: "file with empty content", + opts: &CreateOptions{ + Filenames: []string{emptyFile}, + }, + wantOut: "", + wantStderr: "- Creating gist empty.txt\nX Failed to create gist: a gist file cannot be blank", + wantErr: true, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "empty.txt": map[string]interface{}{}, + }, + }, + responseStatus: http.StatusUnprocessableEntity, }, { name: "stdin arg", @@ -256,6 +279,7 @@ func Test_createRun(t *testing.T) { }, }, }, + responseStatus: http.StatusOK, }, { name: "web arg", @@ -277,14 +301,20 @@ func Test_createRun(t *testing.T) { }, }, }, + responseStatus: http.StatusOK, }, } for _, tt := range tests { reg := &httpmock.Registry{} - reg.Register(httpmock.REST("POST", "gists"), - httpmock.JSONResponse(struct { - Html_url string - }{"https://gist.github.com/aa5a315d61ae9438b18d"})) + if tt.responseStatus == http.StatusUnprocessableEntity { + reg.Register(httpmock.REST("POST", "gists"), + httpmock.StatusStringResponse(http.StatusUnprocessableEntity, "")) + } else { + reg.Register(httpmock.REST("POST", "gists"), + httpmock.JSONResponse(struct { + Html_url string + }{"https://gist.github.com/aa5a315d61ae9438b18d"})) + } mockClient := func() (*http.Client, error) { return &http.Client{Transport: reg}, nil @@ -325,6 +355,32 @@ func Test_createRun(t *testing.T) { } } +func Test_detectEmptyFiles(t *testing.T) { + tests := []struct { + content string + isEmptyFile bool + }{ + { + content: "{}", + isEmptyFile: false, + }, + { + content: "\n\t", + isEmptyFile: true, + }, + } + + for _, tt := range tests { + files := map[string]*shared.GistFile{} + files["file"] = &shared.GistFile{ + Content: tt.content, + } + + isEmptyFile := detectEmptyFiles(files) + assert.Equal(t, tt.isEmptyFile, isEmptyFile) + } +} + func Test_CreateRun_reauth(t *testing.T) { reg := &httpmock.Registry{} reg.Register(httpmock.REST("POST", "gists"), func(req *http.Request) (*http.Response, error) { diff --git a/pkg/cmd/gist/empty.txt b/pkg/cmd/gist/empty.txt new file mode 100644 index 000000000..e69de29bb From 70a9621928bfc9c66ffc057938ab27a928cc0820 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 7 May 2021 13:50:33 +0200 Subject: [PATCH 07/20] :nail_care: cleanup in gist create --- pkg/cmd/gist/create/create.go | 4 +- pkg/cmd/gist/create/create_test.go | 61 +++++++++++++++--------------- pkg/cmd/gist/empty.txt | 0 pkg/cmd/gist/fixture.txt | 1 - pkg/cmd/gist/shared/shared.go | 2 +- 5 files changed, 33 insertions(+), 35 deletions(-) delete mode 100644 pkg/cmd/gist/empty.txt delete mode 100644 pkg/cmd/gist/fixture.txt diff --git a/pkg/cmd/gist/create/create.go b/pkg/cmd/gist/create/create.go index 291162ad4..c10b5d27b 100644 --- a/pkg/cmd/gist/create/create.go +++ b/pkg/cmd/gist/create/create.go @@ -151,11 +151,11 @@ func createRun(opts *CreateOptions) error { var httpError api.HTTPError if errors.As(err, &httpError) { if httpError.OAuthScopes != "" && !strings.Contains(httpError.OAuthScopes, "gist") { - return fmt.Errorf("This command requires the 'gist' OAuth scope.\nPlease re-authenticate by doing `gh config set -h github.com oauth_token ''` and running the command again.") + return fmt.Errorf("This command requires the 'gist' OAuth scope.\nPlease re-authenticate with: gh auth refresh -h %s -s gist", host) } if httpError.StatusCode == http.StatusUnprocessableEntity { if detectEmptyFiles(files) { - fmt.Fprintf(errOut, "%s Failed to create gist: %s", cs.FailureIcon(), "a gist file cannot be blank") + fmt.Fprintf(errOut, "%s Failed to create gist: %s\n", cs.FailureIcon(), "a gist file cannot be blank") return cmdutil.SilentError } } diff --git a/pkg/cmd/gist/create/create_test.go b/pkg/cmd/gist/create/create_test.go index d4069cfdf..b454b1598 100644 --- a/pkg/cmd/gist/create/create_test.go +++ b/pkg/cmd/gist/create/create_test.go @@ -5,9 +5,11 @@ import ( "encoding/json" "io/ioutil" "net/http" + "path" "strings" "testing" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmd/gist/shared" @@ -18,11 +20,6 @@ import ( "github.com/stretchr/testify/assert" ) -const ( - fixtureFile = "../fixture.txt" - emptyFile = "../empty.txt" -) - func Test_processFiles(t *testing.T) { fakeStdin := strings.NewReader("hey cool how is it going") files, err := processFiles(ioutil.NopCloser(fakeStdin), "", []string{"-"}) @@ -165,6 +162,12 @@ func TestNewCmdCreate(t *testing.T) { } func Test_createRun(t *testing.T) { + tempDir := t.TempDir() + fixtureFile := path.Join(tempDir, "fixture.txt") + assert.NoError(t, ioutil.WriteFile(fixtureFile, []byte("{}"), 0644)) + emptyFile := path.Join(tempDir, "empty.txt") + assert.NoError(t, ioutil.WriteFile(emptyFile, []byte(" \t\n"), 0644)) + tests := []struct { name string opts *CreateOptions @@ -247,15 +250,18 @@ func Test_createRun(t *testing.T) { opts: &CreateOptions{ Filenames: []string{emptyFile}, }, - wantOut: "", - wantStderr: "- Creating gist empty.txt\nX Failed to create gist: a gist file cannot be blank", - wantErr: true, + wantOut: "", + wantStderr: heredoc.Doc(` + - Creating gist empty.txt + X Failed to create gist: a gist file cannot be blank + `), + wantErr: true, wantParams: map[string]interface{}{ "description": "", "updated_at": "0001-01-01T00:00:00Z", "public": false, "files": map[string]interface{}{ - "empty.txt": map[string]interface{}{}, + "empty.txt": map[string]interface{}{"content": " \t\n"}, }, }, responseStatus: http.StatusUnprocessableEntity, @@ -306,14 +312,16 @@ func Test_createRun(t *testing.T) { } for _, tt := range tests { reg := &httpmock.Registry{} - if tt.responseStatus == http.StatusUnprocessableEntity { - reg.Register(httpmock.REST("POST", "gists"), - httpmock.StatusStringResponse(http.StatusUnprocessableEntity, "")) + if tt.responseStatus == http.StatusOK { + reg.Register( + httpmock.REST("POST", "gists"), + httpmock.StringResponse(`{ + "html_url": "https://gist.github.com/aa5a315d61ae9438b18d" + }`)) } else { - reg.Register(httpmock.REST("POST", "gists"), - httpmock.JSONResponse(struct { - Html_url string - }{"https://gist.github.com/aa5a315d61ae9438b18d"})) + reg.Register( + httpmock.REST("POST", "gists"), + httpmock.StatusStringResponse(tt.responseStatus, "{}")) } mockClient := func() (*http.Client, error) { @@ -388,33 +396,24 @@ func Test_CreateRun_reauth(t *testing.T) { StatusCode: 404, Request: req, Header: map[string][]string{ - "X-Oauth-Scopes": {"coolScope"}, + "X-Oauth-Scopes": {"repo, read:org"}, }, Body: ioutil.NopCloser(bytes.NewBufferString("oh no")), }, nil }) - mockClient := func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil - } - io, _, _, _ := iostreams.Test() opts := &CreateOptions{ - IO: io, - HttpClient: mockClient, + IO: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, Config: func() (config.Config, error) { return config.NewBlankConfig(), nil }, - Filenames: []string{fixtureFile}, } err := createRun(opts) - if err == nil { - t.Fatalf("expected oauth error") - } - - if !strings.Contains(err.Error(), "Please re-authenticate") { - t.Errorf("got unexpected error: %s", err) - } + assert.EqualError(t, err, "This command requires the 'gist' OAuth scope.\nPlease re-authenticate with: gh auth refresh -h github.com -s gist") } diff --git a/pkg/cmd/gist/empty.txt b/pkg/cmd/gist/empty.txt deleted file mode 100644 index e69de29bb..000000000 diff --git a/pkg/cmd/gist/fixture.txt b/pkg/cmd/gist/fixture.txt deleted file mode 100644 index 9e26dfeeb..000000000 --- a/pkg/cmd/gist/fixture.txt +++ /dev/null @@ -1 +0,0 @@ -{} \ No newline at end of file diff --git a/pkg/cmd/gist/shared/shared.go b/pkg/cmd/gist/shared/shared.go index 98d66a9f8..1eac50d32 100644 --- a/pkg/cmd/gist/shared/shared.go +++ b/pkg/cmd/gist/shared/shared.go @@ -20,7 +20,7 @@ type GistFile struct { Filename string `json:"filename,omitempty"` Type string `json:"type,omitempty"` Language string `json:"language,omitempty"` - Content string `json:"content,omitempty"` + Content string `json:"content"` } type GistOwner struct { From 3cbd5b49346378136f5e85601c42882cf0ef4d06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 10 May 2021 17:09:03 +0200 Subject: [PATCH 08/20] Add `repo fork --org` functionality (#3611) Co-authored-by: Gowtham Munukutla --- api/queries_repo.go | 15 +++++- pkg/cmd/pr/create/create.go | 2 +- pkg/cmd/repo/fork/fork.go | 4 +- pkg/cmd/repo/fork/fork_test.go | 84 ++++++++++++++++++++++++++-------- 4 files changed, 81 insertions(+), 24 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index 90d3949a5..2918ee1d6 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -312,9 +312,20 @@ type repositoryV3 struct { } // ForkRepo forks the repository on GitHub and returns the new repository -func ForkRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { +func ForkRepo(client *Client, repo ghrepo.Interface, org string) (*Repository, error) { path := fmt.Sprintf("repos/%s/forks", ghrepo.FullName(repo)) - body := bytes.NewBufferString(`{}`) + + params := map[string]interface{}{} + if org != "" { + params["organization"] = org + } + + body := &bytes.Buffer{} + enc := json.NewEncoder(body) + if err := enc.Encode(params); err != nil { + return nil, err + } + result := repositoryV3{} err := client.REST(repo.RepoHost(), "POST", path, body, &result) if err != nil { diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index fc2e70e26..5093d53d1 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -674,7 +674,7 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { // one by forking the base repository if headRepo == nil && ctx.IsPushEnabled { opts.IO.StartProgressIndicator() - headRepo, err = api.ForkRepo(client, ctx.BaseRepo) + headRepo, err = api.ForkRepo(client, ctx.BaseRepo, "") opts.IO.StopProgressIndicator() if err != nil { return fmt.Errorf("error forking repo: %w", err) diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index 30df5ae67..1bebcd7df 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -36,6 +36,7 @@ type ForkOptions struct { PromptClone bool PromptRemote bool RemoteName string + Organization string Rename bool } @@ -110,6 +111,7 @@ Additional 'git clone' flags can be passed in by listing them after '--'.`, cmd.Flags().BoolVar(&opts.Clone, "clone", false, "Clone the fork {true|false}") cmd.Flags().BoolVar(&opts.Remote, "remote", false, "Add remote for fork {true|false}") cmd.Flags().StringVar(&opts.RemoteName, "remote-name", "origin", "Specify a name for a fork's new remote.") + cmd.Flags().StringVar(&opts.Organization, "org", "", "Create the fork in an organization") return cmd } @@ -169,7 +171,7 @@ func forkRun(opts *ForkOptions) error { apiClient := api.NewClientFromHTTP(httpClient) opts.IO.StartProgressIndicator() - forkedRepo, err := api.ForkRepo(apiClient, repoToFork) + forkedRepo, err := api.ForkRepo(apiClient, repoToFork, opts.Organization) opts.IO.StopProgressIndicator() if err != nil { return fmt.Errorf("failed to fork: %w", err) diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index d1210048b..26f69aa97 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -2,12 +2,14 @@ package fork import ( "bytes" + "io/ioutil" "net/http" "net/url" "regexp" "testing" "time" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/context" "github.com/cli/cli/git" "github.com/cli/cli/internal/config" @@ -72,8 +74,9 @@ func TestNewCmdFork(t *testing.T) { name: "blank nontty", cli: "", wants: ForkOptions{ - RemoteName: "origin", - Rename: true, + RemoteName: "origin", + Rename: true, + Organization: "", }, }, { @@ -85,6 +88,7 @@ func TestNewCmdFork(t *testing.T) { PromptClone: true, PromptRemote: true, Rename: true, + Organization: "", }, }, { @@ -104,6 +108,16 @@ func TestNewCmdFork(t *testing.T) { Rename: true, }, }, + { + name: "to org", + cli: "--org batmanshome", + wants: ForkOptions{ + RemoteName: "origin", + Remote: false, + Rename: false, + Organization: "batmanshome", + }, + }, } for _, tt := range tests { @@ -141,6 +155,7 @@ func TestNewCmdFork(t *testing.T) { assert.Equal(t, tt.wants.Remote, gotOpts.Remote) assert.Equal(t, tt.wants.PromptRemote, gotOpts.PromptRemote) assert.Equal(t, tt.wants.PromptClone, gotOpts.PromptClone) + assert.Equal(t, tt.wants.Organization, gotOpts.Organization) }) } } @@ -289,6 +304,7 @@ func TestRepoFork_in_parent_tty(t *testing.T) { assert.Equal(t, "✓ Created fork someone/REPO\n✓ Added remote origin\n", output.Stderr()) reg.Verify(t) } + func TestRepoFork_in_parent_nontty(t *testing.T) { defer stubSince(2 * time.Second)() reg := &httpmock.Registry{} @@ -409,37 +425,65 @@ func TestRepoFork_in_parent(t *testing.T) { func TestRepoFork_outside(t *testing.T) { tests := []struct { - name string - args string + name string + args string + postBody string + responseBody string + wantStderr string }{ { - name: "url arg", - args: "--clone=false http://github.com/OWNER/REPO.git", + name: "url arg", + args: "--clone=false http://github.com/OWNER/REPO.git", + postBody: "{}\n", + responseBody: `{"name":"REPO", "owner":{"login":"monalisa"}}`, + wantStderr: heredoc.Doc(` + ✓ Created fork monalisa/REPO + `), }, { - name: "full name arg", - args: "--clone=false OWNER/REPO", + name: "full name arg", + args: "--clone=false OWNER/REPO", + postBody: "{}\n", + responseBody: `{"name":"REPO", "owner":{"login":"monalisa"}}`, + wantStderr: heredoc.Doc(` + ✓ Created fork monalisa/REPO + `), + }, + { + name: "fork to org without clone", + args: "--clone=false OWNER/REPO --org batmanshome", + postBody: "{\"organization\":\"batmanshome\"}\n", + responseBody: `{"name":"REPO", "owner":{"login":"BatmansHome"}}`, + wantStderr: heredoc.Doc(` + ✓ Created fork BatmansHome/REPO + `), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { defer stubSince(2 * time.Second)() + reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + func(req *http.Request) (*http.Response, error) { + bb, err := ioutil.ReadAll(req.Body) + if err != nil { + return nil, err + } + assert.Equal(t, tt.postBody, string(bb)) + return &http.Response{ + Request: req, + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(tt.responseBody)), + }, nil + }) + httpClient := &http.Client{Transport: reg} - output, err := runCommand(httpClient, nil, true, tt.args) - if err != nil { - t.Errorf("error running command `repo fork`: %v", err) - } - + assert.NoError(t, err) assert.Equal(t, "", output.String()) - - r := regexp.MustCompile(`Created fork.*someone/REPO`) - if !r.MatchString(output.Stderr()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } + assert.Equal(t, tt.wantStderr, output.Stderr()) reg.Verify(t) }) } From 26d2e5c5cef62b3667eb2f05051a7b17a9fbe9e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 11 May 2021 17:08:28 +0200 Subject: [PATCH 09/20] Rework our pull request template (#3584) --- .github/PULL_REQUEST_TEMPLATE.md | 4 ++++ .github/PULL_REQUEST_TEMPLATE/bug_fix.md | 19 ------------------- 2 files changed, 4 insertions(+), 19 deletions(-) create mode 100644 .github/PULL_REQUEST_TEMPLATE.md delete mode 100644 .github/PULL_REQUEST_TEMPLATE/bug_fix.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 000000000..aa6662d49 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,4 @@ + diff --git a/.github/PULL_REQUEST_TEMPLATE/bug_fix.md b/.github/PULL_REQUEST_TEMPLATE/bug_fix.md deleted file mode 100644 index ca33dd34c..000000000 --- a/.github/PULL_REQUEST_TEMPLATE/bug_fix.md +++ /dev/null @@ -1,19 +0,0 @@ ---- -name: "\U0001F41B Bug fix" -about: Fix a bug in GitHub CLI - ---- - - - -## Summary - -closes #[issue number] - -## Details - -- From 02b7a7178336628311b75fb59e60f33a6594ab65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 11 May 2021 21:21:57 +0200 Subject: [PATCH 10/20] Add project layout documentation (#3587) --- .github/CONTRIBUTING.md | 2 + docs/project-layout.md | 84 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 docs/project-layout.md diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 56f1248d6..d339a0685 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -36,6 +36,8 @@ Run the new binary as: Run tests with: `go test ./...` +See [project layout documentation](../project-layout.md) for information on where to find specific source files. + ## Submitting a pull request 1. Create a new branch: `git checkout -b my-branch-name` diff --git a/docs/project-layout.md b/docs/project-layout.md new file mode 100644 index 000000000..cf594a2e7 --- /dev/null +++ b/docs/project-layout.md @@ -0,0 +1,84 @@ +# GitHub CLI project layout + +At a high level, these areas make up the `github.com/cli/cli` project: +- [`cmd/`](../cmd) - `main` packages for building binaries such as the `gh` executable +- [`pkg/`](../pkg) - most other packages, including the implementation for individual gh commands +- [`docs/`](../docs) - documentation for maintainers and contributors +- [`script/`](../script) - build and release scripts +- [`internal/`](../internal) - Go packages highly specific to our needs and thus internal +- [`go.mod`](../go.mod) - external Go dependencies for this project, automatically fetched by Go at build time + +Some auxiliary Go packages are at the top level of the project for historical reasons: +- [`api/`](../api) - main utilities for making requests to the GitHub API +- [`context/`](../context) - DEPRECATED: use only for referencing git remotes +- [`git/`](../git) - utilities to gather information from a local git repository +- [`test/`](../test) - DEPRECATED: do not use +- [`utils/`](../utils) - DEPRECATED: use only for printing table output + +## Command-line help text + +Running `gh help issue list` displays help text for a topic. In this case, the topic is a specific command, +and help text for every command is embedded in that command's source code. The naming convention for gh +commands is: +``` +pkg/cmd///.go +``` +Following the above example, the main implementation for the `gh issue list` command, including its help +text, is in [pkg/cmd/issue/view/view.go](../pkg/cmd/issue/view/view.go) + +Other help topics not specific to any command, for example `gh help environment`, are found in +[pkg/cmd/root/help_topic.go](../pkg/cmd/root/help_topic.go). + +During our release process, these help topics are [automatically converted](../cmd/gen-docs/main.go) to +manual pages and published under https://cli.github.com/manual/. + +## How GitHub CLI works + +To illustrate how GitHub CLI works in its typical mode of operation, let's build the project, run a command, +and talk through which code gets run in order. + +1. `go run script/build.go` - Makes sure all external Go depedencies are fetched, then compiles the + `cmd/gh/main.go` file into a `bin/gh` binary. +2. `bin/gh issue list --limit 5` - Runs the newly built `bin/gh` binary (note: on Windows you must use + backslashes like `bin\gh`) and passes the following arguments to the process: `["issue", "list", "--limit", "5"]`. +3. `func main()` inside `cmd/gh/main.go` is the first Go function that runs. The arguments passed to the + process are available through `os.Args`. +4. The `main` package initializes the "root" command with `root.NewCmdRoot()` and dispatches execution to it + with `rootCmd.ExecuteC()`. +5. The [root command](../pkg/cmd/root/root.go) represents the top-level `gh` command and knows how to + dispatch execution to any other gh command nested under it. +6. Based on `["issue", "list"]` arguments, the execution reaches the `RunE` block of the `cobra.Command` + within [pkg/cmd/issue/list/list.go](../pkg/cmd/issue/list/list.go). +7. The `--limit 5` flag originally passed as arguments be automatically parsed and its value stored as + `opts.LimitResults`. +8. `func listRun()` is called, which is responsible for implementing the logic of the `gh issue list` command. +9. The command collects information from sources like the GitHub API then writes the final output to + standard output and standard error [streams](../pkg/iostreams/iostreams.go) available at `opts.IO`. +10. The program execution is now back at `func main()` of `cmd/gh/main.go`. If there were any Go errors as a + result of processing the command, the function will abort the process with a non-zero exit status. + Otherwise, the process ends with status 0 indicating success. + +## How to add a new command + +0. First, check on our issue tracker to verify that our team had approved the plans for a new command. +1. Create a package for the new command, e.g. for a new command `gh boom` create the following directory + structure: `pkg/cmd/boom/` +2. The new package should expose a method, e.g. `NewCmdBoom()`, that accepts a `*cmdutil.Factory` type and + returns a `*cobra.Command`. + * Any logic specific to this command should be kept within the command's package and not added to any + "global" packages like `api` or `utils`. +3. Use the method from the previous step to generate the command and add it to the command tree, typically + somewhere in the `NewCmdRoot()` method. + +## How to write tests + +This task might be tricky. Typically, gh commands do things like look up information from the git repository +in the current directory, query the GitHub API, scan the user's `~/.ssh/config` file, clone or fetch git +repositories, etc. Naturally, none of these things should ever happen for real when running tests, unless +you are sure that any filesystem operations are stricly scoped to a location made for and maintained by the +test itself. To avoid actually running things like making real API requests or shelling out to `git` +commands, we stub them. You should look at how that's done within some existing tests. + +To make your code testable, write small, isolated pieces of functionality that are designed to be composed +together. Prefer table-driven tests for maintaining variations of different test inputs and expectations +when exercising a single piece of functionality. From fddc888a69a8ab6135dc0ec0516df670bed363b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 12 May 2021 16:56:52 +0200 Subject: [PATCH 11/20] Fix "null" display in colored JSON output "null" was previously rendered in "bright black", an ANSI color that is not guaranteed to be visible at all depending on the terminal. Switch the color to cyan to ensure that "null" is visible. --- pkg/jsoncolor/jsoncolor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/jsoncolor/jsoncolor.go b/pkg/jsoncolor/jsoncolor.go index cf3bd064c..d7c808a14 100644 --- a/pkg/jsoncolor/jsoncolor.go +++ b/pkg/jsoncolor/jsoncolor.go @@ -10,7 +10,7 @@ import ( const ( colorDelim = "1;38" // bright white colorKey = "1;34" // bright blue - colorNull = "1;30" // gray + colorNull = "36" // cyan colorString = "32" // green colorBool = "33" // yellow ) From 5f0301c990bcac47cfdb36273032050da40fcaca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 11 May 2021 18:25:20 +0200 Subject: [PATCH 12/20] Have Exporter.Write automatically call ExportData on given data structure --- api/export_pr.go | 20 ++------------- api/export_pr_test.go | 25 ------------------ pkg/cmd/issue/list/list.go | 3 +-- pkg/cmd/issue/status/status.go | 8 +++--- pkg/cmd/issue/view/view.go | 3 +-- pkg/cmd/pr/list/list.go | 3 +-- pkg/cmd/pr/status/status.go | 8 +++--- pkg/cmd/pr/view/view.go | 3 +-- pkg/cmdutil/json_flags.go | 47 +++++++++++++++++++++++++++++++++- pkg/cmdutil/json_flags_test.go | 38 ++++++++++++++++++++++++++- 10 files changed, 97 insertions(+), 61 deletions(-) diff --git a/api/export_pr.go b/api/export_pr.go index dc70ee4fa..8939d9e80 100644 --- a/api/export_pr.go +++ b/api/export_pr.go @@ -6,6 +6,7 @@ import ( ) func (issue *Issue) ExportData(fields []string) *map[string]interface{} { + v := reflect.ValueOf(issue).Elem() data := map[string]interface{}{} for _, f := range fields { @@ -25,7 +26,6 @@ func (issue *Issue) ExportData(fields []string) *map[string]interface{} { case "projectCards": data[f] = issue.ProjectCards.Nodes default: - v := reflect.ValueOf(issue).Elem() sf := fieldByName(v, f) data[f] = sf.Interface() } @@ -35,6 +35,7 @@ func (issue *Issue) ExportData(fields []string) *map[string]interface{} { } func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { + v := reflect.ValueOf(pr).Elem() data := map[string]interface{}{} for _, f := range fields { @@ -75,7 +76,6 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { } data[f] = &requests default: - v := reflect.ValueOf(pr).Elem() sf := fieldByName(v, f) data[f] = sf.Interface() } @@ -84,22 +84,6 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { return &data } -func ExportIssues(issues []Issue, fields []string) *[]interface{} { - data := make([]interface{}, len(issues)) - for i := range issues { - data[i] = issues[i].ExportData(fields) - } - return &data -} - -func ExportPRs(prs []PullRequest, fields []string) *[]interface{} { - data := make([]interface{}, len(prs)) - for i := range prs { - data[i] = prs[i].ExportData(fields) - } - return &data -} - func fieldByName(v reflect.Value, field string) reflect.Value { return v.FieldByNameFunc(func(s string) bool { return strings.EqualFold(field, s) diff --git a/api/export_pr_test.go b/api/export_pr_test.go index eff3c157d..4e02f9f09 100644 --- a/api/export_pr_test.go +++ b/api/export_pr_test.go @@ -90,31 +90,6 @@ func TestIssue_ExportData(t *testing.T) { } } -func TestExportIssues(t *testing.T) { - issues := []Issue{ - {Milestone: Milestone{Title: "hi"}}, - {}, - } - exported := ExportIssues(issues, []string{"milestone"}) - - buf := bytes.Buffer{} - enc := json.NewEncoder(&buf) - enc.SetIndent("", "\t") - require.NoError(t, enc.Encode(exported)) - assert.Equal(t, heredoc.Doc(` - [ - { - "milestone": { - "title": "hi" - } - }, - { - "milestone": null - } - ] - `), buf.String()) -} - func TestPullRequest_ExportData(t *testing.T) { tests := []struct { name string diff --git a/pkg/cmd/issue/list/list.go b/pkg/cmd/issue/list/list.go index ff3aba976..c543a84dc 100644 --- a/pkg/cmd/issue/list/list.go +++ b/pkg/cmd/issue/list/list.go @@ -155,8 +155,7 @@ func listRun(opts *ListOptions) error { defer opts.IO.StopPager() if opts.Exporter != nil { - data := api.ExportIssues(listResult.Issues, opts.Exporter.Fields()) - return opts.Exporter.Write(opts.IO.Out, data, opts.IO.ColorEnabled()) + return opts.Exporter.Write(opts.IO.Out, listResult.Issues, opts.IO.ColorEnabled()) } if isTerminal { diff --git a/pkg/cmd/issue/status/status.go b/pkg/cmd/issue/status/status.go index 764ec52fa..2a8c97ce8 100644 --- a/pkg/cmd/issue/status/status.go +++ b/pkg/cmd/issue/status/status.go @@ -96,11 +96,11 @@ func statusRun(opts *StatusOptions) error { if opts.Exporter != nil { data := map[string]interface{}{ - "createdBy": api.ExportIssues(issuePayload.Authored.Issues, opts.Exporter.Fields()), - "assigned": api.ExportIssues(issuePayload.Assigned.Issues, opts.Exporter.Fields()), - "mentioned": api.ExportIssues(issuePayload.Mentioned.Issues, opts.Exporter.Fields()), + "createdBy": issuePayload.Authored.Issues, + "assigned": issuePayload.Assigned.Issues, + "mentioned": issuePayload.Mentioned.Issues, } - return opts.Exporter.Write(opts.IO.Out, &data, opts.IO.ColorEnabled()) + return opts.Exporter.Write(opts.IO.Out, data, opts.IO.ColorEnabled()) } out := opts.IO.Out diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index e90c6a528..6888cc791 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -116,8 +116,7 @@ func viewRun(opts *ViewOptions) error { defer opts.IO.StopPager() if opts.Exporter != nil { - exportIssue := issue.ExportData(opts.Exporter.Fields()) - return opts.Exporter.Write(opts.IO.Out, exportIssue, opts.IO.ColorEnabled()) + return opts.Exporter.Write(opts.IO.Out, issue, opts.IO.ColorEnabled()) } if opts.IO.IsStdoutTTY() { diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go index f0ccfe11a..faf7a3e24 100644 --- a/pkg/cmd/pr/list/list.go +++ b/pkg/cmd/pr/list/list.go @@ -155,8 +155,7 @@ func listRun(opts *ListOptions) error { defer opts.IO.StopPager() if opts.Exporter != nil { - data := api.ExportPRs(listResult.PullRequests, opts.Exporter.Fields()) - return opts.Exporter.Write(opts.IO.Out, data, opts.IO.ColorEnabled()) + return opts.Exporter.Write(opts.IO.Out, listResult.PullRequests, opts.IO.ColorEnabled()) } if opts.IO.IsStdoutTTY() { diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go index 115ee035b..d14ae5ec2 100644 --- a/pkg/cmd/pr/status/status.go +++ b/pkg/cmd/pr/status/status.go @@ -113,13 +113,13 @@ func statusRun(opts *StatusOptions) error { if opts.Exporter != nil { data := map[string]interface{}{ "currentBranch": nil, - "createdBy": api.ExportPRs(prPayload.ViewerCreated.PullRequests, opts.Exporter.Fields()), - "needsReview": api.ExportPRs(prPayload.ReviewRequested.PullRequests, opts.Exporter.Fields()), + "createdBy": prPayload.ViewerCreated.PullRequests, + "needsReview": prPayload.ReviewRequested.PullRequests, } if prPayload.CurrentPR != nil { - data["currentBranch"] = prPayload.CurrentPR.ExportData(opts.Exporter.Fields()) + data["currentBranch"] = prPayload.CurrentPR } - return opts.Exporter.Write(opts.IO.Out, &data, opts.IO.ColorEnabled()) + return opts.Exporter.Write(opts.IO.Out, data, opts.IO.ColorEnabled()) } out := opts.IO.Out diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index 4e6300297..184892ec5 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -117,8 +117,7 @@ func viewRun(opts *ViewOptions) error { defer opts.IO.StopPager() if opts.Exporter != nil { - exportPR := pr.ExportData(opts.Exporter.Fields()) - return opts.Exporter.Write(opts.IO.Out, exportPR, opts.IO.ColorEnabled()) + return opts.Exporter.Write(opts.IO.Out, pr, opts.IO.ColorEnabled()) } if connectedToTerminal { diff --git a/pkg/cmdutil/json_flags.go b/pkg/cmdutil/json_flags.go index 7b783c3ee..914c5024f 100644 --- a/pkg/cmdutil/json_flags.go +++ b/pkg/cmdutil/json_flags.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "reflect" "sort" "strings" @@ -102,11 +103,14 @@ func (e *exportFormat) Fields() []string { return e.fields } +// Write serializes data into JSON output written to w. If the object passed as data implements exportable, +// or if data is a map or slice of exportable object, ExportData() will be called on each object to obtain +// raw data for serialization. func (e *exportFormat) Write(w io.Writer, data interface{}, colorEnabled bool) error { buf := bytes.Buffer{} encoder := json.NewEncoder(&buf) encoder.SetEscapeHTML(false) - if err := encoder.Encode(data); err != nil { + if err := encoder.Encode(e.exportData(reflect.ValueOf(data))); err != nil { return err } @@ -121,3 +125,44 @@ func (e *exportFormat) Write(w io.Writer, data interface{}, colorEnabled bool) e _, err := io.Copy(w, &buf) return err } + +func (e *exportFormat) exportData(v reflect.Value) interface{} { + switch v.Kind() { + case reflect.Ptr, reflect.Interface: + if !v.IsNil() { + return e.exportData(v.Elem()) + } + case reflect.Slice: + a := make([]interface{}, v.Len()) + for i := 0; i < v.Len(); i++ { + a[i] = e.exportData(v.Index(i)) + } + return a + case reflect.Map: + t := reflect.MapOf(v.Type().Key(), emptyInterfaceType) + m := reflect.MakeMapWithSize(t, v.Len()) + iter := v.MapRange() + for iter.Next() { + ve := reflect.ValueOf(e.exportData(iter.Value())) + m.SetMapIndex(iter.Key(), ve) + } + return m.Interface() + case reflect.Struct: + if v.CanAddr() && reflect.PtrTo(v.Type()).Implements(exportableType) { + ve := v.Addr().Interface().(exportable) + return ve.ExportData(e.fields) + } else if v.Type().Implements(exportableType) { + ve := v.Interface().(exportable) + return ve.ExportData(e.fields) + } + } + return v.Interface() +} + +type exportable interface { + ExportData([]string) *map[string]interface{} +} + +var exportableType = reflect.TypeOf((*exportable)(nil)).Elem() +var sliceOfEmptyInterface []interface{} +var emptyInterfaceType = reflect.TypeOf(sliceOfEmptyInterface).Elem() diff --git a/pkg/cmdutil/json_flags_test.go b/pkg/cmdutil/json_flags_test.go index 61780db96..84825e30a 100644 --- a/pkg/cmdutil/json_flags_test.go +++ b/pkg/cmdutil/json_flags_test.go @@ -2,6 +2,7 @@ package cmdutil import ( "bytes" + "fmt" "io/ioutil" "testing" @@ -137,6 +138,29 @@ func Test_exportFormat_Write(t *testing.T) { wantW: "{\"name\":\"hubot\"}\n", wantErr: false, }, + { + name: "call ExportData", + exporter: exportFormat{fields: []string{"field1", "field2"}}, + args: args{ + data: &exportableItem{"item1"}, + colorEnabled: false, + }, + wantW: "{\"field1\":\"item1:field1\",\"field2\":\"item1:field2\"}\n", + wantErr: false, + }, + { + name: "recursively call ExportData", + exporter: exportFormat{fields: []string{"f1", "f2"}}, + args: args{ + data: map[string]interface{}{ + "s1": []exportableItem{{"i1"}, {"i2"}}, + "s2": []exportableItem{{"i3"}}, + }, + colorEnabled: false, + }, + wantW: "{\"s1\":[{\"f1\":\"i1:f1\",\"f2\":\"i1:f2\"},{\"f1\":\"i2:f1\",\"f2\":\"i2:f2\"}],\"s2\":[{\"f1\":\"i3:f1\",\"f2\":\"i3:f2\"}]}\n", + wantErr: false, + }, { name: "with jq filter", exporter: exportFormat{filter: ".name"}, @@ -166,8 +190,20 @@ func Test_exportFormat_Write(t *testing.T) { return } if gotW := w.String(); gotW != tt.wantW { - t.Errorf("exportFormat.Write() = %v, want %v", gotW, tt.wantW) + t.Errorf("exportFormat.Write() = %q, want %q", gotW, tt.wantW) } }) } } + +type exportableItem struct { + Name string +} + +func (e *exportableItem) ExportData(fields []string) *map[string]interface{} { + m := map[string]interface{}{} + for _, f := range fields { + m[f] = fmt.Sprintf("%s:%s", e.Name, f) + } + return &m +} From 02a2ed2f73789e56963e46a52b07a3ec8a840c72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 11 May 2021 20:00:36 +0200 Subject: [PATCH 13/20] Add `repo view --json` export functionality --- api/export_pr.go | 4 +- api/export_repo.go | 53 +++++++++ api/queries_issue.go | 5 +- api/queries_repo.go | 189 ++++++++++++++++++++++++++++----- api/queries_repo_test.go | 6 +- api/query_builder.go | 130 +++++++++++++++++++++++ context/context.go | 2 +- pkg/cmd/repo/view/http.go | 19 ++++ pkg/cmd/repo/view/view.go | 35 ++++-- pkg/cmd/repo/view/view_test.go | 50 +++++++++ 10 files changed, 447 insertions(+), 46 deletions(-) create mode 100644 api/export_repo.go diff --git a/api/export_pr.go b/api/export_pr.go index 8939d9e80..9a0e10702 100644 --- a/api/export_pr.go +++ b/api/export_pr.go @@ -13,7 +13,7 @@ func (issue *Issue) ExportData(fields []string) *map[string]interface{} { switch f { case "milestone": if issue.Milestone.Title != "" { - data[f] = &issue.Milestone + data[f] = map[string]string{"title": issue.Milestone.Title} } else { data[f] = nil } @@ -44,7 +44,7 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { data[f] = map[string]string{"name": pr.HeadRepository.Name} case "milestone": if pr.Milestone.Title != "" { - data[f] = &pr.Milestone + data[f] = map[string]string{"title": pr.Milestone.Title} } else { data[f] = nil } diff --git a/api/export_repo.go b/api/export_repo.go new file mode 100644 index 000000000..8d4e669ad --- /dev/null +++ b/api/export_repo.go @@ -0,0 +1,53 @@ +package api + +import ( + "reflect" +) + +func (repo *Repository) ExportData(fields []string) *map[string]interface{} { + v := reflect.ValueOf(repo).Elem() + data := map[string]interface{}{} + + for _, f := range fields { + switch f { + case "parent": + data[f] = miniRepoExport(repo.Parent) + case "templateRepository": + data[f] = miniRepoExport(repo.TemplateRepository) + case "languages": + data[f] = repo.Languages.Edges + case "labels": + data[f] = repo.Labels.Nodes + case "assignableUsers": + data[f] = repo.AssignableUsers.Nodes + case "mentionableUsers": + data[f] = repo.MentionableUsers.Nodes + case "milestones": + data[f] = repo.Milestones.Nodes + case "projects": + data[f] = repo.Projects.Nodes + case "repositoryTopics": + var topics []RepositoryTopic + for _, n := range repo.RepositoryTopics.Nodes { + topics = append(topics, n.Topic) + } + data[f] = topics + default: + sf := fieldByName(v, f) + data[f] = sf.Interface() + } + } + + return &data +} + +func miniRepoExport(r *Repository) map[string]interface{} { + if r == nil { + return nil + } + return map[string]interface{}{ + "id": r.ID, + "name": r.Name, + "owner": r.Owner, + } +} diff --git a/api/queries_issue.go b/api/queries_issue.go index 2dfab5742..738f36ff7 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -91,7 +91,10 @@ func (p ProjectCards) ProjectNames() []string { } type Milestone struct { - Title string `json:"title"` + Number int `json:"number"` + Title string `json:"title"` + Description string `json:"description"` + DueOn *time.Time `json:"dueOn"` } type IssuesDisabledError struct { diff --git a/api/queries_repo.go b/api/queries_repo.go index 90d3949a5..c4f285c24 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -16,25 +16,100 @@ import ( // Repository contains information about a GitHub repo type Repository struct { - ID string - Name string - Description string - URL string - CloneURL string - CreatedAt time.Time - Owner RepositoryOwner + ID string + Name string + NameWithOwner string + Owner RepositoryOwner + Parent *Repository + TemplateRepository *Repository + Description string + HomepageURL string + OpenGraphImageURL string + UsesCustomOpenGraphImage bool + URL string + SSHURL string + MirrorURL string + SecurityPolicyURL string - IsPrivate bool - HasIssuesEnabled bool - HasWikiEnabled bool - ViewerPermission string - DefaultBranchRef BranchRef + CreatedAt time.Time + PushedAt *time.Time + UpdatedAt time.Time - Parent *Repository + IsBlankIssuesEnabled bool + IsSecurityPolicyEnabled bool + HasIssuesEnabled bool + HasProjectsEnabled bool + HasWikiEnabled bool + MergeCommitAllowed bool + SquashMergeAllowed bool + RebaseMergeAllowed bool - MergeCommitAllowed bool - RebaseMergeAllowed bool - SquashMergeAllowed bool + ForkCount int + StargazerCount int + Watchers struct { + TotalCount int `json:"totalCount"` + } + Issues struct { + TotalCount int `json:"totalCount"` + } + PullRequests struct { + TotalCount int `json:"totalCount"` + } + + CodeOfConduct *CodeOfConduct + ContactLinks []ContactLink + DefaultBranchRef BranchRef + DeleteBranchOnMerge bool + DiskUsage int + FundingLinks []FundingLink + IsArchived bool + IsEmpty bool + IsFork bool + IsInOrganization bool + IsMirror bool + IsPrivate bool + IsTemplate bool + IsUserConfigurationRepository bool + LicenseInfo *RepositoryLicense + ViewerCanAdminister bool + ViewerDefaultCommitEmail string + ViewerDefaultMergeMethod string + ViewerHasStarred bool + ViewerPermission string + ViewerPossibleCommitEmails []string + ViewerSubscription string + + RepositoryTopics struct { + Nodes []struct { + Topic RepositoryTopic + } + } + PrimaryLanguage *CodingLanguage + Languages struct { + Edges []struct { + Size int `json:"size"` + Node CodingLanguage `json:"node"` + } + } + IssueTemplates []IssueTemplate + PullRequestTemplates []PullRequestTemplate + Labels struct { + Nodes []IssueLabel + } + Milestones struct { + Nodes []Milestone + } + LatestRelease *RepositoryRelease + + AssignableUsers struct { + Nodes []GitHubUser + } + MentionableUsers struct { + Nodes []GitHubUser + } + Projects struct { + Nodes []RepoProject + } // pseudo-field that keeps track of host name of this repo hostname string @@ -42,12 +117,76 @@ type Repository struct { // RepositoryOwner is the owner of a GitHub repository type RepositoryOwner struct { - Login string + ID string `json:"id"` + Login string `json:"login"` +} + +type GitHubUser struct { + ID string `json:"id"` + Login string `json:"login"` + Name string `json:"name"` } // BranchRef is the branch name in a GitHub repository type BranchRef struct { - Name string + Name string `json:"name"` +} + +type CodeOfConduct struct { + Key string `json:"key"` + Name string `json:"name"` + URL string `json:"url"` +} + +type RepositoryLicense struct { + Key string `json:"key"` + Name string `json:"name"` + Nickname string `json:"nickname"` +} + +type ContactLink struct { + About string `json:"about"` + Name string `json:"name"` + URL string `json:"url"` +} + +type FundingLink struct { + Platform string `json:"platform"` + URL string `json:"url"` +} + +type CodingLanguage struct { + Name string `json:"name"` +} + +type IssueTemplate struct { + Name string `json:"name"` + Title string `json:"title"` + Body string `json:"body"` + About string `json:"about"` +} + +type PullRequestTemplate struct { + Filename string `json:"filename"` + Body string `json:"body"` +} + +type RepositoryTopic struct { + Name string `json:"name"` +} + +type RepositoryRelease struct { + Name string `json:"name"` + TagName string `json:"tagName"` + URL string `json:"url"` + PublishedAt time.Time `json:"publishedAt"` +} + +type IssueLabel struct { + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + Color string `json:"color"` } // RepoOwner is the login name of the owner @@ -65,11 +204,6 @@ func (r Repository) RepoHost() string { return r.hostname } -// IsFork is true when this repository has a parent repository -func (r Repository) IsFork() bool { - return r.Parent != nil -} - // ViewerCanPush is true when the requesting user has push access func (r Repository) ViewerCanPush() bool { switch r.ViewerPermission { @@ -305,7 +439,6 @@ type repositoryV3 struct { NodeID string Name string CreatedAt time.Time `json:"created_at"` - CloneURL string `json:"clone_url"` Owner struct { Login string } @@ -324,7 +457,6 @@ func ForkRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { return &Repository{ ID: result.NodeID, Name: result.Name, - CloneURL: result.CloneURL, CreatedAt: result.CreatedAt, Owner: RepositoryOwner{ Login: result.Owner.Login, @@ -707,9 +839,10 @@ func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoRes } type RepoProject struct { - ID string - Name string - ResourcePath string + ID string `json:"id"` + Name string `json:"name"` + Number int `json:"number"` + ResourcePath string `json:"resourcePath"` } // RepoProjects fetches all open projects for a repository diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index a9d535e6e..50a2067f1 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -144,9 +144,9 @@ func Test_RepoMetadata(t *testing.T) { func Test_ProjectsToPaths(t *testing.T) { expectedProjectPaths := []string{"OWNER/REPO/PROJECT_NUMBER", "ORG/PROJECT_NUMBER"} projects := []RepoProject{ - {"id1", "My Project", "/OWNER/REPO/projects/PROJECT_NUMBER"}, - {"id2", "Org Project", "/orgs/ORG/projects/PROJECT_NUMBER"}, - {"id3", "Project", "/orgs/ORG/projects/PROJECT_NUMBER_2"}, + {ID: "id1", Name: "My Project", ResourcePath: "/OWNER/REPO/projects/PROJECT_NUMBER"}, + {ID: "id2", Name: "Org Project", ResourcePath: "/orgs/ORG/projects/PROJECT_NUMBER"}, + {ID: "id3", Name: "Project", ResourcePath: "/orgs/ORG/projects/PROJECT_NUMBER_2"}, } projectNames := []string{"My Project", "Org Project"} diff --git a/api/query_builder.go b/api/query_builder.go index 84d9d8f91..25862e179 100644 --- a/api/query_builder.go +++ b/api/query_builder.go @@ -186,3 +186,133 @@ func PullRequestGraphQL(fields []string) string { } return strings.Join(q, ",") } + +var RepositoryFields = []string{ + "id", + "name", + "nameWithOwner", + "owner", + "parent", + "templateRepository", + "description", + "homepageUrl", + "openGraphImageUrl", + "usesCustomOpenGraphImage", + "url", + "sshUrl", + "mirrorUrl", + "securityPolicyUrl", + + "createdAt", + "pushedAt", + "updatedAt", + + "isBlankIssuesEnabled", + "isSecurityPolicyEnabled", + "hasIssuesEnabled", + "hasProjectsEnabled", + "hasWikiEnabled", + "mergeCommitAllowed", + "squashMergeAllowed", + "rebaseMergeAllowed", + + "forkCount", + "stargazerCount", + "watchers", + "issues", + "pullRequests", + + "codeOfConduct", + "contactLinks", + "defaultBranchRef", + "deleteBranchOnMerge", + "diskUsage", + "fundingLinks", + "isArchived", + "isEmpty", + "isFork", + "isInOrganization", + "isMirror", + "isPrivate", + "isTemplate", + "isUserConfigurationRepository", + "licenseInfo", + "viewerCanAdminister", + "viewerDefaultCommitEmail", + "viewerDefaultMergeMethod", + "viewerHasStarred", + "viewerPermission", + "viewerPossibleCommitEmails", + "viewerSubscription", + + "repositoryTopics", + "primaryLanguage", + "languages", + "issueTemplates", + "pullRequestTemplates", + "labels", + "milestones", + "latestRelease", + + "assignableUsers", + "mentionableUsers", + "projects", + + // "branchProtectionRules", // too complex to expose + // "collaborators", // does it make sense to expose without affiliation filter? +} + +func RepositoryGraphQL(fields []string) string { + var q []string + for _, field := range fields { + switch field { + case "codeOfConduct": + q = append(q, "codeOfConduct{key,name,url}") + case "contactLinks": + q = append(q, "contactLinks{about,name,url}") + case "fundingLinks": + q = append(q, "fundingLinks{platform,url}") + case "licenseInfo": + q = append(q, "licenseInfo{key,name,nickname}") + case "owner": + q = append(q, "owner{id,login}") + case "parent": + q = append(q, "parent{id,name,owner{id,login}}") + case "templateRepository": + q = append(q, "templateRepository{id,name,owner{id,login}}") + case "repositoryTopics": + q = append(q, "repositoryTopics(first:100){nodes{topic{name}}}") + case "issueTemplates": + q = append(q, "issueTemplates{name,title,body,about}") + case "pullRequestTemplates": + q = append(q, "pullRequestTemplates{body,filename}") + case "labels": + q = append(q, "labels(first:100){nodes{id,color,name,description}}") + case "languages": + q = append(q, "languages(first:100){edges{size,node{name}}}") + case "primaryLanguage": + q = append(q, "primaryLanguage{name}") + case "latestRelease": + q = append(q, "latestRelease{publishedAt,tagName,name,url}") + case "milestones": + q = append(q, "milestones(first:100,states:OPEN){nodes{number,title,description,dueOn}}") + case "assignableUsers": + q = append(q, "assignableUsers(first:100){nodes{id,login,name}}") + case "mentionableUsers": + q = append(q, "mentionableUsers(first:100){nodes{id,login,name}}") + case "projects": + q = append(q, "projects(first:100,states:OPEN){nodes{id,name,number,body,resourcePath}}") + case "watchers": + q = append(q, "watchers{totalCount}") + case "issues": + q = append(q, "issues(states:OPEN){totalCount}") + case "pullRequests": + q = append(q, "pullRequests(states:OPEN){totalCount}") + case "defaultBranchRef": + q = append(q, "defaultBranchRef{name}") + default: + q = append(q, field) + } + } + return strings.Join(q, ",") +} diff --git a/context/context.go b/context/context.go index babd51f55..4c1a64c73 100644 --- a/context/context.go +++ b/context/context.go @@ -104,7 +104,7 @@ func (r *ResolvedRemotes) BaseRepo(io *iostreams.IOStreams) (ghrepo.Interface, e if repo == nil { continue } - if repo.IsFork() { + if repo.Parent != nil { add(repo.Parent) } add(repo) diff --git a/pkg/cmd/repo/view/http.go b/pkg/cmd/repo/view/http.go index 24eaaa676..3ef1dc784 100644 --- a/pkg/cmd/repo/view/http.go +++ b/pkg/cmd/repo/view/http.go @@ -12,6 +12,25 @@ import ( var NotFoundError = errors.New("not found") +func fetchRepository(apiClient *api.Client, repo ghrepo.Interface, fields []string) (*api.Repository, error) { + query := fmt.Sprintf(`query RepositoryInfo($owner: String!, $name: String!) { + repository(owner: $owner, name: $name) {%s} + }`, api.RepositoryGraphQL(fields)) + + variables := map[string]interface{}{ + "owner": repo.RepoOwner(), + "name": repo.RepoName(), + } + + var result struct { + Repository api.Repository + } + if err := apiClient.GraphQL(repo.RepoHost(), query, variables, &result); err != nil { + return nil, err + } + return api.InitRepoHostname(&result.Repository, repo.RepoHost()), nil +} + type RepoReadme struct { Filename string Content string diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index fcc785e62..7e507c182 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -29,6 +29,7 @@ type ViewOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) Browser browser + Exporter cmdutil.Exporter RepoArg string Web bool @@ -67,10 +68,13 @@ With '--branch', view a specific branch of the repository.`, cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open a repository in the browser") cmd.Flags().StringVarP(&opts.Branch, "branch", "b", "", "View a specific branch of the repository") + cmdutil.AddJSONFlags(cmd, &opts.Exporter, api.RepositoryFields) return cmd } +var defaultFields = []string{"name", "owner", "description"} + func viewRun(opts *ViewOptions) error { httpClient, err := opts.HttpClient() if err != nil { @@ -101,11 +105,24 @@ func viewRun(opts *ViewOptions) error { } } - repo, err := api.GitHubRepo(apiClient, toView) + var readme *RepoReadme + fields := defaultFields + if opts.Exporter != nil { + fields = opts.Exporter.Fields() + } + + repo, err := fetchRepository(apiClient, toView, fields) if err != nil { return err } + if !opts.Web && opts.Exporter == nil { + readme, err = RepositoryReadme(httpClient, toView, opts.Branch) + if err != nil && !errors.Is(err, NotFoundError) { + return err + } + } + openURL := generateBranchURL(toView, opts.Branch) if opts.Web { if opts.IO.IsStdoutTTY() { @@ -114,21 +131,17 @@ func viewRun(opts *ViewOptions) error { return opts.Browser.Browse(openURL) } - fullName := ghrepo.FullName(toView) - - readme, err := RepositoryReadme(httpClient, toView, opts.Branch) - if err != nil && err != NotFoundError { - return err - } - opts.IO.DetectTerminalTheme() - - err = opts.IO.StartPager() - if err != nil { + if err := opts.IO.StartPager(); err != nil { return err } defer opts.IO.StopPager() + if opts.Exporter != nil { + return opts.Exporter.Write(opts.IO.Out, repo, opts.IO.ColorEnabled()) + } + + fullName := ghrepo.FullName(toView) stdout := opts.IO.Out if !opts.IO.IsStdoutTTY() { diff --git a/pkg/cmd/repo/view/view_test.go b/pkg/cmd/repo/view/view_test.go index fd91dcd70..b208b1f5a 100644 --- a/pkg/cmd/repo/view/view_test.go +++ b/pkg/cmd/repo/view/view_test.go @@ -3,10 +3,12 @@ package view import ( "bytes" "fmt" + "io" "net/http" "testing" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmdutil" @@ -625,3 +627,51 @@ func Test_ViewRun_HandlesSpecialCharacters(t *testing.T) { }) } } + +func Test_viewRun_json(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(false) + + reg := &httpmock.Registry{} + defer reg.Verify(t) + reg.StubRepoInfoResponse("OWNER", "REPO", "main") + + opts := &ViewOptions{ + IO: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Exporter: &testExporter{ + fields: []string{"name", "defaultBranchRef"}, + }, + } + + _, teardown := run.Stub() + defer teardown(t) + + err := viewRun(opts) + assert.NoError(t, err) + assert.Equal(t, heredoc.Doc(` + name: REPO + defaultBranchRef: main + `), stdout.String()) + assert.Equal(t, "", stderr.String()) +} + +type testExporter struct { + fields []string +} + +func (e *testExporter) Fields() []string { + return e.fields +} + +func (e *testExporter) Write(w io.Writer, data interface{}, colorize bool) error { + r := data.(*api.Repository) + fmt.Fprintf(w, "name: %s\n", r.Name) + fmt.Fprintf(w, "defaultBranchRef: %s\n", r.DefaultBranchRef.Name) + return nil +} From df2ae17b548a6c1e38190bafb3edbbe9d02475dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 12 May 2021 17:35:02 +0200 Subject: [PATCH 14/20] Bump Cobra to v1.1.3 --- go.mod | 2 +- go.sum | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index b44012c93..96b74584c 100644 --- a/go.mod +++ b/go.mod @@ -27,7 +27,7 @@ require ( github.com/rivo/uniseg v0.1.0 github.com/shurcooL/githubv4 v0.0.0-20200928013246-d292edc3691b github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f - github.com/spf13/cobra v1.1.1 + github.com/spf13/cobra v1.1.3 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.6.1 golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897 diff --git a/go.sum b/go.sum index 523a32a85..d1f1f19db 100644 --- a/go.sum +++ b/go.sum @@ -248,8 +248,8 @@ github.com/soheilhy/cmux v0.1.4/go.mod h1:IM3LyeVVIOuxMH7sFAkER9+bJ4dT7Ms6E4xg4k github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA= github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B0CQ= github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= -github.com/spf13/cobra v1.1.1 h1:KfztREH0tPxJJ+geloSLaAkaPkr4ki2Er5quFV1TDo4= -github.com/spf13/cobra v1.1.1/go.mod h1:WnodtKOvamDL/PwE2M4iKs8aMDBZ5Q5klgD3qfVJQMI= +github.com/spf13/cobra v1.1.3 h1:xghbfqPkxzxP3C/f3n5DdpAbdKLj4ZE4BWQI362l53M= +github.com/spf13/cobra v1.1.3/go.mod h1:pGADOWyqRD/YMrPZigI/zbliZ2wVD/23d+is3pSWzOo= github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo= github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= @@ -419,7 +419,7 @@ gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo= gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bli9HhUf9+ttbYbLASfIpnQbh74= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo= gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From b09c1f7a6f54651353e95a5fc59af9f1aab671d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 12 May 2021 17:35:17 +0200 Subject: [PATCH 15/20] Add shell completion for the `--json` flag --- pkg/cmdutil/json_flags.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pkg/cmdutil/json_flags.go b/pkg/cmdutil/json_flags.go index 7b783c3ee..e0abec093 100644 --- a/pkg/cmdutil/json_flags.go +++ b/pkg/cmdutil/json_flags.go @@ -26,6 +26,21 @@ func AddJSONFlags(cmd *cobra.Command, exportTarget *Exporter, fields []string) { f.StringP("jq", "q", "", "Filter JSON output using a jq `expression`") f.StringP("template", "t", "", "Format JSON output using a Go template") + _ = cmd.RegisterFlagCompletionFunc("json", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + var results []string + if idx := strings.IndexRune(toComplete, ','); idx >= 0 { + toComplete = toComplete[idx+1:] + } + toComplete = strings.ToLower(toComplete) + for _, f := range fields { + if strings.HasPrefix(strings.ToLower(f), toComplete) { + results = append(results, f) + } + } + sort.Strings(results) + return results, cobra.ShellCompDirectiveNoSpace + }) + oldPreRun := cmd.PreRunE cmd.PreRunE = func(c *cobra.Command, args []string) error { if oldPreRun != nil { From adbfb6e8deb49667376f53ec60b9bd21dde0658a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 17 May 2021 15:37:39 +0200 Subject: [PATCH 16/20] Merge pull request #3638 from cli/release-discussion Create a Release Discussion on every new release --- .github/workflows/releases.yml | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index ccb42c8df..4f593f36a 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -133,9 +133,11 @@ jobs: - name: Build MSI id: buildmsi shell: bash + env: + ZIP_FILE: ${{ steps.download_exe.outputs.zip }} run: | mkdir -p build - msi="$(basename "${{ steps.download_exe.outputs.zip }}" ".zip").msi" + msi="$(basename "$ZIP_FILE" ".zip").msi" printf "::set-output name=msi::%s\n" "$msi" go-msi make --msi "$PWD/$msi" --out "$PWD/build" --version "${GITHUB_REF#refs/tags/}" - name: Obtain signing cert @@ -145,14 +147,24 @@ jobs: run: .\script\setup-windows-certificate.ps1 - name: Sign MSI env: + CERT_FILE: ${{ steps.obtain_cert.outputs.cert-file }} + EXE_FILE: ${{ steps.buildmsi.outputs.msi }} GITHUB_CERT_PASSWORD: ${{ secrets.GITHUB_CERT_PASSWORD }} - run: | - .\script\sign.ps1 -Certificate "${{ steps.obtain_cert.outputs.cert-file }}" ` - -Executable "${{ steps.buildmsi.outputs.msi }}" + run: .\script\sign.ps1 -Certificate $env:CERT_FILE -Executable $env:EXE_FILE - name: Upload MSI shell: bash - run: hub release edit "${GITHUB_REF#refs/tags/}" -m "" --draft=false -a "${{ steps.buildmsi.outputs.msi }}" + run: | + tag_name="${GITHUB_REF#refs/tags/}" + hub release edit "$tag_name" -m "" -a "$MSI_FILE" + release_url="$(gh api repos/:owner/:repo/releases -q ".[]|select(.tag_name==\"${tag_name}\")|.url")" + publish_args=( -F draft=false ) + if [[ $GITHUB_REF != *-* ]]; then + publish_args+=( -f discussion_category_name="$DISCUSSION_CATEGORY" ) + fi + gh api -X PATCH "$release_url" "${publish_args[@]}" env: + MSI_FILE: ${{ steps.buildmsi.outputs.msi }} + DISCUSSION_CATEGORY: General GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} - name: Bump homebrew-core formula uses: mislav/bump-homebrew-formula-action@v1 From a2307e357dfa74423c8641794e728b1017bc03b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 17 May 2021 16:32:01 +0200 Subject: [PATCH 17/20] Add `repo list --json` support --- pkg/cmd/repo/list/http.go | 133 ++++++++++++++++---------------------- pkg/cmd/repo/list/list.go | 42 ++++++++++-- 2 files changed, 93 insertions(+), 82 deletions(-) diff --git a/pkg/cmd/repo/list/http.go b/pkg/cmd/repo/list/http.go index 90e31fa4a..1dbe372e4 100644 --- a/pkg/cmd/repo/list/http.go +++ b/pkg/cmd/repo/list/http.go @@ -1,48 +1,18 @@ package list import ( - "context" + "fmt" "net/http" - "reflect" "strings" - "time" - "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/api" "github.com/cli/cli/pkg/githubsearch" "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 + Repositories []api.Repository TotalCount int FromSearch bool } @@ -54,6 +24,7 @@ type FilterOptions struct { Language string Archived bool NonArchived bool + Fields []string } func listRepos(client *http.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { @@ -67,62 +38,65 @@ func listRepos(client *http.Client, hostname string, limit int, owner string, fi } variables := map[string]interface{}{ - "perPage": githubv4.Int(perPage), - "endCursor": (*githubv4.String)(nil), + "perPage": githubv4.Int(perPage), } 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) } + inputs := []string{"$perPage:Int!", "$endCursor:String", "$privacy:RepositoryPrivacy", "$fork:Boolean"} var ownerConnection string if owner == "" { - ownerConnection = `graphql:"repositoryOwner: viewer"` + ownerConnection = "repositoryOwner: viewer" } else { - ownerConnection = `graphql:"repositoryOwner(login: $owner)"` + ownerConnection = "repositoryOwner(login: $owner)" variables["owner"] = githubv4.String(owner) + inputs = append(inputs, "$owner:String!") } - type repositoryOwner struct { - Login string - Repositories struct { - Nodes []Repository - TotalCount int - PageInfo struct { - HasNextPage bool - EndCursor string + type result struct { + RepositoryOwner struct { + Login string + Repositories struct { + Nodes []api.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) + query := fmt.Sprintf(`query RepositoryList(%s) { + %s { + login + repositories(first: $perPage, after: $endCursor, privacy: $privacy, isFork: $fork, ownerAffiliations: OWNER, orderBy: { field: PUSHED_AT, direction: DESC }) { + nodes{%s} + totalCount + pageInfo{hasNextPage,endCursor} + } + } + }`, strings.Join(inputs, ","), ownerConnection, api.RepositoryGraphQL(filter.Fields)) + + apiClient := api.NewClientFromHTTP(client) listResult := RepositoryList{} pagination: for { - result := reflect.New(query) - err := gql.QueryNamed(context.Background(), "RepositoryList", result.Interface(), variables) + var res result + err := apiClient.GraphQL(hostname, query, variables, &res) if err != nil { return nil, err } - owner := result.Elem().FieldByName("RepositoryOwner").Interface().(repositoryOwner) + owner := res.RepositoryOwner listResult.TotalCount = owner.Repositories.TotalCount listResult.Owner = owner.Login @@ -143,47 +117,52 @@ pagination: } func searchRepos(client *http.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { - type query struct { + type result struct { Search struct { RepositoryCount int - Nodes []struct { - Repository Repository `graphql:"...on Repository"` - } - PageInfo struct { + Nodes []api.Repository + PageInfo struct { HasNextPage bool EndCursor string } - } `graphql:"search(type: REPOSITORY, query: $query, first: $perPage, after: $endCursor)"` + } } + query := fmt.Sprintf(`query RepositoryListSearch($query:String!,$perPage:Int!,$endCursor:String) { + search(type: REPOSITORY, query: $query, first: $perPage, after: $endCursor) { + repositoryCount + nodes{...on Repository{%s}} + pageInfo{hasNextPage,endCursor} + } + }`, api.RepositoryGraphQL(filter.Fields)) + 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), + "query": githubv4.String(searchQuery(owner, filter)), + "perPage": githubv4.Int(perPage), } - gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) + apiClient := api.NewClientFromHTTP(client) listResult := RepositoryList{FromSearch: true} pagination: for { - var result query - err := gql.QueryNamed(context.Background(), "RepositoryListSearch", &result, variables) + var result result + err := apiClient.GraphQL(hostname, query, variables, &result) if err != nil { return nil, err } listResult.TotalCount = result.Search.RepositoryCount - for _, node := range result.Search.Nodes { + for _, repo := range result.Search.Nodes { if listResult.Owner == "" { - idx := strings.IndexRune(node.Repository.NameWithOwner, '/') - listResult.Owner = node.Repository.NameWithOwner[:idx] + idx := strings.IndexRune(repo.NameWithOwner, '/') + listResult.Owner = repo.NameWithOwner[:idx] } - listResult.Repositories = append(listResult.Repositories, node.Repository) + listResult.Repositories = append(listResult.Repositories, repo) if len(listResult.Repositories) >= limit { break pagination } diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go index 5b0af46f0..3f1bf64f3 100644 --- a/pkg/cmd/repo/list/list.go +++ b/pkg/cmd/repo/list/list.go @@ -3,8 +3,10 @@ package list import ( "fmt" "net/http" + "strings" "time" + "github.com/cli/cli/api" "github.com/cli/cli/internal/config" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" @@ -17,6 +19,7 @@ type ListOptions struct { HttpClient func() (*http.Client, error) Config func() (config.Config, error) IO *iostreams.IOStreams + Exporter cmdutil.Exporter Limit int Owner string @@ -88,10 +91,13 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman 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") + cmdutil.AddJSONFlags(cmd, &opts.Exporter, api.RepositoryFields) return cmd } +var defaultFields = []string{"nameWithOwner", "description", "isPrivate", "isFork", "isArchived", "createdAt", "pushedAt"} + func listRun(opts *ListOptions) error { httpClient, err := opts.HttpClient() if err != nil { @@ -105,6 +111,10 @@ func listRun(opts *ListOptions) error { Language: opts.Language, Archived: opts.Archived, NonArchived: opts.NonArchived, + Fields: defaultFields, + } + if opts.Exporter != nil { + filter.Fields = opts.Exporter.Fields() } cfg, err := opts.Config() @@ -127,27 +137,31 @@ func listRun(opts *ListOptions) error { } defer opts.IO.StopPager() + if opts.Exporter != nil { + return opts.Exporter.Write(opts.IO.Out, listResult.Repositories, opts.IO.ColorEnabled()) + } + cs := opts.IO.ColorScheme() tp := utils.NewTablePrinter(opts.IO) now := opts.Now() for _, repo := range listResult.Repositories { - info := repo.Info() + info := repoInfo(repo) infoColor := cs.Gray if repo.IsPrivate { infoColor = cs.Yellow } t := repo.PushedAt - // if listResult.FromSearch { - // t = repo.UpdatedAt - // } + if repo.PushedAt == nil { + t = &repo.CreatedAt + } 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) + tp.AddField(utils.FuzzyAgoAbbr(now, *t), nil, cs.Gray) } else { tp.AddField(t.Format(time.RFC3339), nil, nil) } @@ -179,3 +193,21 @@ func listHeader(owner string, matchCount, totalMatchCount int, hasFilters bool) } return fmt.Sprintf("Showing %d of %d repositories in @%s%s", matchCount, totalMatchCount, owner, matchStr) } + +func repoInfo(r api.Repository) 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, ", ") +} From 3f3d4e38d44e21727e55fc628113357f9918c268 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 17 May 2021 16:43:39 +0200 Subject: [PATCH 18/20] Avoid crash when `--json` doesn't request `nameWithOwner` --- pkg/cmd/repo/list/http.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/repo/list/http.go b/pkg/cmd/repo/list/http.go index 1dbe372e4..ef574b43c 100644 --- a/pkg/cmd/repo/list/http.go +++ b/pkg/cmd/repo/list/http.go @@ -158,7 +158,7 @@ pagination: listResult.TotalCount = result.Search.RepositoryCount for _, repo := range result.Search.Nodes { - if listResult.Owner == "" { + if listResult.Owner == "" && repo.NameWithOwner != "" { idx := strings.IndexRune(repo.NameWithOwner, '/') listResult.Owner = repo.NameWithOwner[:idx] } From eb35a3457c4cc7fc726b633c06eb4dea03937d93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 17 May 2021 17:00:25 +0200 Subject: [PATCH 19/20] Make sure docs URLs are linked in web docs --- pkg/cmd/actions/actions.go | 2 +- pkg/cmd/completion/completion.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/actions/actions.go b/pkg/cmd/actions/actions.go index 33ba4b1a7..b618166ee 100644 --- a/pkg/cmd/actions/actions.go +++ b/pkg/cmd/actions/actions.go @@ -67,7 +67,7 @@ func actionsExplainer(cs *iostreams.ColorScheme) string { To see more help, run 'gh help workflow ' For more in depth help including examples, see online documentation at: - https://docs.github.com/en/actions/guides/managing-github-actions-with-github-cli + `, header, runHeader, workflowHeader) } diff --git a/pkg/cmd/completion/completion.go b/pkg/cmd/completion/completion.go index 24414a5fc..7af5271ca 100644 --- a/pkg/cmd/completion/completion.go +++ b/pkg/cmd/completion/completion.go @@ -21,7 +21,7 @@ func NewCmdCompletion(io *iostreams.IOStreams) *cobra.Command { When installing GitHub CLI through a package manager, it's possible that no additional shell configuration is necessary to gain completion support. For - Homebrew, see https://docs.brew.sh/Shell-Completion + Homebrew, see If you need to set up completions manually, follow the instructions below. The exact config file locations might vary based on your system. Make sure to restart your From 42d2da812c83bc0a6a108f55b2476fd7fba5b070 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 17 May 2021 17:01:33 +0200 Subject: [PATCH 20/20] Preserve list fomatting in web docs for `gh actions` --- pkg/cmd/actions/actions.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/actions/actions.go b/pkg/cmd/actions/actions.go index b618166ee..356ca1581 100644 --- a/pkg/cmd/actions/actions.go +++ b/pkg/cmd/actions/actions.go @@ -48,20 +48,20 @@ func actionsExplainer(cs *iostreams.ColorScheme) string { GitHub CLI integrates with Actions to help you manage runs and workflows. - %s - gh run list: List recent workflow runs - gh run view: View details for a workflow run or one of its jobs - gh run watch: Watch a workflow run while it executes - gh run rerun: Rerun a failed workflow run + %s + gh run list: List recent workflow runs + gh run view: View details for a workflow run or one of its jobs + gh run watch: Watch a workflow run while it executes + gh run rerun: Rerun a failed workflow run gh run download: Download artifacts generated by runs To see more help, run 'gh help run ' - %s - gh workflow list: List all the workflow files in your repository - gh workflow view: View details for a workflow file - gh workflow enable: Enable a workflow file - gh workflow disable: Disable a workflow file + %s + gh workflow list: List all the workflow files in your repository + gh workflow view: View details for a workflow file + gh workflow enable: Enable a workflow file + gh workflow disable: Disable a workflow file gh workflow run: Trigger a workflow_dispatch run for a workflow file To see more help, run 'gh help workflow '