diff --git a/cmd/gh/main.go b/cmd/gh/main.go index fdde7acd3..73b153ffa 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -51,12 +51,14 @@ func main() { os.Exit(2) } - prompt, _ := cfg.Get("", "prompt") - - if prompt == config.PromptsDisabled { + if prompt, _ := cfg.Get("", "prompt"); prompt == config.PromptsDisabled { cmdFactory.IOStreams.SetNeverPrompt(true) } + if pager, _ := cfg.Get("", "pager"); pager != "" { + cmdFactory.IOStreams.SetPager(pager) + } + expandedArgs := []string{} if len(os.Args) > 0 { expandedArgs = os.Args[1:] diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index 1eb97ab10..f40cb9097 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -110,14 +110,14 @@ github.com: _, err := ParseConfig("config.yml") assert.Nil(t, err) - expectedMain := "# What protocol to use when performing git operations. Supported values: ssh, https\ngit_protocol: https\n# What editor gh should run when creating issues, pull requests, etc. If blank, will refer to environment.\neditor:\n# When to interactively prompt. This is a global config that cannot be overriden by hostname. Supported values: enabled, disabled\nprompt: enabled\n# Aliases allow you to create nicknames for gh commands\naliases:\n co: pr checkout\n" expectedHosts := `github.com: user: keiyuri oauth_token: "123456" ` - assert.Equal(t, expectedMain, mainBuf.String()) assert.Equal(t, expectedHosts, hostsBuf.String()) + assert.NotContains(t, mainBuf.String(), "github.com") + assert.NotContains(t, mainBuf.String(), "oauth_token") } func Test_parseConfigFile(t *testing.T) { diff --git a/internal/config/config_type.go b/internal/config/config_type.go index 740955539..2ff23e58c 100644 --- a/internal/config/config_type.go +++ b/internal/config/config_type.go @@ -180,6 +180,15 @@ func NewBlankRoot() *yaml.Node { Kind: yaml.ScalarNode, Value: PromptsEnabled, }, + { + HeadComment: "A pager program to send command output to. Example value: less", + Kind: yaml.ScalarNode, + Value: "pager", + }, + { + Kind: yaml.ScalarNode, + Value: "", + }, { HeadComment: "Aliases allow you to create nicknames for gh commands", Kind: yaml.ScalarNode, diff --git a/internal/config/config_type_test.go b/internal/config/config_type_test.go index 6e561f634..50e2b9f5a 100644 --- a/internal/config/config_type_test.go +++ b/internal/config/config_type_test.go @@ -4,6 +4,7 @@ import ( "bytes" "testing" + "github.com/MakeNowJust/heredoc" "github.com/stretchr/testify/assert" ) @@ -19,8 +20,8 @@ func Test_fileConfig_Set(t *testing.T) { assert.NoError(t, c.Set("github.com", "user", "hubot")) assert.NoError(t, c.Write()) - expected := "# What protocol to use when performing git operations. Supported values: ssh, https\ngit_protocol: https\n# What editor gh should run when creating issues, pull requests, etc. If blank, will refer to environment.\neditor: nano\n# When to interactively prompt. This is a global config that cannot be overriden by hostname. Supported values: enabled, disabled\nprompt: enabled\n# Aliases allow you to create nicknames for gh commands\naliases:\n co: pr checkout\n" - assert.Equal(t, expected, mainBuf.String()) + assert.Contains(t, mainBuf.String(), "editor: nano") + assert.Contains(t, mainBuf.String(), "git_protocol: https") assert.Equal(t, `github.com: git_protocol: ssh user: hubot @@ -37,7 +38,19 @@ func Test_defaultConfig(t *testing.T) { cfg := NewBlankConfig() assert.NoError(t, cfg.Write()) - expected := "# What protocol to use when performing git operations. Supported values: ssh, https\ngit_protocol: https\n# What editor gh should run when creating issues, pull requests, etc. If blank, will refer to environment.\neditor:\n# When to interactively prompt. This is a global config that cannot be overriden by hostname. Supported values: enabled, disabled\nprompt: enabled\n# Aliases allow you to create nicknames for gh commands\naliases:\n co: pr checkout\n" + expected := heredoc.Doc(` + # What protocol to use when performing git operations. Supported values: ssh, https + git_protocol: https + # What editor gh should run when creating issues, pull requests, etc. If blank, will refer to environment. + editor: + # When to interactively prompt. This is a global config that cannot be overriden by hostname. Supported values: enabled, disabled + prompt: enabled + # A pager program to send command output to. Example value: less + pager: + # Aliases allow you to create nicknames for gh commands + aliases: + co: pr checkout + `) assert.Equal(t, expected, mainBuf.String()) assert.Equal(t, "", hostsBuf.String()) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index f051883a8..9ec4de435 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -13,6 +13,7 @@ import ( "sort" "strconv" "strings" + "syscall" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/ghinstance" @@ -196,6 +197,12 @@ func apiRun(opts *ApiOptions) error { headersOutputStream := opts.IO.Out if opts.Silent { opts.IO.Out = ioutil.Discard + } else { + err := opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() } host := ghinstance.OverridableDefault() @@ -265,12 +272,13 @@ func processResponse(resp *http.Response, opts *ApiOptions, headersOutputStream if isJSON && opts.IO.ColorEnabled() { err = jsoncolor.Write(opts.IO.Out, responseBody, " ") - if err != nil { - return - } } else { _, err = io.Copy(opts.IO.Out, responseBody) - if err != nil { + } + if err != nil { + if errors.Is(err, syscall.EPIPE) { + err = nil + } else { return } } 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 aeec309a0..2795ca23b 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -88,10 +88,16 @@ func viewRun(opts *ViewOptions) error { } 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/diff/diff.go b/pkg/cmd/pr/diff/diff.go index c6902e5b5..2bea8072b 100644 --- a/pkg/cmd/pr/diff/diff.go +++ b/pkg/cmd/pr/diff/diff.go @@ -6,9 +6,8 @@ import ( "fmt" "io" "net/http" - "os" - "os/exec" "strings" + "syscall" "github.com/cli/cli/api" "github.com/cli/cli/context" @@ -16,7 +15,6 @@ import ( "github.com/cli/cli/pkg/cmd/pr/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" - "github.com/google/shlex" "github.com/spf13/cobra" ) @@ -93,15 +91,18 @@ func diffRun(opts *DiffOptions) error { } defer diff.Close() - if opts.UseColor == "never" { - _, err = io.Copy(opts.IO.Out, diff) + err = opts.IO.StartPager() + if err != nil { return err } + defer opts.IO.StopPager() - if opts.IO.IsStdoutTTY() { - if pager := os.Getenv("PAGER"); pager != "" { - return runPager(pager, diff, opts.IO.Out) + if opts.UseColor == "never" { + _, err = io.Copy(opts.IO.Out, diff) + if errors.Is(err, syscall.EPIPE) { + return nil } + return err } diffLines := bufio.NewScanner(diff) @@ -148,14 +149,3 @@ func isRemovalLine(dl string) bool { func validColorFlag(c string) bool { return c == "auto" || c == "always" || c == "never" } - -var runPager = func(pager string, diff io.Reader, out io.Writer) error { - args, err := shlex.Split(pager) - if err != nil { - return err - } - pagerCmd := exec.Command(args[0], args[1:]...) - pagerCmd.Stdin = diff - pagerCmd.Stdout = out - return pagerCmd.Run() -} diff --git a/pkg/cmd/pr/diff/diff_test.go b/pkg/cmd/pr/diff/diff_test.go index dc93970b6..5e362ac4d 100644 --- a/pkg/cmd/pr/diff/diff_test.go +++ b/pkg/cmd/pr/diff/diff_test.go @@ -2,10 +2,8 @@ package diff import ( "bytes" - "io" "io/ioutil" "net/http" - "os" "testing" "github.com/cli/cli/context" @@ -214,13 +212,8 @@ func TestPRDiff_notty(t *testing.T) { } func TestPRDiff_tty(t *testing.T) { - pager := os.Getenv("PAGER") http := &httpmock.Registry{} - defer func() { - os.Setenv("PAGER", pager) - http.Verify(t) - }() - os.Setenv("PAGER", "") + defer http.Verify(t) http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ { "url": "https://github.com/OWNER/REPO/pull/123", @@ -237,38 +230,6 @@ func TestPRDiff_tty(t *testing.T) { assert.Contains(t, output.String(), "\x1b[32m+site: bin/gh\x1b[m") } -func TestPRDiff_pager(t *testing.T) { - realRunPager := runPager - pager := os.Getenv("PAGER") - http := &httpmock.Registry{} - defer func() { - runPager = realRunPager - os.Setenv("PAGER", pager) - http.Verify(t) - }() - runPager = func(pager string, diff io.Reader, out io.Writer) error { - _, err := io.Copy(out, diff) - return err - } - os.Setenv("PAGER", "fakepager") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes": [ - { "url": "https://github.com/OWNER/REPO/pull/123", - "number": 123, - "id": "foobar123", - "headRefName": "feature", - "baseRefName": "master" } - ] } } } }`)) - http.StubResponse(200, bytes.NewBufferString(testDiff)) - output, err := runCommand(http, nil, true, "") - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - if diff := cmp.Diff(testDiff, output.String()); diff != "" { - t.Errorf("command output did not match:\n%s", diff) - } -} - const testDiff = `diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index 73974448..b7fc0154 100644 --- a/.github/workflows/releases.yml 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 c42048043..a1a15ec42 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 7896d1d71..95da1ad0c 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -1,9 +1,11 @@ package view import ( + "errors" "fmt" "net/http" "strings" + "syscall" "text/template" "github.com/MakeNowJust/heredoc" @@ -107,6 +109,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() { @@ -166,7 +174,7 @@ func viewRun(opts *ViewOptions) error { } err = tmpl.Execute(stdout, repoData) - if err != nil { + if err != nil && !errors.Is(err, syscall.EPIPE) { return err } diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go index 1357825a2..a1acf5a40 100644 --- a/pkg/cmd/root/help_topic.go +++ b/pkg/cmd/root/help_topic.go @@ -28,6 +28,8 @@ func NewHelpTopic(topic string) *cobra.Command { DEBUG: set to any value to enable verbose output to standard error. Include values "api" or "oauth" to print detailed information about HTTP requests or authentication flow. + PAGER: a paging program to send standard output to, e.g. "less". + GLAMOUR_STYLE: the style to use for rendering Markdown. See https://github.com/charmbracelet/glamour#styles diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index 9a0ce4ff8..d6c8ae48f 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -12,6 +12,7 @@ import ( "time" "github.com/briandowns/spinner" + "github.com/google/shlex" "github.com/mattn/go-colorable" "github.com/mattn/go-isatty" "golang.org/x/crypto/ssh/terminal" @@ -36,6 +37,9 @@ type IOStreams struct { stderrTTYOverride bool stderrIsTTY bool + pagerCommand string + pagerProcess *os.Process + neverPrompt bool } @@ -88,6 +92,60 @@ func (s *IOStreams) IsStderrTTY() bool { return false } +func (s *IOStreams) SetPager(cmd string) { + s.pagerCommand = cmd +} + +func (s *IOStreams) StartPager() error { + if s.pagerCommand == "" || !s.IsStdoutTTY() { + return nil + } + + pagerArgs, err := shlex.Split(s.pagerCommand) + if err != nil { + return err + } + + pagerEnv := os.Environ() + for i := len(pagerEnv) - 1; i >= 0; i-- { + if strings.HasPrefix(pagerEnv[i], "PAGER=") { + pagerEnv = append(pagerEnv[0:i], pagerEnv[i+1:]...) + } + } + if _, ok := os.LookupEnv("LESS"); !ok { + pagerEnv = append(pagerEnv, "LESS=FRX") + } + if _, ok := os.LookupEnv("LV"); !ok { + pagerEnv = append(pagerEnv, "LV=-c") + } + + pagerCmd := exec.Command(pagerArgs[0], pagerArgs[1:]...) + pagerCmd.Env = pagerEnv + pagerCmd.Stdout = s.Out + pagerCmd.Stderr = s.ErrOut + pagedOut, err := pagerCmd.StdinPipe() + if err != nil { + return err + } + s.Out = pagedOut + err = pagerCmd.Start() + if err != nil { + return err + } + s.pagerProcess = pagerCmd.Process + return nil +} + +func (s *IOStreams) StopPager() { + if s.pagerProcess == nil { + return + } + + s.Out.(io.ReadCloser).Close() + _, _ = s.pagerProcess.Wait() + s.pagerProcess = nil +} + func (s *IOStreams) CanPrompt() bool { if s.neverPrompt { return false @@ -155,6 +213,7 @@ func System() *IOStreams { Out: colorable.NewColorable(os.Stdout), ErrOut: colorable.NewColorable(os.Stderr), colorEnabled: os.Getenv("NO_COLOR") == "" && stdoutIsTTY, + pagerCommand: os.Getenv("PAGER"), } if stdoutIsTTY && stderrIsTTY {