Re-enable label colors for issue list (#4106)

* Re-enable label colors for issue list
* Drop parentheses wrapping issue labels
* Support ANSI escape codes in TablePrinter cells
* Switch to a Truncate implementation that correctly measures ANSI escape codes
* Only output RGB color if terminal has truecolor capabilities
* Enable `ENABLE_VIRTUAL_TERMINAL_PROCESSING` on Windows - fixes wrapping issues with full lines and allows truecolor rendering

Co-authored-by: Mislav Marohnić <mislav@github.com>
This commit is contained in:
Heath Stewart 2021-08-23 10:55:12 -07:00 committed by GitHub
parent 25b150ad6e
commit 88af63d36f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 101 additions and 138 deletions

3
go.mod
View file

@ -21,10 +21,9 @@ require (
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
github.com/mattn/go-colorable v0.1.8
github.com/mattn/go-isatty v0.0.13
github.com/mattn/go-runewidth v0.0.10
github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d
github.com/muesli/reflow v0.2.1-0.20210502190812-c80126ec2ad5
github.com/muesli/termenv v0.8.1
github.com/rivo/uniseg v0.2.0
github.com/shurcooL/githubv4 v0.0.0-20200928013246-d292edc3691b
github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f
github.com/spf13/cobra v1.2.1

3
go.sum
View file

@ -268,8 +268,9 @@ github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RR
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
github.com/muesli/reflow v0.2.0 h1:2o0UBJPHHH4fa2GCXU4Rg4DwOtWPMekCeyc5EWbAQp0=
github.com/muesli/reflow v0.2.0/go.mod h1:qT22vjVmM9MIUeLgsVYe/Ye7eZlbv9dZjL3dVhUqLX8=
github.com/muesli/reflow v0.2.1-0.20210502190812-c80126ec2ad5 h1:T+Fc6qGlSfM+z0JPlp+n5rijvlg6C6JYFSNaqnCifDU=
github.com/muesli/reflow v0.2.1-0.20210502190812-c80126ec2ad5/go.mod h1:Xk+z4oIWdQqJzsxyjgl3P22oYZnHdZ8FFTHAQQt5BMQ=
github.com/muesli/termenv v0.8.1 h1:9q230czSP3DHVpkaPDXGp0TOfAwyjyYwXlUCQxQSaBk=
github.com/muesli/termenv v0.8.1/go.mod h1:kzt/D/4a88RoheZmwfqorY3A+tnsSMA9HJC/fQSFKo0=
github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec=

View file

@ -450,7 +450,6 @@ func Test_promptGists(t *testing.T) {
}
io, _, _, _ := iostreams.Test()
cs := iostreams.NewColorScheme(io.ColorEnabled(), io.ColorSupport256())
for _, tt := range tests {
reg := &httpmock.Registry{}
@ -472,7 +471,7 @@ func Test_promptGists(t *testing.T) {
as.StubOne(tt.gistIndex)
t.Run(tt.name, func(t *testing.T) {
gistID, err := promptGists(client, ghinstance.Default(), cs)
gistID, err := promptGists(client, ghinstance.Default(), io.ColorScheme())
assert.NoError(t, err)
assert.Equal(t, tt.wantOut, gistID)
reg.Verify(t)

View file

@ -100,9 +100,9 @@ func TestIssueList_tty(t *testing.T) {
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
#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())
}

View file

@ -22,10 +22,6 @@ func PrintIssues(io *iostreams.IOStreams, prefix string, totalCount int, issues
issueNum = "#" + issueNum
}
issueNum = prefix + issueNum
labels := issueLabelList(&issue)
if labels != "" && table.IsTTY() {
labels = fmt.Sprintf("(%s)", labels)
}
now := time.Now()
ago := now.Sub(issue.UpdatedAt)
table.AddField(issueNum, nil, cs.ColorFromString(prShared.ColorForState(issue.State)))
@ -33,7 +29,7 @@ func PrintIssues(io *iostreams.IOStreams, prefix string, totalCount int, issues
table.AddField(issue.State, nil, nil)
}
table.AddField(text.ReplaceExcessiveWhitespace(issue.Title), nil, nil)
table.AddField(labels, truncateLabels, cs.Gray)
table.AddField(issueLabelList(&issue, cs, table.IsTTY()), nil, nil)
if table.IsTTY() {
table.AddField(utils.FuzzyAgo(ago), nil, cs.Gray)
} else {
@ -48,22 +44,18 @@ func PrintIssues(io *iostreams.IOStreams, prefix string, totalCount int, issues
}
}
func truncateLabels(w int, t string) string {
if len(t) < 2 {
return t
}
truncated := text.Truncate(w-2, t[1:len(t)-1])
return fmt.Sprintf("(%s)", truncated)
}
func issueLabelList(issue *api.Issue) string {
func issueLabelList(issue *api.Issue, cs *iostreams.ColorScheme, colorize bool) string {
if len(issue.Labels.Nodes) == 0 {
return ""
}
labelNames := make([]string, len(issue.Labels.Nodes))
for i, label := range issue.Labels.Nodes {
labelNames[i] = label.Name
labelNames := make([]string, 0, len(issue.Labels.Nodes))
for _, label := range issue.Labels.Nodes {
if colorize {
labelNames = append(labelNames, cs.HexToRGB(label.Color, label.Name))
} else {
labelNames = append(labelNames, label.Name)
}
}
return strings.Join(labelNames, ", ")

View file

@ -1,15 +0,0 @@
package shared
import "testing"
func Test_truncateLabels(t *testing.T) {
got := truncateLabels(12, "(one, two, three)")
expected := "(one, tw...)"
if got != expected {
t.Errorf("expected %q, got %q", expected, got)
}
if truncateLabels(10, "") != "" {
t.Error("blank value error")
}
}

View file

@ -135,9 +135,10 @@ func watchRun(opts *WatchOptions) error {
prNumber = fmt.Sprintf(" #%d", number)
}
opts.IO.EnableVirtualTerminalProcessing()
// clear entire screen
fmt.Fprintf(out, "\x1b[2J")
if err := opts.IO.EnableVirtualTerminalProcessing(); err == nil {
// clear entire screen
fmt.Fprintf(out, "\x1b[2J")
}
annotationCache := map[int64][]shared.Annotation{}

View file

@ -34,27 +34,33 @@ func EnvColorForced() bool {
}
func Is256ColorSupported() bool {
return IsTrueColorSupported() ||
strings.Contains(os.Getenv("TERM"), "256") ||
strings.Contains(os.Getenv("COLORTERM"), "256")
}
func IsTrueColorSupported() bool {
term := os.Getenv("TERM")
colorterm := os.Getenv("COLORTERM")
return strings.Contains(term, "256") ||
strings.Contains(term, "24bit") ||
return strings.Contains(term, "24bit") ||
strings.Contains(term, "truecolor") ||
strings.Contains(colorterm, "256") ||
strings.Contains(colorterm, "24bit") ||
strings.Contains(colorterm, "truecolor")
}
func NewColorScheme(enabled, is256enabled bool) *ColorScheme {
func NewColorScheme(enabled, is256enabled bool, trueColor bool) *ColorScheme {
return &ColorScheme{
enabled: enabled,
is256enabled: is256enabled,
hasTrueColor: trueColor,
}
}
type ColorScheme struct {
enabled bool
is256enabled bool
hasTrueColor bool
}
func (c *ColorScheme) Bold(t string) string {
@ -205,7 +211,7 @@ func (c *ColorScheme) ColorFromString(s string) func(string) string {
}
func (c *ColorScheme) HexToRGB(hex string, x string) string {
if !c.enabled || !c.is256enabled {
if !c.enabled || !c.hasTrueColor {
return x
}

View file

@ -148,51 +148,37 @@ func TestEnvColorForced(t *testing.T) {
func Test_HextoRGB(t *testing.T) {
tests := []struct {
name string
hex string
arg string
expectedOutput string
expectedError bool
cs *ColorScheme
name string
hex string
text string
wants string
cs *ColorScheme
}{
{
name: "Colored red enabled color",
hex: "fc0303",
arg: "red",
expectedOutput: "\033[38;2;252;3;3mred\033[0m",
cs: NewColorScheme(true, true),
name: "truecolor",
hex: "fc0303",
text: "red",
wants: "\033[38;2;252;3;3mred\033[0m",
cs: NewColorScheme(true, true, true),
},
{
name: "Failed colored red enabled color",
hex: "fc0303",
arg: "red",
expectedOutput: "\033[38;2;252;2;3mred\033[0m",
expectedError: true,
cs: NewColorScheme(true, true),
name: "no truecolor",
hex: "fc0303",
text: "red",
wants: "red",
cs: NewColorScheme(true, true, false),
},
{
name: "Colored red disabled color",
hex: "fc0303",
arg: "red",
expectedOutput: "red",
cs: NewColorScheme(false, false),
},
{
name: "Failed colored red disabled color",
hex: "fc0303",
arg: "red",
expectedOutput: "\033[38;2;252;3;3mred\033[0m",
expectedError: true,
cs: NewColorScheme(false, false),
name: "no color",
hex: "fc0303",
text: "red",
wants: "red",
cs: NewColorScheme(false, false, false),
},
}
for _, tt := range tests {
output := tt.cs.HexToRGB(tt.hex, tt.arg)
if tt.expectedError {
assert.NotEqual(t, tt.expectedOutput, output)
} else {
assert.Equal(t, tt.expectedOutput, output)
}
output := tt.cs.HexToRGB(tt.hex, tt.text)
assert.Equal(t, tt.wants, output)
}
}

View file

@ -2,4 +2,15 @@
package iostreams
func (s *IOStreams) EnableVirtualTerminalProcessing() {}
import (
"errors"
"os"
)
func (s *IOStreams) EnableVirtualTerminalProcessing() error {
return nil
}
func enableVirtualTerminalProcessing(f *os.File) error {
return errors.New("not implemented")
}

View file

@ -8,14 +8,23 @@ import (
"golang.org/x/sys/windows"
)
func (s *IOStreams) EnableVirtualTerminalProcessing() {
func (s *IOStreams) EnableVirtualTerminalProcessing() error {
if !s.IsStdoutTTY() {
return
return nil
}
stdout := windows.Handle(s.originalOut.(*os.File).Fd())
f, ok := s.originalOut.(*os.File)
if !ok {
return nil
}
return enableVirtualTerminalProcessing(f)
}
func enableVirtualTerminalProcessing(f *os.File) error {
stdout := windows.Handle(f.Fd())
var originalMode uint32
windows.GetConsoleMode(stdout, &originalMode)
windows.SetConsoleMode(stdout, originalMode|windows.ENABLE_VIRTUAL_TERMINAL_PROCESSING)
return windows.SetConsoleMode(stdout, originalMode|windows.ENABLE_VIRTUAL_TERMINAL_PROCESSING)
}

View file

@ -30,6 +30,7 @@ type IOStreams struct {
originalOut io.Writer
colorEnabled bool
is256enabled bool
hasTrueColor bool
terminalTheme string
progressIndicatorEnabled bool
@ -60,6 +61,10 @@ func (s *IOStreams) ColorSupport256() bool {
return s.is256enabled
}
func (s *IOStreams) HasTrueColor() bool {
return s.hasTrueColor
}
func (s *IOStreams) DetectTerminalTheme() string {
if !s.ColorEnabled() {
s.terminalTheme = "none"
@ -289,7 +294,7 @@ func (s *IOStreams) ForceTerminal(spec string) {
}
func (s *IOStreams) ColorScheme() *ColorScheme {
return NewColorScheme(s.ColorEnabled(), s.ColorSupport256())
return NewColorScheme(s.ColorEnabled(), s.ColorSupport256(), s.HasTrueColor())
}
func (s *IOStreams) ReadUserFile(fn string) ([]byte, error) {
@ -318,13 +323,21 @@ func System() *IOStreams {
stdoutIsTTY := isTerminal(os.Stdout)
stderrIsTTY := isTerminal(os.Stderr)
assumeTrueColor := false
if stdoutIsTTY {
if err := enableVirtualTerminalProcessing(os.Stdout); err == nil {
assumeTrueColor = true
}
}
io := &IOStreams{
In: os.Stdin,
originalOut: os.Stdout,
Out: colorable.NewColorable(os.Stdout),
ErrOut: colorable.NewColorable(os.Stderr),
colorEnabled: EnvColorForced() || (!EnvColorDisabled() && stdoutIsTTY),
is256enabled: Is256ColorSupported(),
is256enabled: assumeTrueColor || Is256ColorSupported(),
hasTrueColor: assumeTrueColor || IsTrueColorSupported(),
pagerCommand: os.Getenv("PAGER"),
ttySize: ttySize,
}

View file

@ -1,23 +1,18 @@
package text
import (
runewidth "github.com/mattn/go-runewidth"
"github.com/rivo/uniseg"
"github.com/muesli/reflow/ansi"
"github.com/muesli/reflow/truncate"
)
const (
ellipsisWidth = 3
ellipsis = "..."
minWidthForEllipsis = 5
)
// DisplayWidth calculates what the rendered width of a string may be
func DisplayWidth(s string) int {
g := uniseg.NewGraphemes(s)
w := 0
for g.Next() {
w += graphemeWidth(g)
}
return w
return ansi.PrintableRuneWidth(s)
}
// Truncate shortens a string to fit the maximum display width
@ -27,49 +22,15 @@ func Truncate(maxWidth int, s string) string {
return s
}
useEllipsis := false
tail := ""
if maxWidth >= minWidthForEllipsis {
useEllipsis = true
maxWidth -= ellipsisWidth
tail = ellipsis
}
g := uniseg.NewGraphemes(s)
r := ""
rWidth := 0
for {
g.Next()
gWidth := graphemeWidth(g)
if rWidth+gWidth <= maxWidth {
r += g.Str()
rWidth += gWidth
continue
} else {
break
}
}
if useEllipsis {
r += "..."
}
if rWidth < maxWidth {
r := truncate.StringWithTail(s, uint(maxWidth), tail)
if DisplayWidth(r) < maxWidth {
r += " "
}
return r
}
// graphemeWidth calculates what the rendered width of a grapheme may be
func graphemeWidth(g *uniseg.Graphemes) int {
// If grapheme spans one rune use that rune width
// If grapheme spans multiple runes use the first non-zero rune width
w := 0
for _, r := range g.Runes() {
w = runewidth.RuneWidth(r)
if w > 0 {
break
}
}
return w
}