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

#285 add color output for mage list #301

Merged
merged 12 commits into from
Jul 8, 2020
Merged

#285 add color output for mage list #301

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9495507
#285 add color output for mage list
mirogta Apr 25, 2020
0a042a3
added color configuration via MAGEFILE_ENABLE_COLOR and MAGEFILE_TARG…
mirogta May 19, 2020
9a98a25
fixed template
mirogta May 19, 2020
90314f9
typo
mirogta May 20, 2020
d1da6c9
more typos
mirogta May 20, 2020
50d2587
fix test
mirogta May 20, 2020
a894948
simplify color terminal detection
mirogta Jul 8, 2020
d4fdf58
fix: color output test in CI
mirogta Jul 8, 2020
71bfbb0
fix: test in CI
mirogta Jul 8, 2020
7afad3c
Update mage/main_test.go
natefinch Jul 8, 2020
a1f2a7c
Update mg/runtime.go
natefinch Jul 8, 2020
7254217
Update mg/runtime.go
natefinch Jul 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions mage/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,21 @@ func testmain(m *testing.M) int {
if err := os.Unsetenv(mg.IgnoreDefaultEnv); err != nil {
log.Fatal(err)
}
if err := os.Setenv(mg.CacheEnv, dir); err != nil {
log.Fatal(err)
}
resetTerm()
return m.Run()
}

func resetTerm() {
// unset TERM env var in order to disable color output to make the tests simpler
// there is a specific test for colorized output, so all the other tests can use non-colorized one
if err := os.Unsetenv("TERM"); err != nil {
log.Fatal(err)
}
}

