From 82661c197e681d79e5658387ee00e0b8468982a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 4 Aug 2020 18:38:06 +0200 Subject: [PATCH] Isolate `pr list` command --- command/alias.go | 5 +- command/issue.go | 92 +------ command/issue_test.go | 179 ------------- command/pr.go | 159 ----------- command/pr_test.go | 197 -------------- command/root.go | 2 + .../cmd/pr/list}/fixtures/prList.json | 0 .../list}/fixtures/prListWithDuplicates.json | 0 pkg/cmd/pr/list/list.go | 168 ++++++++++++ pkg/cmd/pr/list/list_test.go | 246 ++++++++++++++++++ pkg/cmd/pr/shared/display.go | 19 ++ pkg/cmd/pr/shared/display_test.go | 114 ++++++++ pkg/cmd/pr/shared/params.go | 51 ++++ pkg/cmd/pr/shared/params_test.go | 71 +++++ pkg/iostreams/iostreams.go | 42 +++ utils/table_printer.go | 27 +- 16 files changed, 732 insertions(+), 640 deletions(-) rename {test => pkg/cmd/pr/list}/fixtures/prList.json (100%) rename {test => pkg/cmd/pr/list}/fixtures/prListWithDuplicates.json (100%) create mode 100644 pkg/cmd/pr/list/list.go create mode 100644 pkg/cmd/pr/list/list_test.go create mode 100644 pkg/cmd/pr/shared/display_test.go create mode 100644 pkg/cmd/pr/shared/params_test.go diff --git a/command/alias.go b/command/alias.go index 7f02b03d5..bc3a9cf15 100644 --- a/command/alias.go +++ b/command/alias.go @@ -166,9 +166,8 @@ func aliasList(cmd *cobra.Command, args []string) error { return nil } - stdout := colorableOut(cmd) - - tp := utils.NewTablePrinter(stdout) + // TODO: connect to per-command io streams + tp := utils.NewTablePrinter(defaultStreams) aliasMap := aliasCfg.All() keys := []string{} diff --git a/command/issue.go b/command/issue.go index cca81db2f..02c5ba776 100644 --- a/command/issue.go +++ b/command/issue.go @@ -3,7 +3,6 @@ package command import ( "fmt" "io" - "net/url" "strconv" "strings" "time" @@ -122,57 +121,6 @@ var issueReopenCmd = &cobra.Command{ RunE: issueReopen, } -type filterOptions struct { - entity string - state string - assignee string - labels []string - author string - baseBranch string - mention string - milestone string -} - -func listURLWithQuery(listURL string, options filterOptions) (string, error) { - u, err := url.Parse(listURL) - if err != nil { - return "", err - } - query := fmt.Sprintf("is:%s ", options.entity) - if options.state != "all" { - query += fmt.Sprintf("is:%s ", options.state) - } - if options.assignee != "" { - query += fmt.Sprintf("assignee:%s ", options.assignee) - } - for _, label := range options.labels { - query += fmt.Sprintf("label:%s ", quoteValueForQuery(label)) - } - if options.author != "" { - query += fmt.Sprintf("author:%s ", options.author) - } - if options.baseBranch != "" { - query += fmt.Sprintf("base:%s ", options.baseBranch) - } - if options.mention != "" { - query += fmt.Sprintf("mentions:%s ", options.mention) - } - if options.milestone != "" { - query += fmt.Sprintf("milestone:%s ", quoteValueForQuery(options.milestone)) - } - q := u.Query() - q.Set("q", strings.TrimSuffix(query, " ")) - u.RawQuery = q.Encode() - return u.String(), nil -} - -func quoteValueForQuery(v string) string { - if strings.ContainsAny(v, " \"\t\r\n") { - return fmt.Sprintf("%q", v) - } - return v -} - func issueList(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) apiClient, err := apiClientForContext(ctx) @@ -230,14 +178,14 @@ func issueList(cmd *cobra.Command, args []string) error { if web { issueListURL := ghrepo.GenerateRepoURL(baseRepo, "issues") - openURL, err := listURLWithQuery(issueListURL, filterOptions{ - entity: "issue", - state: state, - assignee: assignee, - labels: labels, - author: author, - mention: mention, - milestone: milestone, + openURL, err := shared.ListURLWithQuery(issueListURL, shared.FilterOptions{ + Entity: "issue", + State: state, + Assignee: assignee, + Labels: labels, + Author: author, + Mention: mention, + Milestone: milestone, }) if err != nil { return err @@ -259,7 +207,7 @@ func issueList(cmd *cobra.Command, args []string) error { } }) - title := listHeader(ghrepo.FullName(baseRepo), "issue", len(listResult.Issues), listResult.TotalCount, hasFilters) + title := shared.ListHeader(ghrepo.FullName(baseRepo), "issue", len(listResult.Issues), listResult.TotalCount, hasFilters) if connectedToTerminal(cmd) { fmt.Fprintf(colorableErr(cmd), "\n%s\n\n", title) } @@ -362,25 +310,6 @@ func issueStateTitleWithColor(state string) string { return colorFunc(strings.Title(strings.ToLower(state))) } -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, fmt.Sprintf("open %s", itemName)), repoName) -} - func printRawIssuePreview(out io.Writer, issue *api.Issue) error { assignees := issueAssigneeList(*issue) labels := issueLabelList(*issue) @@ -619,7 +548,8 @@ func issueCreate(cmd *cobra.Command, args []string) error { } func printIssues(w io.Writer, prefix string, totalCount int, issues []api.Issue) { - table := utils.NewTablePrinter(w) + // TODO: accept io streams via argument + table := utils.NewTablePrinter(defaultStreams) for _, issue := range issues { issueNum := strconv.Itoa(issue.Number) if table.IsTTY() { diff --git a/command/issue_test.go b/command/issue_test.go index 8cde92d6e..16d613d72 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -803,117 +803,6 @@ func TestIssueCreate_webTitleBody(t *testing.T) { 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 open 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 open 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 open 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) - } - }) - } -} - func TestIssueStateTitleWithColor(t *testing.T) { tests := map[string]struct { state string @@ -1071,71 +960,3 @@ func TestIssueReopen_issuesDisabled(t *testing.T) { t.Fatalf("got error: %v", err) } } - -func Test_listURLWithQuery(t *testing.T) { - type args struct { - listURL string - options filterOptions - } - tests := []struct { - name string - args args - want string - wantErr bool - }{ - { - name: "blank", - args: args{ - listURL: "https://example.com/path?a=b", - options: filterOptions{ - entity: "issue", - state: "open", - }, - }, - want: "https://example.com/path?a=b&q=is%3Aissue+is%3Aopen", - wantErr: false, - }, - { - name: "all", - args: args{ - listURL: "https://example.com/path", - options: filterOptions{ - entity: "issue", - state: "open", - assignee: "bo", - author: "ka", - baseBranch: "trunk", - mention: "nu", - }, - }, - want: "https://example.com/path?q=is%3Aissue+is%3Aopen+assignee%3Abo+author%3Aka+base%3Atrunk+mentions%3Anu", - wantErr: false, - }, - { - name: "spaces in values", - args: args{ - listURL: "https://example.com/path", - options: filterOptions{ - entity: "pr", - state: "open", - labels: []string{"docs", "help wanted"}, - milestone: `Codename "What Was Missing"`, - }, - }, - want: "https://example.com/path?q=is%3Apr+is%3Aopen+label%3Adocs+label%3A%22help+wanted%22+milestone%3A%22Codename+%5C%22What+Was+Missing%5C%22%22", - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := listURLWithQuery(tt.args.listURL, tt.args.options) - if (err != nil) != tt.wantErr { - t.Errorf("listURLWithQuery() error = %v, wantErr %v", err, tt.wantErr) - return - } - if got != tt.want { - t.Errorf("listURLWithQuery() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/command/pr.go b/command/pr.go index 2ff6b4286..083400419 100644 --- a/command/pr.go +++ b/command/pr.go @@ -2,17 +2,11 @@ package command import ( "fmt" - "strconv" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" - "github.com/cli/cli/internal/ghrepo" - "github.com/cli/cli/pkg/cmd/pr/shared" - "github.com/cli/cli/pkg/cmdutil" - "github.com/cli/cli/pkg/text" "github.com/cli/cli/utils" "github.com/spf13/cobra" - "github.com/spf13/pflag" ) func init() { @@ -22,14 +16,6 @@ func init() { prCmd.AddCommand(prCloseCmd) prCmd.AddCommand(prReopenCmd) prCmd.AddCommand(prReadyCmd) - - prCmd.AddCommand(prListCmd) - prListCmd.Flags().BoolP("web", "w", false, "Open the browser to list the pull request(s)") - prListCmd.Flags().IntP("limit", "L", 30, "Maximum number of items to fetch") - prListCmd.Flags().StringP("state", "s", "open", "Filter by state: {open|closed|merged|all}") - prListCmd.Flags().StringP("base", "B", "", "Filter by base branch") - prListCmd.Flags().StringSliceP("label", "l", nil, "Filter by labels") - prListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") } var prCmd = &cobra.Command{ @@ -48,18 +34,6 @@ var prCmd = &cobra.Command{ - by URL, e.g. "https://github.com/OWNER/REPO/pull/123"; or - by the name of its head branch, e.g. "patch-1" or "OWNER:patch-1".`}, } -var prListCmd = &cobra.Command{ - Use: "list", - Short: "List and filter pull requests in this repository", - Args: cmdutil.NoArgsQuoteReminder, - Example: heredoc.Doc(` - $ gh pr list --limit 999 - $ gh pr list --state closed - $ gh pr list --label "priority 1" --label "bug" - $ gh pr list --web - `), - RunE: prList, -} var prCloseCmd = &cobra.Command{ Use: "close { | | }", Short: "Close a pull request", @@ -79,131 +53,6 @@ var prReadyCmd = &cobra.Command{ RunE: prReady, } -func prList(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) - if err != nil { - return err - } - - web, err := cmd.Flags().GetBool("web") - if err != nil { - return err - } - - limit, err := cmd.Flags().GetInt("limit") - if err != nil { - return err - } - if limit <= 0 { - return fmt.Errorf("invalid limit: %v", limit) - } - - state, err := cmd.Flags().GetString("state") - if err != nil { - return err - } - baseBranch, err := cmd.Flags().GetString("base") - if err != nil { - return err - } - labels, err := cmd.Flags().GetStringSlice("label") - if err != nil { - return err - } - assignee, err := cmd.Flags().GetString("assignee") - if err != nil { - return err - } - - if web { - prListURL := ghrepo.GenerateRepoURL(baseRepo, "pulls") - openURL, err := listURLWithQuery(prListURL, filterOptions{ - entity: "pr", - state: state, - assignee: assignee, - labels: labels, - baseBranch: baseBranch, - }) - if err != nil { - return err - } - fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", utils.DisplayURL(openURL)) - return utils.OpenInBrowser(openURL) - } - - var graphqlState []string - switch state { - case "open": - graphqlState = []string{"OPEN"} - case "closed": - graphqlState = []string{"CLOSED", "MERGED"} - case "merged": - graphqlState = []string{"MERGED"} - case "all": - graphqlState = []string{"OPEN", "CLOSED", "MERGED"} - default: - return fmt.Errorf("invalid state: %s", state) - } - - params := map[string]interface{}{ - "state": graphqlState, - } - if len(labels) > 0 { - params["labels"] = labels - } - if baseBranch != "" { - params["baseBranch"] = baseBranch - } - if assignee != "" { - params["assignee"] = assignee - } - - listResult, err := api.PullRequestList(apiClient, baseRepo, params, limit) - if err != nil { - return err - } - - hasFilters := false - cmd.Flags().Visit(func(f *pflag.Flag) { - switch f.Name { - case "state", "label", "base", "assignee": - hasFilters = true - } - }) - - title := listHeader(ghrepo.FullName(baseRepo), "pull request", len(listResult.PullRequests), listResult.TotalCount, hasFilters) - if connectedToTerminal(cmd) { - fmt.Fprintf(colorableErr(cmd), "\n%s\n\n", title) - } - - table := utils.NewTablePrinter(cmd.OutOrStdout()) - for _, pr := range listResult.PullRequests { - prNum := strconv.Itoa(pr.Number) - if table.IsTTY() { - prNum = "#" + prNum - } - table.AddField(prNum, nil, shared.ColorFuncForPR(pr)) - table.AddField(text.ReplaceExcessiveWhitespace(pr.Title), nil, nil) - table.AddField(pr.HeadLabel(), nil, utils.Cyan) - if !table.IsTTY() { - table.AddField(prStateWithDraft(&pr), nil, nil) - } - table.EndRow() - } - err = table.Render() - if err != nil { - return err - } - - return nil -} - func prClose(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) apiClient, err := apiClientForContext(ctx) @@ -266,14 +115,6 @@ func prReopen(cmd *cobra.Command, args []string) error { return nil } -func prStateWithDraft(pr *api.PullRequest) string { - if pr.IsDraft && pr.State == "OPEN" { - return "DRAFT" - } - - return pr.State -} - func prReady(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) apiClient, err := apiClientForContext(ctx) diff --git a/command/pr_test.go b/command/pr_test.go index f02a2e9f2..3ac35d624 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -2,16 +2,9 @@ package command import ( "bytes" - "os/exec" "reflect" "regexp" - "strings" "testing" - - "github.com/cli/cli/internal/run" - "github.com/cli/cli/pkg/httpmock" - "github.com/cli/cli/test" - "github.com/stretchr/testify/assert" ) func eq(t *testing.T, got interface{}, expected interface{}) { @@ -21,196 +14,6 @@ func eq(t *testing.T, got interface{}, expected interface{}) { } } -func TestPRList(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestList\b`), httpmock.FileResponse("../test/fixtures/prList.json")) - - output, err := RunCommand("pr list") - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, ` -Showing 3 of 3 open pull requests in OWNER/REPO - -`, output.Stderr()) - - lines := strings.Split(output.String(), "\n") - res := []*regexp.Regexp{ - regexp.MustCompile(`#32.*New feature.*feature`), - regexp.MustCompile(`#29.*Fixed bad bug.*hubot:bug-fix`), - regexp.MustCompile(`#28.*Improve documentation.*docs`), - } - - for i, r := range res { - if !r.MatchString(lines[i]) { - t.Errorf("%s did not match %s", lines[i], r) - } - } -} - -func TestPRList_nontty(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(false)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestList\b`), httpmock.FileResponse("../test/fixtures/prList.json")) - - output, err := RunCommand("pr list") - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, "", output.Stderr()) - - assert.Equal(t, `32 New feature feature DRAFT -29 Fixed bad bug hubot:bug-fix OPEN -28 Improve documentation docs MERGED -`, output.String()) -} - -func TestPRList_filtering(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestList\b`), - httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { - assert.Equal(t, []interface{}{"OPEN", "CLOSED", "MERGED"}, params["state"].([]interface{})) - assert.Equal(t, []interface{}{"one", "two", "three"}, params["labels"].([]interface{})) - })) - - output, err := RunCommand(`pr list -s all -l one,two -l three`) - if err != nil { - t.Fatal(err) - } - - eq(t, output.String(), "") - eq(t, output.Stderr(), ` -No pull requests match your search in OWNER/REPO - -`) -} - -func TestPRList_filteringRemoveDuplicate(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestList\b`), - httpmock.FileResponse("../test/fixtures/prListWithDuplicates.json")) - - output, err := RunCommand("pr list -l one,two") - if err != nil { - t.Fatal(err) - } - - lines := strings.Split(output.String(), "\n") - - res := []*regexp.Regexp{ - regexp.MustCompile(`#32.*New feature.*feature`), - regexp.MustCompile(`#29.*Fixed bad bug.*hubot:bug-fix`), - regexp.MustCompile(`#28.*Improve documentation.*docs`), - } - - for i, r := range res { - if !r.MatchString(lines[i]) { - t.Errorf("%s did not match %s", lines[i], r) - } - } -} - -func TestPRList_filteringClosed(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestList\b`), - httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { - assert.Equal(t, []interface{}{"CLOSED", "MERGED"}, params["state"].([]interface{})) - })) - - _, err := RunCommand(`pr list -s closed`) - if err != nil { - t.Fatal(err) - } -} - -func TestPRList_filteringAssignee(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestList\b`), - httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { - assert.Equal(t, `repo:OWNER/REPO assignee:hubot is:pr sort:created-desc is:merged label:"needs tests" base:"develop"`, params["q"].(string)) - })) - - _, err := RunCommand(`pr list -s merged -l "needs tests" -a hubot -B develop`) - if err != nil { - t.Fatal(err) - } -} - -func TestPRList_filteringAssigneeLabels(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - initFakeHTTP() - - _, err := RunCommand(`pr list -l one,two -a hubot`) - if err == nil && err.Error() != "multiple labels with --assignee are not supported" { - t.Fatal(err) - } -} - -func TestPRList_withInvalidLimitFlag(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - initFakeHTTP() - - _, err := RunCommand(`pr list --limit=0`) - if err == nil && err.Error() != "invalid limit: 0" { - t.Errorf("error running command `issue list`: %v", err) - } -} - -func TestPRList_web(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand("pr list --web -a peter -l bug -l docs -L 10 -s merged -B trunk") - if err != nil { - t.Errorf("error running command `pr list` with `--web` flag: %v", err) - } - - expectedURL := "https://github.com/OWNER/REPO/pulls?q=is%3Apr+is%3Amerged+assignee%3Apeter+label%3Abug+label%3Adocs+base%3Atrunk" - - eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening github.com/OWNER/REPO/pulls in your browser.\n") - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, expectedURL) -} - func TestPrClose(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() diff --git a/command/root.go b/command/root.go index 4c9412445..f24a0233d 100644 --- a/command/root.go +++ b/command/root.go @@ -26,6 +26,7 @@ import ( prCheckoutCmd "github.com/cli/cli/pkg/cmd/pr/checkout" prCreateCmd "github.com/cli/cli/pkg/cmd/pr/create" prDiffCmd "github.com/cli/cli/pkg/cmd/pr/diff" + prListCmd "github.com/cli/cli/pkg/cmd/pr/list" prMergeCmd "github.com/cli/cli/pkg/cmd/pr/merge" prReviewCmd "github.com/cli/cli/pkg/cmd/pr/review" prStatusCmd "github.com/cli/cli/pkg/cmd/pr/status" @@ -181,6 +182,7 @@ func init() { prCmd.AddCommand(prMergeCmd.NewCmdMerge(&repoResolvingCmdFactory, nil)) prCmd.AddCommand(prStatusCmd.NewCmdStatus(&repoResolvingCmdFactory, nil)) prCmd.AddCommand(prCreateCmd.NewCmdCreate(&repoResolvingCmdFactory, nil)) + prCmd.AddCommand(prListCmd.NewCmdList(&repoResolvingCmdFactory, nil)) RootCmd.AddCommand(creditsCmd.NewCmdCredits(cmdFactory, nil)) } diff --git a/test/fixtures/prList.json b/pkg/cmd/pr/list/fixtures/prList.json similarity index 100% rename from test/fixtures/prList.json rename to pkg/cmd/pr/list/fixtures/prList.json diff --git a/test/fixtures/prListWithDuplicates.json b/pkg/cmd/pr/list/fixtures/prListWithDuplicates.json similarity index 100% rename from test/fixtures/prListWithDuplicates.json rename to pkg/cmd/pr/list/fixtures/prListWithDuplicates.json diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go new file mode 100644 index 000000000..da8a04857 --- /dev/null +++ b/pkg/cmd/pr/list/list.go @@ -0,0 +1,168 @@ +package list + +import ( + "fmt" + "net/http" + "strconv" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/text" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type ListOptions struct { + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + + WebMode bool + LimitResults int + State string + BaseBranch string + Labels []string + Assignee string +} + +func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { + opts := &ListOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + BaseRepo: f.BaseRepo, + } + + cmd := &cobra.Command{ + Use: "list", + Short: "List and filter pull requests in this repository", + Example: heredoc.Doc(` + $ gh pr list --limit 999 + $ gh pr list --state closed + $ gh pr list --label "priority 1" --label "bug" + $ gh pr list --web + `), + Args: cmdutil.NoArgsQuoteReminder, + RunE: func(cmd *cobra.Command, args []string) error { + if opts.LimitResults < 1 { + return &cmdutil.FlagError{Err: fmt.Errorf("invalid value for --limit: %v", opts.LimitResults)} + } + + if runF != nil { + return runF(opts) + } + return listRun(opts) + }, + } + + cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the browser to list the pull requests") + cmd.Flags().IntVarP(&opts.LimitResults, "limit", "L", 30, "Maximum number of items to fetch") + cmd.Flags().StringVarP(&opts.State, "state", "s", "open", "Filter by state: {open|closed|merged|all}") + cmd.Flags().StringVarP(&opts.BaseBranch, "base", "B", "", "Filter by base branch") + cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Filter by labels") + cmd.Flags().StringVarP(&opts.Assignee, "assignee", "a", "", "Filter by assignee") + + return cmd +} + +func listRun(opts *ListOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + baseRepo, err := opts.BaseRepo() + if err != nil { + return err + } + + if opts.WebMode { + prListURL := ghrepo.GenerateRepoURL(baseRepo, "pulls") + openURL, err := shared.ListURLWithQuery(prListURL, shared.FilterOptions{ + Entity: "pr", + State: opts.State, + Assignee: opts.Assignee, + Labels: opts.Labels, + BaseBranch: opts.BaseBranch, + }) + if err != nil { + return err + } + + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL)) + } + return utils.OpenInBrowser(openURL) + } + + var graphqlState []string + switch opts.State { + case "open": + graphqlState = []string{"OPEN"} + case "closed": + graphqlState = []string{"CLOSED", "MERGED"} + case "merged": + graphqlState = []string{"MERGED"} + case "all": + graphqlState = []string{"OPEN", "CLOSED", "MERGED"} + default: + return fmt.Errorf("invalid state: %s", opts.State) + } + + params := map[string]interface{}{ + "state": graphqlState, + } + if len(opts.Labels) > 0 { + params["labels"] = opts.Labels + } + if opts.BaseBranch != "" { + params["baseBranch"] = opts.BaseBranch + } + if opts.Assignee != "" { + params["assignee"] = opts.Assignee + } + + listResult, err := api.PullRequestList(apiClient, baseRepo, params, opts.LimitResults) + if err != nil { + return err + } + + if opts.IO.IsStdoutTTY() { + hasFilters := opts.State != "open" || len(opts.Labels) > 0 || opts.BaseBranch != "" || opts.Assignee != "" + title := shared.ListHeader(ghrepo.FullName(baseRepo), "pull request", len(listResult.PullRequests), listResult.TotalCount, hasFilters) + fmt.Fprintf(opts.IO.ErrOut, "\n%s\n\n", title) + } + + table := utils.NewTablePrinter(opts.IO) + for _, pr := range listResult.PullRequests { + prNum := strconv.Itoa(pr.Number) + if table.IsTTY() { + prNum = "#" + prNum + } + table.AddField(prNum, nil, shared.ColorFuncForPR(pr)) + table.AddField(text.ReplaceExcessiveWhitespace(pr.Title), nil, nil) + table.AddField(pr.HeadLabel(), nil, utils.Cyan) + if !table.IsTTY() { + table.AddField(prStateWithDraft(&pr), nil, nil) + } + table.EndRow() + } + err = table.Render() + if err != nil { + return err + } + + return nil +} + +func prStateWithDraft(pr *api.PullRequest) string { + if pr.IsDraft && pr.State == "OPEN" { + return "DRAFT" + } + + return pr.State +} diff --git a/pkg/cmd/pr/list/list_test.go b/pkg/cmd/pr/list/list_test.go new file mode 100644 index 000000000..5faf42b57 --- /dev/null +++ b/pkg/cmd/pr/list/list_test.go @@ -0,0 +1,246 @@ +package list + +import ( + "bytes" + "io/ioutil" + "net/http" + "os/exec" + "reflect" + "regexp" + "strings" + "testing" + + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/internal/run" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func eq(t *testing.T, got interface{}, expected interface{}) { + t.Helper() + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected: %v, got: %v", expected, got) + } +} +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + } + + cmd := NewCmdList(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func initFakeHTTP() *httpmock.Registry { + return &httpmock.Registry{} +} + +func TestPRList(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + http.Register(httpmock.GraphQL(`query PullRequestList\b`), httpmock.FileResponse("./fixtures/prList.json")) + + output, err := runCommand(http, true, "") + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, ` +Showing 3 of 3 open pull requests in OWNER/REPO + +`, output.Stderr()) + + lines := strings.Split(output.String(), "\n") + res := []*regexp.Regexp{ + regexp.MustCompile(`#32.*New feature.*feature`), + regexp.MustCompile(`#29.*Fixed bad bug.*hubot:bug-fix`), + regexp.MustCompile(`#28.*Improve documentation.*docs`), + } + + for i, r := range res { + if !r.MatchString(lines[i]) { + t.Errorf("%s did not match %s", lines[i], r) + } + } +} + +func TestPRList_nontty(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + http.Register(httpmock.GraphQL(`query PullRequestList\b`), httpmock.FileResponse("./fixtures/prList.json")) + + output, err := runCommand(http, false, "") + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, "", output.Stderr()) + + assert.Equal(t, `32 New feature feature DRAFT +29 Fixed bad bug hubot:bug-fix OPEN +28 Improve documentation docs MERGED +`, output.String()) +} + +func TestPRList_filtering(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query PullRequestList\b`), + httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { + assert.Equal(t, []interface{}{"OPEN", "CLOSED", "MERGED"}, params["state"].([]interface{})) + assert.Equal(t, []interface{}{"one", "two", "three"}, params["labels"].([]interface{})) + })) + + output, err := runCommand(http, true, `-s all -l one,two -l three`) + if err != nil { + t.Fatal(err) + } + + eq(t, output.String(), "") + eq(t, output.Stderr(), ` +No pull requests match your search in OWNER/REPO + +`) +} + +func TestPRList_filteringRemoveDuplicate(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query PullRequestList\b`), + httpmock.FileResponse("./fixtures/prListWithDuplicates.json")) + + output, err := runCommand(http, true, "-l one,two") + if err != nil { + t.Fatal(err) + } + + lines := strings.Split(output.String(), "\n") + + res := []*regexp.Regexp{ + regexp.MustCompile(`#32.*New feature.*feature`), + regexp.MustCompile(`#29.*Fixed bad bug.*hubot:bug-fix`), + regexp.MustCompile(`#28.*Improve documentation.*docs`), + } + + for i, r := range res { + if !r.MatchString(lines[i]) { + t.Errorf("%s did not match %s", lines[i], r) + } + } +} + +func TestPRList_filteringClosed(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query PullRequestList\b`), + httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { + assert.Equal(t, []interface{}{"CLOSED", "MERGED"}, params["state"].([]interface{})) + })) + + _, err := runCommand(http, true, `-s closed`) + if err != nil { + t.Fatal(err) + } +} + +func TestPRList_filteringAssignee(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query PullRequestList\b`), + httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { + assert.Equal(t, `repo:OWNER/REPO assignee:hubot is:pr sort:created-desc is:merged label:"needs tests" base:"develop"`, params["q"].(string)) + })) + + _, err := runCommand(http, true, `-s merged -l "needs tests" -a hubot -B develop`) + if err != nil { + t.Fatal(err) + } +} + +func TestPRList_filteringAssigneeLabels(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + _, err := runCommand(http, true, `-l one,two -a hubot`) + if err == nil && err.Error() != "multiple labels with --assignee are not supported" { + t.Fatal(err) + } +} + +func TestPRList_withInvalidLimitFlag(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + _, err := runCommand(http, true, `--limit=0`) + if err == nil && err.Error() != "invalid limit: 0" { + t.Errorf("error running command `issue list`: %v", err) + } +} + +func TestPRList_web(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + var seenCmd *exec.Cmd + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { + seenCmd = cmd + return &test.OutputStub{} + }) + defer restoreCmd() + + output, err := runCommand(http, true, "--web -a peter -l bug -l docs -L 10 -s merged -B trunk") + if err != nil { + t.Errorf("error running command `pr list` with `--web` flag: %v", err) + } + + expectedURL := "https://github.com/OWNER/REPO/pulls?q=is%3Apr+is%3Amerged+assignee%3Apeter+label%3Abug+label%3Adocs+base%3Atrunk" + + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening github.com/OWNER/REPO/pulls in your browser.\n") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, expectedURL) +} diff --git a/pkg/cmd/pr/shared/display.go b/pkg/cmd/pr/shared/display.go index f43b7fa67..31e7228b6 100644 --- a/pkg/cmd/pr/shared/display.go +++ b/pkg/cmd/pr/shared/display.go @@ -45,3 +45,22 @@ func PrintHeader(w io.Writer, s string) { func PrintMessage(w io.Writer, s string) { fmt.Fprintln(w, utils.Gray(s)) } + +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, fmt.Sprintf("open %s", itemName)), repoName) +} diff --git a/pkg/cmd/pr/shared/display_test.go b/pkg/cmd/pr/shared/display_test.go new file mode 100644 index 000000000..d958d2265 --- /dev/null +++ b/pkg/cmd/pr/shared/display_test.go @@ -0,0 +1,114 @@ +package shared + +import "testing" + +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 open 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 open 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 open 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/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index d2556cafe..616b4b358 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -112,3 +112,54 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par return nil } + +type FilterOptions struct { + Entity string + State string + Assignee string + Labels []string + Author string + BaseBranch string + Mention string + Milestone string +} + +func ListURLWithQuery(listURL string, options FilterOptions) (string, error) { + u, err := url.Parse(listURL) + if err != nil { + return "", err + } + query := fmt.Sprintf("is:%s ", options.Entity) + if options.State != "all" { + query += fmt.Sprintf("is:%s ", options.State) + } + if options.Assignee != "" { + query += fmt.Sprintf("assignee:%s ", options.Assignee) + } + for _, label := range options.Labels { + query += fmt.Sprintf("label:%s ", quoteValueForQuery(label)) + } + if options.Author != "" { + query += fmt.Sprintf("author:%s ", options.Author) + } + if options.BaseBranch != "" { + query += fmt.Sprintf("base:%s ", options.BaseBranch) + } + if options.Mention != "" { + query += fmt.Sprintf("mentions:%s ", options.Mention) + } + if options.Milestone != "" { + query += fmt.Sprintf("milestone:%s ", quoteValueForQuery(options.Milestone)) + } + q := u.Query() + q.Set("q", strings.TrimSuffix(query, " ")) + u.RawQuery = q.Encode() + return u.String(), nil +} + +func quoteValueForQuery(v string) string { + if strings.ContainsAny(v, " \"\t\r\n") { + return fmt.Sprintf("%q", v) + } + return v +} diff --git a/pkg/cmd/pr/shared/params_test.go b/pkg/cmd/pr/shared/params_test.go new file mode 100644 index 000000000..177ae0148 --- /dev/null +++ b/pkg/cmd/pr/shared/params_test.go @@ -0,0 +1,71 @@ +package shared + +import "testing" + +func Test_listURLWithQuery(t *testing.T) { + type args struct { + listURL string + options FilterOptions + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "blank", + args: args{ + listURL: "https://example.com/path?a=b", + options: FilterOptions{ + Entity: "issue", + State: "open", + }, + }, + want: "https://example.com/path?a=b&q=is%3Aissue+is%3Aopen", + wantErr: false, + }, + { + name: "all", + args: args{ + listURL: "https://example.com/path", + options: FilterOptions{ + Entity: "issue", + State: "open", + Assignee: "bo", + Author: "ka", + BaseBranch: "trunk", + Mention: "nu", + }, + }, + want: "https://example.com/path?q=is%3Aissue+is%3Aopen+assignee%3Abo+author%3Aka+base%3Atrunk+mentions%3Anu", + wantErr: false, + }, + { + name: "spaces in values", + args: args{ + listURL: "https://example.com/path", + options: FilterOptions{ + Entity: "pr", + State: "open", + Labels: []string{"docs", "help wanted"}, + Milestone: `Codename "What Was Missing"`, + }, + }, + want: "https://example.com/path?q=is%3Apr+is%3Aopen+label%3Adocs+label%3A%22help+wanted%22+milestone%3A%22Codename+%5C%22What+Was+Missing%5C%22%22", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ListURLWithQuery(tt.args.listURL, tt.args.options) + if (err != nil) != tt.wantErr { + t.Errorf("listURLWithQuery() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("listURLWithQuery() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index a1ffdad32..cf69ecd8c 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -2,12 +2,17 @@ package iostreams import ( "bytes" + "fmt" "io" "io/ioutil" "os" + "os/exec" + "strconv" + "strings" "github.com/mattn/go-colorable" "github.com/mattn/go-isatty" + "golang.org/x/crypto/ssh/terminal" ) type IOStreams struct { @@ -74,6 +79,29 @@ func (s *IOStreams) IsStderrTTY() bool { return false } +func (s *IOStreams) TerminalWidth() int { + defaultWidth := 80 + if s.stdoutTTYOverride { + return defaultWidth + } + + if w, _, err := terminalSize(s.Out); err == nil { + return w + } + + if isCygwinTerminal(s.Out) { + tputCmd := exec.Command("tput", "cols") + tputCmd.Stdin = os.Stdin + if out, err := tputCmd.Output(); err == nil { + if w, err := strconv.Atoi(strings.TrimSpace(string(out))); err == nil { + return w + } + } + } + + return defaultWidth +} + func System() *IOStreams { var out io.Writer = os.Stdout var colorEnabled bool @@ -104,3 +132,17 @@ func Test() (*IOStreams, *bytes.Buffer, *bytes.Buffer, *bytes.Buffer) { func isTerminal(f *os.File) bool { return isatty.IsTerminal(f.Fd()) || isatty.IsCygwinTerminal(f.Fd()) } + +func isCygwinTerminal(w io.Writer) bool { + if f, isFile := w.(*os.File); isFile { + return isatty.IsCygwinTerminal(f.Fd()) + } + return false +} + +func terminalSize(w io.Writer) (int, int, error) { + if f, isFile := w.(*os.File); isFile { + return terminal.GetSize(int(f.Fd())) + } + return 0, 0, fmt.Errorf("%v is not a file", w) +} diff --git a/utils/table_printer.go b/utils/table_printer.go index 823a4b533..3047e2d81 100644 --- a/utils/table_printer.go +++ b/utils/table_printer.go @@ -3,11 +3,9 @@ package utils import ( "fmt" "io" - "os" - "os/exec" - "strconv" "strings" + "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/text" ) @@ -18,28 +16,15 @@ type TablePrinter interface { Render() error } -func NewTablePrinter(w io.Writer) TablePrinter { - if IsTerminal(w) { - isCygwin := IsCygwinTerminal(w) - ttyWidth := 80 - if termWidth, _, err := TerminalSize(w); err == nil { - ttyWidth = termWidth - } else if isCygwin { - tputCmd := exec.Command("tput", "cols") - tputCmd.Stdin = os.Stdin - if out, err := tputCmd.Output(); err == nil { - if w, err := strconv.Atoi(strings.TrimSpace(string(out))); err == nil { - ttyWidth = w - } - } - } +func NewTablePrinter(io *iostreams.IOStreams) TablePrinter { + if io.IsStdoutTTY() { return &ttyTablePrinter{ - out: NewColorable(w), - maxWidth: ttyWidth, + out: io.Out, + maxWidth: io.TerminalWidth(), } } return &tsvTablePrinter{ - out: w, + out: io.Out, } }