STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #1630 from cli/pager-for-everyone

Browse source 
Extend PAGER support to all commands producing significant output
This commit is contained in:
Mislav Marohnić 2020-09-16 16:22:58 +02:00 • committed by GitHub
commit 3049db646f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 197 additions and 115 deletions
Download patch file Download diff file Expand all files Collapse all files

8
cmd/gh/main.go
View file

@ -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:]

4
internal/config/config_file_test.go
View file

@ -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) {

9
internal/config/config_type.go
View file

@ -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,

19
internal/config/config_type_test.go
View file

@ -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())

16
pkg/cmd/api/api.go
View file

@ -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
}
}

3
pkg/cmd/issue/list/fixtures/issueList.json
View file

@ -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": [
{

8
pkg/cmd/issue/list/list.go
View file

@ -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)

24
pkg/cmd/issue/list/list_test.go
View file

@ -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
`)

6
pkg/cmd/issue/status/status.go
View file

@ -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, "")

8
pkg/cmd/issue/view/view.go
View file

@ -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)
}

28
pkg/cmd/pr/diff/diff.go
View file

@ -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()
}

41
pkg/cmd/pr/diff/diff_test.go
View file

@ -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

8
pkg/cmd/pr/list/list.go
View file

@ -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)

47
pkg/cmd/pr/list/list_test.go
View file

@ -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) {

6
pkg/cmd/pr/status/status.go
View file

@ -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, "")

6
pkg/cmd/pr/view/view.go
View file

@ -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)
}

10
pkg/cmd/repo/view/view.go
View file

@ -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
}

2
pkg/cmd/root/help_topic.go
View file

@ -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

59
pkg/iostreams/iostreams.go
View file

@ -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 {