From 25a926d5cb36dd3b6da17e249c825e7b64338f7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 11 Oct 2022 14:29:50 +0200 Subject: [PATCH 1/5] Dogfood term package from go-gh --- cmd/gh/main.go | 4 - go.mod | 4 +- pkg/iostreams/color.go | 25 ------ pkg/iostreams/color_test.go | 121 -------------------------- pkg/iostreams/console.go | 7 +- pkg/iostreams/console_windows.go | 14 +-- pkg/iostreams/iostreams.go | 136 ++++++------------------------ pkg/iostreams/iostreams_test.go | 68 --------------- pkg/iostreams/tty_size.go | 20 ----- pkg/iostreams/tty_size_windows.go | 16 ---- utils/table_printer.go | 4 +- 11 files changed, 38 insertions(+), 381 deletions(-) delete mode 100644 pkg/iostreams/tty_size.go delete mode 100644 pkg/iostreams/tty_size_windows.go diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 750233ba7fa..f867d7c7082 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -64,10 +64,6 @@ func mainRun() exitCode { cmdFactory := factory.New(buildVersion) stderr := cmdFactory.IOStreams.ErrOut - - if spec := os.Getenv("GH_FORCE_TTY"); spec != "" { - cmdFactory.IOStreams.ForceTerminal(spec) - } if !cmdFactory.IOStreams.ColorEnabled() { surveyCore.DisableColor = true } else { diff --git a/go.mod b/go.mod index 08d4c245bca..e2cbf9a23f8 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,6 @@ require ( github.com/mattn/go-colorable v0.1.13 github.com/mattn/go-isatty v0.0.16 github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d - github.com/muesli/termenv v0.12.0 github.com/muhammadmuzzammil1998/jsonc v0.0.0-20201229145248-615b0916ca38 github.com/opentracing/opentracing-go v1.1.0 github.com/shurcooL/githubv4 v0.0.0-20220520033151-0b4e3294ff00 @@ -35,7 +34,6 @@ require ( github.com/stretchr/testify v1.7.5 golang.org/x/crypto v0.0.0-20210817164053-32db794688a5 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c - golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 golang.org/x/text v0.3.7 gopkg.in/yaml.v3 v3.0.1 @@ -59,6 +57,7 @@ require ( github.com/mattn/go-runewidth v0.0.13 // indirect github.com/microcosm-cc/bluemonday v1.0.19 // indirect github.com/muesli/reflow v0.3.0 // indirect + github.com/muesli/termenv v0.12.0 // indirect github.com/olekukonko/tablewriter v0.0.5 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rivo/uniseg v0.2.0 // indirect @@ -70,6 +69,7 @@ require ( github.com/yuin/goldmark-emoji v1.0.1 // indirect golang.org/x/net v0.0.0-20220722155237-a158d28d115b // indirect golang.org/x/oauth2 v0.0.0-20220309155454-6242fa91716a // indirect + golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab // indirect gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect ) diff --git a/pkg/iostreams/color.go b/pkg/iostreams/color.go index e2c2c1c9cf8..1d35c05d937 100644 --- a/pkg/iostreams/color.go +++ b/pkg/iostreams/color.go @@ -2,7 +2,6 @@ package iostreams import ( "fmt" - "os" "strconv" "strings" @@ -25,30 +24,6 @@ var ( } ) -func EnvColorDisabled() bool { - return os.Getenv("NO_COLOR") != "" || os.Getenv("CLICOLOR") == "0" -} - -func EnvColorForced() bool { - return os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" -} - -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, "24bit") || - strings.Contains(term, "truecolor") || - strings.Contains(colorterm, "24bit") || - strings.Contains(colorterm, "truecolor") -} - func NewColorScheme(enabled, is256enabled bool, trueColor bool) *ColorScheme { return &ColorScheme{ enabled: enabled, diff --git a/pkg/iostreams/color_test.go b/pkg/iostreams/color_test.go index e88fe79e35c..59fea53ed1e 100644 --- a/pkg/iostreams/color_test.go +++ b/pkg/iostreams/color_test.go @@ -6,127 +6,6 @@ import ( "github.com/stretchr/testify/assert" ) -func TestEnvColorDisabled(t *testing.T) { - tests := []struct { - name string - NO_COLOR string - CLICOLOR string - CLICOLOR_FORCE string - want bool - }{ - { - name: "pristine env", - NO_COLOR: "", - CLICOLOR: "", - CLICOLOR_FORCE: "", - want: false, - }, - { - name: "NO_COLOR enabled", - NO_COLOR: "1", - CLICOLOR: "", - CLICOLOR_FORCE: "", - want: true, - }, - { - name: "CLICOLOR disabled", - NO_COLOR: "", - CLICOLOR: "0", - CLICOLOR_FORCE: "", - want: true, - }, - { - name: "CLICOLOR enabled", - NO_COLOR: "", - CLICOLOR: "1", - CLICOLOR_FORCE: "", - want: false, - }, - { - name: "CLICOLOR_FORCE has no effect", - NO_COLOR: "", - CLICOLOR: "", - CLICOLOR_FORCE: "1", - want: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Setenv("NO_COLOR", tt.NO_COLOR) - t.Setenv("CLICOLOR", tt.CLICOLOR) - t.Setenv("CLICOLOR_FORCE", tt.CLICOLOR_FORCE) - - if got := EnvColorDisabled(); got != tt.want { - t.Errorf("EnvColorDisabled(): want %v, got %v", tt.want, got) - } - }) - } -} - -func TestEnvColorForced(t *testing.T) { - tests := []struct { - name string - NO_COLOR string - CLICOLOR string - CLICOLOR_FORCE string - want bool - }{ - { - name: "pristine env", - NO_COLOR: "", - CLICOLOR: "", - CLICOLOR_FORCE: "", - want: false, - }, - { - name: "NO_COLOR enabled", - NO_COLOR: "1", - CLICOLOR: "", - CLICOLOR_FORCE: "", - want: false, - }, - { - name: "CLICOLOR disabled", - NO_COLOR: "", - CLICOLOR: "0", - CLICOLOR_FORCE: "", - want: false, - }, - { - name: "CLICOLOR enabled", - NO_COLOR: "", - CLICOLOR: "1", - CLICOLOR_FORCE: "", - want: false, - }, - { - name: "CLICOLOR_FORCE enabled", - NO_COLOR: "", - CLICOLOR: "", - CLICOLOR_FORCE: "1", - want: true, - }, - { - name: "CLICOLOR_FORCE disabled", - NO_COLOR: "", - CLICOLOR: "", - CLICOLOR_FORCE: "0", - want: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Setenv("NO_COLOR", tt.NO_COLOR) - t.Setenv("CLICOLOR", tt.CLICOLOR) - t.Setenv("CLICOLOR_FORCE", tt.CLICOLOR_FORCE) - - if got := EnvColorForced(); got != tt.want { - t.Errorf("EnvColorForced(): want %v, got %v", tt.want, got) - } - }) - } -} - func TestColorFromRGB(t *testing.T) { tests := []struct { name string diff --git a/pkg/iostreams/console.go b/pkg/iostreams/console.go index 73126787d65..89bdd1daabb 100644 --- a/pkg/iostreams/console.go +++ b/pkg/iostreams/console.go @@ -3,8 +3,9 @@ package iostreams -import "errors" +import "os" -func enableVirtualTerminalProcessing(fd uintptr) error { - return errors.New("not implemented") +func hasAlternateScreenBuffer(hasTrueColor bool) bool { + // on non-Windows, we just assume that alternate screen buffer is supported in most cases + return os.Getenv("TERM") != "dumb" } diff --git a/pkg/iostreams/console_windows.go b/pkg/iostreams/console_windows.go index 12f4e28509c..ee0238e4935 100644 --- a/pkg/iostreams/console_windows.go +++ b/pkg/iostreams/console_windows.go @@ -3,14 +3,8 @@ package iostreams -import ( - "golang.org/x/sys/windows" -) - -func enableVirtualTerminalProcessing(fd uintptr) error { - stdout := windows.Handle(fd) - - var originalMode uint32 - windows.GetConsoleMode(stdout, &originalMode) - return windows.SetConsoleMode(stdout, originalMode|windows.ENABLE_VIRTUAL_TERMINAL_PROCESSING) +func hasAlternateScreenBuffer(hasTrueColor bool) bool { + // on Windows we just assume that alternate screen buffer is supported if we + // enabled virtual terminal processing, which in turn enables truecolor + return hasTrueColor } diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index 1a6399ae876..892f1716256 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -8,18 +8,16 @@ import ( "os" "os/exec" "os/signal" - "strconv" "strings" "sync" "time" "github.com/briandowns/spinner" + ghTerm "github.com/cli/go-gh/pkg/term" "github.com/cli/safeexec" "github.com/google/shlex" "github.com/mattn/go-colorable" "github.com/mattn/go-isatty" - "github.com/muesli/termenv" - "golang.org/x/term" ) const DefaultWidth = 80 @@ -40,13 +38,12 @@ type fileReader interface { } type IOStreams struct { + term ghTerm.Term + In fileReader Out fileWriter ErrOut io.Writer - colorEnabled bool - is256enabled bool - hasTrueColor bool terminalTheme string progressIndicatorEnabled bool @@ -63,8 +60,6 @@ type IOStreams struct { stdoutIsTTY bool stderrTTYOverride bool stderrIsTTY bool - termWidthOverride int - ttySize func() (int, int, error) pagerCommand string pagerProcess *os.Process @@ -75,42 +70,33 @@ type IOStreams struct { } func (s *IOStreams) ColorEnabled() bool { - return s.colorEnabled + return s.term.IsColorEnabled() } func (s *IOStreams) ColorSupport256() bool { - return s.is256enabled + return s.term.Is256ColorSupported() } func (s *IOStreams) HasTrueColor() bool { - return s.hasTrueColor + return s.term.IsTrueColorSupported() } // DetectTerminalTheme is a utility to call before starting the output pager so that the terminal background // can be reliably detected. func (s *IOStreams) DetectTerminalTheme() { - if !s.ColorEnabled() { - s.terminalTheme = "none" - return - } - - if s.pagerProcess != nil { + if !s.ColorEnabled() || s.pagerProcess != nil { s.terminalTheme = "none" return } style := os.Getenv("GLAMOUR_STYLE") if style != "" && style != "auto" { + // ensure GLAMOUR_STYLE takes precedence over "light" and "dark" themes s.terminalTheme = "none" return } - if termenv.HasDarkBackground() { - s.terminalTheme = "dark" - return - } - - s.terminalTheme = "light" + s.terminalTheme = s.term.Theme() } // TerminalTheme returns "light", "dark", or "none" depending on the background color of the terminal. @@ -123,7 +109,7 @@ func (s *IOStreams) TerminalTheme() string { } func (s *IOStreams) SetColorEnabled(colorEnabled bool) { - s.colorEnabled = colorEnabled + //s.colorEnabled = colorEnabled } func (s *IOStreams) SetStdinTTY(isTTY bool) { @@ -339,65 +325,13 @@ func (s *IOStreams) RefreshScreen() { } } -// TerminalWidth returns the width of the terminal that stdout is attached to. -// TODO: investigate whether ProcessTerminalWidth could replace all this. +// TerminalWidth returns the width of the terminal that controls the process func (s *IOStreams) TerminalWidth() int { - if s.termWidthOverride > 0 { - return s.termWidthOverride - } - - defaultWidth := DefaultWidth - - if w, _, err := terminalSize(s.Out.Fd()); err == nil { + w, _, err := s.term.Size() + if err == nil && w > 0 { return w } - - if isCygwinTerminal(s.Out.Fd()) { - tputExe, err := safeexec.LookPath("tput") - if err != nil { - return defaultWidth - } - tputCmd := exec.Command(tputExe, "cols") - tputCmd.Stdin = os.Stdin - if out, err := tputCmd.Output(); err == nil { - if w, err := strconv.Atoi(strings.TrimSpace(string(out))); err == nil { - return w - } - } - } - - return defaultWidth -} - -// ProcessTerminalWidth returns the width of the terminal that the process is attached to. -func (s *IOStreams) ProcessTerminalWidth() int { - w, _, err := s.ttySize() - if err != nil { - return DefaultWidth - } - return w -} - -func (s *IOStreams) ForceTerminal(spec string) { - s.colorEnabled = !EnvColorDisabled() - s.SetStdoutTTY(true) - - if w, err := strconv.Atoi(spec); err == nil { - s.termWidthOverride = w - return - } - - ttyWidth, _, err := s.ttySize() - if err != nil { - return - } - s.termWidthOverride = ttyWidth - - if strings.HasSuffix(spec, "%") { - if p, err := strconv.Atoi(spec[:len(spec)-1]); err == nil { - s.termWidthOverride = int(float64(s.termWidthOverride) * (float64(p) / 100)) - } - } + return DefaultWidth } func (s *IOStreams) ColorScheme() *ColorScheme { @@ -427,25 +361,20 @@ func (s *IOStreams) TempFile(dir, pattern string) (*os.File, error) { } func System() *IOStreams { + terminal := ghTerm.FromEnv() + + // we avoid using terminal.IsTerminalOutput() in order to additionally support cygwin stdoutIsTTY := isTerminal(os.Stdout) stderrIsTTY := isTerminal(os.Stderr) var stdout fileWriter = os.Stdout - isVirtualTerminal := false - if stdoutIsTTY { - // enables ANSI escape sequences on modern Windows - if err := enableVirtualTerminalProcessing(os.Stdout.Fd()); err == nil { - isVirtualTerminal = true - } else { - // as a fallback, translate ANSI escape sequences to Windows console syscalls - colorableStdout := colorable.NewColorable(os.Stdout) - if colorableStdout != os.Stdout { - // ensure that the file descriptor of the original stdout is preserved - stdout = &fdWriter{ - fd: os.Stdout.Fd(), - Writer: colorableStdout, - } - } + // On Windows with no virtual terminal processing support, translate ANSI escape + // sequences to console syscalls + if colorableStdout := colorable.NewColorable(os.Stdout); colorableStdout != os.Stdout { + // ensure that the file descriptor of the original stdout is preserved + stdout = &fdWriter{ + fd: os.Stdout.Fd(), + Writer: colorableStdout, } } @@ -453,18 +382,15 @@ func System() *IOStreams { In: os.Stdin, Out: stdout, ErrOut: colorable.NewColorable(os.Stderr), - colorEnabled: EnvColorForced() || (!EnvColorDisabled() && stdoutIsTTY), - is256enabled: isVirtualTerminal || Is256ColorSupported(), - hasTrueColor: isVirtualTerminal || IsTrueColorSupported(), pagerCommand: os.Getenv("PAGER"), - ttySize: ttySize, + term: terminal, } if stdoutIsTTY && stderrIsTTY { io.progressIndicatorEnabled = true } - if stdoutIsTTY && isVirtualTerminal { + if stdoutIsTTY && hasAlternateScreenBuffer(terminal.IsTrueColorSupported()) { io.alternateScreenBufferEnabled = true } @@ -485,9 +411,6 @@ func Test() (*IOStreams, *bytes.Buffer, *bytes.Buffer, *bytes.Buffer) { }, Out: &fdWriter{fd: 1, Writer: out}, ErrOut: errOut, - ttySize: func() (int, int, error) { - return -1, -1, errors.New("ttySize not implemented in tests") - }, } io.SetStdinTTY(false) io.SetStdoutTTY(false) @@ -496,18 +419,13 @@ func Test() (*IOStreams, *bytes.Buffer, *bytes.Buffer, *bytes.Buffer) { } func isTerminal(f *os.File) bool { - return isatty.IsTerminal(f.Fd()) || isatty.IsCygwinTerminal(f.Fd()) + return ghTerm.IsTerminal(f) || isCygwinTerminal(f.Fd()) } func isCygwinTerminal(fd uintptr) bool { return isatty.IsCygwinTerminal(fd) } -// terminalSize measures the viewport of the terminal that the output stream is connected to -func terminalSize(fd uintptr) (int, int, error) { - return term.GetSize(int(fd)) -} - // pagerWriter implements a WriteCloser that wraps all EPIPE errors in an ErrClosedPagerPipe type. type pagerWriter struct { io.WriteCloser diff --git a/pkg/iostreams/iostreams_test.go b/pkg/iostreams/iostreams_test.go index 772df9bf673..eefc07b9cf4 100644 --- a/pkg/iostreams/iostreams_test.go +++ b/pkg/iostreams/iostreams_test.go @@ -2,79 +2,11 @@ package iostreams import ( "bufio" - "errors" "fmt" - "io" "os" "testing" ) -func TestIOStreams_ForceTerminal(t *testing.T) { - tests := []struct { - name string - iostreams *IOStreams - arg string - wantTTY bool - wantWidth int - }{ - { - name: "explicit width", - iostreams: &IOStreams{}, - arg: "72", - wantTTY: true, - wantWidth: 72, - }, - { - name: "measure width", - iostreams: &IOStreams{ - ttySize: func() (int, int, error) { - return 72, 0, nil - }, - }, - arg: "true", - wantTTY: true, - wantWidth: 72, - }, - { - name: "measure width fails", - iostreams: &IOStreams{ - Out: &fdWriter{ - Writer: io.Discard, - fd: 1, - }, - ttySize: func() (int, int, error) { - return -1, -1, errors.New("ttySize sabotage!") - }, - }, - arg: "true", - wantTTY: true, - wantWidth: 80, - }, - { - name: "apply percentage", - iostreams: &IOStreams{ - ttySize: func() (int, int, error) { - return 72, 0, nil - }, - }, - arg: "50%", - wantTTY: true, - wantWidth: 36, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tt.iostreams.ForceTerminal(tt.arg) - if isTTY := tt.iostreams.IsStdoutTTY(); isTTY != tt.wantTTY { - t.Errorf("IOStreams.IsStdoutTTY() = %v, want %v", isTTY, tt.wantTTY) - } - if tw := tt.iostreams.TerminalWidth(); tw != tt.wantWidth { - t.Errorf("IOStreams.TerminalWidth() = %v, want %v", tw, tt.wantWidth) - } - }) - } -} - func TestStopAlternateScreenBuffer(t *testing.T) { ios, _, stdout, _ := Test() ios.SetAlternateScreenBufferEnabled(true) diff --git a/pkg/iostreams/tty_size.go b/pkg/iostreams/tty_size.go deleted file mode 100644 index c6cbd9c6fcd..00000000000 --- a/pkg/iostreams/tty_size.go +++ /dev/null @@ -1,20 +0,0 @@ -//go:build !windows -// +build !windows - -package iostreams - -import ( - "os" - - "golang.org/x/term" -) - -// ttySize measures the size of the controlling terminal for the current process -func ttySize() (int, int, error) { - f, err := os.Open("/dev/tty") - if err != nil { - return -1, -1, err - } - defer f.Close() - return term.GetSize(int(f.Fd())) -} diff --git a/pkg/iostreams/tty_size_windows.go b/pkg/iostreams/tty_size_windows.go deleted file mode 100644 index 72fb8a7ba17..00000000000 --- a/pkg/iostreams/tty_size_windows.go +++ /dev/null @@ -1,16 +0,0 @@ -package iostreams - -import ( - "os" - - "golang.org/x/term" -) - -func ttySize() (int, int, error) { - f, err := os.Open("CONOUT$") - if err != nil { - return -1, -1, err - } - defer f.Close() - return term.GetSize(int(f.Fd())) -} diff --git a/utils/table_printer.go b/utils/table_printer.go index 2ccef943ce2..1dfd8ef2560 100644 --- a/utils/table_printer.go +++ b/utils/table_printer.go @@ -40,10 +40,8 @@ func NewTablePrinterWithOptions(ios *iostreams.IOStreams, opts TablePrinterOptio var maxWidth int if opts.MaxWidth > 0 { maxWidth = opts.MaxWidth - } else if ios.IsStdoutTTY() { - maxWidth = ios.TerminalWidth() } else { - maxWidth = ios.ProcessTerminalWidth() + maxWidth = ios.TerminalWidth() } return &ttyTablePrinter{ out: out, From 66282b681994414a55b9e4a26716f7d4825b96a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 24 Oct 2022 17:15:06 +0200 Subject: [PATCH 2/5] Restore color override --- pkg/iostreams/iostreams.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index 892f1716256..b888c5b5cc5 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -61,6 +61,9 @@ type IOStreams struct { stderrTTYOverride bool stderrIsTTY bool + colorOverride bool + colorEnabled bool + pagerCommand string pagerProcess *os.Process @@ -70,14 +73,23 @@ type IOStreams struct { } func (s *IOStreams) ColorEnabled() bool { + if s.colorOverride { + return s.colorEnabled + } return s.term.IsColorEnabled() } func (s *IOStreams) ColorSupport256() bool { + if s.colorOverride { + return s.colorEnabled + } return s.term.Is256ColorSupported() } func (s *IOStreams) HasTrueColor() bool { + if s.colorOverride { + return s.colorEnabled + } return s.term.IsTrueColorSupported() } @@ -109,7 +121,8 @@ func (s *IOStreams) TerminalTheme() string { } func (s *IOStreams) SetColorEnabled(colorEnabled bool) { - //s.colorEnabled = colorEnabled + s.colorOverride = true + s.colorEnabled = colorEnabled } func (s *IOStreams) SetStdinTTY(isTTY bool) { From 58b12bfd1356f131f395cf3bd2c21c4766eeb5f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 25 Oct 2022 16:56:28 +0200 Subject: [PATCH 3/5] Fake terminal-like object in tests --- pkg/iostreams/iostreams.go | 59 +++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index b888c5b5cc5..3c3f1edf7f5 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -37,8 +37,17 @@ type fileReader interface { Fd() uintptr } +type term interface { + IsTerminalOutput() bool + IsColorEnabled() bool + Is256ColorSupported() bool + IsTrueColorSupported() bool + Theme() string + Size() (int, int, error) +} + type IOStreams struct { - term ghTerm.Term + term term In fileReader Out fileWriter @@ -149,10 +158,12 @@ func (s *IOStreams) IsStdoutTTY() bool { if s.stdoutTTYOverride { return s.stdoutIsTTY } - if stdout, ok := s.Out.(*os.File); ok { - return isTerminal(stdout) + // support GH_FORCE_TTY + if s.term.IsTerminalOutput() { + return true } - return false + stdout, ok := s.Out.(*os.File) + return ok && isCygwinTerminal(stdout.Fd()) } func (s *IOStreams) SetStderrTTY(isTTY bool) { @@ -376,10 +387,6 @@ func (s *IOStreams) TempFile(dir, pattern string) (*os.File, error) { func System() *IOStreams { terminal := ghTerm.FromEnv() - // we avoid using terminal.IsTerminalOutput() in order to additionally support cygwin - stdoutIsTTY := isTerminal(os.Stdout) - stderrIsTTY := isTerminal(os.Stderr) - var stdout fileWriter = os.Stdout // On Windows with no virtual terminal processing support, translate ANSI escape // sequences to console syscalls @@ -396,10 +403,12 @@ func System() *IOStreams { Out: stdout, ErrOut: colorable.NewColorable(os.Stderr), pagerCommand: os.Getenv("PAGER"), - term: terminal, + term: &terminal, } - if stdoutIsTTY && stderrIsTTY { + stdoutIsTTY := io.IsStdoutTTY() + + if stdoutIsTTY && io.IsStderrTTY() { io.progressIndicatorEnabled = true } @@ -407,12 +416,35 @@ func System() *IOStreams { io.alternateScreenBufferEnabled = true } - // prevent duplicate isTerminal queries now that we know the answer - io.SetStdoutTTY(stdoutIsTTY) - io.SetStderrTTY(stderrIsTTY) return io } +type fakeTerm struct{} + +func (t fakeTerm) IsTerminalOutput() bool { + return false +} + +func (t fakeTerm) IsColorEnabled() bool { + return false +} + +func (t fakeTerm) Is256ColorSupported() bool { + return false +} + +func (t fakeTerm) IsTrueColorSupported() bool { + return false +} + +func (t fakeTerm) Theme() string { + return "" +} + +func (t fakeTerm) Size() (int, int, error) { + return 80, -1, nil +} + func Test() (*IOStreams, *bytes.Buffer, *bytes.Buffer, *bytes.Buffer) { in := &bytes.Buffer{} out := &bytes.Buffer{} @@ -424,6 +456,7 @@ func Test() (*IOStreams, *bytes.Buffer, *bytes.Buffer, *bytes.Buffer) { }, Out: &fdWriter{fd: 1, Writer: out}, ErrOut: errOut, + term: &fakeTerm{}, } io.SetStdinTTY(false) io.SetStdoutTTY(false) From ba028b9044a2c4e34072fff0978d9597a16e2aef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 2 Nov 2022 15:53:26 +0100 Subject: [PATCH 4/5] Fix setting up git credential helper on Windows --- pkg/cmd/auth/shared/git_credential.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/auth/shared/git_credential.go b/pkg/cmd/auth/shared/git_credential.go index 5ddac14d552..8624cf00bef 100644 --- a/pkg/cmd/auth/shared/git_credential.go +++ b/pkg/cmd/auth/shared/git_credential.go @@ -180,7 +180,7 @@ func isGitMissing(err error) bool { } func shellQuote(s string) string { - if strings.ContainsAny(s, " $") { + if strings.ContainsAny(s, " $\\") { return "'" + s + "'" } return s From c2793b62c4bc71f6561b716d2199f8e314ba0459 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 3 Nov 2022 12:51:51 +0100 Subject: [PATCH 5/5] Bump go-gh Features: - Support garage.github.com - Resolve ssh hostname aliases with `ssh -G` - Correctly measure terminal size when stdout is redirected --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index a681c2c951f..54e55904fe9 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/briandowns/spinner v1.18.1 github.com/charmbracelet/glamour v0.5.1-0.20220727184942-e70ff2d969da github.com/charmbracelet/lipgloss v0.5.0 - github.com/cli/go-gh v0.1.2 + github.com/cli/go-gh v0.1.3-0.20221102170023-e3ec45fb1d1b github.com/cli/oauth v0.9.0 github.com/cli/safeexec v1.0.0 github.com/cpuguy83/go-md2man/v2 v2.0.2 diff --git a/go.sum b/go.sum index e15581064f0..01ce18b9780 100644 --- a/go.sum +++ b/go.sum @@ -58,8 +58,8 @@ github.com/cli/browser v1.1.0 h1:xOZBfkfY9L9vMBgqb1YwRirGu6QFaQ5dP/vXt5ENSOY= github.com/cli/browser v1.1.0/go.mod h1:HKMQAt9t12kov91Mn7RfZxyJQQgWgyS/3SZswlZ5iTI= github.com/cli/crypto v0.0.0-20210929142629-6be313f59b03 h1:3f4uHLfWx4/WlnMPXGai03eoWAI+oGHJwr+5OXfxCr8= github.com/cli/crypto v0.0.0-20210929142629-6be313f59b03/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -github.com/cli/go-gh v0.1.2 h1:DoiHIo7uuK51Tw5dmawHfIMcBq9CsNNZ2uQTPkP4pLU= -github.com/cli/go-gh v0.1.2/go.mod h1:bqxLdCoTZ73BuiPEJx4olcO/XKhVZaFDchFagYRBweE= +github.com/cli/go-gh v0.1.3-0.20221102170023-e3ec45fb1d1b h1:W17Cf1UmOvLPbrHcFs9InoY3VPxC0TJWx3QwnjnD4TY= +github.com/cli/go-gh v0.1.3-0.20221102170023-e3ec45fb1d1b/go.mod h1:bqxLdCoTZ73BuiPEJx4olcO/XKhVZaFDchFagYRBweE= github.com/cli/oauth v0.9.0 h1:nxBC0Df4tUzMkqffAB+uZvisOwT3/N9FpkfdTDtafxc= github.com/cli/oauth v0.9.0/go.mod h1:qd/FX8ZBD6n1sVNQO3aIdRxeu5LGw9WhKnYhIIoC2A4= github.com/cli/safeexec v1.0.0 h1:0VngyaIyqACHdcMNWfo6+KdUYnqEr2Sg+bSP1pdF+dI=