Refactor internals of Command and State flag tracking

Author: mfridman

README.md

@@ -2,27 +2,35 @@
 
 [![GoDoc](https://godoc.org/github.com/mfridman/cli?status.svg)](https://godoc.org/github.com/mfridman/cli)
 [![CI](https://github.com/mfridman/cli/actions/workflows/ci.yaml/badge.svg)](https://github.com/mfridman/cli/actions/workflows/ci.yaml)
-[![Go Report
-Card](https://goreportcard.com/badge/github.com/mfridman/cli)](https://goreportcard.com/report/github.com/mfridman/cli)
 
-A lightweight framework for building Go CLI applications with nested subcommands.
-
-Supports flexible flag placement ([allowing flags anywhere on the
-CLI](https://mfridman.com/blog/2024/allowing-flags-anywhere-on-the-cli/)), since Go's standard
-library requires flags before arguments.
+A Go framework for building CLI applications with flexible flag placement. Extends the standard
+library's `flag` package to support [flags
+anywhere](https://mfridman.com/blog/2024/allowing-flags-anywhere-on-the-cli/) in command arguments.
 
 ## Features
 
 - Nested subcommands for organizing complex CLIs
-- Flexible flag parsing, allowing flags anywhere on the CLI
+- Flexible flag parsing, allowing flags anywhere
 - Subcommands inherit flags from parent commands
 - Type-safe flag access
 - Automatic generation of help text and usage information
 - Suggestions for misspelled or incomplete commands
 
-And that's it! It's the bare minimum to build a CLI application in Go while leveraging the standard
+And that's it! It's the **bare minimum to build a CLI application** while leveraging the standard
 library's `flag` package.
 
+### But why?
+
+This framework embraces minimalism while maintaining functionality. It provides essential building
+blocks for CLI applications without the bloat, allowing you to:
+
+- Build maintainable command-line tools quickly
+- Focus on application logic rather than framework complexity
+- Extend functionality **only when needed**
+
+Sometimes less is more. While other frameworks offer extensive features, this package focuses on
+core functionality.
+
 ## Installation
 
 ```bash
@@ -100,16 +108,25 @@ a slice of child commands.
 
 > [!TIP]
 >
-> There's a top-level convenience function `FlagsFunc` that allows you to define flags inline:
+> There's a convenience function `FlagsFunc` that allows you to define flags inline:
 
 ```go
-cmd.Flags = cli.FlagsFunc(func(fs *flag.FlagSet) {
-	fs.Bool("verbose", false, "enable verbose output")
-	fs.String("output", "", "output file")
-	fs.Int("count", 0, "number of items")
-})
+root := &cli.Command{
+	Flags: cli.FlagsFunc(func(f *flag.FlagSet) {
+		fs.Bool("verbose", false, "enable verbose output")
+		fs.String("output", "", "output file")
+		fs.Int("count", 0, "number of items")
+	}),
+	FlagsMetadata: []cli.FlagMetadata{
+		{Name: "c", Required: true},
+	},
+}
 ```
 
+The `FlagsMetadata` field is a slice of `FlagMetadata` structs that define metadata for each flag.
+Unfortunatly, the `flag.FlagSet` package alone is a bit limiting, so this package adds a layer on
+top to provide the most common features.
+
 The `Exec` field is a function that is called when the command is executed. This is where you put
 your business logic.
 

command.go

@@ -47,8 +47,7 @@ type Command struct {
 	// is called.
 	Exec func(ctx context.Context, s *State) error
 
-	state *State
-	// TODO(mf): remove this in favor of tracking the selected *Command in the state
+	state    *State
 	selected *Command
 }
 
@@ -94,15 +93,13 @@ func (c *Command) showHelp() error {
 		return nil
 	}
 
-	// Display command description first if available, with wrapping
 	if c.ShortHelp != "" {
 		for _, line := range wrapText(c.ShortHelp, 80) {
 			fmt.Fprintf(w, "%s\n", line)
 		}
 		fmt.Fprintln(w)
 	}
 
-	// Show usage pattern
 	fmt.Fprintf(w, "Usage:\n  ")
 	if c.Usage != "" {
 		fmt.Fprintf(w, "%s\n", c.Usage)
@@ -118,7 +115,6 @@ func (c *Command) showHelp() error {
 	}
 	fmt.Fprintln(w)
 
-	// Show available subcommands
 	if len(c.SubCommands) > 0 {
 		fmt.Fprintf(w, "Available Commands:\n")
 
@@ -155,7 +151,6 @@ func (c *Command) showHelp() error {
 		fmt.Fprintln(w)
 	}
 
-	// Collect and format all flags
 	type flagInfo struct {
 		name   string
 		usage  string
@@ -176,12 +171,12 @@ func (c *Command) showHelp() error {
 		})
 	}
 
-	// Global flags
-	if c.state.parent != nil {
+	// Global flags from parent commands
+	if c.state != nil && c.state.parent != nil {
 		p := c.state.parent
 		for p != nil {
-			if p.flags != nil {
-				p.flags.VisitAll(func(f *flag.Flag) {
+			if p.cmd != nil && p.cmd.Flags != nil {
+				p.cmd.Flags.VisitAll(func(f *flag.Flag) {
 					flags = append(flags, flagInfo{
 						name:   "-" + f.Name,
 						usage:  f.Usage,
@@ -195,12 +190,10 @@ func (c *Command) showHelp() error {
 	}
 
 	if len(flags) > 0 {
-		// Sort flags by name
 		slices.SortFunc(flags, func(a, b flagInfo) int {
 			return cmp.Compare(a.name, b.name)
 		})
 
-		// Find the longest flag name for alignment
 		maxLen := 0
 		for _, f := range flags {
 			if len(f.name) > maxLen {
@@ -218,28 +211,22 @@ func (c *Command) showHelp() error {
 			}
 		}
 
-		// Print local flags first
 		if hasLocal {
 			fmt.Fprintf(w, "Flags:\n")
 			for _, f := range flags {
 				if !f.global {
 					nameWidth := maxLen + 4
 					wrapWidth := 80 - nameWidth
 
-					// Prepare the usage text with default value if needed
 					usageText := f.usage
 					if f.defval != "" && f.defval != "false" {
 						usageText += fmt.Sprintf(" (default %s)", f.defval)
 					}
 
-					// Wrap the usage text
 					lines := wrapText(usageText, wrapWidth)
-
-					// Print first line with flag name
 					padding := strings.Repeat(" ", maxLen-len(f.name)+4)
 					fmt.Fprintf(w, "  %s%s%s\n", f.name, padding, lines[0])
 
-					// Print subsequent lines with proper padding
 					indentPadding := strings.Repeat(" ", nameWidth+2)
 					for _, line := range lines[1:] {
 						fmt.Fprintf(w, "%s%s\n", indentPadding, line)
@@ -249,28 +236,22 @@ func (c *Command) showHelp() error {
 			fmt.Fprintln(w)
 		}
 
-		// Then print global flags
 		if hasGlobal {
 			fmt.Fprintf(w, "Global Flags:\n")
 			for _, f := range flags {
 				if f.global {
 					nameWidth := maxLen + 4
 					wrapWidth := 80 - nameWidth
 
-					// Prepare the usage text with default value if needed
 					usageText := f.usage
 					if f.defval != "" && f.defval != "false" {
 						usageText += fmt.Sprintf(" (default %s)", f.defval)
 					}
 
-					// Wrap the usage text
 					lines := wrapText(usageText, wrapWidth)
-
-					// Print first line with flag name
 					padding := strings.Repeat(" ", maxLen-len(f.name)+4)
 					fmt.Fprintf(w, "  %s%s%s\n", f.name, padding, lines[0])
 
-					// Print subsequent lines with proper padding
 					indentPadding := strings.Repeat(" ", nameWidth+2)
 					for _, line := range lines[1:] {
 						fmt.Fprintf(w, "%s%s\n", indentPadding, line)
@@ -281,7 +262,6 @@ func (c *Command) showHelp() error {
 		}
 	}
 
-	// Show help hint for subcommands
 	if len(c.SubCommands) > 0 {
 		fmt.Fprintf(w, "Use \"%s [command] --help\" for more information about a command.\n", c.Name)
 	}

parse.go

@@ -26,13 +26,7 @@ func Parse(root *Command, args []string) error {
 
 	// Initialize root state
 	if root.state == nil {
-		root.state = &State{}
-	}
-	if root.state.flags == nil {
-		if root.Flags == nil {
-			root.Flags = flag.NewFlagSet(root.Name, flag.ContinueOnError)
-		}
-		root.state.flags = root.Flags
+		root.state = &State{cmd: root}
 	}
 
 	// First split args at the -- delimiter if present
@@ -55,31 +49,29 @@ func Parse(root *Command, args []string) error {
 
 	// Create combined flags with all parent flags
 	combinedFlags := flag.NewFlagSet(root.Name, flag.ContinueOnError)
+	// TODO(mf): revisit this
 	combinedFlags.SetOutput(io.Discard)
 
-	// First pass: process commands and build the flag set This lets us capture help requests before
-	// any flag parsing errors
+	// First pass: process commands and build the flag set. This lets us capture help requests
+	// before any flag parsing errors
 	for _, arg := range argsToParse {
 		if arg == "-h" || arg == "--h" || arg == "-help" || arg == "--help" {
 			combinedFlags.Usage = func() { _ = current.showHelp() }
 			return current.showHelp()
 		}
-
 		// Skip anything that looks like a flag
 		if strings.HasPrefix(arg, "-") {
 			continue
 		}
-
 		// Try to traverse to subcommand
 		if len(current.SubCommands) > 0 {
 			if sub := current.findSubCommand(arg); sub != nil {
 				if sub.state == nil {
-					sub.state = &State{}
+					sub.state = &State{cmd: sub}
 				}
 				if sub.Flags == nil {
 					sub.Flags = flag.NewFlagSet(sub.Name, flag.ContinueOnError)
 				}
-				sub.state.flags = sub.Flags
 				sub.state.parent = current.state
 				current = sub
 				commandChain = append(commandChain, sub)
@@ -122,7 +114,6 @@ func Parse(root *Command, args []string) error {
 			if flag == nil {
 				return fmt.Errorf("command %q: internal error: required flag %q not found in flag set", current.Name, flagMetadata.Name)
 			}
-
 			// Look for the flag in the original args before any delimiter
 			found := false
 			for _, arg := range argsToParse {

state.go

@@ -17,8 +17,7 @@ type State struct {
 	Stdin          io.Reader
 	Stdout, Stderr io.Writer
 
-	// TODO(mf): remove flags in favor of tracking the selected *Command
-	flags  *flag.FlagSet
+	cmd    *Command // Reference to the command this state belongs to
 	parent *State
 }
 
@@ -36,7 +35,7 @@ type State struct {
 // unexpected behavior.
 func GetFlag[T any](s *State, name string) T {
 	// TODO(mf): we should have a way to get the selected command here to improve error messages
-	if f := s.flags.Lookup(name); f != nil {
+	if f := s.cmd.Flags.Lookup(name); f != nil {
 		if getter, ok := f.Value.(flag.Getter); ok {
 			value := getter.Get()
 			if v, ok := value.(T); ok {
@@ -52,6 +51,6 @@ func GetFlag[T any](s *State, name string) T {
 		return GetFlag[T](s.parent, name)
 	}
 	// If flag not found anywhere in hierarchy, panic with helpful message
-	msg := fmt.Sprintf("internal error: flag not found: %q in %s flag set", name, s.flags.Name())
+	msg := fmt.Sprintf("internal error: flag not found: %q in %s flag set", name, s.cmd.Name)
 	panic(msg)
 }

state_test.go

@@ -13,7 +13,10 @@ func TestGetFlag(t *testing.T) {
 
 	t.Run("flag not found", func(t *testing.T) {
 		st := &State{
-			flags: flag.NewFlagSet("root", flag.ContinueOnError),
+			cmd: &Command{
+				Name:  "root",
+				Flags: flag.NewFlagSet("root", flag.ContinueOnError),
+			},
 		}
 		// Capture the panic
 		defer func() {
@@ -27,9 +30,11 @@ func TestGetFlag(t *testing.T) {
 	})
 	t.Run("flag type mismatch", func(t *testing.T) {
 		st := &State{
-			flags: flag.NewFlagSet("root", flag.ContinueOnError),
+			cmd: &Command{
+				Name:  "root",
+				Flags: FlagsFunc(func(f *flag.FlagSet) { f.String("version", "1.0.0", "show version") }),
+			},
 		}
-		st.flags.String("version", "1.0", "version")
 		defer func() {
 			r := recover()
 			require.NotNil(t, r)