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] 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, }, }