Rework error and debug handling

cmd/platform/main.go

@@ -1,11 +1,10 @@
 package main
 
 import (
-	"log"
+	"fmt"
 	"os"
 	"strings"
 
-	"github.com/fatih/color"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 
@@ -14,16 +13,16 @@ import (
 )
 
 func main() {
-	log.SetOutput(color.Error)
-
 	// Load configuration.
 	cnfYAML, err := config.LoadYAML()
 	if err != nil {
-		log.Fatal(err)
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
 	}
 	cnf, err := config.FromYAML(cnfYAML)
 	if err != nil {
-		log.Fatal(err)
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
 	}
 
 	// When Cobra starts, load Viper config from the environment.

commands/completion.go

@@ -9,7 +9,6 @@ import (
 	"path"
 	"strings"
 
-	"github.com/fatih/color"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 
@@ -19,9 +18,10 @@ import (
 
 func newCompletionCommand(cnf *config.Config) *cobra.Command {
 	return &cobra.Command{
-		Use:   "completion",
-		Short: "Print the completion script for your shell",
-		Args:  cobra.MaximumNArgs(1),
+		Use:           "completion",
+		Short:         "Print the completion script for your shell",
+		Args:          cobra.MaximumNArgs(1),
+		SilenceErrors: true,
 		Run: func(cmd *cobra.Command, args []string) {
 			completionArgs := []string{"_completion", "-g", "--program", cnf.Application.Executable}
 			if len(args) > 0 {
@@ -33,20 +33,21 @@ func newCompletionCommand(cnf *config.Config) *cobra.Command {
 				Version:            version,
 				CustomPharPath:     viper.GetString("phar-path"),
 				Debug:              viper.GetBool("debug"),
+				DebugLogFunc:       debugLog,
 				DisableInteraction: viper.GetBool("no-interaction"),
 				Stdout:             &b,
 				Stderr:             cmd.ErrOrStderr(),
 				Stdin:              cmd.InOrStdin(),
 			}
 
 			if err := c.Init(); err != nil {
-				debugLog("%s\n", color.RedString(err.Error()))
+				debugLog(err.Error())
 				os.Exit(1)
 				return
 			}
 
 			if err := c.Exec(cmd.Context(), completionArgs...); err != nil {
-				debugLog("%s\n", color.RedString(err.Error()))
+				debugLog(err.Error())
 				exitCode := 1
 				var execErr *exec.ExitError
 				if errors.As(err, &execErr) {

commands/list.go

@@ -3,12 +3,8 @@ package commands
 import (
 	"bytes"
 	"encoding/json"
-	"errors"
 	"fmt"
-	"os"
-	"os/exec"
 
-	"github.com/fatih/color"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 
@@ -28,14 +24,14 @@ func newListCommand(cnf *config.Config) *cobra.Command {
 				Version:            version,
 				CustomPharPath:     viper.GetString("phar-path"),
 				Debug:              viper.GetBool("debug"),
+				DebugLogFunc:       debugLog,
 				DisableInteraction: viper.GetBool("no-interaction"),
 				Stdout:             &b,
 				Stderr:             cmd.ErrOrStderr(),
 				Stdin:              cmd.InOrStdin(),
 			}
 			if err := c.Init(); err != nil {
-				debugLog("%s\n", color.RedString(err.Error()))
-				os.Exit(1)
+				exitWithError(cmd, err)
 				return
 			}
 
@@ -47,20 +43,13 @@ func newListCommand(cnf *config.Config) *cobra.Command {
 				arguments = append(arguments, args[0])
 			}
 			if err := c.Exec(cmd.Context(), arguments...); err != nil {
-				debugLog("%s\n", color.RedString(err.Error()))
-				exitCode := 1
-				var execErr *exec.ExitError
-				if errors.As(err, &execErr) {
-					exitCode = execErr.ExitCode()
-				}
-				os.Exit(exitCode)
+				exitWithError(cmd, err)
 				return
 			}
 
 			var list List
 			if err := json.Unmarshal(b.Bytes(), &list); err != nil {
-				debugLog("%s\n", color.RedString(err.Error()))
-				os.Exit(1)
+				exitWithError(cmd, err)
 				return
 			}
 
@@ -99,21 +88,14 @@ func newListCommand(cnf *config.Config) *cobra.Command {
 				c.Stdout = cmd.OutOrStdout()
 				arguments := []string{"list", "--format=" + format}
 				if err := c.Exec(cmd.Context(), arguments...); err != nil {
-					debugLog("%s\n", color.RedString(err.Error()))
-					exitCode := 1
-					var execErr *exec.ExitError
-					if errors.As(err, &execErr) {
-						exitCode = execErr.ExitCode()
-					}
-					os.Exit(exitCode)
+					exitWithError(cmd, err)
 				}
 				return
 			}
 
 			result, err := formatter.Format(&list, config.FromContext(cmd.Context()))
 			if err != nil {
-				debugLog("%s\n", color.RedString(err.Error()))
-				os.Exit(1)
+				exitWithError(cmd, err)
 				return
 			}
 

commands/root.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"log"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -51,11 +50,19 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob
 		DisableFlagParsing: false,
 		FParseErrWhitelist: cobra.FParseErrWhitelist{UnknownFlags: true},
 		SilenceUsage:       true,
-		SilenceErrors:      true,
+		SilenceErrors:      false,
 		PersistentPreRun: func(cmd *cobra.Command, _ []string) {
 			if viper.GetBool("quiet") && !viper.GetBool("debug") && !viper.GetBool("verbose") {
 				viper.Set("no-interaction", true)
 				cmd.SetErr(io.Discard)
+			} else {
+				// Ensure the command's output writers can handle colors.
+				if cmd.OutOrStdout() == os.Stdout {
+					cmd.SetOut(color.Output)
+				}
+				if cmd.ErrOrStderr() == os.Stderr {
+					cmd.SetErr(color.Error)
+				}
 			}
 			if viper.GetBool("version") {
 				versionCommand.Run(cmd, []string{})
@@ -71,11 +78,11 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob
 		Run: func(cmd *cobra.Command, _ []string) {
 			runLegacyCLI(cmd.Context(), cnf, cmd.OutOrStdout(), cmd.ErrOrStderr(), cmd.InOrStdin(), os.Args[1:])
 		},
-		PersistentPostRun: func(_ *cobra.Command, _ []string) {
-			checkShellConfigLeftovers(cnf)
+		PersistentPostRun: func(cmd *cobra.Command, _ []string) {
+			checkShellConfigLeftovers(cmd.ErrOrStderr(), cnf)
 			select {
 			case rel := <-updateMessageChan:
-				printUpdateMessage(rel, cnf)
+				printUpdateMessage(cmd.ErrOrStderr(), rel, cnf)
 			default:
 			}
 		},
@@ -142,7 +149,7 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob
 }
 
 // checkShellConfigLeftovers checks .zshrc and .bashrc for any leftovers from the legacy CLI
-func checkShellConfigLeftovers(cnf *config.Config) {
+func checkShellConfigLeftovers(w io.Writer, cnf *config.Config) {
 	start := fmt.Sprintf("# BEGIN SNIPPET: %s configuration", cnf.Application.Name)
 	end := "# END SNIPPET"
 	shellConfigSnippet := regexp.MustCompile(regexp.QuoteMeta(start) + "(?s).+?" + regexp.QuoteMeta(end))
@@ -168,23 +175,23 @@ func checkShellConfigLeftovers(cnf *config.Config) {
 		}
 
 		if shellConfigSnippet.Match(shellConfig) {
-			fmt.Fprintf(color.Error, "%s Your %s file contains code that is no longer needed for the New %s\n",
-				color.YellowString("Warning:"),
+			fmt.Fprintf(w, "%s Your %s file contains code that is no longer needed for the New %s\n",
+				color.YellowString("Notice:"),
 				shellConfigFile,
 				cnf.Application.Name,
 			)
-			fmt.Fprintf(color.Error, "%s %s\n", color.YellowString("Please remove the following lines from:"), shellConfigFile)
-			fmt.Fprintf(color.Error, "\t%s\n", strings.ReplaceAll(string(shellConfigSnippet.Find(shellConfig)), "\n", "\n\t"))
+			fmt.Fprintf(w, "%s %s\n", color.YellowString("Please remove the following lines from:"), shellConfigFile)
+			fmt.Fprintf(w, "\t%s\n", strings.ReplaceAll(string(shellConfigSnippet.Find(shellConfig)), "\n", "\n\t"))
 		}
 	}
 }
 
-func printUpdateMessage(newRelease *internal.ReleaseInfo, cnf *config.Config) {
+func printUpdateMessage(w io.Writer, newRelease *internal.ReleaseInfo, cnf *config.Config) {
 	if newRelease == nil {
 		return
 	}
 
-	fmt.Fprintf(color.Error, "\n\n%s %s → %s\n",
+	fmt.Fprintf(w, "\n\n%s %s → %s\n",
 		color.YellowString(fmt.Sprintf("A new release of the %s is available:", cnf.Application.Name)),
 		color.CyanString(version),
 		color.CyanString(newRelease.Version),
@@ -193,19 +200,19 @@ func printUpdateMessage(newRelease *internal.ReleaseInfo, cnf *config.Config) {
 	executable, err := os.Executable()
 	if err == nil && cnf.Wrapper.HomebrewTap != "" && isUnderHomebrew(executable) {
 		fmt.Fprintf(
-			color.Error,
+			w,
 			"To upgrade, run: brew update && brew upgrade %s\n",
 			color.YellowString(cnf.Wrapper.HomebrewTap),
 		)
 	} else if cnf.Wrapper.GitHubRepo != "" {
 		fmt.Fprintf(
-			color.Error,
+			w,
 			"To upgrade, follow the instructions at: https://github.com/%s#upgrade\n",
 			cnf.Wrapper.GitHubRepo,
 		)
 	}
 
-	fmt.Fprintf(color.Error, "%s\n\n", color.YellowString(newRelease.URL))
+	fmt.Fprintf(w, "%s\n\n", color.YellowString(newRelease.URL))
 }
 
 func isUnderHomebrew(binary string) bool {
@@ -228,7 +235,18 @@ func debugLog(format string, v ...any) {
 		return
 	}
 
-	log.Printf(format, v...)
+	prefix := color.New(color.ReverseVideo).Sprintf("DEBUG")
+	fmt.Fprintf(color.Error, prefix+" "+strings.TrimSpace(format)+"\n", v...)
+}
+
+func exitWithError(cmd *cobra.Command, err error) {
+	cmd.PrintErrln(color.RedString(err.Error()))
+	exitCode := 1
+	var execErr *exec.ExitError
+	if errors.As(err, &execErr) {
+		exitCode = execErr.ExitCode()
+	}
+	os.Exit(exitCode)
 }
 
 func runLegacyCLI(ctx context.Context, cnf *config.Config, stdout, stderr io.Writer, stdin io.Reader, args []string) {
@@ -237,6 +255,7 @@ func runLegacyCLI(ctx context.Context, cnf *config.Config, stdout, stderr io.Wri
 		Version:            version,
 		CustomPharPath:     viper.GetString("phar-path"),
 		Debug:              viper.GetBool("debug"),
+		DebugLogFunc:       debugLog,
 		DisableInteraction: viper.GetBool("no-interaction"),
 		Stdout:             stdout,
 		Stderr:             stderr,

internal/legacy/legacy.go

@@ -6,7 +6,6 @@ import (
 	_ "embed"
 	"fmt"
 	"io"
-	"log"
 	"os"
 	"os/exec"
 	"path"
@@ -77,6 +76,13 @@ type CLIWrapper struct {
 	CustomPharPath     string
 	Debug              bool
 	DisableInteraction bool
+	DebugLogFunc       func(string, ...any)
+}
+
+func (c *CLIWrapper) debug(msg string, args ...any) {
+	if c.DebugLogFunc != nil {
+		c.DebugLogFunc(msg, args...)
+	}
 }
 
 func (c *CLIWrapper) cacheDir() string {
@@ -86,7 +92,7 @@ func (c *CLIWrapper) cacheDir() string {
 // Init the CLI wrapper, creating a temporary directory and copying over files
 func (c *CLIWrapper) Init() error {
 	if _, err := os.Stat(c.cacheDir()); os.IsNotExist(err) {
-		c.debugLog("cache directory does not exist, creating: %s", c.cacheDir())
+		c.debug("Cache directory does not exist, creating: %s", c.cacheDir())
 		if err := os.Mkdir(c.cacheDir(), 0o700); err != nil {
 			return fmt.Errorf("could not create temporary directory: %w", err)
 		}
@@ -95,7 +101,7 @@ func (c *CLIWrapper) Init() error {
 	if err := fileLock.Lock(); err != nil {
 		return fmt.Errorf("could not acquire lock: %w", err)
 	}
-	c.debugLog("lock acquired: %s", fileLock.Path())
+	c.debug("Lock acquired: %s", fileLock.Path())
 	//nolint:errcheck
 	defer fileLock.Unlock()
 
@@ -104,7 +110,7 @@ func (c *CLIWrapper) Init() error {
 			return fmt.Errorf("legacy CLI phar file not found: %w", err)
 		}
 
-		c.debugLog("phar file does not exist, copying: %s", c.PharPath())
+		c.debug("Phar file does not exist, copying: %s", c.PharPath())
 		if err := copyFile(c.PharPath(), phar); err != nil {
 			return fmt.Errorf("could not copy phar file: %w", err)
 		}
@@ -126,7 +132,7 @@ func (c *CLIWrapper) Init() error {
 	}
 
 	if _, err := os.Stat(c.PHPPath()); os.IsNotExist(err) {
-		c.debugLog("PHP binary does not exist, copying: %s", c.PHPPath())
+		c.debug("PHP binary does not exist, copying: %s", c.PHPPath())
 		if err := c.copyPHP(); err != nil {
 			return fmt.Errorf("could not copy files: %w", err)
 		}
@@ -182,7 +188,7 @@ func (c *CLIWrapper) Exec(ctx context.Context, args ...string) error {
 	))
 	if err := cmd.Run(); err != nil {
 		// Cleanup cache directory
-		c.debugLog("removing cache directory: %s", c.cacheDir())
+		c.debug("Removing cache directory: %s", c.cacheDir())
 		os.RemoveAll(c.cacheDir())
 		return fmt.Errorf("could not run legacy CLI command: %w", err)
 	}
@@ -215,10 +221,3 @@ func (c *CLIWrapper) PharPath() string {
 func (c *CLIWrapper) ConfigPath() string {
 	return path.Join(c.cacheDir(), "config.yaml")
 }
-
-// debugLog logs a debugging message, if debug is enabled
-func (c *CLIWrapper) debugLog(msg string, v ...any) {
-	if c.Debug {
-		log.Printf(msg, v...)
-	}
-}