Skip to content

feat: support NO_COLOR environment variable #411

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

Merged
merged 1 commit into from
Mar 25, 2025
Merged

feat: support NO_COLOR environment variable #411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
72 changes: 51 additions & 21 deletions logger/log.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,63 @@ import (
"fmt"
"io"
"os"
"strings"

"github.com/motemen/go-colorine"
)

var logger = colorine.NewLogger(
colorine.Prefixes{
"git": colorine.Verbose,
"hg": colorine.Verbose,
"svn": colorine.Verbose,
"darcs": colorine.Verbose,
"pijul": colorine.Verbose,
"bzr": colorine.Verbose,
"fossil": colorine.Verbose,
"skip": colorine.Verbose,
"cd": colorine.Verbose,
"resolved": colorine.Verbose,

"open": colorine.Warn,
"exists": colorine.Warn,
"warning": colorine.Warn,
var (
NoColor = colorine.TextStyle{Foreground: colorine.None, Background: colorine.None}
VerboseColor = colorine.Verbose // white
InfoColor = colorine.Info // green
NoticeColor = colorine.Notice // blue
WarnColor = colorine.Warn // yellow
ErrorColor = colorine.Error // red
)

"authorized": colorine.Notice,
var (
logger = colorine.NewLogger( // default logger with color
colorine.Prefixes{
// verbose
"git": VerboseColor,
"hg": VerboseColor,
"svn": VerboseColor,
"darcs": VerboseColor,
"pijul": VerboseColor,
"bzr": VerboseColor,
"fossil": VerboseColor,
"skip": VerboseColor,
"cd": VerboseColor,
"resolved": VerboseColor,
// notice
"authorized": NoticeColor,
// warn
"open": WarnColor,
"exists": WarnColor,
"warning": WarnColor,
// error
"error": ErrorColor,
},
InfoColor, // default is info
)

"error": colorine.Error,
}, colorine.Info)
loggerWithoutColor = colorine.NewLogger(
colorine.Prefixes{},
NoColor,
)
)

func init() {
SelectLogger()
}

func SelectLogger() {
v := os.Getenv("NO_COLOR")

if strings.ToLower(v) == "true" {
logger = loggerWithoutColor
}
Comment on lines +58 to +62
Copy link

Choose a reason for hiding this comment

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

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?

Copy link

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll try to replace colorine with fatih/color .

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

Copy link

Choose a reason for hiding this comment

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

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.

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


SetOutput(os.Stderr)
}

Expand All @@ -39,12 +69,12 @@ func SetOutput(w io.Writer) {
logger.SetOutput(w)
}

// Log output
// Log outputs log
func Log(prefix, message string) {
logger.Log(prefix, message)
}

// Logf output log with format
// Logf outputs log with format
func Logf(prefix, msg string, args ...interface{}) {
Log(prefix, fmt.Sprintf(msg, args...))
}
43 changes: 36 additions & 7 deletions logger/log_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,41 @@
package logger

import "testing"
import (
"os"
"testing"
)

func TestLog(t *testing.T) {
Log("default", "shows this color")
Log("error", "shows this color")
Log("open", "shows this color")
Log("authorized", "shows this color")
Log("skip", "shows this color")
Log("git", "shows this color")
t.Run("with color", func(t *testing.T) {
t.Logf("NO_COLOR: %s", os.Getenv("NO_COLOR"))
SelectLogger()
// info
Log("default", "should be green")
// verbose
Log("git", "should be white")
Log("skip", "should be white")
// notice
Log("authorized", "should be blue")
// warn
Log("open", "should be yellow")
// error
Log("error", "should be red")
})

t.Run("without color", func(t *testing.T) {
t.Setenv("NO_COLOR", "true")
t.Logf("NO_COLOR: %s", os.Getenv("NO_COLOR"))
SelectLogger()
// info
Log("default", "should be none")
// verbose
Log("git", "should be none")
Log("skip", "should be none")
// notice
Log("authorized", "should be none")
// warn
Log("open", "should be none")
// error
Log("error", "should be none")
})
}
Loading