Add FORCE_COLOR environment variable support#280
Conversation
|
just following up here, happy to tweak anything if needed! |
|
just checking in on this one |
345c765 to
b78d43c
Compare
| os.Setenv("FORCE_COLOR", "1") | ||
| t.Cleanup(func() { | ||
| os.Unsetenv("FORCE_COLOR") | ||
| }) |
There was a problem hiding this comment.
| os.Setenv("FORCE_COLOR", "1") | |
| t.Cleanup(func() { | |
| os.Unsetenv("FORCE_COLOR") | |
| }) | |
| t.Setenv("FORCE_COLOR", "1") |
| os.Setenv("FORCE_COLOR", "1") | ||
| os.Setenv("NO_COLOR", "1") | ||
| t.Cleanup(func() { | ||
| os.Unsetenv("FORCE_COLOR") | ||
| os.Unsetenv("NO_COLOR") | ||
| }) |
There was a problem hiding this comment.
| os.Setenv("FORCE_COLOR", "1") | |
| os.Setenv("NO_COLOR", "1") | |
| t.Cleanup(func() { | |
| os.Unsetenv("FORCE_COLOR") | |
| os.Unsetenv("NO_COLOR") | |
| }) | |
| t.Setenv("FORCE_COLOR", "1") | |
| t.Setenv("NO_COLOR", "1") |
| func Test_forceColorIsSet(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| act func() | ||
| want bool | ||
| }{ | ||
| { | ||
| name: "default", | ||
| act: func() {}, | ||
| want: false, | ||
| }, | ||
| { | ||
| name: "FORCE_COLOR=1", | ||
| act: func() { os.Setenv("FORCE_COLOR", "1") }, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "FORCE_COLOR=0", | ||
| act: func() { os.Setenv("FORCE_COLOR", "0") }, | ||
| want: false, | ||
| }, | ||
| { | ||
| name: "FORCE_COLOR=true", | ||
| act: func() { os.Setenv("FORCE_COLOR", "true") }, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "FORCE_COLOR=", | ||
| act: func() { os.Setenv("FORCE_COLOR", "") }, | ||
| want: true, | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Cleanup(func() { | ||
| os.Unsetenv("FORCE_COLOR") | ||
| }) | ||
| tt.act() | ||
| if got := forceColorIsSet(); got != tt.want { | ||
| t.Errorf("forceColorIsSet() = %v, want %v", got, tt.want) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
I would like to recommend this
| func Test_forceColorIsSet(t *testing.T) { | |
| tests := []struct { | |
| name string | |
| act func() | |
| want bool | |
| }{ | |
| { | |
| name: "default", | |
| act: func() {}, | |
| want: false, | |
| }, | |
| { | |
| name: "FORCE_COLOR=1", | |
| act: func() { os.Setenv("FORCE_COLOR", "1") }, | |
| want: true, | |
| }, | |
| { | |
| name: "FORCE_COLOR=0", | |
| act: func() { os.Setenv("FORCE_COLOR", "0") }, | |
| want: false, | |
| }, | |
| { | |
| name: "FORCE_COLOR=true", | |
| act: func() { os.Setenv("FORCE_COLOR", "true") }, | |
| want: true, | |
| }, | |
| { | |
| name: "FORCE_COLOR=", | |
| act: func() { os.Setenv("FORCE_COLOR", "") }, | |
| want: true, | |
| }, | |
| } | |
| for _, tt := range tests { | |
| t.Run(tt.name, func(t *testing.T) { | |
| t.Cleanup(func() { | |
| os.Unsetenv("FORCE_COLOR") | |
| }) | |
| tt.act() | |
| if got := forceColorIsSet(); got != tt.want { | |
| t.Errorf("forceColorIsSet() = %v, want %v", got, tt.want) | |
| } | |
| }) | |
| } | |
| } | |
| func Test_forceColorIsSet(t *testing.T) { | |
| t.Run("default", func(t *testing.T){ | |
| if forceColorIsSet() { | |
| t.Errorf("forceColorIsSet() = true, want false") | |
| } | |
| }) | |
| tests := []struct { | |
| name string | |
| value string | |
| want bool | |
| }{ | |
| { | |
| name: "FORCE_COLOR=1", | |
| value: "1", | |
| want: true, | |
| }, | |
| { | |
| name: "FORCE_COLOR=0", | |
| value: "0", | |
| want: false, | |
| }, | |
| { | |
| name: "FORCE_COLOR=true", | |
| value: "true", | |
| want: true, | |
| }, | |
| { | |
| name: "FORCE_COLOR=", | |
| value: "", | |
| want: true, | |
| }, | |
| } | |
| for _, tt := range tests { | |
| t.Run(tt.name, func(t *testing.T) { | |
| t.SetEnv("FORCE_COLOR", tt.value) | |
| if got := forceColorIsSet(); got != tt.want { | |
| t.Errorf("forceColorIsSet() = %v, want %v", got, tt.want) | |
| } | |
| }) | |
| } | |
| } |
There was a problem hiding this comment.
You could also test a value like "whatever"
Add support for the FORCE_COLOR environment variable (https://force-color.org). When FORCE_COLOR is set to any value other than "0", colors are enabled regardless of terminal detection. NO_COLOR takes precedence over FORCE_COLOR. This is useful in CI environments like GitHub Actions and GitLab CI that support ANSI colors but are not detected as TTYs. Fixes fatih#155
b78d43c to
2b93d01
Compare
|
Rebased on latest main and addressed all your feedback — switched to t.Setenv, restructured the test with a separate default subtest, and added a "whatever" test case. Thanks for the suggestions! |
| { | ||
| name: "FORCE_COLOR=", | ||
| value: "", | ||
| want: true, | ||
| }, |
There was a problem hiding this comment.
This surprised me
I went to read the spec you quoted
Command-line software which outputs colored text should check for a FORCE_COLOR environment variable. When this variable is present and not an empty string (regardless of its value), it should force the addition of ANSI color.
So here it state that empty string should not enable color.
But then it also state that having FORCE_COLOR=0 implies having colors?
Which mean your lookup env should check for values different than empty string, and not "0"
Note: I understand the spec is unclear
Because this activates colors
FORCE_COLOR=" "While this doesn't
FORCE_COLOR=""I'm unsure what is the best here, because I would expect this to disable colors
FORCE_COLOR=0So here as it's open to interpretation, but that the spec seems pretty clear I would state with the spec and use a lookup and check for != "" and add tests for "0" " " "0 "
Fixes #155
Adds support for the
FORCE_COLORenvironment variable. WhenFORCE_COLORis set to any value other than"0", colors are enabled regardless of TTY detection.NO_COLORtakes precedence overFORCE_COLOR.This is useful for CI systems like GitHub Actions and GitLab CI that support ANSI colors but aren't detected as TTYs. The
FORCE_COLORconvention has been standardized at https://force-color.org and is already widely supported in the Node.js ecosystem (chalk, etc.) and increasingly in other languages.Changes
color.go: AddedforceColorIsSet()helper and integrated it into theNoColorglobal initialization andNew()constructorcolor_test.go: Added tests forforceColorIsSet(), color output withFORCE_COLOR, andNO_COLORprecedence overFORCE_COLORREADME.md: DocumentedFORCE_COLORsupport and updated the GitHub Actions sectionTesting