From 37d651c1f2847d8514b79d1dd7389be06ec60447 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Fri, 13 Jul 2018 17:33:25 +0200 Subject: [PATCH 1/4] Add CLICOLOR support This implement CLICOLOR and CLICOLOR_FORCE check on terminal coloring as defined in https://bixense.com/clicolors/ --- text_formatter.go | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/text_formatter.go b/text_formatter.go index 3e55040..cdf3185 100644 --- a/text_formatter.go +++ b/text_formatter.go @@ -3,6 +3,7 @@ package logrus import ( "bytes" "fmt" + "os" "sort" "strings" "sync" @@ -35,6 +36,9 @@ type TextFormatter struct { // Force disabling colors. DisableColors bool + // Override coloring based on CLICOLOR and CLICOLOR_FORCE. - https://bixense.com/clicolors/ + OverrideColors bool + // Disable timestamp logging. useful when output is redirected to logging // system that already adds timestamps. DisableTimestamp bool @@ -78,6 +82,22 @@ func (f *TextFormatter) init(entry *Entry) { } } +func (f *TextFormatter) isColored() bool { + isColored := f.ForceColors || f.isTerminal + + if f.OverrideColors { + if force, ok := os.LookupEnv("CLICOLOR_FORCE"); ok && force != "0" { + isColored = true + } + + if os.Getenv("CLICOLOR") == "0" { + isColored = false + } + } + + return isColored && !f.DisableColors +} + // Format renders a single log entry func (f *TextFormatter) Format(entry *Entry) ([]byte, error) { prefixFieldClashes(entry.Data, f.FieldMap) @@ -100,13 +120,11 @@ func (f *TextFormatter) Format(entry *Entry) ([]byte, error) { f.Do(func() { f.init(entry) }) - isColored := (f.ForceColors || f.isTerminal) && !f.DisableColors - timestampFormat := f.TimestampFormat if timestampFormat == "" { timestampFormat = defaultTimestampFormat } - if isColored { + if f.isColored() { f.printColored(b, entry, keys, timestampFormat) } else { if !f.DisableTimestamp { From eb968b65069b7b415d83ba5a6451718d3f93763e Mon Sep 17 00:00:00 2001 From: David Bariod Date: Thu, 9 Aug 2018 15:00:46 +0200 Subject: [PATCH 2/4] Fix for CLICOLOR_FORCE handling --- text_formatter.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/text_formatter.go b/text_formatter.go index cdf3185..beb628f 100644 --- a/text_formatter.go +++ b/text_formatter.go @@ -37,7 +37,7 @@ type TextFormatter struct { DisableColors bool // Override coloring based on CLICOLOR and CLICOLOR_FORCE. - https://bixense.com/clicolors/ - OverrideColors bool + EnvironmentOverrideColors bool // Disable timestamp logging. useful when output is redirected to logging // system that already adds timestamps. @@ -85,12 +85,12 @@ func (f *TextFormatter) init(entry *Entry) { func (f *TextFormatter) isColored() bool { isColored := f.ForceColors || f.isTerminal - if f.OverrideColors { + if f.EnvironmentOverrideColors { if force, ok := os.LookupEnv("CLICOLOR_FORCE"); ok && force != "0" { isColored = true - } - - if os.Getenv("CLICOLOR") == "0" { + } else if ok && force == "0" { + isColored = false + } else if os.Getenv("CLICOLOR") == "0" { isColored = false } } From cadf2ceaf8580dddc21b34895f5fed8d2c3b2e60 Mon Sep 17 00:00:00 2001 From: David Bariod Date: Thu, 9 Aug 2018 15:01:49 +0200 Subject: [PATCH 3/4] Add unit test for TextFormatter.isColored --- text_formatter_test.go | 214 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 214 insertions(+) diff --git a/text_formatter_test.go b/text_formatter_test.go index 921d052..023f346 100644 --- a/text_formatter_test.go +++ b/text_formatter_test.go @@ -4,6 +4,7 @@ import ( "bytes" "errors" "fmt" + "os" "strings" "testing" "time" @@ -216,5 +217,218 @@ func TestTextFormatterFieldMap(t *testing.T) { "Formatted output doesn't respect FieldMap") } +func TestTextFormatterIsColored(t *testing.T) { + params := []struct { + name string + expectedResult bool + isTerminal bool + disableColor bool + forceColor bool + envColor bool + clicolorIsSet bool + clicolorForceIsSet bool + clicolorVal string + clicolorForceVal string + }{ + // Default values + { + name: "testcase1", + expectedResult: false, + isTerminal: false, + disableColor: false, + forceColor: false, + envColor: false, + clicolorIsSet: false, + clicolorForceIsSet: false, + }, + // Output on terminal + { + name: "testcase2", + expectedResult: true, + isTerminal: true, + disableColor: false, + forceColor: false, + envColor: false, + clicolorIsSet: false, + clicolorForceIsSet: false, + }, + // Output on terminal with color disabled + { + name: "testcase3", + expectedResult: false, + isTerminal: true, + disableColor: true, + forceColor: false, + envColor: false, + clicolorIsSet: false, + clicolorForceIsSet: false, + }, + // Output not on terminal with color disabled + { + name: "testcase4", + expectedResult: false, + isTerminal: false, + disableColor: true, + forceColor: false, + envColor: false, + clicolorIsSet: false, + clicolorForceIsSet: false, + }, + // Output not on terminal with color forced + { + name: "testcase5", + expectedResult: true, + isTerminal: false, + disableColor: false, + forceColor: true, + envColor: false, + clicolorIsSet: false, + clicolorForceIsSet: false, + }, + // Output on terminal with clicolor set to "0" + { + name: "testcase6", + expectedResult: false, + isTerminal: true, + disableColor: false, + forceColor: false, + envColor: true, + clicolorIsSet: true, + clicolorForceIsSet: false, + clicolorVal: "0", + }, + // Output on terminal with clicolor set to "1" + { + name: "testcase7", + expectedResult: true, + isTerminal: true, + disableColor: false, + forceColor: false, + envColor: true, + clicolorIsSet: true, + clicolorForceIsSet: false, + clicolorVal: "1", + }, + // Output not on terminal with clicolor set to "0" + { + name: "testcase8", + expectedResult: false, + isTerminal: false, + disableColor: false, + forceColor: false, + envColor: true, + clicolorIsSet: true, + clicolorForceIsSet: false, + clicolorVal: "0", + }, + // Output not on terminal with clicolor set to "1" + { + name: "testcase9", + expectedResult: false, + isTerminal: false, + disableColor: false, + forceColor: false, + envColor: true, + clicolorIsSet: true, + clicolorForceIsSet: false, + clicolorVal: "1", + }, + // Output not on terminal with clicolor set to "1" and force color + { + name: "testcase10", + expectedResult: true, + isTerminal: false, + disableColor: false, + forceColor: true, + envColor: true, + clicolorIsSet: true, + clicolorForceIsSet: false, + clicolorVal: "1", + }, + // Output not on terminal with clicolor set to "0" and force color + { + name: "testcase11", + expectedResult: false, + isTerminal: false, + disableColor: false, + forceColor: true, + envColor: true, + clicolorIsSet: true, + clicolorForceIsSet: false, + clicolorVal: "0", + }, + // Output not on terminal with clicolor set to "1" + { + name: "testcase12", + expectedResult: false, + isTerminal: false, + disableColor: false, + forceColor: false, + envColor: true, + clicolorIsSet: true, + clicolorForceIsSet: false, + clicolorVal: "1", + }, + // Output not on terminal with clicolor_force set to "1" + { + name: "testcase13", + expectedResult: true, + isTerminal: false, + disableColor: false, + forceColor: false, + envColor: true, + clicolorIsSet: false, + clicolorForceIsSet: true, + clicolorForceVal: "1", + }, + // Output not on terminal with clicolor_force set to "0" + { + name: "testcase14", + expectedResult: false, + isTerminal: false, + disableColor: false, + forceColor: false, + envColor: true, + clicolorIsSet: false, + clicolorForceIsSet: true, + clicolorForceVal: "0", + }, + // Output on terminal with clicolor_force set to "0" + { + name: "testcase14", + expectedResult: false, + isTerminal: true, + disableColor: false, + forceColor: false, + envColor: true, + clicolorIsSet: false, + clicolorForceIsSet: true, + clicolorForceVal: "0", + }, + } + + defer os.Clearenv() + + for _, val := range params { + t.Run("textformatter_"+val.name, func(subT *testing.T) { + tf := TextFormatter{ + isTerminal: val.isTerminal, + DisableColors: val.disableColor, + ForceColors: val.forceColor, + EnvironmentOverrideColors: val.envColor, + } + os.Clearenv() + if val.clicolorIsSet { + os.Setenv("CLICOLOR", val.clicolorVal) + } + if val.clicolorForceIsSet { + os.Setenv("CLICOLOR_FORCE", val.clicolorForceVal) + } + res := tf.isColored() + assert.Equal(subT, val.expectedResult, res) + }) + } +} + // TODO add tests for sorting etc., this requires a parser for the text // formatter output. From b5e6fae4fba49f90e609d6379cc6409fa8f85e2e Mon Sep 17 00:00:00 2001 From: David Bariod Date: Mon, 13 Aug 2018 17:27:32 +0200 Subject: [PATCH 4/4] Cleanup on unit test on isColored --- text_formatter_test.go | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/text_formatter_test.go b/text_formatter_test.go index 023f346..652102d 100644 --- a/text_formatter_test.go +++ b/text_formatter_test.go @@ -357,21 +357,9 @@ func TestTextFormatterIsColored(t *testing.T) { clicolorForceIsSet: false, clicolorVal: "0", }, - // Output not on terminal with clicolor set to "1" - { - name: "testcase12", - expectedResult: false, - isTerminal: false, - disableColor: false, - forceColor: false, - envColor: true, - clicolorIsSet: true, - clicolorForceIsSet: false, - clicolorVal: "1", - }, // Output not on terminal with clicolor_force set to "1" { - name: "testcase13", + name: "testcase12", expectedResult: true, isTerminal: false, disableColor: false, @@ -383,7 +371,7 @@ func TestTextFormatterIsColored(t *testing.T) { }, // Output not on terminal with clicolor_force set to "0" { - name: "testcase14", + name: "testcase13", expectedResult: false, isTerminal: false, disableColor: false, @@ -407,7 +395,12 @@ func TestTextFormatterIsColored(t *testing.T) { }, } - defer os.Clearenv() + cleanenv := func() { + os.Unsetenv("CLICOLOR") + os.Unsetenv("CLICOLOR_FORCE") + } + + defer cleanenv() for _, val := range params { t.Run("textformatter_"+val.name, func(subT *testing.T) { @@ -417,7 +410,7 @@ func TestTextFormatterIsColored(t *testing.T) { ForceColors: val.forceColor, EnvironmentOverrideColors: val.envColor, } - os.Clearenv() + cleanenv() if val.clicolorIsSet { os.Setenv("CLICOLOR", val.clicolorVal) }