diff --git a/api/queries_issue.go b/api/queries_issue.go index f83e43d92..ae27878a6 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -171,7 +171,7 @@ func IssueStatus(client *Client, repo ghrepo.Interface, currentUsername string) return &payload, nil } -func IssueList(client *Client, repo ghrepo.Interface, state string, labels []string, assigneeString string, limit int, authorString string) ([]Issue, error) { +func IssueList(client *Client, repo ghrepo.Interface, state string, labels []string, assigneeString string, limit int, authorString string) (*IssuesAndTotalCount, error) { var states []string switch state { case "open", "": @@ -189,6 +189,7 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str repository(owner: $owner, name: $repo) { hasIssuesEnabled issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee, createdBy: $author}) { + totalCount nodes { ...issue } @@ -219,8 +220,9 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str var response struct { Repository struct { Issues struct { - Nodes []Issue - PageInfo struct { + TotalCount int + Nodes []Issue + PageInfo struct { HasNextPage bool EndCursor string } @@ -258,7 +260,8 @@ loop: } } - return issues, nil + res := IssuesAndTotalCount{Issues: issues, TotalCount: response.Repository.Issues.TotalCount} + return &res, nil } func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, error) { diff --git a/api/queries_pr.go b/api/queries_pr.go index ee80c2f7f..be86dc679 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -437,7 +437,7 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter return &result.CreatePullRequest.PullRequest, nil } -func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([]PullRequest, error) { +func PullRequestList(client *Client, vars map[string]interface{}, limit int) (*PullRequestAndTotalCount, error) { type prBlock struct { Edges []struct { Node PullRequest @@ -446,6 +446,8 @@ func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([] HasNextPage bool EndCursor string } + TotalCount int + IssueCount int } type response struct { Repository struct { @@ -489,24 +491,26 @@ func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([] first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC} - ) { - edges { - node { - ...pr - } - } - pageInfo { - hasNextPage - endCursor - } - } - } + ) { + totalCount + edges { + node { + ...pr + } + } + pageInfo { + hasNextPage + endCursor + } + } + } }` var check = make(map[int]struct{}) var prs []PullRequest pageLimit := min(limit, 100) variables := map[string]interface{}{} + res := PullRequestAndTotalCount{} // If assignee was specified, use the `search` API rather than // `Repository.pullRequests`, but this mode doesn't support multiple labels @@ -518,6 +522,7 @@ func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([] $endCursor: String, ) { search(query: $q, type: ISSUE, first: $limit, after: $endCursor) { + issueCount edges { node { ...pr @@ -571,8 +576,10 @@ loop: return nil, err } prData := data.Repository.PullRequests + res.TotalCount = prData.TotalCount if _, ok := variables["q"]; ok { prData = data.Search + res.TotalCount = prData.IssueCount } for _, edge := range prData.Edges { @@ -594,8 +601,8 @@ loop: break } } - - return prs, nil + res.PullRequests = prs + return &res, nil } func min(a, b int) int { diff --git a/command/issue.go b/command/issue.go index d0d5fb7fc..11e5bc6c2 100644 --- a/command/issue.go +++ b/command/issue.go @@ -33,7 +33,7 @@ func init() { issueCmd.AddCommand(issueListCmd) issueListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") issueListCmd.Flags().StringSliceP("label", "l", nil, "Filter by label") - issueListCmd.Flags().StringP("state", "s", "", "Filter by state: {open|closed|all}") + issueListCmd.Flags().StringP("state", "s", "open", "Filter by state: {open|closed|all}") issueListCmd.Flags().IntP("limit", "L", 30, "Maximum number of issues to fetch") issueListCmd.Flags().StringP("author", "A", "", "Filter by author") @@ -114,30 +114,27 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableErr(cmd), "\nIssues for %s\n\n", ghrepo.FullName(baseRepo)) - - issues, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit, author) + listResult, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit, author) if err != nil { return err } - if len(issues) == 0 { - colorErr := colorableErr(cmd) // Send to stderr because otherwise when piping this command it would seem like the "no open issues" message is actually an issue - msg := "There are no open issues" - - userSetFlags := false - cmd.Flags().Visit(func(f *pflag.Flag) { - userSetFlags = true - }) - if userSetFlags { - msg = "No issues match your search" + hasFilters := false + cmd.Flags().Visit(func(f *pflag.Flag) { + switch f.Name { + case "state", "label", "assignee", "author": + hasFilters = true } - printMessage(colorErr, msg) - return nil - } + }) + + title := listHeader(ghrepo.FullName(baseRepo), "issue", len(listResult.Issues), listResult.TotalCount, hasFilters) + // TODO: avoid printing header if piped to a script + fmt.Fprintf(colorableErr(cmd), "\n%s\n\n", title) out := cmd.OutOrStdout() - printIssues(out, "", len(issues), issues) + + printIssues(out, "", len(listResult.Issues), listResult.Issues) + return nil } @@ -231,6 +228,25 @@ func issueView(cmd *cobra.Command, args []string) error { } +func listHeader(repoName string, itemName string, matchCount int, totalMatchCount int, hasFilters bool) string { + if totalMatchCount == 0 { + if hasFilters { + return fmt.Sprintf("No %ss match your search in %s", itemName, repoName) + } + return fmt.Sprintf("There are no open %ss in %s", itemName, repoName) + } + + if hasFilters { + matchVerb := "match" + if totalMatchCount == 1 { + matchVerb = "matches" + } + return fmt.Sprintf("Showing %d of %s in %s that %s your search", matchCount, utils.Pluralize(totalMatchCount, itemName), repoName, matchVerb) + } + + return fmt.Sprintf("Showing %d of %s in %s", matchCount, utils.Pluralize(totalMatchCount, itemName), repoName) +} + func printIssuePreview(out io.Writer, issue *api.Issue) error { coloredLabels := labelList(*issue) if coloredLabels != "" { diff --git a/command/issue_test.go b/command/issue_test.go index 141147e8a..0d839d5a2 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -111,6 +111,11 @@ func TestIssueList(t *testing.T) { t.Errorf("error running command `issue list`: %v", err) } + eq(t, output.Stderr(), ` +Showing 3 of 3 issues in OWNER/REPO + +`) + expectedIssues := []*regexp.Regexp{ regexp.MustCompile(`(?m)^1\t.*won`), regexp.MustCompile(`(?m)^2\t.*too`), @@ -144,9 +149,8 @@ func TestIssueList_withFlags(t *testing.T) { eq(t, output.String(), "") eq(t, output.Stderr(), ` -Issues for OWNER/REPO +No issues match your search in OWNER/REPO -No issues match your search `) bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) @@ -558,3 +562,114 @@ func TestIssueCreate_webTitleBody(t *testing.T) { eq(t, url, "https://github.com/OWNER/REPO/issues/new?title=mytitle&body=mybody") eq(t, output.String(), "Opening github.com/OWNER/REPO/issues/new in your browser.\n") } + +func Test_listHeader(t *testing.T) { + type args struct { + repoName string + itemName string + matchCount int + totalMatchCount int + hasFilters bool + } + tests := []struct { + name string + args args + want string + }{ + { + name: "no results", + args: args{ + repoName: "REPO", + itemName: "table", + matchCount: 0, + totalMatchCount: 0, + hasFilters: false, + }, + want: "There are no open tables in REPO", + }, + { + name: "no matches after filters", + args: args{ + repoName: "REPO", + itemName: "Luftballon", + matchCount: 0, + totalMatchCount: 0, + hasFilters: true, + }, + want: "No Luftballons match your search in REPO", + }, + { + name: "one result", + args: args{ + repoName: "REPO", + itemName: "genie", + matchCount: 1, + totalMatchCount: 23, + hasFilters: false, + }, + want: "Showing 1 of 23 genies in REPO", + }, + { + name: "one result after filters", + args: args{ + repoName: "REPO", + itemName: "tiny cup", + matchCount: 1, + totalMatchCount: 23, + hasFilters: true, + }, + want: "Showing 1 of 23 tiny cups in REPO that match your search", + }, + { + name: "one result in total", + args: args{ + repoName: "REPO", + itemName: "chip", + matchCount: 1, + totalMatchCount: 1, + hasFilters: false, + }, + want: "Showing 1 of 1 chip in REPO", + }, + { + name: "one result in total after filters", + args: args{ + repoName: "REPO", + itemName: "spicy noodle", + matchCount: 1, + totalMatchCount: 1, + hasFilters: true, + }, + want: "Showing 1 of 1 spicy noodle in REPO that matches your search", + }, + { + name: "multiple results", + args: args{ + repoName: "REPO", + itemName: "plant", + matchCount: 4, + totalMatchCount: 23, + hasFilters: false, + }, + want: "Showing 4 of 23 plants in REPO", + }, + { + name: "multiple results after filters", + args: args{ + repoName: "REPO", + itemName: "boomerang", + matchCount: 4, + totalMatchCount: 23, + hasFilters: true, + }, + want: "Showing 4 of 23 boomerangs in REPO that match your search", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := listHeader(tt.args.repoName, tt.args.itemName, tt.args.matchCount, tt.args.totalMatchCount, tt.args.hasFilters); got != tt.want { + t.Errorf("listHeader() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/command/pr.go b/command/pr.go index 6c9501a3e..bd7743f46 100644 --- a/command/pr.go +++ b/command/pr.go @@ -140,8 +140,6 @@ func prList(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableErr(cmd), "\nPull requests for %s\n\n", ghrepo.FullName(baseRepo)) - limit, err := cmd.Flags().GetInt("limit") if err != nil { return err @@ -192,28 +190,25 @@ func prList(cmd *cobra.Command, args []string) error { params["assignee"] = assignee } - prs, err := api.PullRequestList(apiClient, params, limit) + listResult, err := api.PullRequestList(apiClient, params, limit) if err != nil { return err } - if len(prs) == 0 { - colorErr := colorableErr(cmd) // Send to stderr because otherwise when piping this command it would seem like the "no open prs" message is actually a pr - msg := "There are no open pull requests" - - userSetFlags := false - cmd.Flags().Visit(func(f *pflag.Flag) { - userSetFlags = true - }) - if userSetFlags { - msg = "No pull requests match your search" + hasFilters := false + cmd.Flags().Visit(func(f *pflag.Flag) { + switch f.Name { + case "state", "label", "base", "assignee": + hasFilters = true } - printMessage(colorErr, msg) - return nil - } + }) + + title := listHeader(ghrepo.FullName(baseRepo), "pull request", len(listResult.PullRequests), listResult.TotalCount, hasFilters) + // TODO: avoid printing header if piped to a script + fmt.Fprintf(colorableErr(cmd), "\n%s\n\n", title) table := utils.NewTablePrinter(cmd.OutOrStdout()) - for _, pr := range prs { + for _, pr := range listResult.PullRequests { prNum := strconv.Itoa(pr.Number) if table.IsTTY() { prNum = "#" + prNum diff --git a/command/pr_test.go b/command/pr_test.go index c2199013d..4b9d0c6de 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -58,7 +58,7 @@ func RunCommand(cmd *cobra.Command, args string) (*cmdOut, error) { v.Replace([]string{}) default: switch v.Type() { - case "bool", "string": + case "bool", "string", "int": v.Set(f.DefValue) } } @@ -276,6 +276,10 @@ func TestPRList(t *testing.T) { t.Fatal(err) } + eq(t, output.Stderr(), ` +Showing 3 of 3 pull requests in OWNER/REPO + +`) eq(t, output.String(), `32 New feature feature 29 Fixed bad bug hubot:bug-fix 28 Improve documentation docs @@ -297,9 +301,8 @@ func TestPRList_filtering(t *testing.T) { eq(t, output.String(), "") eq(t, output.Stderr(), ` -Pull requests for OWNER/REPO +No pull requests match your search in OWNER/REPO -No pull requests match your search `) bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) diff --git a/test/fixtures/issueList.json b/test/fixtures/issueList.json index 40537f12d..4b19f3930 100644 --- a/test/fixtures/issueList.json +++ b/test/fixtures/issueList.json @@ -3,6 +3,7 @@ "repository": { "hasIssuesEnabled": true, "issues": { + "totalCount": 3, "nodes": [ { "number": 1, diff --git a/test/fixtures/prList.json b/test/fixtures/prList.json index 2808a5a8e..abd0e4ee9 100644 --- a/test/fixtures/prList.json +++ b/test/fixtures/prList.json @@ -2,6 +2,7 @@ "data": { "repository": { "pullRequests": { + "totalCount": 3, "edges": [ { "node": {