From 41460b9425137013a5507f3780289f5d7e8294be Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Wed, 25 Jan 2023 16:49:01 -0800 Subject: [PATCH 1/2] Improve AutoColor functionality Cleanup AutoColor to make it never fail, instead do it's best to activate color if possible only. Fixes #122 --- colorize_unix.go | 32 ++++++++++++++++++++++---------- colorize_windows.go | 38 +++++++++++++++++++------------------- intlogger.go | 11 ----------- logger.go | 7 +++++++ 4 files changed, 48 insertions(+), 40 deletions(-) diff --git a/colorize_unix.go b/colorize_unix.go index 99cc176..80aa7b1 100644 --- a/colorize_unix.go +++ b/colorize_unix.go @@ -7,23 +7,35 @@ import ( "github.com/mattn/go-isatty" ) +// hasFD is used to check if the writer has an Fd value to check +// if it's a terminal. +type hasFD interface { + Fd() uintptr +} + // setColorization will mutate the values of this logger // to appropriately configure colorization options. It provides // a wrapper to the output stream on Windows systems. func (l *intLogger) setColorization(opts *LoggerOptions) { - switch opts.Color { - case ColorOff: - fallthrough - case ForceColor: + if opts.Color != AutoColor { return - case AutoColor: - fi := l.checkWriterIsFile() - isUnixTerm := isatty.IsTerminal(fi.Fd()) - isCygwinTerm := isatty.IsCygwinTerminal(fi.Fd()) - isTerm := isUnixTerm || isCygwinTerm - if !isTerm { + } + + if sc, ok := l.writer.w.(SupportsColor); ok { + if !sc.SupportsColor() { l.headerColor = ColorOff l.writer.color = ColorOff } + return + } + + fi, ok := l.writer.w.(hasFD) + if !ok { + return + } + + if !isatty.IsTerminal(fi.Fd()) { + l.headerColor = ColorOff + l.writer.color = ColorOff } } diff --git a/colorize_windows.go b/colorize_windows.go index 26f8cef..67a8b69 100644 --- a/colorize_windows.go +++ b/colorize_windows.go @@ -7,32 +7,32 @@ import ( "os" colorable "github.com/mattn/go-colorable" - "github.com/mattn/go-isatty" ) // setColorization will mutate the values of this logger // to appropriately configure colorization options. It provides // a wrapper to the output stream on Windows systems. func (l *intLogger) setColorization(opts *LoggerOptions) { - switch opts.Color { - case ColorOff: + if opts.Color == ColorOff { return - case ForceColor: - fi := l.checkWriterIsFile() - l.writer.w = colorable.NewColorable(fi) - case AutoColor: - fi := l.checkWriterIsFile() - isUnixTerm := isatty.IsTerminal(os.Stdout.Fd()) - isCygwinTerm := isatty.IsCygwinTerminal(os.Stdout.Fd()) - isTerm := isUnixTerm || isCygwinTerm - if !isTerm { - l.writer.color = ColorOff - l.headerColor = ColorOff - return - } + } + + fi, ok := l.writer.w.(*os.File) + if !ok { + l.writer.color = ColorOff + l.headerColor = ColorOff + return + } + + cfi := colorable.NewColorable(fi) - if l.headerColor == ColorOff { - l.writer.w = colorable.NewColorable(fi) - } + // NewColorable detects if color is possible and if it's not, then it + // returns the original value. So we can test if we got the original + // value back to know if color is possible. + if cfi == fi { + l.writer.color = ColorOff + l.headerColor = ColorOff + } else { + l.writer.w = cfi } } diff --git a/intlogger.go b/intlogger.go index 89d26c9..b61476b 100644 --- a/intlogger.go +++ b/intlogger.go @@ -8,7 +8,6 @@ import ( "fmt" "io" "log" - "os" "reflect" "runtime" "sort" @@ -876,16 +875,6 @@ func (l *intLogger) StandardWriter(opts *StandardLoggerOptions) io.Writer { } } -// checks if the underlying io.Writer is a file, and -// panics if not. For use by colorization. -func (l *intLogger) checkWriterIsFile() *os.File { - fi, ok := l.writer.w.(*os.File) - if !ok { - panic("Cannot enable coloring of non-file Writers") - } - return fi -} - // Accept implements the SinkAdapter interface func (i *intLogger) Accept(name string, level Level, msg string, args ...interface{}) { i.log(name, level, msg, args...) diff --git a/logger.go b/logger.go index 3cdb283..7e08aa6 100644 --- a/logger.go +++ b/logger.go @@ -89,6 +89,13 @@ const ( ForceColor ) +// SupportsColor is an optional interface that can be implemented by the output +// value. If implemented and SupportsColor() returns true, then AutoColor will +// enable colorization. +type SupportsColor interface { + SupportsColor() bool +} + // LevelFromString returns a Level type for the named log level, or "NoLevel" if // the level string is invalid. This facilitates setting the log level via // config or environment variable by name in a predictable way. From 5763c2427a51df5b2eee464dda5f9e1c5257e629 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Wed, 25 Jan 2023 16:51:39 -0800 Subject: [PATCH 2/2] Bump up versions tested --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 65defaf..043696a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,7 +11,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - go-version: [1.14.x, 1.15.x, 1.16.x] + go-version: [1.14.x, 1.15.x, 1.16.x, 1.17.x, 1.18.x, 1.19.x] os: [ubuntu-latest, windows-latest, macOS-latest] steps: - name: Install Go