Fix cli output to remove control chars

Author: akclace

cmd/openrun/app_cmds.go

@@ -16,11 +16,12 @@ import (
 	"github.com/urfave/cli/v2"
 )
 
+var DRY_RUN_MESSAGE = "\n" + YELLOW + "*** dry-run mode, changes have NOT been committed. ***" + RESET + "\n"
+
 const (
-	DRY_RUN_FLAG    = "dry-run"
-	DRY_RUN_ARG     = "dryRun"
-	DRY_RUN_MESSAGE = "\n" + YELLOW + "*** dry-run mode, changes have NOT been committed. ***" + RESET + "\n"
-	PATH_SPEC_HELP  = `The (optional) domain and path are separated by a ":". appPathGlob supports a glob pattern.
+	DRY_RUN_FLAG   = "dry-run"
+	DRY_RUN_ARG    = "dryRun"
+	PATH_SPEC_HELP = `The (optional) domain and path are separated by a ":". appPathGlob supports a glob pattern.
 In the glob, * matches any number of characters, ** matches any number of characters including /.
 all is a shortcut for "*:**", which matches all apps across all domains, including no domain.
 To prevent shell expansion for *, placing the path in quotes is recommended.
@@ -259,7 +260,7 @@ func appListCommand(commonFlags []cli.Flag, clientConfig *types.ClientConfig) *c
 	flags := make([]cli.Flag, 0, len(commonFlags)+2)
 	flags = append(flags, commonFlags...)
 	flags = append(flags, newBoolFlag("internal", "i", "Include internal apps", false))
-	flags = append(flags, newStringFlag("format", "f", "The display format. Valid options are table, basic, csv, json, jsonl and jsonl_pretty", ""))
+	flags = append(flags, newFormatFlag())
 
 	return &cli.Command{
 		Name:      "list",

cmd/openrun/bindings.go

@@ -225,7 +225,7 @@ Examples:
 func bindingGetCommand(commonFlags []cli.Flag, clientConfig *types.ClientConfig) *cli.Command {
 	flags := make([]cli.Flag, 0, len(commonFlags)+1)
 	flags = append(flags, commonFlags...)
-	flags = append(flags, newStringFlag("format", "f", "The display format. Valid options are table, basic, csv, json, jsonl and jsonl_pretty", ""))
+	flags = append(flags, newFormatFlag())
 
 	return &cli.Command{
 		Name:      "get",
@@ -306,7 +306,7 @@ func bindingListCommand(commonFlags []cli.Flag, clientConfig *types.ClientConfig
 	flags := make([]cli.Flag, 0, len(commonFlags)+2)
 	flags = append(flags, commonFlags...)
 	flags = append(flags, newStringFlag(SOURCE_FLAG, "s", "Filter bindings by source", ""))
-	flags = append(flags, newStringFlag("format", "f", "The display format. Valid options are table, basic, csv, json, jsonl and jsonl_pretty", ""))
+	flags = append(flags, newFormatFlag())
 
 	return &cli.Command{
 		Name:  "list",

cmd/openrun/color_unix.go

@@ -0,0 +1,12 @@
+// Copyright (c) ClaceIO, LLC
+// SPDX-License-Identifier: Apache-2.0
+
+//go:build !windows
+
+package main
+
+// enableVirtualTerminal is a no-op on non-Windows platforms, the terminal
+// handles ANSI escape sequences when the output is a tty
+func enableVirtualTerminal() bool {
+	return true
+}

cmd/openrun/color_windows.go

@@ -0,0 +1,32 @@
+// Copyright (c) ClaceIO, LLC
+// SPDX-License-Identifier: Apache-2.0
+
+//go:build windows
+
+package main
+
+import (
+	"os"
+
+	"golang.org/x/sys/windows"
+)
+
+// enableVirtualTerminal enables ANSI escape sequence processing on the
+// consoles attached to stdout and stderr. Returns false if the console does
+// not support it, in which case colored output is disabled
+func enableVirtualTerminal() bool {
+	for _, f := range []*os.File{os.Stdout, os.Stderr} {
+		handle := windows.Handle(f.Fd())
+		var mode uint32
+		if err := windows.GetConsoleMode(handle, &mode); err != nil {
+			return false
+		}
+		if mode&windows.ENABLE_VIRTUAL_TERMINAL_PROCESSING != 0 {
+			continue
+		}
+		if err := windows.SetConsoleMode(handle, mode|windows.ENABLE_VIRTUAL_TERMINAL_PROCESSING); err != nil {
+			return false
+		}
+	}
+	return true
+}

cmd/openrun/flags.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 
 	"github.com/openrundev/openrun/internal/system"
@@ -22,13 +23,54 @@ const (
 	FORMAT_JSONL_PRETTY = "jsonl_pretty"
 	FORMAT_CSV          = "csv"
 )
-const (
-	//Terminal colors
-	RESET  = "\033[0m"
-	RED    = "\033[31m"
-	GREEN  = "\033[32m"
-	YELLOW = "\033[33m"
-)
+
+var validFormats = []string{FORMAT_TABLE, FORMAT_BASIC, FORMAT_CSV, FORMAT_JSON, FORMAT_JSONL, FORMAT_JSONL_PRETTY}
+
+// newFormatFlag creates the output format flag, validating the value at parse
+// time so an invalid format is reported as an error instead of a panic
+func newFormatFlag() *cli.StringFlag {
+	return &cli.StringFlag{
+		Name:    "format",
+		Aliases: []string{"f"},
+		Usage:   "The display format. Valid options are table, basic, csv, json, jsonl and jsonl_pretty",
+		Action: func(_ *cli.Context, value string) error {
+			if !slices.Contains(validFormats, value) {
+				return fmt.Errorf("invalid format %q: valid options are %s", value, strings.Join(validFormats, ", "))
+			}
+			return nil
+		},
+	}
+}
+// Terminal colors, empty strings when the terminal does not support ANSI escape sequences
+var RESET, RED, GREEN, YELLOW = initColors()
+
+func initColors() (string, string, string, string) {
+	if colorsSupported() {
+		return "\033[0m", "\033[31m", "\033[32m", "\033[33m"
+	}
+	return "", "", "", ""
+}
+
+// colorsSupported reports whether colored output should be used: disabled if
+// NO_COLOR is set, TERM is dumb, stdout/stderr are not terminals, or the
+// platform cannot process ANSI escape sequences
+func colorsSupported() bool {
+	if _, ok := os.LookupEnv("NO_COLOR"); ok {
+		return false
+	}
+	if os.Getenv("TERM") == "dumb" {
+		return false
+	}
+	return isTerminal(os.Stdout) && isTerminal(os.Stderr) && enableVirtualTerminal()
+}
+
+func isTerminal(f *os.File) bool {
+	info, err := f.Stat()
+	if err != nil {
+		return false
+	}
+	return info.Mode()&os.ModeCharDevice != 0
+}
 
 func newStringFlag(name, alias, usage, value string) *cli.StringFlag {
 	var aliases []string

cmd/openrun/main.go

@@ -5,11 +5,11 @@ package main
 
 import (
 	"fmt"
-	"log"
 	"os"
 	"path"
 	"path/filepath"
 	"runtime"
+	"slices"
 	"strings"
 
 	"github.com/urfave/cli/v2"
@@ -199,13 +199,50 @@ func parseConfig(cCtx *cli.Context, globalConfig *types.GlobalConfig, clientConf
 	if err := system.LoadClientConfig(string(buf), clientConfig); err != nil {
 		return err
 	}
+	if !slices.Contains(validFormats, clientConfig.Client.DefaultFormat) {
+		return fmt.Errorf("invalid client.default_format %q in config: valid options are %s",
+			clientConfig.Client.DefaultFormat, strings.Join(validFormats, ", "))
+	}
 	if err := system.LoadServerConfig(string(buf), serverConfig); err != nil {
 		return err
 	}
 
 	return nil
 }
 
+// fatalError prints the error to stderr and exits
+func fatalError(err error) {
+	fmt.Fprintf(os.Stderr, RED+"error: %s"+RESET+"\n", err) //nolint:errcheck
+	system.NotifyServiceFailed(1)
+	os.Exit(1)
+}
+
+// setUsageErrorHandlers routes flag parsing errors through the app
+// ExitErrHandler so they are printed to stderr instead of the urfave/cli
+// default of printing usage errors on stdout
+func setUsageErrorHandlers(commands []*cli.Command, helpPath string) {
+	for _, command := range commands {
+		commandPath := helpPath + " " + command.Name
+		if command.OnUsageError == nil {
+			command.OnUsageError = func(_ *cli.Context, err error, _ bool) error {
+				return usageError(command.Flags, commandPath, err)
+			}
+		}
+		setUsageErrorHandlers(command.Subcommands, commandPath)
+	}
+}
+
+// usageError adds a did-you-mean suggestion for unknown flags and a --help
+// hint to flag parsing errors
+func usageError(flags []cli.Flag, helpPath string, err error) error {
+	if flagName, ok := strings.CutPrefix(err.Error(), "flag provided but not defined: -"); ok && cli.SuggestFlag != nil {
+		if suggestion := cli.SuggestFlag(flags, flagName, false); suggestion != "" {
+			return fmt.Errorf("%w (did you mean %q?)\nrun '%s --help' for usage", err, suggestion, helpPath)
+		}
+	}
+	return fmt.Errorf("%w\nrun '%s --help' for usage", err, helpPath)
+}
+
 func main() {
 	// Start the OS service control handler if launched by the Windows
 	// service control manager. Must run before any long initialization so
@@ -214,16 +251,17 @@ func main() {
 
 	globalConfig, clientConfig, serverConfig, err := system.GetDefaultConfigs()
 	if err != nil {
-		log.Fatal(err)
+		fatalError(err)
 	}
 	globalFlags, err := globalFlags(globalConfig, clientConfig)
 	if err != nil {
-		log.Fatal(err)
+		fatalError(err)
 	}
 	allCommands, err := getAllCommands(clientConfig, serverConfig)
 	if err != nil {
-		log.Fatal(err)
+		fatalError(err)
 	}
+	setUsageErrorHandlers(allCommands, "openrun")
 
 	app := &cli.App{
 		Name:                 "openrun",
@@ -249,9 +287,7 @@ func main() {
 		},
 		ExitErrHandler: func(c *cli.Context, err error) {
 			if err != nil {
-				fmt.Fprintf(cli.ErrWriter, RED+"error: %s\n"+RESET, err) //nolint:errcheck
-				system.NotifyServiceFailed(1)
-				os.Exit(1)
+				fatalError(err)
 			}
 		},
 		Commands: allCommands,
@@ -262,14 +298,26 @@ func main() {
 				os.Exit(0)
 				return nil
 			}
+			if ctx.Args().Present() {
+				// An unknown command should fail instead of showing the help and exiting 0
+				arg := ctx.Args().First()
+				if cli.SuggestCommand != nil {
+					if suggestion := cli.SuggestCommand(ctx.App.Commands, arg); suggestion != "" {
+						return fmt.Errorf("unknown command %q. %s\nrun 'openrun --help' for usage", arg, suggestion)
+					}
+				}
+				return fmt.Errorf("unknown command %q\nrun 'openrun --help' for usage", arg)
+			}
 			return cli.ShowAppHelp(ctx)
 		},
 	}
 
+	app.OnUsageError = func(_ *cli.Context, err error, _ bool) error {
+		return usageError(app.Flags, "openrun", err)
+	}
+
 	if err := app.Run(normalizeInterspersedFlags(app, os.Args)); err != nil {
-		fmt.Fprintf(os.Stderr, "error: %s", err) //nolint:errcheck
-		system.NotifyServiceFailed(1)
-		os.Exit(1)
+		fatalError(err)
 	}
 }
 

cmd/openrun/misc_cmds.go

@@ -101,17 +101,19 @@ func bootstrapConfigFile(clHome, configFile string) error {
 	return nil
 }
 
+// promptPassword prompts on stderr so the command output can be redirected
 func promptPassword(prompt string) (string, error) {
-	fmt.Print(prompt)
+	fmt.Fprint(os.Stderr, prompt)
 	password, err := readPassword()
 	if err != nil {
 		return "", err
 	}
-	fmt.Print("\nConfirm password: ")
+	fmt.Fprint(os.Stderr, "\nConfirm password: ")
 	confirmPassword, err := readPassword()
 	if err != nil {
 		return "", err
 	}
+	fmt.Fprintln(os.Stderr)
 	if password != confirmPassword {
 		return "", fmt.Errorf("passwords do not match")
 	}

cmd/openrun/provider_cmds.go

@@ -119,7 +119,7 @@ func providerUninstallCommand(commonFlags []cli.Flag, clientConfig *types.Client
 func providerListCommand(commonFlags []cli.Flag, clientConfig *types.ClientConfig) *cli.Command {
 	flags := make([]cli.Flag, 0, len(commonFlags)+1)
 	flags = append(flags, commonFlags...)
-	flags = append(flags, newStringFlag("format", "f", "The display format. Valid options are table, basic, csv, json, jsonl and jsonl_pretty", ""))
+	flags = append(flags, newFormatFlag())
 
 	return &cli.Command{
 		Name:  "list",

cmd/openrun/replication_cmds.go

@@ -28,7 +28,7 @@ func initReplicationCommand(commonFlags []cli.Flag, clientConfig *types.ClientCo
 func replicationStatusCommand(commonFlags []cli.Flag, clientConfig *types.ClientConfig) *cli.Command {
 	flags := make([]cli.Flag, 0, len(commonFlags)+1)
 	flags = append(flags, commonFlags...)
-	flags = append(flags, newStringFlag("format", "f", "The display format. Valid options are table, basic, csv, json, jsonl and jsonl_pretty", ""))
+	flags = append(flags, newFormatFlag())
 
 	return &cli.Command{
 		Name:  "status",

cmd/openrun/secret_cmds.go

@@ -166,7 +166,7 @@ func secretListCommand(commonFlags []cli.Flag, clientConfig *types.ClientConfig)
 	flags := make([]cli.Flag, 0, len(commonFlags)+2)
 	flags = append(flags, commonFlags...)
 	flags = append(flags, secretProviderFlag())
-	flags = append(flags, newStringFlag("format", "f", "The display format. Valid options are table, basic, csv, json, jsonl and jsonl_pretty", ""))
+	flags = append(flags, newFormatFlag())
 
 	return &cli.Command{
 		Name:      "list",