Merge pull request #1917 from cli/fix-pager-glamour-bug

Check for terminal background color before starting pager
This commit is contained in:
Sam 2020-09-28 11:05:39 +02:00 committed by GitHub
commit 1b029c766d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 256 additions and 37 deletions

1
go.mod
View file

@ -18,6 +18,7 @@ require (
github.com/mattn/go-runewidth v0.0.9
github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d
github.com/mitchellh/go-homedir v1.1.0
github.com/muesli/termenv v0.7.2
github.com/rivo/uniseg v0.1.0
github.com/shurcooL/githubv4 v0.0.0-20200802174311-f27d2ca7f6d5
github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f

2
go.sum
View file

@ -132,6 +132,8 @@ github.com/muesli/reflow v0.1.0 h1:oQdpLfO56lr5pgLvqD0TcjW85rDjSYSBVdiG1Ch1ddM=
github.com/muesli/reflow v0.1.0/go.mod h1:I9bWAt7QTg/que/qmUCJBGlj7wEq8OAFBjPNjc6xK4I=
github.com/muesli/termenv v0.6.0 h1:zxvzTBmo4ZcxhNGGWeMz+Tttm51eF5bmPjfy4MCRYlk=
github.com/muesli/termenv v0.6.0/go.mod h1:SohX91w6swWA4AYU+QmPx+aSgXhWO0juiyID9UZmbpA=
github.com/muesli/termenv v0.7.2 h1:r1raklL3uKE7rOvWgSenmEm2px+dnc33OTisZ8YR1fw=
github.com/muesli/termenv v0.7.2/go.mod h1:ct2L5N2lmix82RaY3bMWwVu/jUFc9Ule0KGDCiKYPh8=
github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U=
github.com/olekukonko/tablewriter v0.0.4 h1:vHD/YYe1Wolo78koG299f7V/VAS08c6IpCLn+Ejf/w8=

View file

@ -10,6 +10,7 @@ import (
"github.com/cli/cli/pkg/cmd/gist/shared"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
"github.com/cli/cli/pkg/markdown"
"github.com/cli/cli/utils"
"github.com/spf13/cobra"
)
@ -115,7 +116,8 @@ func viewRun(opts *ViewOptions) error {
}
content := gistFile.Content
if strings.Contains(gistFile.Type, "markdown") && !opts.Raw {
rendered, err := utils.RenderMarkdown(gistFile.Content)
style := markdown.GetStyle(opts.IO.DetectTerminalTheme())
rendered, err := markdown.Render(gistFile.Content, style)
if err == nil {
content = rendered
}

View file

@ -16,6 +16,7 @@ import (
prShared "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/markdown"
"github.com/cli/cli/utils"
"github.com/spf13/cobra"
)
@ -89,6 +90,8 @@ func viewRun(opts *ViewOptions) error {
return utils.OpenInBrowser(openURL)
}
opts.IO.DetectTerminalTheme()
err = opts.IO.StartPager()
if err != nil {
return err
@ -96,7 +99,7 @@ func viewRun(opts *ViewOptions) error {
defer opts.IO.StopPager()
if opts.IO.IsStdoutTTY() {
return printHumanIssuePreview(opts.IO.Out, issue)
return printHumanIssuePreview(opts.IO, issue)
}
return printRawIssuePreview(opts.IO.Out, issue)
}
@ -122,7 +125,8 @@ func printRawIssuePreview(out io.Writer, issue *api.Issue) error {
return nil
}
func printHumanIssuePreview(out io.Writer, issue *api.Issue) error {
func printHumanIssuePreview(io *iostreams.IOStreams, issue *api.Issue) error {
out := io.Out
now := time.Now()
ago := now.Sub(issue.CreatedAt)
@ -158,7 +162,8 @@ func printHumanIssuePreview(out io.Writer, issue *api.Issue) error {
// Body
if issue.Body != "" {
fmt.Fprintln(out)
md, err := utils.RenderMarkdown(issue.Body)
style := markdown.GetStyle(io.TerminalTheme())
md, err := markdown.Render(issue.Body, style)
if err != nil {
return err
}

View file

@ -14,6 +14,7 @@ import (
"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/markdown"
"github.com/cli/cli/pkg/prompt"
"github.com/cli/cli/pkg/surveyext"
"github.com/cli/cli/utils"
@ -252,7 +253,8 @@ func reviewSurvey(io *iostreams.IOStreams, editorCommand string) (*api.PullReque
}
if len(bodyAnswers.Body) > 0 {
renderedBody, err := utils.RenderMarkdown(bodyAnswers.Body)
style := markdown.GetStyle(io.DetectTerminalTheme())
renderedBody, err := markdown.Render(bodyAnswers.Body, style)
if err != nil {
return nil, err
}

View file

@ -16,6 +16,7 @@ import (
"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/markdown"
"github.com/cli/cli/utils"
"github.com/spf13/cobra"
)
@ -99,6 +100,8 @@ func viewRun(opts *ViewOptions) error {
return utils.OpenInBrowser(openURL)
}
opts.IO.DetectTerminalTheme()
err = opts.IO.StartPager()
if err != nil {
return err
@ -106,7 +109,7 @@ func viewRun(opts *ViewOptions) error {
defer opts.IO.StopPager()
if connectedToTerminal {
return printHumanPrPreview(opts.IO.Out, pr)
return printHumanPrPreview(opts.IO, pr)
}
return printRawPrPreview(opts.IO.Out, pr)
}
@ -134,7 +137,9 @@ func printRawPrPreview(out io.Writer, pr *api.PullRequest) error {
return nil
}
func printHumanPrPreview(out io.Writer, pr *api.PullRequest) error {
func printHumanPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error {
out := io.Out
// Header (Title and State)
fmt.Fprintln(out, utils.Bold(pr.Title))
fmt.Fprintf(out, "%s", shared.StateTitleWithColor(*pr))
@ -172,7 +177,8 @@ func printHumanPrPreview(out io.Writer, pr *api.PullRequest) error {
// Body
if pr.Body != "" {
fmt.Fprintln(out)
md, err := utils.RenderMarkdown(pr.Body)
style := markdown.GetStyle(io.TerminalTheme())
md, err := markdown.Render(pr.Body, style)
if err != nil {
return err
}

View file

@ -12,6 +12,7 @@ import (
"github.com/cli/cli/pkg/cmd/release/shared"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
"github.com/cli/cli/pkg/markdown"
"github.com/cli/cli/utils"
"github.com/spf13/cobra"
)
@ -122,7 +123,8 @@ func renderReleaseTTY(io *iostreams.IOStreams, release *shared.Release) error {
fmt.Fprintf(w, "%s\n", iofmt.Gray(fmt.Sprintf("%s released this %s", release.Author.Login, utils.FuzzyAgo(time.Since(release.PublishedAt)))))
}
renderedDescription, err := utils.RenderMarkdown(release.Body)
style := markdown.GetStyle(io.DetectTerminalTheme())
renderedDescription, err := markdown.Render(release.Body, style)
if err != nil {
return err
}

View file

@ -14,6 +14,7 @@ import (
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
"github.com/cli/cli/pkg/markdown"
"github.com/cli/cli/utils"
"github.com/enescakir/emoji"
"github.com/spf13/cobra"
@ -114,6 +115,8 @@ func viewRun(opts *ViewOptions) error {
return err
}
opts.IO.DetectTerminalTheme()
err = opts.IO.StartPager()
if err != nil {
return err
@ -153,7 +156,8 @@ func viewRun(opts *ViewOptions) error {
readmeContent = utils.Gray("This repository does not have a README")
} else if isMarkdownFile(readme.Filename) {
var err error
readmeContent, err = utils.RenderMarkdown(readme.Content)
style := markdown.GetStyle(opts.IO.TerminalTheme())
readmeContent, err = markdown.Render(readme.Content, style)
if err != nil {
return fmt.Errorf("error rendering markdown: %w", err)
}

View file

@ -15,6 +15,7 @@ import (
"github.com/google/shlex"
"github.com/mattn/go-colorable"
"github.com/mattn/go-isatty"
"github.com/muesli/termenv"
"golang.org/x/crypto/ssh/terminal"
)
@ -24,9 +25,10 @@ type IOStreams struct {
ErrOut io.Writer
// the original (non-colorable) output stream
originalOut io.Writer
colorEnabled bool
is256enabled bool
originalOut io.Writer
colorEnabled bool
is256enabled bool
terminalTheme string
progressIndicatorEnabled bool
progressIndicator *spinner.Spinner
@ -52,6 +54,40 @@ func (s *IOStreams) ColorSupport256() bool {
return s.is256enabled
}
func (s *IOStreams) DetectTerminalTheme() string {
if !s.ColorEnabled() {
s.terminalTheme = "none"
return "none"
}
if s.pagerProcess != nil {
s.terminalTheme = "none"
return "none"
}
style := os.Getenv("GLAMOUR_STYLE")
if style != "" && style != "auto" {
s.terminalTheme = "none"
return "none"
}
if termenv.HasDarkBackground() {
s.terminalTheme = "dark"
return "dark"
}
s.terminalTheme = "light"
return "light"
}
func (s *IOStreams) TerminalTheme() string {
if s.terminalTheme == "" {
return "none"
}
return s.terminalTheme
}
func (s *IOStreams) SetStdinTTY(isTTY bool) {
s.stdinTTYOverride = true
s.stdinIsTTY = isTTY

42
pkg/markdown/markdown.go Normal file
View file

@ -0,0 +1,42 @@
package markdown
import (
"os"
"strings"
"github.com/charmbracelet/glamour"
)
func Render(text, style string) (string, error) {
// Glamour rendering preserves carriage return characters in code blocks, but
// we need to ensure that no such characters are present in the output.
text = strings.ReplaceAll(text, "\r\n", "\n")
tr, err := glamour.NewTermRenderer(
glamour.WithStylePath(style),
// glamour.WithBaseURL(""), // TODO: make configurable
// glamour.WithWordWrap(80), // TODO: make configurable
)
if err != nil {
return "", err
}
return tr.Render(text)
}
func GetStyle(defaultStyle string) string {
style := fromEnv()
if style != "" && style != "auto" {
return style
}
if defaultStyle == "light" || defaultStyle == "dark" {
return defaultStyle
}
return "notty"
}
var fromEnv = func() string {
return os.Getenv("GLAMOUR_STYLE")
}

View file

@ -0,0 +1,141 @@
package markdown
import (
"testing"
"github.com/stretchr/testify/assert"
)
func Test_Render(t *testing.T) {
type input struct {
text string
style string
}
type output struct {
wantsErr bool
}
tests := []struct {
name string
input input
output output
}{
{
name: "invalid glamour style",
input: input{
text: "some text",
style: "invalid",
},
output: output{
wantsErr: true,
},
},
{
name: "valid glamour style",
input: input{
text: "some text",
style: "dark",
},
output: output{
wantsErr: false,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := Render(tt.input.text, tt.input.style)
if tt.output.wantsErr {
assert.Error(t, err)
return
}
assert.NoError(t, err)
})
}
}
func Test_GetStyle(t *testing.T) {
type input struct {
glamourStyle string
terminalTheme string
}
type output struct {
style string
}
tests := []struct {
name string
input input
output output
}{
{
name: "no glamour style and no terminal theme",
input: input{
glamourStyle: "",
terminalTheme: "none",
},
output: output{
style: "notty",
},
},
{
name: "auto glamour style and no terminal theme",
input: input{
glamourStyle: "auto",
terminalTheme: "none",
},
output: output{
style: "notty",
},
},
{
name: "user glamour style and no terminal theme",
input: input{
glamourStyle: "somestyle",
terminalTheme: "none",
},
output: output{
style: "somestyle",
},
},
{
name: "no glamour style and light terminal theme",
input: input{
glamourStyle: "",
terminalTheme: "light",
},
output: output{
style: "light",
},
},
{
name: "no glamour style and dark terminal theme",
input: input{
glamourStyle: "",
terminalTheme: "dark",
},
output: output{
style: "dark",
},
},
{
name: "no glamour style and unknown terminal theme",
input: input{
glamourStyle: "",
terminalTheme: "unknown",
},
output: output{
style: "notty",
},
},
}
for _, tt := range tests {
fromEnv = func() string {
return tt.input.glamourStyle
}
t.Run(tt.name, func(t *testing.T) {
style := GetStyle(tt.input.terminalTheme)
assert.Equal(t, tt.output.style, style)
})
}
}

View file

@ -8,7 +8,6 @@ import (
"time"
"github.com/briandowns/spinner"
"github.com/charmbracelet/glamour"
"github.com/cli/cli/internal/run"
"github.com/cli/cli/pkg/browser"
)
@ -29,29 +28,6 @@ func OpenInBrowser(url string) error {
return err
}
func RenderMarkdown(text string) (string, error) {
// Glamour rendering preserves carriage return characters in code blocks, but
// we need to ensure that no such characters are present in the output.
text = strings.ReplaceAll(text, "\r\n", "\n")
renderStyle := glamour.WithStandardStyle("notty")
// TODO: make color an input parameter
if isColorEnabled() {
renderStyle = glamour.WithEnvironmentConfig()
}
tr, err := glamour.NewTermRenderer(
renderStyle,
// glamour.WithBaseURL(""), // TODO: make configurable
// glamour.WithWordWrap(80), // TODO: make configurable
)
if err != nil {
return "", err
}
return tr.Render(text)
}
func Pluralize(num int, thing string) string {
if num == 1 {
return fmt.Sprintf("%d %s", num, thing)