feat: support NO_COLOR environment variable #411
Conversation
| v := os.Getenv("NO_COLOR") | ||
|
|
||
| if strings.ToLower(v) == "true" { | ||
| logger = loggerWithoutColor | ||
| } |
There was a problem hiding this comment.
I should have expected colorine to support NO_COLOR, not eaxh user of the lib to implement that.
I mean it's what fatih/color does
https://github.com/fatih/color/blob/62b78f82738d9b72cd6a159cb33c7be523c0899b/color.go#L40
Did you consider opening the PR on colorine?
There was a problem hiding this comment.
colorine is abandoned apparently, last commit is years old.
Maybe using another lib for color could be a better move than adding a band on a wooden leg
There was a problem hiding this comment.
I'll try to replace colorine with fatih/color .
There was a problem hiding this comment.
I agree that it's a good idea to leave it up to the coloring logger library whether or not to colorize in order to avoid making the changes too large.
However, changing the library at this stage would have too much impact.
So, at this stage, I would like to propose changing ghq/logger.logger to an interface like type interface logger { Log(string, string) } and replacing it depending on whether "NO_COLOR" is truthy or not.
At this time, you will need to implement something like rawLogger that satisfies the logger interface, but that will not be a major implementation.
There was a problem hiding this comment.
By the way, it is common to specify things like NO_COLOR=1, and I thought that the truthiness of this could be determined simply by checking whether the string is empty. https://no-color.org also says this.
However, there is probably some debate about whether values like “false,” “off,” “0,” and spaces should be considered true, but I think it is OK to treat them as true, following no-color.org.
There was a problem hiding this comment.
I agree that it's a good idea to leave it up to the coloring logger library whether or not to colorize in order to avoid making the changes too large.
However, changing the library at this stage would have too much impact.
So, at this stage, I would like to propose changing
ghq/logger.loggerto an interface liketype interface logger { Log(string, string) }and replacing it depending on whether "NO_COLOR" is truthy or not.At this time, you will need to implement something like rawLogger that satisfies the logger interface, but that will not be a major implementation.
I agree. Another PR would be a good thing. This one can focus on the nocolor env checking only.
Also if you only need color for the logs, and only the logs, there are slog implementation that comes with colors
Implement #330