From ccfe38de260f06bd399eaf6daa53d60980cf4397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 4 Sep 2020 23:08:09 +0200 Subject: [PATCH] Add PAGER support to commands that produce significant output --- pkg/cmd/issue/list/fixtures/issueList.json | 3 ++ pkg/cmd/issue/list/list.go | 8 +++- pkg/cmd/issue/list/list_test.go | 24 ++++++----- pkg/cmd/issue/status/status.go | 6 +++ pkg/cmd/issue/view/view.go | 8 +++- pkg/cmd/pr/list/list.go | 8 +++- pkg/cmd/pr/list/list_test.go | 47 ++++++++-------------- pkg/cmd/pr/status/status.go | 6 +++ pkg/cmd/pr/view/view.go | 6 +++ pkg/cmd/repo/view/view.go | 14 ++++++- 10 files changed, 86 insertions(+), 44 deletions(-) diff --git a/pkg/cmd/issue/list/fixtures/issueList.json b/pkg/cmd/issue/list/fixtures/issueList.json index 4b19f3930..1f878f7a5 100644 --- a/pkg/cmd/issue/list/fixtures/issueList.json +++ b/pkg/cmd/issue/list/fixtures/issueList.json @@ -9,6 +9,7 @@ "number": 1, "title": "number won", "url": "https://wow.com", + "updatedAt": "2011-01-26T19:01:12Z", "labels": { "nodes": [ { @@ -22,6 +23,7 @@ "number": 2, "title": "number too", "url": "https://wow.com", + "updatedAt": "2011-01-26T19:01:12Z", "labels": { "nodes": [ { @@ -35,6 +37,7 @@ "number": 4, "title": "number fore", "url": "https://wow.com", + "updatedAt": "2011-01-26T19:01:12Z", "labels": { "nodes": [ { diff --git a/pkg/cmd/issue/list/list.go b/pkg/cmd/issue/list/list.go index 1065e61be..017556af6 100644 --- a/pkg/cmd/issue/list/list.go +++ b/pkg/cmd/issue/list/list.go @@ -116,10 +116,16 @@ func listRun(opts *ListOptions) error { return err } + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + if isTerminal { hasFilters := opts.State != "open" || len(opts.Labels) > 0 || opts.Assignee != "" || opts.Author != "" || opts.Mention != "" || opts.Milestone != "" title := prShared.ListHeader(ghrepo.FullName(baseRepo), "issue", len(listResult.Issues), listResult.TotalCount, hasFilters) - fmt.Fprintf(opts.IO.ErrOut, "\n%s\n\n", title) + fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) } issueShared.PrintIssues(opts.IO, "", len(listResult.Issues), listResult.Issues) diff --git a/pkg/cmd/issue/list/list_test.go b/pkg/cmd/issue/list/list_test.go index 6646a10f7..ff202b6f3 100644 --- a/pkg/cmd/issue/list/list_test.go +++ b/pkg/cmd/issue/list/list_test.go @@ -7,8 +7,10 @@ import ( "net/http" "os/exec" "reflect" + "regexp" "testing" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/run" @@ -97,15 +99,19 @@ func TestIssueList_tty(t *testing.T) { t.Errorf("error running command `issue list`: %v", err) } - eq(t, output.Stderr(), ` -Showing 3 of 3 open issues in OWNER/REPO + out := output.String() + timeRE := regexp.MustCompile(`\d+ years`) + out = timeRE.ReplaceAllString(out, "X years") -`) + assert.Equal(t, heredoc.Doc(` - test.ExpectLines(t, output.String(), - "number won", - "number too", - "number fore") + Showing 3 of 3 open issues in OWNER/REPO + + #1 number won (label) about X years ago + #2 number too (label) about X years ago + #4 number fore (label) about X years ago + `), out) + assert.Equal(t, ``, output.Stderr()) } func TestIssueList_tty_withFlags(t *testing.T) { @@ -141,8 +147,8 @@ func TestIssueList_tty_withFlags(t *testing.T) { t.Errorf("error running command `issue list`: %v", err) } - eq(t, output.String(), "") - eq(t, output.Stderr(), ` + eq(t, output.Stderr(), "") + eq(t, output.String(), ` No issues match your search in OWNER/REPO `) diff --git a/pkg/cmd/issue/status/status.go b/pkg/cmd/issue/status/status.go index d1b68bc0d..42ba91823 100644 --- a/pkg/cmd/issue/status/status.go +++ b/pkg/cmd/issue/status/status.go @@ -68,6 +68,12 @@ func statusRun(opts *StatusOptions) error { return err } + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + out := opts.IO.Out fmt.Fprintln(out, "") diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 6d2b0eeaa..7ff7a0e43 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -86,10 +86,16 @@ func viewRun(opts *ViewOptions) error { fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", openURL) return utils.OpenInBrowser(openURL) } + + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + if opts.IO.IsStdoutTTY() { return printHumanIssuePreview(opts.IO.Out, issue) } - return printRawIssuePreview(opts.IO.Out, issue) } diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go index fd1ebb2b9..e4fe318c8 100644 --- a/pkg/cmd/pr/list/list.go +++ b/pkg/cmd/pr/list/list.go @@ -133,10 +133,16 @@ func listRun(opts *ListOptions) error { return err } + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + 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) + fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) } table := utils.NewTablePrinter(opts.IO) diff --git a/pkg/cmd/pr/list/list_test.go b/pkg/cmd/pr/list/list_test.go index 5faf42b57..ab99e4aa9 100644 --- a/pkg/cmd/pr/list/list_test.go +++ b/pkg/cmd/pr/list/list_test.go @@ -6,10 +6,10 @@ import ( "net/http" "os/exec" "reflect" - "regexp" "strings" "testing" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmdutil" @@ -76,23 +76,15 @@ func TestPRList(t *testing.T) { t.Fatal(err) } - assert.Equal(t, ` -Showing 3 of 3 open pull requests in OWNER/REPO + assert.Equal(t, heredoc.Doc(` -`, 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) - } - } + Showing 3 of 3 open pull requests in OWNER/REPO + + #32 New feature feature + #29 Fixed bad bug hubot:bug-fix + #28 Improve documentation docs + `), output.String()) + assert.Equal(t, ``, output.Stderr()) } func TestPRList_nontty(t *testing.T) { @@ -130,8 +122,8 @@ func TestPRList_filtering(t *testing.T) { t.Fatal(err) } - eq(t, output.String(), "") - eq(t, output.Stderr(), ` + eq(t, output.Stderr(), "") + eq(t, output.String(), ` No pull requests match your search in OWNER/REPO `) @@ -150,19 +142,12 @@ func TestPRList_filteringRemoveDuplicate(t *testing.T) { 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) - } + out := output.String() + idx := strings.Index(out, "New feature") + if idx < 0 { + t.Fatalf("text %q not found in %q", "New feature", out) } + assert.Equal(t, idx, strings.LastIndex(out, "New feature")) } func TestPRList_filteringClosed(t *testing.T) { diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go index dffd2b412..2c012d712 100644 --- a/pkg/cmd/pr/status/status.go +++ b/pkg/cmd/pr/status/status.go @@ -97,6 +97,12 @@ func statusRun(opts *StatusOptions) error { return err } + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + out := opts.IO.Out fmt.Fprintln(out, "") diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index b653e5ffe..e8b5398b2 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -99,6 +99,12 @@ func viewRun(opts *ViewOptions) error { return utils.OpenInBrowser(openURL) } + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + if connectedToTerminal { return printHumanPrPreview(opts.IO.Out, pr) } diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index adcf384a4..f5a70f54b 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -1,10 +1,13 @@ package view import ( + "errors" "fmt" "html/template" "net/http" + "os" "strings" + "syscall" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" @@ -107,6 +110,12 @@ func viewRun(opts *ViewOptions) error { return err } + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + stdout := opts.IO.Out if !opts.IO.IsStdoutTTY() { @@ -167,7 +176,10 @@ func viewRun(opts *ViewOptions) error { err = tmpl.Execute(stdout, repoData) if err != nil { - return err + var pathError *os.PathError + if !errors.As(err, &pathError) || pathError.Op != "write" || pathError.Unwrap() != syscall.EPIPE { + return err + } } return nil