From fa0d2a82ff4d29942688ebd241548a6daa971b09 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 5 Jun 2019 00:10:46 -0700 Subject: [PATCH 03/13] add implementation and tests --- text_formatter.go | 9 ++++- text_formatter_test.go | 88 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/text_formatter.go b/text_formatter.go index e01587c..fb13499 100644 --- a/text_formatter.go +++ b/text_formatter.go @@ -57,6 +57,10 @@ type TextFormatter struct { // Disables the truncation of the level text to 4 characters. DisableLevelTruncation bool + // PadLevelText Adds padding the level text so that all the levels output at the same length + // PadLevelText is a superset of the DisableLevelTruncation option + PadLevelText bool + // QuoteEmptyFields will wrap empty fields in quotes if true QuoteEmptyFields bool @@ -217,9 +221,12 @@ func (f *TextFormatter) printColored(b *bytes.Buffer, entry *Entry, keys []strin } levelText := strings.ToUpper(entry.Level.String()) - if !f.DisableLevelTruncation { + if !f.DisableLevelTruncation && !f.PadLevelText { levelText = levelText[0:4] } + if f.PadLevelText { + levelText = fmt.Sprintf("%-7s", levelText) + } // Remove a single newline if it already exists in the message to keep // the behavior of logrus text_formatter the same as the stdlib log package diff --git a/text_formatter_test.go b/text_formatter_test.go index 9c5e6f0..e48e430 100644 --- a/text_formatter_test.go +++ b/text_formatter_test.go @@ -172,6 +172,94 @@ func TestDisableLevelTruncation(t *testing.T) { checkDisableTruncation(false, InfoLevel) } +func TestPadLevelText(t *testing.T) { + // A note for future maintainers / committers: + // + // This test denormalizes the level text as a part of its assertions. + // Because of that, its not really a "unit test" of the PadLevelText functionality. + // So! Many apologies to the potential future person who has to rewrite this test + // when they are changing some completely unrelated functionality. + params := []struct { + name string + level Level + paddedLevelText string + }{ + { + name: "PanicLevel", + level: PanicLevel, + paddedLevelText: "PANIC ", // 2 extra spaces + }, + { + name: "FatalLevel", + level: FatalLevel, + paddedLevelText: "FATAL ", // 2 extra spaces + }, + { + name: "ErrorLevel", + level: ErrorLevel, + paddedLevelText: "ERROR ", // 2 extra spaces + }, + { + name: "WarnLevel", + level: WarnLevel, + // WARNING is already the max length, so we don't need to assert a paddedLevelText + }, + { + name: "DebugLevel", + level: DebugLevel, + paddedLevelText: "DEBUG ", // 2 extra spaces + }, + { + name: "TraceLevel", + level: TraceLevel, + paddedLevelText: "TRACE ", // 2 extra spaces + }, + { + name: "InfoLevel", + level: InfoLevel, + paddedLevelText: "INFO ", // 3 extra spaces + }, + } + + // We create a "default" TextFormatter to do a control case test + // and a TextFormatter with PadLevelText, which is the flag we are testing here + tfDefault := TextFormatter{} + tfWithPadding := TextFormatter{PadLevelText: true} + + for _, val := range params { + t.Run(val.name, func(t *testing.T) { + // TextFormatter writes into these bytes.Buffers, and we make assertions about their contents later + var bytesDefault bytes.Buffer + var bytesWithPadding bytes.Buffer + + // The TextFormatter instance and the bytes.Buffer instance are different here + // all the other arguments are the same + tfDefault.printColored(&bytesDefault, &Entry{Level: val.level}, []string{}, nil, "") + tfWithPadding.printColored(&bytesWithPadding, &Entry{Level: val.level}, []string{}, nil, "") + + // turn the bytes back into a string so that we can actually work with the data + logLineDefault := (&bytesDefault).String() + logLineWithPadding := (&bytesWithPadding).String() + + // Control: the level text should not be padded by default + if val.paddedLevelText != "" && strings.Contains(logLineDefault, val.paddedLevelText) { + t.Errorf("log line \"%s\" should not contain the padded level text \"%s\" by default", logLineDefault, val.paddedLevelText) + } + + // Assertion: the level text should still contain the string representation of the level + if !strings.Contains(strings.ToLower(logLineWithPadding), val.level.String()) { + t.Errorf("log line \"%s\" should contain the level text \"%s\" when padding is enabled", logLineWithPadding, val.level.String()) + } + + // Assertion: the level text should be in its padded form now + if val.paddedLevelText != "" && !strings.Contains(logLineWithPadding, val.paddedLevelText) { + t.Errorf("log line \"%s\" should contain the padded level text \"%s\" when padding is enabled", logLineWithPadding, val.paddedLevelText) + } + + }) + } +} + func TestDisableTimestampWithColoredOutput(t *testing.T) { tf := &TextFormatter{DisableTimestamp: true, ForceColors: true} From 58f7e00129989b7799aa4ed9097f98383bb579ac Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 5 Jun 2019 00:27:10 -0700 Subject: [PATCH 04/13] update comments --- text_formatter_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text_formatter_test.go b/text_formatter_test.go index e48e430..775f4aa 100644 --- a/text_formatter_test.go +++ b/text_formatter_test.go @@ -221,8 +221,8 @@ func TestPadLevelText(t *testing.T) { }, } - // We create a "default" TextFormatter to do a control case test - // and a TextFormatter with PadLevelText, which is the flag we are testing here + // We create a "default" TextFormatter to do a control test. + // We also create a TextFormatter with PadLevelText, which is the parameter we want to do our most relevant assertions against. tfDefault := TextFormatter{} tfWithPadding := TextFormatter{PadLevelText: true} From 2d641d1668b752bae18eff57cbaa4f04f14236b2 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 5 Jun 2019 00:33:48 -0700 Subject: [PATCH 05/13] update the readme --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 2040b42..48d64a0 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,7 @@ time="2015-03-26T01:27:38-04:00" level=fatal method=github.com/sirupsen/arcticcr ``` Note that this does add measurable overhead - the cost will depend on the version of Go, but is between 20 and 40% in recent tests with 1.6 and 1.7. You can validate this in your -environment via benchmarks: +environment via benchmarks: ``` go test -bench=.*CallerTracing ``` @@ -354,6 +354,7 @@ The built-in logging formatters are: [github.com/mattn/go-colorable](https://github.com/mattn/go-colorable). * When colors are enabled, levels are truncated to 4 characters by default. To disable truncation set the `DisableLevelTruncation` field to `true`. + * When outputting to a TTY, its often helpful to visually scan down a column where all the levels are the same width. Setting the `PadLevelText` field to `true` enables this functionality, by adding padding to the level text. * All options are listed in the [generated docs](https://godoc.org/github.com/sirupsen/logrus#TextFormatter). * `logrus.JSONFormatter`. Logs fields as JSON. * All options are listed in the [generated docs](https://godoc.org/github.com/sirupsen/logrus#JSONFormatter). From 691f1b6074511f5bcf95dda280c8da89b6a7b3da Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 5 Jun 2019 00:36:14 -0700 Subject: [PATCH 06/13] add a space back --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 48d64a0..c5cd0e4 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,7 @@ time="2015-03-26T01:27:38-04:00" level=fatal method=github.com/sirupsen/arcticcr ``` Note that this does add measurable overhead - the cost will depend on the version of Go, but is between 20 and 40% in recent tests with 1.6 and 1.7. You can validate this in your -environment via benchmarks: +environment via benchmarks: ``` go test -bench=.*CallerTracing ``` From bef31a5df9d1b50314d7611e392c706d6e6d695b Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 5 Jun 2019 00:41:05 -0700 Subject: [PATCH 07/13] wording shift --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c5cd0e4..08c3269 100644 --- a/README.md +++ b/README.md @@ -354,7 +354,7 @@ The built-in logging formatters are: [github.com/mattn/go-colorable](https://github.com/mattn/go-colorable). * When colors are enabled, levels are truncated to 4 characters by default. To disable truncation set the `DisableLevelTruncation` field to `true`. - * When outputting to a TTY, its often helpful to visually scan down a column where all the levels are the same width. Setting the `PadLevelText` field to `true` enables this functionality, by adding padding to the level text. + * When outputting to a TTY, its often helpful to visually scan down a column where all the levels are the same width. Setting the `PadLevelText` field to `true` enables this behavior, by adding padding to the level text. * All options are listed in the [generated docs](https://godoc.org/github.com/sirupsen/logrus#TextFormatter). * `logrus.JSONFormatter`. Logs fields as JSON. * All options are listed in the [generated docs](https://godoc.org/github.com/sirupsen/logrus#JSONFormatter). From 693469de8f5d1e17ff6f3d8b479f36e5f97c5b7d Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Mon, 24 Jun 2019 20:42:20 -0700 Subject: [PATCH 09/13] dynamically space the level text --- text_formatter.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/text_formatter.go b/text_formatter.go index fb13499..a4b65e8 100644 --- a/text_formatter.go +++ b/text_formatter.go @@ -6,6 +6,7 @@ import ( "os" "runtime" "sort" + "strconv" "strings" "sync" "time" @@ -83,12 +84,22 @@ type TextFormatter struct { CallerPrettyfier func(*runtime.Frame) (function string, file string) terminalInitOnce sync.Once + + // The max length of the level text, generated dynamically on init + levelTextMaxLength int } func (f *TextFormatter) init(entry *Entry) { if entry.Logger != nil { f.isTerminal = checkIfTerminal(entry.Logger.Out) } + // Get the max length of the level text + for _, level := range AllLevels { + levelTextLength := len(level.String()) + if levelTextLength > f.levelTextMaxLength { + f.levelTextMaxLength = levelTextLength + } + } } func (f *TextFormatter) isColored() bool { @@ -225,7 +236,13 @@ func (f *TextFormatter) printColored(b *bytes.Buffer, entry *Entry, keys []strin levelText = levelText[0:4] } if f.PadLevelText { - levelText = fmt.Sprintf("%-7s", levelText) + // Generates the format string used in the next line, for example "%-6s" or "%-7s". + // Based on the max level text length. + formatString := "%-" + strconv.Itoa(f.levelTextMaxLength) + "s" + // Formats the level text by appending spaces up to the max length, for example: + // - "INFO " + // - "WARNING" + levelText = fmt.Sprintf(formatString, levelText) } // Remove a single newline if it already exists in the message to keep From 8ba442aca65c8333a2c78c566e064cc0e8ad1f88 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Mon, 24 Jun 2019 20:50:37 -0700 Subject: [PATCH 10/13] init the loggers in tests --- text_formatter_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/text_formatter_test.go b/text_formatter_test.go index 775f4aa..d04fbcd 100644 --- a/text_formatter_test.go +++ b/text_formatter_test.go @@ -233,8 +233,11 @@ func TestPadLevelText(t *testing.T) { var bytesWithPadding bytes.Buffer // The TextFormatter instance and the bytes.Buffer instance are different here - // all the other arguments are the same + // all the other arguments are the same. We also initialize them so that they + // fill in the value of levelTextMaxLength. + tfDefault.init(&Entry{}) tfDefault.printColored(&bytesDefault, &Entry{Level: val.level}, []string{}, nil, "") + tfWithPadding.init(&Entry{}) tfWithPadding.printColored(&bytesWithPadding, &Entry{Level: val.level}, []string{}, nil, "") // turn the bytes back into a string so that we can actually work with the data From dcce32597dee9bd34880bf1e7a42a328eaec13d1 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 26 Jun 2019 20:37:17 -0700 Subject: [PATCH 11/13] avoid escapes! h/t @therealplato --- text_formatter_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/text_formatter_test.go b/text_formatter_test.go index d04fbcd..99b5d8c 100644 --- a/text_formatter_test.go +++ b/text_formatter_test.go @@ -246,17 +246,17 @@ func TestPadLevelText(t *testing.T) { // Control: the level text should not be padded by default if val.paddedLevelText != "" && strings.Contains(logLineDefault, val.paddedLevelText) { - t.Errorf("log line \"%s\" should not contain the padded level text \"%s\" by default", logLineDefault, val.paddedLevelText) + t.Errorf("log line %q should not contain the padded level text %q by default", logLineDefault, val.paddedLevelText) } // Assertion: the level text should still contain the string representation of the level if !strings.Contains(strings.ToLower(logLineWithPadding), val.level.String()) { - t.Errorf("log line \"%s\" should contain the level text \"%s\" when padding is enabled", logLineWithPadding, val.level.String()) + t.Errorf("log line %q should contain the level text %q when padding is enabled", logLineWithPadding, val.level.String()) } // Assertion: the level text should be in its padded form now if val.paddedLevelText != "" && !strings.Contains(logLineWithPadding, val.paddedLevelText) { - t.Errorf("log line \"%s\" should contain the padded level text \"%s\" when padding is enabled", logLineWithPadding, val.paddedLevelText) + t.Errorf("log line %q should contain the padded level text %q when padding is enabled", logLineWithPadding, val.paddedLevelText) } }) From af6ed964ef540511b4982e50cee85e6fc9715763 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 26 Jun 2019 20:48:04 -0700 Subject: [PATCH 12/13] len => RuneCount note: this is not currently easily testable without a larger diff that refactors the levels --- text_formatter.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/text_formatter.go b/text_formatter.go index a4b65e8..f08563e 100644 --- a/text_formatter.go +++ b/text_formatter.go @@ -10,6 +10,7 @@ import ( "strings" "sync" "time" + "unicode/utf8" ) const ( @@ -95,7 +96,7 @@ func (f *TextFormatter) init(entry *Entry) { } // Get the max length of the level text for _, level := range AllLevels { - levelTextLength := len(level.String()) + levelTextLength := utf8.RuneCount([]byte(level.String())) if levelTextLength > f.levelTextMaxLength { f.levelTextMaxLength = levelTextLength } From 539b8af839f189b659e6c7f62b4524a460fbae20 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Thu, 27 Jun 2019 18:35:00 -0700 Subject: [PATCH 13/13] its => it's --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 08c3269..e7450a8 100644 --- a/README.md +++ b/README.md @@ -354,7 +354,7 @@ The built-in logging formatters are: [github.com/mattn/go-colorable](https://github.com/mattn/go-colorable). * When colors are enabled, levels are truncated to 4 characters by default. To disable truncation set the `DisableLevelTruncation` field to `true`. - * When outputting to a TTY, its often helpful to visually scan down a column where all the levels are the same width. Setting the `PadLevelText` field to `true` enables this behavior, by adding padding to the level text. + * When outputting to a TTY, it's often helpful to visually scan down a column where all the levels are the same width. Setting the `PadLevelText` field to `true` enables this behavior, by adding padding to the level text. * All options are listed in the [generated docs](https://godoc.org/github.com/sirupsen/logrus#TextFormatter). * `logrus.JSONFormatter`. Logs fields as JSON. * All options are listed in the [generated docs](https://godoc.org/github.com/sirupsen/logrus#JSONFormatter).