Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dogfood term package from go-gh #6421

Merged
merged 8 commits into from Nov 3, 2022
Merged

Dogfood term package from go-gh #6421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
25a926d
Dogfood term package from go-gh
mislav Oct 11, 2022
76ad94c
Merge remote-tracking branch 'origin' into go-gh-term
mislav Oct 24, 2022
66282b6
Restore color override
mislav Oct 24, 2022
58b12bf
Fake terminal-like object in tests
mislav Oct 25, 2022
38d465b
Merge remote-tracking branch 'origin' into go-gh-term
mislav Oct 31, 2022
ba028b9
Fix setting up git credential helper on Windows
mislav Nov 2, 2022
57b3ef1
Merge pull request #6558 from cli/setup-git-windows
mislav Nov 2, 2022
c2793b6
Bump go-gh
mislav Nov 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions cmd/gh/main.go
View file
Open in desktop
Expand Up @@ -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
ansi.DisableColors(true)
Expand Down
6 changes: 3 additions & 3 deletions go.mod
View file
Open in desktop
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.8
google.golang.org/grpc v1.49.0
Expand All @@ -61,6 +59,7 @@ require (
github.com/mattn/go-runewidth v0.0.13 // indirect
github.com/microcosm-cc/bluemonday v1.0.20 // 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
Expand All @@ -72,6 +71,7 @@ require (
github.com/yuin/goldmark-emoji v1.0.1 // indirect
golang.org/x/net v0.0.0-20220923203811-8be639271d50 // indirect
golang.org/x/oauth2 v0.0.0-20220309155454-6242fa91716a // indirect
golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab // indirect
google.golang.org/genproto v0.0.0-20200825200019-8632dd797987 // indirect
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
)
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Expand Up @@ -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=
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/auth/shared/git_credential.go
View file
Open in desktop
Expand Up @@ -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
Expand Down
25 changes: 0 additions & 25 deletions pkg/iostreams/color.go
View file
Open in desktop
Expand Up @@ -2,7 +2,6 @@ package iostreams

import (
"fmt"
"os"
"strconv"
"strings"

Expand All @@ -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,
Expand Down
121 changes: 0 additions & 121 deletions pkg/iostreams/color_test.go
View file
Open in desktop
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions pkg/iostreams/console.go
View file
Open in desktop
Expand Up @@ -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"
}
14 changes: 4 additions & 10 deletions pkg/iostreams/console_windows.go
View file
Open in desktop
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does windows not support os.Getenv("TERM")?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my knowledge, Windows typically doesn't advertise terminal features via the TERM environment variable, but it does expose them programmatically through Console APIs. Hence, if we successfully enabled virtual terminal processing via these APIs, I think we can safely assume that alternate screen buffers are enabled. This is why the check isn't symmetrical with non-Windows platforms.

}