func TestTransitiveDepCache(t *testing.T) {
cache, err := internal.OutputDebug("go", "env", "GOCACHE")
if err != nil {
Expand Down Expand Up @@ -292,6 +304,7 @@ func TestListMagefilesLib(t *testing.T) {
}

func TestMixedMageImports(t *testing.T) {
resetTerm()
stderr := &bytes.Buffer{}
stdout := &bytes.Buffer{}
inv := Invocation{
Expand Down Expand Up @@ -420,7 +433,81 @@ Targets:
}
}

var terminals = []struct {
code string
supportsColor bool
}{
{"", false},
{"vt100", false},
{"cygwin", false},
{"xterm", true},
{"xterm-vt220", true},
{"xterm-16color", true},
{"xterm-256color", true},
{"screen-256color", true},
{"tmux-256color", true},
{"rxvt-unicode-256color", true},
}

func TestListWithColor(t *testing.T) {

expectedPlainText := `
This is a comment on the package which should get turned into output with the list of targets.

Targets:
somePig* This is the synopsis for SomePig.
testVerbose

* default target
`[1:]

// NOTE: using the literal string would be complicated because I would need to break it
// in the middle and join with a normal string for the target names,
// otherwise the single backslash would be taken literally and encoded as \\
expectedColorizedText := "" +
"This is a comment on the package which should get turned into output with the list of targets.\n" +
"\n" +
"Targets:\n" +
" \x1b[36msomePig*\x1b[0m This is the synopsis for SomePig.\n" +
" \x1b[36mtestVerbose\x1b[0m \n" +
"\n" +
"* default target\n"

for _, terminal := range terminals {
t.Run(terminal.code, func(t *testing.T) {
os.Setenv("TERM", terminal.code)

stdout := &bytes.Buffer{}
inv := Invocation{
Dir: "./testdata/list",
Stdout: stdout,
Stderr: ioutil.Discard,
List: true,
}

code := Invoke(inv)
if code != 0 {
t.Errorf("expected to exit with code 0, but got %v", code)
}
actual := stdout.String()
var expected string
if terminal.supportsColor {
expected = expectedColorizedText
} else {
expected = expectedPlainText
}

if actual != expected {
t.Logf("expected: %q", expected)
t.Logf(" actual: %q", actual)
t.Fatalf("expected:\n%v\n\ngot:\n%v", expected, actual)
}
})
}
}

func TestNoArgNoDefaultList(t *testing.T) {
resetTerm()
stdout := &bytes.Buffer{}
stderr := &bytes.Buffer{}
inv := Invocation{
Expand Down Expand Up @@ -458,6 +545,7 @@ func TestIgnoreDefault(t *testing.T) {
if err := os.Setenv(mg.IgnoreDefaultEnv, "1"); err != nil {
t.Fatal(err)
}
resetTerm()

code := Invoke(inv)
if code != 0 {
Expand Down Expand Up @@ -1286,6 +1374,7 @@ func TestGoCmd(t *testing.T) {
var runtimeVer = regexp.MustCompile(`go1\.([0-9]+)`)

func TestGoModules(t *testing.T) {
resetTerm()
matches := runtimeVer.FindStringSubmatch(runtime.Version())
if len(matches) < 2 || minorVer(t, matches[1]) < 11 {
t.Skipf("Skipping Go modules test because go version %q is less than go1.11", runtime.Version())
Expand Down
56 changes: 55 additions & 1 deletion mage/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,60 @@ Options:
return
}

// Terminals with color support:
// "TERM=xterm"
// "TERM=xterm-vt220"
// "TERM=xterm-256color"
// "TERM=screen-256color"
// "TERM=tmux-256color"
// "TERM=rxvt-unicode-256color"
// Don't support color:
// "TERM=cygwin"
var specialColorTerms = map[string]bool{
"screen-256color": true,
"tmux-256color": true,
"rxvt-unicode-256color": true,
}

// terminalSupportsColor checks if the current console supports color output
//
// Supported:
// linux, mac, or windows's ConEmu, Cmder, putty, git-bash.exe
// Not supported:
// windows cmd.exe, powerShell.exe
terminalSupportsColor := func() bool {
envTerm := os.Getenv("TERM")
if strings.Contains(envTerm, "xterm") {
Copy link
Member

Choose a reason for hiding this comment

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

This kinda skeeves me out. Are we sure there are no existing terminals that include the substring xterm that don't support color? (I ask because I have no clue)

Copy link
Member Author

@mirogta mirogta May 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, the best I could find was that there's "xterm-mono". I've added a condition to disable terminal support if this is detected.

See rust-lang/cargo#7646

Copy link

@arcticicestudio arcticicestudio Jun 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I can say from experience that this topic could be a separate study course 🙉
Here's a list of possible terminal identifiers from my color theme project for GNU dircolors (the tool that provides the color database for the GNU ls command by populating the LS_COLORS environment variable).
The list contains a lot more IDs and I'm sure it still misses some, e.g. new terminals like Alacritty bring their own new identifiers and terminfo definitions.
Anyway, maybe it's easier to only check for terminal IDs that are definitely known to not support colors and let all other IDs pass. This way the list could be extended from time to time when users report terminal IDs that are not checked yet. Since this is a opt-in feature it should not break anyone's output.

Copy link
Member Author

Choose a reason for hiding this comment

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

only check of terminal IDs that definitely known to not support colors and let all others IDs pass

Great idea! @natefinch any objections? The code would get significantly simpler.

	// Terminals which  don't support color:
	// 	TERM=vt100
	// 	TERM=cygwin
	// 	TERM=xterm-mono
        var noColorTerms := [3]string{"vt100", "cygwin", "xterm-mono"}

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I missed this was a question for me. Yes, let's do that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool - done, see the update.

I've used a map rather than a list for performance, so that I don't need to iterate over it and just use the terminal name as a key.

Copy link
Member Author

Choose a reason for hiding this comment

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

"It works on my machine" including all tests, but having some trouble with tests in Travis CI. Trying to find out what is the TERM set to...

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking good now. Ready for a merge & release?

return true
}

// it's special color term
if _, ok := specialColorTerms[envTerm]; ok {
return true
}

// like on ConEmu software, e.g "ConEmuANSI=ON"
if os.Getenv("ConEmuANSI") == "ON" {
return true
}

// like on ConEmu software, e.g "ANSICON=189x2000 (189x43)"
if os.Getenv("ANSICON") != "" {
return true
}

return false
}

colorizeOutputOnColorTerminal := func(str string) string {
if terminalSupportsColor() {
// chosing CYAN (code 36) as an arbitrary color, because it has a neutral meaning
return fmt.Sprintf("\033[36m%s\033[0m", str)
} else {
return str
}
}

list := func() error {
{{with .Description}}fmt.Println(` + "`{{.}}\n`" + `)
{{- end}}
Expand All @@ -117,7 +171,7 @@ Options:
fmt.Println("Targets:")
w := tabwriter.NewWriter(os.Stdout, 0, 4, 4, ' ', 0)
for _, name := range keys {
fmt.Fprintf(w, " %v\t%v\n", name, targets[name])
fmt.Fprintf(w, " %v\t%v\n", colorizeOutputOnColorTerminal(name), targets[name])
}
err := w.Flush()
{{- if .DefaultFunc.Name}}
Expand Down