From 6a13fc0abfe6dd320866528362491f4a5c29d842 Mon Sep 17 00:00:00 2001
From: William Murphy <willmurphyscode@users.noreply.github.com>
Date: Fri, 15 Nov 2024 09:40:52 -0500
Subject: [PATCH] Fix: still warn on zero verbosity (#68)

* fix: allocate stderr logger if log.Warn needed

Previously, there was an issue where if there was no TTY, and verbosity
was zero, the logger would be initialized with io.Discard instead of
stderr. Log to stderr unless "--quiet" is passed.

Signed-off-by: Will Murphy <willmurphyscode@users.noreply.github.com>

* feat: respect NO_TTY env var when setting up UI

Signed-off-by: Will Murphy <willmurphyscode@users.noreply.github.com>

* test: unit test for warn logging

Signed-off-by: Will Murphy <willmurphyscode@users.noreply.github.com>

---------

Signed-off-by: Will Murphy <willmurphyscode@users.noreply.github.com>
---
 logging.go      | 16 +++++++++++++++-
 logging_test.go | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/logging.go b/logging.go
index d889013..2e17d77 100644
--- a/logging.go
+++ b/logging.go
@@ -41,7 +41,7 @@ func DefaultLogger(clioCfg Config, store redact.Store) (logger.Logger, error) {
 
 	l, err := logrus.New(
 		logrus.Config{
-			EnableConsole: cfg.Verbosity > 0 && !cfg.Quiet,
+			EnableConsole: !cfg.Quiet,
 			FileLocation:  cfg.FileLocation,
 			Level:         cfg.Level,
 			Formatter:     adaptLogFormatter(logrus.DefaultTextFormatter()),
@@ -163,6 +163,10 @@ func (l *LoggingConfig) selectLevel() (logger.Level, error) {
 }
 
 func (l *LoggingConfig) AllowUI(stdin fs.File) bool {
+	if forceNoTTY(os.Getenv("NO_TTY")) {
+		return false
+	}
+
 	pipedInput, err := isPipedInput(stdin)
 	if err != nil || pipedInput {
 		// since we can't tell if there was piped input we assume that there could be to disable the ETUI
@@ -188,6 +192,16 @@ func (l *LoggingConfig) AllowUI(stdin fs.File) bool {
 	return l.Verbosity == 0
 }
 
+func forceNoTTY(noTTYenvVar string) bool {
+	switch strings.ToLower(noTTYenvVar) {
+	case "1":
+		return true
+	case "true":
+		return true
+	}
+	return false
+}
+
 // isPipedInput returns true if there is no input device, which means the user **may** be providing input via a pipe.
 func isPipedInput(stdin fs.File) (bool, error) {
 	if stdin == nil {
diff --git a/logging_test.go b/logging_test.go
index ecb3ee6..0c86857 100644
--- a/logging_test.go
+++ b/logging_test.go
@@ -83,6 +83,19 @@ func Test_newLogger(t *testing.T) {
 				assert.Equal(t, "[0000]  INFO test *******\n", stripAnsi(buf.String()))
 			},
 		},
+		{
+			name: "warn logging is not discarded",
+			cfg:  &LoggingConfig{Level: "warn"},
+			assertLogger: func(log logger.Logger) {
+				c, ok := log.(logger.Controller)
+				require.True(t, ok)
+				out := c.GetOutput()
+				// attempt to cast as *os.File, which will fail if output
+				// has defaulted to io.Discard
+				_, ok = out.(*os.File)
+				require.True(t, ok)
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -258,6 +271,26 @@ func TestLoggingConfig_AllowUI(t *testing.T) {
 	}
 }
 
+func TestForceNoTTY(t *testing.T) {
+	tests := []struct {
+		envVarValue string
+		want        bool
+	}{
+		{"", false},
+		{"0", false},
+		{"false", false},
+		{"other-string", false},
+		{"1", true},
+		{"true", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.envVarValue, func(t *testing.T) {
+			assert.Equal(t, tt.want, forceNoTTY(tt.envVarValue))
+		})
+	}
+}
+
 func Test_isPipedInput(t *testing.T) {
 
 	tests := []struct {