remove global re-init hack for Stdout message prints using functions …

…with writers instead

Signed-off-by: Kit Patella <[email protected]>

src/cmd/common/table.go

@@ -45,14 +45,8 @@ func PrintFindings(lintErr *lint.LintError) {
 		}
 
 		// Print table to our OutputWriter
-		// HACK(mkcp): Setting a PTerm global isn't ideal or thread-safe. However, it lets us render even when message
-		// is disabled.
-		lastWriter := *message.PTermWriter.Load()
-		message.InitializePTerm(OutputWriter)
 		message.Notef("Linting package %q at %s", findings[0].PackageNameOverride, packagePathFromUser)
-		message.Table([]string{"Type", "Path", "Message"}, lintData)
-		// Reset pterm output
-		message.InitializePTerm(lastWriter)
+		message.TableWithWriter(OutputWriter, []string{"Type", "Path", "Message"}, lintData)
 	}
 }
 

src/cmd/package.go

@@ -275,14 +275,8 @@ var packageListCmd = &cobra.Command{
 			})
 		}
 
-		// NOTE(mkcp): Renders table with message.
 		header := []string{"Package", "Version", "Components"}
-		// HACK(mkcp): Re-initializing PTerm globally isn't ideal or thread-safe. However, it lets us render even when
-		// message is disabled.
-		lastWriter := *message.PTermWriter.Load()
-		message.InitializePTerm(OutputWriter)
-		message.Table(header, packageData)
-		message.InitializePTerm(lastWriter)
+		message.TableWithWriter(message.OutputWriter, header, packageData)
 
 		// Print out any unmarshalling errors
 		if err != nil {

src/pkg/message/connect.go

@@ -6,7 +6,6 @@ package message
 
 import (
 	"fmt"
-	"os"
 
 	"github.com/zarf-dev/zarf/src/types"
 )
@@ -21,13 +20,8 @@ func PrintConnectStringTable(connectStrings types.ConnectStrings) {
 			connectData = append(connectData, []string{name, connect.Description})
 		}
 
-		// HACK(mkcp): Setting a PTerm global during runtime isn't ideal or thread-safe. However, it lets us render
-		// even when message is disabled.
-		lastWriter := *PTermWriter.Load()
-		InitializePTerm(os.Stdout)
 		// Create the table output with the data
 		header := []string{"Connect Command", "Description"}
-		Table(header, connectData)
-		InitializePTerm(lastWriter)
+		TableWithWriter(OutputWriter, header, connectData)
 	}
 }

src/pkg/message/credentials.go

@@ -55,7 +55,7 @@ func PrintCredentialTable(state *types.ZarfState, componentsToDeploy []types.Dep
 
 	if len(loginData) > 0 {
 		header := []string{"Application", "Username", "Password", "Connect", "Get-Creds Key"}
-		Table(header, loginData)
+		TableWithWriter(OutputWriter, header, loginData)
 	}
 }
 

src/pkg/message/message.go

@@ -9,7 +9,6 @@ import (
 	"io"
 	"os"
 	"strings"
-	"sync/atomic"
 	"time"
 
 	"github.com/defenseunicorns/pkg/helpers/v2"
@@ -34,23 +33,18 @@ const (
 	TermWidth = 100
 )
 
-// NoProgress tracks whether spinner/progress bars show updates.
-var NoProgress bool
-
-// RuleLine creates a line of ━ as wide as the terminal
-var RuleLine = strings.Repeat("━", TermWidth)
-
-// logLevel holds the pterm compatible log level integer
-var logLevel = InfoLevel
-
-// logFile acts as a buffer for logFile generation
-var logFile *PausableWriter
-
-// PTermWriter is an unholy hack that allows us to retrieve the writer passed into InitializePTerm. Under no
-// circumstances should this be considered stable or supported. It's only purpose is so we change the writer to
-// Stdout for specific logs, then back to its original intended value that we set at the start of the command.
-// It should be replaced by something threadsafe as soon as possible. Blame mkcp for this contraption.
-var PTermWriter atomic.Pointer[io.Writer]
+var (
+	// NoProgress tracks whether spinner/progress bars show updates.
+	NoProgress bool
+	// RuleLine creates a line of ━ as wide as the terminal
+	RuleLine = strings.Repeat("━", TermWidth)
+	// OutputWriter provides a default writer to Stdout for user-focused output like tables and yaml
+	OutputWriter = os.Stdout
+	// logLevel holds the pterm compatible log level integer
+	logLevel = InfoLevel
+	// logFile acts as a buffer for logFile generation
+	logFile *PausableWriter
+)
 
 // DebugWriter represents a writer interface that writes to message.Debug
 type DebugWriter struct{}
@@ -80,10 +74,6 @@ func InitializePTerm(w io.Writer) {
 		Text: " •",
 	}
 
-	// HACK(mkcp): See the comments on the var above but this should not be used for anything other than its intended
-	// use and should be removed as soon as possible.
-	PTermWriter.Store(&w)
-
 	pterm.SetDefaultOutput(w)
 }
 
@@ -260,6 +250,11 @@ func Paragraphn(n int, format string, a ...any) string {
 
 // Table prints a padded table containing the specified header and data
 func Table(header []string, data [][]string) {
+	TableWithWriter(nil, header, data)
+}
+
+// TableWithWriter prints a padded table containing the specified header and data to the optional writer.
+func TableWithWriter(writer io.Writer, header []string, data [][]string) {
 	pterm.Println()
 
 	// To avoid side effects make copies of the header and data before adding padding
@@ -282,8 +277,12 @@ func Table(header []string, data [][]string) {
 		table = append(table, pterm.TableData{row}...)
 	}
 
-	//nolint:errcheck // never returns an error
-	pterm.DefaultTable.WithHasHeader().WithData(table).Render()
+	// Use DefaultTable writer if none is provided
+	tPrinter := pterm.DefaultTable
+	if writer != nil {
+		tPrinter.Writer = writer
+	}
+	_ = tPrinter.WithHasHeader().WithData(table).Render() //nolint:errcheck
 }
 
 func debugPrinter(offset int, a ...any) {

src/pkg/utils/yaml.go

@@ -103,13 +103,8 @@ func ColorPrintYAML(data any, hints map[string]string, spaceRootLists bool) erro
 		outputYAML = ansiRegex.ReplaceAllString(outputYAML, "")
 	}
 
-	// HACK(mkcp): Re-initializing a PTerm global isn't ideal or thread-safe. However, it lets us render even when
-	// message is disabled.
-	lastWriter := *message.PTermWriter.Load()
-	message.InitializePTerm(os.Stdout)
-	pterm.Println()
-	pterm.Println(outputYAML)
-	message.InitializePTerm(lastWriter)
+	content := strings.Join([]string{"\n", outputYAML}, "")
+	pterm.Fprintln(message.OutputWriter, content)
 	return nil
 }