chore: do not call os exit on usage to allow for resource cleaup

Calling os.Exit from a library can be problematic for some users,
especially when cleanup is necessary. Whenever usage is rendered, return
error ErrShowUsage and let the caller handle it appropriately.

Add tests.

Author: brandon1024

execute.go

@@ -60,7 +60,7 @@ var ErrEnvironmentBindFailure = errors.New("cmder: failed to update flag from en
 //
 // If the command also implements [FlagInitializer], InitializeFlags() will be invoked to register additional
 // command-line flags. Each command/subcommand is given a unique [flag.FlagSet]. Help flags ('-h', '--help') are
-// configured automatically and must not be set by the application.
+// configured automatically if not defined and will instruct Execute to render command usage.
 //
 // Execute parses getopt-style (GNU/POSIX) command-line arguments with the help of package [getopt]. To use the standard
 // [flag] syntax instead, see [WithNativeFlags]. Flags and arguments cannot be interspersed by default. You can change
@@ -70,12 +70,13 @@ var ErrEnvironmentBindFailure = errors.New("cmder: failed to update flag from en
 //
 // # Usage and Help Texts
 //
-// Whenever the user provides the '-h' or '--help' flag at the command line, [Execute] will display command usage and
-// exit. The format of the help text can be adjusted with [WithUsageTemplate]. By default, usage information will
-// be written to stderr, but this can be adjusted by setting [WithUsageOutput].
+// Whenever the user provides the '-h' or '--help' flag at the command line and the command doesn't register custom help
+// flags, Execute will display command usage and return [ErrShowUsage]. The format of the help text can be adjusted with
+// [WithUsageTemplate]. By default, usage information will be written to stderr, but this can be adjusted by setting
+// [WithUsageOutput].
 //
-// If a command's [Run] routine returns [ErrShowUsage] (or an error wrapping [ErrShowUsage]), [Execute] will render
-// help text and exit with status 2.
+// If a command's Run routine returns [ErrShowUsage] (or an error wrapping [ErrShowUsage]), Execute will render
+// help text and return the error.
 func Execute(ctx context.Context, cmd Command, op ...ExecuteOption) error {
 	// do some checks
 	if cmd == nil {
@@ -98,11 +99,6 @@ func Execute(ctx context.Context, cmd Command, op ...ExecuteOption) error {
 		return err
 	}
 
-	// if help was requested, display and exit
-	if cmd, ok := helpRequested(stack); ok {
-		return usage(*cmd, ops)
-	}
-
 	return execute(ctx, stack, ops)
 }
 
@@ -157,13 +153,16 @@ type command struct {
 func (c command) onInit(ctx context.Context, ops *ExecuteOptions) error {
 	var err error
 
+	if c.showHelp {
+		return errors.Join(ErrShowUsage, usage(c, ops))
+	}
+
 	if cmd, ok := c.Command.(Initializer); ok {
 		err = cmd.Initialize(ctx, c.args)
 	}
 
 	if errors.Is(err, ErrShowUsage) {
-		_ = usage(c, ops)
-		os.Exit(2)
+		return errors.Join(err, usage(c, ops))
 	}
 
 	return err
@@ -173,8 +172,7 @@ func (c command) onInit(ctx context.Context, ops *ExecuteOptions) error {
 func (c command) run(ctx context.Context, ops *ExecuteOptions) error {
 	err := c.Run(ctx, c.args)
 	if errors.Is(err, ErrShowUsage) {
-		_ = usage(c, ops)
-		os.Exit(2)
+		return errors.Join(err, usage(c, ops))
 	}
 
 	return err
@@ -189,8 +187,7 @@ func (c command) onDestroy(ctx context.Context, ops *ExecuteOptions) error {
 	}
 
 	if errors.Is(err, ErrShowUsage) {
-		_ = usage(c, ops)
-		os.Exit(2)
+		return errors.Join(err, usage(c, ops))
 	}
 
 	return err
@@ -212,14 +209,16 @@ func buildCallStack(cmd Command, ops *ExecuteOptions) ([]command, error) {
 			fs:      flag.NewFlagSet(cmd.Name(), flag.ContinueOnError),
 		}
 
-		// add help flags
-		this.fs.BoolVar(&this.showHelp, "h", false, "show command help and usage information")
-		this.fs.BoolVar(&this.showHelp, "help", false, "show command help and usage information")
-
 		if c, ok := cmd.(FlagInitializer); ok {
 			c.InitializeFlags(this.fs)
 		}
 
+		// add help flags
+		if this.fs.Lookup("h") == nil && this.fs.Lookup("help") == nil {
+			this.fs.BoolVar(&this.showHelp, "h", false, "show command help and usage information")
+			this.fs.BoolVar(&this.showHelp, "help", false, "show command help and usage information")
+		}
+
 		// bind environment variables
 		if ops.bindEnv {
 			if err := bindEnvironmentFlags(stack, this, ops); err != nil {
@@ -330,19 +329,3 @@ func formatEnvvar(flagpath []string) string {
 
 	return strings.Join(flagpath, "_")
 }
-
-// helpRequested traverses the command stack and returns whether help text was requested with '-h' or '--help' flags,
-// returning the leaf command from stack and true.
-func helpRequested(stack []command) (*command, bool) {
-	if len(stack) == 0 {
-		return nil, false
-	}
-
-	for _, cmd := range stack {
-		if cmd.showHelp {
-			return &stack[len(stack)-1], true
-		}
-	}
-
-	return nil, false
-}

execute_test.go

@@ -2,8 +2,13 @@ package cmder
 
 import (
 	"context"
+	"errors"
 	"flag"
+	"net/http"
 	"testing"
+	"time"
+
+	"github.com/brandon1024/cmder/getopt"
 )
 
 func TestExecute(t *testing.T) {
@@ -86,4 +91,93 @@ func TestExecute(t *testing.T) {
 			assert(t, match([]string{"000", "--l2f1", "25", "111", "--", "--l2f0=255"}, result))
 		})
 	})
+
+	t.Run("native flags", func(t *testing.T) {
+		var (
+			addr           string
+			readTimeout    time.Duration
+			writeTimeout   time.Duration
+			maxHeaderBytes int
+			maxBodySize    int64
+			basicAuth      string
+			noAuth         bool
+		)
+
+		var args []string
+
+		cmd := &BaseCommand{
+			CommandName: "native-flags",
+			InitFlagsFunc: func(fs *flag.FlagSet) {
+				fs.StringVar(&addr, "http.bind-addr", ":8080", "bind address for the web server")
+				fs.DurationVar(&readTimeout, "http.read-timeout", time.Duration(0), "read timeout for requests")
+				fs.DurationVar(&writeTimeout, "http.write-timeout", time.Duration(0), "write timeout for responses")
+				fs.IntVar(&maxHeaderBytes, "http.max-header-size", http.DefaultMaxHeaderBytes, "max permitted size of the headers in a request")
+				fs.Int64Var(&maxBodySize, "http.max-body-size", 1<<26, "max permitted size of the headers in a request")
+				fs.StringVar(&basicAuth, "http.auth-basic", "", "basic auth credentials (in format user:pass)")
+				fs.BoolVar(&noAuth, "http.no-auth", false, "disable basic auth")
+
+				getopt.Alias(fs, "http.bind-addr", "a")
+				getopt.Alias(fs, "http.read-timeout", "r")
+				getopt.Alias(fs, "http.write-timeout", "w")
+				getopt.Alias(fs, "http.max-header-size", "h")
+				getopt.Alias(fs, "http.max-body-size", "b")
+				getopt.Alias(fs, "http.auth-basic", "C")
+				getopt.Alias(fs, "http.no-auth", "E")
+			},
+			RunFunc: func(ctx context.Context, a []string) error {
+				args = a
+				return nil
+			},
+		}
+
+		t.Run("should correctly parse flags in standard flag libs format", func(t *testing.T) {
+			err := Execute(t.Context(), cmd, WithNativeFlags(), WithArgs([]string{
+				"-http.bind-addr", "0.0.0.0:8000",
+				"--http.read-timeout", "10s",
+				"-http.write-timeout=5s",
+				"-h", "8096",
+				"-b=65536",
+				"-http.auth-basic", "U:P",
+				"--",
+				"-http.no-auth", "true",
+			}))
+
+			assert(t, nilerr(err))
+			assert(t, eq("0.0.0.0:8000", addr))
+			assert(t, eq(10*time.Second, readTimeout))
+			assert(t, eq(5*time.Second, writeTimeout))
+			assert(t, eq(8096, maxHeaderBytes))
+			assert(t, eq(65536, maxBodySize))
+			assert(t, eq("U:P", basicAuth))
+			assert(t, eq(false, noAuth))
+			assert(t, match([]string{"-http.no-auth", "true"}, args))
+		})
+	})
+
+	t.Run("help flags", func(t *testing.T) {
+		t.Run("should not register help flags if defined by command", func(t *testing.T) {
+			var showHelp bool
+
+			cmd := &BaseCommand{
+				CommandName: "help-cmd",
+				InitFlagsFunc: func(fs *flag.FlagSet) {
+					fs.BoolVar(&showHelp, "h", showHelp, "show help")
+					fs.BoolVar(&showHelp, "help", showHelp, "show help")
+				},
+			}
+
+			err := Execute(t.Context(), cmd, WithArgs([]string{"--help"}))
+			assert(t, nilerr(err))
+			assert(t, eq(true, showHelp))
+		})
+
+		t.Run("should return ErrShowUsage if help flags not defined", func(t *testing.T) {
+			cmd := &BaseCommand{
+				CommandName: "help-cmd",
+			}
+
+			err := Execute(t.Context(), cmd, WithArgs([]string{"--help"}))
+			assert(t, eq(true, errors.Is(err, ErrShowUsage)))
+		})
+	})
 }