Add support for FORCE_COLOR - #204
Conversation
| defer func() { | ||
| os.Unsetenv("CLICOLOR_FORCE") | ||
| os.Unsetenv("FORCE_COLOR") | ||
| }() | ||
| for i := 0; i < len(test.environ); i += 2 { | ||
| os.Setenv(test.environ[i], test.environ[i+1]) | ||
| } |
There was a problem hiding this comment.
Using t.SetEnv would avoid the defer and race conditions
Go 1.17+ supports it, and Go 1.17 is the minimal version of the repository, so it should be OK
There was a problem hiding this comment.
was just following the existing tests, but agreed; updated.
| tests := []struct { | ||
| name string | ||
| environ []string | ||
| expected Profile | ||
| }{ | ||
| // FORCE_COLOR and CLICOLOR_FORCE both force at least ANSI on a non-TTY. | ||
| {"force_color", []string{"FORCE_COLOR", "1"}, ANSI}, | ||
| {"clicolor_force", []string{"CLICOLOR_FORCE", "1"}, ANSI}, |
There was a problem hiding this comment.
Wouldn't this be simpler with envName and envValue being string in the struct instead of a slice that lead you to loop on I with I+2 and use i and i+1
There was a problem hiding this comment.
was following the existing pattern, but agreed; changed the test a little bit so that it's using a map instead.
| expected Profile | ||
| }{ | ||
| // FORCE_COLOR and CLICOLOR_FORCE both force at least ANSI on a non-TTY. | ||
| {"force_color", []string{"FORCE_COLOR", "1"}, ANSI}, |
There was a problem hiding this comment.
Ypu should also test FORCE_COLOR being "whatever" and "0"
The force color sites explains it is activated with any non empty value
It's what your code does, BTW
|
|
||
| - [`NO_COLOR`](https://no-color.org/) — disables color output when set | ||
| - [`CLICOLOR`/`CLICOLOR_FORCE`](https://bixense.com/clicolors/) — controls and forces color output | ||
| - [`FORCE_COLOR`](https://force-color.org/) — forces a minimum color level: `1` for ANSI, `2` for 256-color, `3` for TrueColor |
There was a problem hiding this comment.
This is unclear to me
I would expect the code to do something when FORCE_COLOR is 1, 2, or 3.
But it's not what the code does, and not what https://force-color.org "specs" mention.
There was a problem hiding this comment.
you're right; was refactoring and dropped this in the code but not here; fixed.
2 participants
Ref: https://force-color.org/