From 89e1ee321746e77ac768621bbdbf495ce89518df Mon Sep 17 00:00:00 2001 From: Matthew Gleich Date: Thu, 17 Sep 2020 16:27:23 -0400 Subject: [PATCH 1/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Refactor=20gist=20list?= =?UTF-8?q?=20to=20use=20graphQL?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Matthew Gleich --- pkg/cmd/gist/list/http.go | 87 +++++++++++++++++++++++++++++++-------- 1 file changed, 70 insertions(+), 17 deletions(-) diff --git a/pkg/cmd/gist/list/http.go b/pkg/cmd/gist/list/http.go index aa75eef25..2474dd5fd 100644 --- a/pkg/cmd/gist/list/http.go +++ b/pkg/cmd/gist/list/http.go @@ -1,41 +1,94 @@ package list import ( - "fmt" "net/http" - "net/url" "sort" + "strings" + "time" "github.com/cli/cli/api" "github.com/cli/cli/pkg/cmd/gist/shared" ) func listGists(client *http.Client, hostname string, limit int, visibility string) ([]shared.Gist, error) { - result := []shared.Gist{} - - query := url.Values{} - if visibility == "all" { - query.Add("per_page", fmt.Sprintf("%d", limit)) - } else { - query.Add("per_page", "100") + type response struct { + Viewer struct { + Gists struct { + Nodes []struct { + Description string + Files []struct { + Name string + Language struct { + Name string + } + Extension string + } + IsPublic bool + Name string + UpdatedAt time.Time + } + } + } + } + + query := ` + query ListGists($visibility: GistPrivacy!, $per_page: Int = 10) { + viewer { + gists(first: $per_page, privacy: $visibility) { + nodes { + name + files { + name + language { + name + } + extension + } + description + updatedAt + isPublic + } + } + } + }` + + if visibility != "all" { + limit = 100 + } + variables := map[string]interface{}{ + "per_page": limit, + "visibility": strings.ToUpper(visibility), } - // TODO switch to graphql apiClient := api.NewClientFromHTTP(client) - err := apiClient.REST(hostname, "GET", "gists?"+query.Encode(), nil, &result) + var result response + err := apiClient.GraphQL(hostname, query, variables, &result) if err != nil { return nil, err } gists := []shared.Gist{} + for _, gist := range result.Viewer.Gists.Nodes { - for _, gist := range result { - if len(gists) == limit { - break - } - if visibility == "all" || (visibility == "secret" && !gist.Public) || (visibility == "public" && gist.Public) { - gists = append(gists, gist) + files := map[string]*shared.GistFile{} + for _, file := range gist.Files { + files[file.Name] = &shared.GistFile{ + Filename: file.Name, + Type: file.Extension, + Language: file.Language.Name, + } } + + gists = append( + gists, + shared.Gist{ + ID: gist.Name, + Description: gist.Description, + Files: files, + UpdatedAt: gist.UpdatedAt, + Public: gist.IsPublic, + }, + ) } sort.SliceStable(gists, func(i, j int) bool { From d8ef8b836e5d166efa0c07b2af66545a35762cdc Mon Sep 17 00:00:00 2001 From: Matthew Gleich Date: Sun, 4 Oct 2020 01:57:36 -0400 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=90=9B=20Fix=20stubs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Matthew Gleich --- pkg/cmd/gist/list/list_test.go | 305 ++++++++++++++++++++++++--------- 1 file changed, 222 insertions(+), 83 deletions(-) diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index 497f68edc..435ee2f37 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -2,11 +2,11 @@ package list import ( "bytes" + "fmt" "net/http" "testing" "time" - "github.com/cli/cli/pkg/cmd/gist/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" @@ -88,115 +88,254 @@ func TestNewCmdList(t *testing.T) { } func Test_listRun(t *testing.T) { + const query = `query ListGists\b` + sixHoursAgo, _ := time.ParseDuration("-6h") + timeSixHoursAgo := time.Now().Add(sixHoursAgo).Format(time.RFC3339) + blankTime := time.Time{}.Format(time.RFC3339) tests := []struct { - name string - opts *ListOptions - wantOut string - stubs func(*httpmock.Registry) - nontty bool - updatedAt *time.Time + name string + opts *ListOptions + wantOut string + stubs func(*httpmock.Registry) + nontty bool }{ { name: "no gists", opts: &ListOptions{}, stubs: func(reg *httpmock.Registry) { - reg.Register(httpmock.REST("GET", "gists"), - httpmock.JSONResponse([]shared.Gist{})) - + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(`{ "data": { "viewer": { + "gists": { "nodes": [] } + } } }`)) }, wantOut: "", }, { - name: "default behavior", - opts: &ListOptions{}, + name: "default behavior", + opts: &ListOptions{}, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "1234567890", + "files": [{ "name": "cool.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "description": "", + "updatedAt": "%v", + "isPublic": true + }, + { + "name": "4567890123", + "files": [{ "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "description": "", + "updatedAt": "%v", + "isPublic": true + }, + { + "name": "2345678901", + "files": [ + { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" } + ], + "description": "tea leaves thwart those who court catastrophe", + "updatedAt": "%v", + "isPublic": false + }, + { + "name": "3456789012", + "files": [ + { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile2.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile3.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile4.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile5.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile6.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile7.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile9.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile10.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile11.txt", "languages": { "name": "None" }, "extension": ".txt" } + ], + "description": "short desc", + "updatedAt": "%v", + "isPublic": false + } + ] } } } }`, + timeSixHoursAgo, + timeSixHoursAgo, + timeSixHoursAgo, + timeSixHoursAgo, + )), + ) + }, wantOut: "1234567890 cool.txt 1 file public about 6 hours ago\n4567890123 1 file public about 6 hours ago\n2345678901 tea leaves thwart... 2 files secret about 6 hours ago\n3456789012 short desc 11 files secret about 6 hours ago\n", }, { - name: "with public filter", - opts: &ListOptions{Visibility: "public"}, + name: "with public filter", + opts: &ListOptions{Visibility: "public"}, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "1234567890", + "files": [{ "name": "cool.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "description": "", + "updatedAt": "%v", + "isPublic": true + }, + { + "name": "4567890123", + "files": [{ "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "description": "", + "updatedAt": "%v", + "isPublic": true + } + ] } } } }`, + timeSixHoursAgo, + timeSixHoursAgo, + )), + ) + }, wantOut: "1234567890 cool.txt 1 file public about 6 hours ago\n4567890123 1 file public about 6 hours ago\n", }, { - name: "with secret filter", - opts: &ListOptions{Visibility: "secret"}, + name: "with secret filter", + opts: &ListOptions{Visibility: "secret"}, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "2345678901", + "files": [ + { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" } + ], + "description": "tea leaves thwart those who court catastrophe", + "updatedAt": "%v", + "isPublic": false + }, + { + "name": "3456789012", + "files": [ + { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile2.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile3.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile4.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile5.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile6.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile7.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile9.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile10.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile11.txt", "languages": { "name": "None" }, "extension": ".txt" } + ], + "description": "short desc", + "updatedAt": "%v", + "isPublic": false + } + ] } } } }`, + timeSixHoursAgo, + timeSixHoursAgo, + )), + ) + }, wantOut: "2345678901 tea leaves thwart... 2 files secret about 6 hours ago\n3456789012 short desc 11 files secret about 6 hours ago\n", }, { - name: "with limit", - opts: &ListOptions{Limit: 1}, + name: "with limit", + opts: &ListOptions{Limit: 1}, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "1234567890", + "files": [{ "name": "cool.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "description": "", + "updatedAt": "%v", + "isPublic": true + } + ] } } } }`, + timeSixHoursAgo, + )), + ) + }, wantOut: "1234567890 cool.txt 1 file public about 6 hours ago\n", }, { - name: "nontty output", - opts: &ListOptions{}, - updatedAt: &time.Time{}, - wantOut: "1234567890\tcool.txt\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n4567890123\t\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n2345678901\ttea leaves thwart those who court catastrophe\t2 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n3456789012\tshort desc\t11 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n", - nontty: true, + name: "nontty output", + opts: &ListOptions{}, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(query), + httpmock.StringResponse(fmt.Sprintf( + `{ "data": { "viewer": { "gists": { "nodes": [ + { + "name": "1234567890", + "files": [{ "name": "cool.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "description": "", + "updatedAt": "%v", + "isPublic": true + }, + { + "name": "4567890123", + "files": [{ "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "description": "", + "updatedAt": "%v", + "isPublic": true + }, + { + "name": "2345678901", + "files": [ + { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" } + ], + "description": "tea leaves thwart those who court catastrophe", + "updatedAt": "%v", + "isPublic": false + }, + { + "name": "3456789012", + "files": [ + { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile2.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile3.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile4.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile5.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile6.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile7.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile9.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile10.txt", "languages": { "name": "None" }, "extension": ".txt" }, + { "name": "gistfile11.txt", "languages": { "name": "None" }, "extension": ".txt" } + ], + "description": "short desc", + "updatedAt": "%v", + "isPublic": false + } + ] } } } }`, + blankTime, + blankTime, + blankTime, + blankTime, + )), + ) + }, + wantOut: "1234567890\tcool.txt\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n4567890123\t\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n2345678901\ttea leaves thwart those who court catastrophe\t2 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n3456789012\tshort desc\t11 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n", + nontty: true, }, } for _, tt := range tests { - sixHoursAgo, _ := time.ParseDuration("-6h") - updatedAt := time.Now().Add(sixHoursAgo) - if tt.updatedAt != nil { - updatedAt = *tt.updatedAt - } - reg := &httpmock.Registry{} - if tt.stubs == nil { - reg.Register(httpmock.REST("GET", "gists"), - httpmock.JSONResponse([]shared.Gist{ - { - ID: "1234567890", - UpdatedAt: updatedAt, - Description: "", - Files: map[string]*shared.GistFile{ - "cool.txt": {}, - }, - Public: true, - }, - { - ID: "4567890123", - UpdatedAt: updatedAt, - Description: "", - Files: map[string]*shared.GistFile{ - "gistfile0.txt": {}, - }, - Public: true, - }, - { - ID: "2345678901", - UpdatedAt: updatedAt, - Description: "tea leaves thwart those who court catastrophe", - Files: map[string]*shared.GistFile{ - "gistfile0.txt": {}, - "gistfile1.txt": {}, - }, - Public: false, - }, - { - ID: "3456789012", - UpdatedAt: updatedAt, - Description: "short desc", - Files: map[string]*shared.GistFile{ - "gistfile0.txt": {}, - "gistfile1.txt": {}, - "gistfile2.txt": {}, - "gistfile3.txt": {}, - "gistfile4.txt": {}, - "gistfile5.txt": {}, - "gistfile6.txt": {}, - "gistfile7.txt": {}, - "gistfile8.txt": {}, - "gistfile9.txt": {}, - "gistfile10.txt": {}, - }, - Public: false, - }, - })) - } else { - tt.stubs(reg) - } + tt.stubs(reg) tt.opts.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: reg}, nil From f12437015417beec598a02b3720ef2621ceb0392 Mon Sep 17 00:00:00 2001 From: Matthew Gleich Date: Sun, 4 Oct 2020 15:02:13 -0400 Subject: [PATCH 3/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Refactor=20to=20only?= =?UTF-8?q?=20pass=20time=20in=20once=20for=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Matthew Gleich --- pkg/cmd/gist/list/list_test.go | 58 +++++++++++++++------------------- 1 file changed, 25 insertions(+), 33 deletions(-) diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index 435ee2f37..f47289c55 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -119,31 +119,31 @@ func Test_listRun(t *testing.T) { httpmock.GraphQL(query), httpmock.StringResponse(fmt.Sprintf( `{ "data": { "viewer": { "gists": { "nodes": [ - { + { "name": "1234567890", "files": [{ "name": "cool.txt", "languages": { "name": "None" }, "extension": ".txt" }], "description": "", - "updatedAt": "%v", + "updatedAt": "%[1]v", "isPublic": true }, - { + { "name": "4567890123", "files": [{ "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }], "description": "", - "updatedAt": "%v", + "updatedAt": "%[1]v", "isPublic": true }, - { + { "name": "2345678901", "files": [ { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" } ], "description": "tea leaves thwart those who court catastrophe", - "updatedAt": "%v", + "updatedAt": "%[1]v", "isPublic": false }, - { + { "name": "3456789012", "files": [ { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, @@ -159,14 +159,11 @@ func Test_listRun(t *testing.T) { { "name": "gistfile11.txt", "languages": { "name": "None" }, "extension": ".txt" } ], "description": "short desc", - "updatedAt": "%v", + "updatedAt": "%[1]v", "isPublic": false } ] } } } }`, timeSixHoursAgo, - timeSixHoursAgo, - timeSixHoursAgo, - timeSixHoursAgo, )), ) }, @@ -180,23 +177,22 @@ func Test_listRun(t *testing.T) { httpmock.GraphQL(query), httpmock.StringResponse(fmt.Sprintf( `{ "data": { "viewer": { "gists": { "nodes": [ - { + { "name": "1234567890", "files": [{ "name": "cool.txt", "languages": { "name": "None" }, "extension": ".txt" }], "description": "", - "updatedAt": "%v", + "updatedAt": "%[1]v", "isPublic": true }, - { + { "name": "4567890123", "files": [{ "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }], "description": "", - "updatedAt": "%v", + "updatedAt": "%[1]v", "isPublic": true } ] } } } }`, timeSixHoursAgo, - timeSixHoursAgo, )), ) }, @@ -210,17 +206,17 @@ func Test_listRun(t *testing.T) { httpmock.GraphQL(query), httpmock.StringResponse(fmt.Sprintf( `{ "data": { "viewer": { "gists": { "nodes": [ - { + { "name": "2345678901", "files": [ { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" } ], "description": "tea leaves thwart those who court catastrophe", - "updatedAt": "%v", + "updatedAt": "%[1]v", "isPublic": false }, - { + { "name": "3456789012", "files": [ { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, @@ -236,12 +232,11 @@ func Test_listRun(t *testing.T) { { "name": "gistfile11.txt", "languages": { "name": "None" }, "extension": ".txt" } ], "description": "short desc", - "updatedAt": "%v", + "updatedAt": "%[1]v", "isPublic": false } ] } } } }`, timeSixHoursAgo, - timeSixHoursAgo, )), ) }, @@ -255,7 +250,7 @@ func Test_listRun(t *testing.T) { httpmock.GraphQL(query), httpmock.StringResponse(fmt.Sprintf( `{ "data": { "viewer": { "gists": { "nodes": [ - { + { "name": "1234567890", "files": [{ "name": "cool.txt", "languages": { "name": "None" }, "extension": ".txt" }], "description": "", @@ -277,31 +272,31 @@ func Test_listRun(t *testing.T) { httpmock.GraphQL(query), httpmock.StringResponse(fmt.Sprintf( `{ "data": { "viewer": { "gists": { "nodes": [ - { + { "name": "1234567890", "files": [{ "name": "cool.txt", "languages": { "name": "None" }, "extension": ".txt" }], "description": "", - "updatedAt": "%v", + "updatedAt": "%[1]v", "isPublic": true }, - { + { "name": "4567890123", "files": [{ "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }], "description": "", - "updatedAt": "%v", + "updatedAt": "%[1]v", "isPublic": true }, - { + { "name": "2345678901", "files": [ { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" } ], "description": "tea leaves thwart those who court catastrophe", - "updatedAt": "%v", + "updatedAt": "%[1]v", "isPublic": false }, - { + { "name": "3456789012", "files": [ { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, @@ -317,14 +312,11 @@ func Test_listRun(t *testing.T) { { "name": "gistfile11.txt", "languages": { "name": "None" }, "extension": ".txt" } ], "description": "short desc", - "updatedAt": "%v", + "updatedAt": "%[1]v", "isPublic": false } ] } } } }`, blankTime, - blankTime, - blankTime, - blankTime, )), ) }, From 1859728f7e83d8eda6bf5da7bac7824a081ef3db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 5 Oct 2020 20:27:05 +0200 Subject: [PATCH 4/5] Fix parsing `gist list --public/--secret` flags It's not sufficient to use `Changed("public")` to test if a boolean flag was activated, since the user might have passed `--public=false`. Instead, check the true value of the flag. The `--public` and `--secret` flags should be mutually exclusive, so now if both are activated, `--secret` takes precedence. --- pkg/cmd/gist/list/list.go | 17 ++++++++--------- pkg/cmd/gist/list/list_test.go | 10 +++++++++- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index e1142bdb7..bc02cc033 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -27,6 +27,9 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman HttpClient: f.HttpClient, } + var flagPublic bool + var flagSecret bool + cmd := &cobra.Command{ Use: "list", Short: "List your gists", @@ -36,27 +39,23 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman return &cmdutil.FlagError{Err: fmt.Errorf("invalid limit: %v", opts.Limit)} } - pub := cmd.Flags().Changed("public") - secret := cmd.Flags().Changed("secret") - opts.Visibility = "all" - if pub && !secret { - opts.Visibility = "public" - } else if secret && !pub { + if flagSecret { opts.Visibility = "secret" + } else if flagPublic { + opts.Visibility = "public" } if runF != nil { return runF(opts) } - return listRun(opts) }, } cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 10, "Maximum number of gists to fetch") - cmd.Flags().Bool("public", false, "Show only public gists") - cmd.Flags().Bool("secret", false, "Show only secret gists") + cmd.Flags().BoolVar(&flagPublic, "public", false, "Show only public gists") + cmd.Flags().BoolVar(&flagSecret, "secret", false, "Show only secret gists") return cmd } diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index f47289c55..6eb8f96d3 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -43,12 +43,20 @@ func TestNewCmdList(t *testing.T) { Visibility: "secret", }, }, + { + name: "secret with explicit false value", + cli: "--secret=false", + wants: ListOptions{ + Limit: 10, + Visibility: "all", + }, + }, { name: "public and secret", cli: "--secret --public", wants: ListOptions{ Limit: 10, - Visibility: "all", + Visibility: "secret", }, }, { From cd2adfeba039e91f46f04ee3955d4034e3fbb734 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 5 Oct 2020 20:33:31 +0200 Subject: [PATCH 5/5] Port listing gists to githubv4 GraphQL client - Fetching more than 100 gists is now supported - The GraphQL query name is now `GistList` instead of `ListGists` for consistency with other queries - Avoid fetching unnecessary Files fields - Gists are now rendered in the order that the API returned them in - The gist timestamp for machine-readable output is now rendered in RFC3339 format instead of in `time.Time.String()` format which is only meant for debugging and is not considered stable - Ensure newlines in gist description are rendered as spaces --- pkg/cmd/gist/list/http.go | 111 ++++++++++++++---------------- pkg/cmd/gist/list/list.go | 20 +++--- pkg/cmd/gist/list/list_test.go | 121 +++++++++++++++++---------------- 3 files changed, 123 insertions(+), 129 deletions(-) diff --git a/pkg/cmd/gist/list/http.go b/pkg/cmd/gist/list/http.go index 2474dd5fd..435f19d4d 100644 --- a/pkg/cmd/gist/list/http.go +++ b/pkg/cmd/gist/list/http.go @@ -1,13 +1,15 @@ package list import ( + "context" "net/http" - "sort" "strings" "time" - "github.com/cli/cli/api" + "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/pkg/cmd/gist/shared" + "github.com/shurcooL/githubv4" + "github.com/shurcooL/graphql" ) func listGists(client *http.Client, hostname string, limit int, visibility string) ([]shared.Gist, error) { @@ -17,83 +19,70 @@ func listGists(client *http.Client, hostname string, limit int, visibility strin Nodes []struct { Description string Files []struct { - Name string - Language struct { - Name string - } - Extension string + Name string } IsPublic bool Name string UpdatedAt time.Time } - } - } - } - - query := ` - query ListGists($visibility: GistPrivacy!, $per_page: Int = 10) { - viewer { - gists(first: $per_page, privacy: $visibility) { - nodes { - name - files { - name - language { - name - } - extension - } - description - updatedAt - isPublic + PageInfo struct { + HasNextPage bool + EndCursor string } - } + } `graphql:"gists(first: $per_page, after: $endCursor, privacy: $visibility, orderBy: {field: CREATED_AT, direction: DESC})"` } - }` - - if visibility != "all" { - limit = 100 } + + perPage := limit + if perPage > 100 { + perPage = 100 + } + variables := map[string]interface{}{ - "per_page": limit, - "visibility": strings.ToUpper(visibility), + "per_page": githubv4.Int(perPage), + "endCursor": (*githubv4.String)(nil), + "visibility": githubv4.GistPrivacy(strings.ToUpper(visibility)), } - apiClient := api.NewClientFromHTTP(client) - var result response - err := apiClient.GraphQL(hostname, query, variables, &result) - if err != nil { - return nil, err - } + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) gists := []shared.Gist{} - for _, gist := range result.Viewer.Gists.Nodes { +pagination: + for { + var result response + err := gql.QueryNamed(context.Background(), "GistList", &result, variables) + if err != nil { + return nil, err + } - files := map[string]*shared.GistFile{} - for _, file := range gist.Files { - files[file.Name] = &shared.GistFile{ - Filename: file.Name, - Type: file.Extension, - Language: file.Language.Name, + for _, gist := range result.Viewer.Gists.Nodes { + files := map[string]*shared.GistFile{} + for _, file := range gist.Files { + files[file.Name] = &shared.GistFile{ + Filename: file.Name, + } + } + + gists = append( + gists, + shared.Gist{ + ID: gist.Name, + Description: gist.Description, + Files: files, + UpdatedAt: gist.UpdatedAt, + Public: gist.IsPublic, + }, + ) + if len(gists) == limit { + break pagination } } - gists = append( - gists, - shared.Gist{ - ID: gist.Name, - Description: gist.Description, - Files: files, - UpdatedAt: gist.UpdatedAt, - Public: gist.IsPublic, - }, - ) + if !result.Viewer.Gists.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(result.Viewer.Gists.PageInfo.EndCursor) } - sort.SliceStable(gists, func(i, j int) bool { - return gists[i].UpdatedAt.After(gists[j].UpdatedAt) - }) - return gists, nil } diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index bc02cc033..375c08372 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/text" "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -76,10 +77,7 @@ func listRun(opts *ListOptions) error { tp := utils.NewTablePrinter(opts.IO) for _, gist := range gists { - fileCount := 0 - for range gist.Files { - fileCount++ - } + fileCount := len(gist.Files) visibility := "public" visColor := cs.Green @@ -98,16 +96,16 @@ func listRun(opts *ListOptions) error { } } + gistTime := gist.UpdatedAt.Format(time.RFC3339) + if tp.IsTTY() { + gistTime = utils.FuzzyAgo(time.Since(gist.UpdatedAt)) + } + tp.AddField(gist.ID, nil, nil) - tp.AddField(description, nil, cs.Bold) + tp.AddField(text.ReplaceExcessiveWhitespace(description), nil, cs.Bold) tp.AddField(utils.Pluralize(fileCount, "file"), nil, nil) tp.AddField(visibility, nil, visColor) - if tp.IsTTY() { - updatedAt := utils.FuzzyAgo(time.Since(gist.UpdatedAt)) - tp.AddField(updatedAt, nil, cs.Gray) - } else { - tp.AddField(gist.UpdatedAt.String(), nil, nil) - } + tp.AddField(gistTime, nil, cs.Gray) tp.EndRow() } diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index 6eb8f96d3..43787916f 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" @@ -96,10 +97,11 @@ func TestNewCmdList(t *testing.T) { } func Test_listRun(t *testing.T) { - const query = `query ListGists\b` - sixHoursAgo, _ := time.ParseDuration("-6h") - timeSixHoursAgo := time.Now().Add(sixHoursAgo).Format(time.RFC3339) - blankTime := time.Time{}.Format(time.RFC3339) + const query = `query GistList\b` + sixHours, _ := time.ParseDuration("6h") + sixHoursAgo := time.Now().Add(-sixHours) + absTime, _ := time.Parse(time.RFC3339, "2020-07-30T15:24:28Z") + tests := []struct { name string opts *ListOptions @@ -129,14 +131,14 @@ func Test_listRun(t *testing.T) { `{ "data": { "viewer": { "gists": { "nodes": [ { "name": "1234567890", - "files": [{ "name": "cool.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "files": [{ "name": "cool.txt" }], "description": "", "updatedAt": "%[1]v", "isPublic": true }, { "name": "4567890123", - "files": [{ "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "files": [{ "name": "gistfile0.txt" }], "description": "", "updatedAt": "%[1]v", "isPublic": true @@ -144,8 +146,8 @@ func Test_listRun(t *testing.T) { { "name": "2345678901", "files": [ - { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" } + { "name": "gistfile0.txt" }, + { "name": "gistfile1.txt" } ], "description": "tea leaves thwart those who court catastrophe", "updatedAt": "%[1]v", @@ -154,24 +156,24 @@ func Test_listRun(t *testing.T) { { "name": "3456789012", "files": [ - { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile2.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile3.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile4.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile5.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile6.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile7.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile9.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile10.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile11.txt", "languages": { "name": "None" }, "extension": ".txt" } + { "name": "gistfile0.txt" }, + { "name": "gistfile1.txt" }, + { "name": "gistfile2.txt" }, + { "name": "gistfile3.txt" }, + { "name": "gistfile4.txt" }, + { "name": "gistfile5.txt" }, + { "name": "gistfile6.txt" }, + { "name": "gistfile7.txt" }, + { "name": "gistfile9.txt" }, + { "name": "gistfile10.txt" }, + { "name": "gistfile11.txt" } ], "description": "short desc", "updatedAt": "%[1]v", "isPublic": false } ] } } } }`, - timeSixHoursAgo, + sixHoursAgo.Format(time.RFC3339), )), ) }, @@ -187,20 +189,20 @@ func Test_listRun(t *testing.T) { `{ "data": { "viewer": { "gists": { "nodes": [ { "name": "1234567890", - "files": [{ "name": "cool.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "files": [{ "name": "cool.txt" }], "description": "", "updatedAt": "%[1]v", "isPublic": true }, { "name": "4567890123", - "files": [{ "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "files": [{ "name": "gistfile0.txt" }], "description": "", "updatedAt": "%[1]v", "isPublic": true } ] } } } }`, - timeSixHoursAgo, + sixHoursAgo.Format(time.RFC3339), )), ) }, @@ -217,8 +219,8 @@ func Test_listRun(t *testing.T) { { "name": "2345678901", "files": [ - { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" } + { "name": "gistfile0.txt" }, + { "name": "gistfile1.txt" } ], "description": "tea leaves thwart those who court catastrophe", "updatedAt": "%[1]v", @@ -227,24 +229,24 @@ func Test_listRun(t *testing.T) { { "name": "3456789012", "files": [ - { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile2.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile3.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile4.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile5.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile6.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile7.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile9.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile10.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile11.txt", "languages": { "name": "None" }, "extension": ".txt" } + { "name": "gistfile0.txt" }, + { "name": "gistfile1.txt" }, + { "name": "gistfile2.txt" }, + { "name": "gistfile3.txt" }, + { "name": "gistfile4.txt" }, + { "name": "gistfile5.txt" }, + { "name": "gistfile6.txt" }, + { "name": "gistfile7.txt" }, + { "name": "gistfile9.txt" }, + { "name": "gistfile10.txt" }, + { "name": "gistfile11.txt" } ], "description": "short desc", "updatedAt": "%[1]v", "isPublic": false } ] } } } }`, - timeSixHoursAgo, + sixHoursAgo.Format(time.RFC3339), )), ) }, @@ -260,13 +262,13 @@ func Test_listRun(t *testing.T) { `{ "data": { "viewer": { "gists": { "nodes": [ { "name": "1234567890", - "files": [{ "name": "cool.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "files": [{ "name": "cool.txt" }], "description": "", "updatedAt": "%v", "isPublic": true } ] } } } }`, - timeSixHoursAgo, + sixHoursAgo.Format(time.RFC3339), )), ) }, @@ -282,14 +284,14 @@ func Test_listRun(t *testing.T) { `{ "data": { "viewer": { "gists": { "nodes": [ { "name": "1234567890", - "files": [{ "name": "cool.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "files": [{ "name": "cool.txt" }], "description": "", "updatedAt": "%[1]v", "isPublic": true }, { "name": "4567890123", - "files": [{ "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }], + "files": [{ "name": "gistfile0.txt" }], "description": "", "updatedAt": "%[1]v", "isPublic": true @@ -297,8 +299,8 @@ func Test_listRun(t *testing.T) { { "name": "2345678901", "files": [ - { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" } + { "name": "gistfile0.txt" }, + { "name": "gistfile1.txt" } ], "description": "tea leaves thwart those who court catastrophe", "updatedAt": "%[1]v", @@ -307,29 +309,34 @@ func Test_listRun(t *testing.T) { { "name": "3456789012", "files": [ - { "name": "gistfile0.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile1.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile2.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile3.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile4.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile5.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile6.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile7.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile9.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile10.txt", "languages": { "name": "None" }, "extension": ".txt" }, - { "name": "gistfile11.txt", "languages": { "name": "None" }, "extension": ".txt" } + { "name": "gistfile0.txt" }, + { "name": "gistfile1.txt" }, + { "name": "gistfile2.txt" }, + { "name": "gistfile3.txt" }, + { "name": "gistfile4.txt" }, + { "name": "gistfile5.txt" }, + { "name": "gistfile6.txt" }, + { "name": "gistfile7.txt" }, + { "name": "gistfile9.txt" }, + { "name": "gistfile10.txt" }, + { "name": "gistfile11.txt" } ], "description": "short desc", "updatedAt": "%[1]v", "isPublic": false } ] } } } }`, - blankTime, + absTime.Format(time.RFC3339), )), ) }, - wantOut: "1234567890\tcool.txt\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n4567890123\t\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n2345678901\ttea leaves thwart those who court catastrophe\t2 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n3456789012\tshort desc\t11 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n", - nontty: true, + wantOut: heredoc.Doc(` + 1234567890 cool.txt 1 file public 2020-07-30T15:24:28Z + 4567890123 1 file public 2020-07-30T15:24:28Z + 2345678901 tea leaves thwart those who court catastrophe 2 files secret 2020-07-30T15:24:28Z + 3456789012 short desc 11 files secret 2020-07-30T15:24:28Z + `), + nontty: true, }, }