Move flag usage block writing into command.go

Author: FollowTheProcess

command.go

@@ -11,6 +11,8 @@ import (
 	"strings"
 	"unicode/utf8"
 
+	publicflag "go.followtheprocess.codes/cli/flag"
+
 	"go.followtheprocess.codes/cli/internal/arg"
 	"go.followtheprocess.codes/cli/internal/flag"
 	"go.followtheprocess.codes/cli/internal/style"
@@ -462,11 +464,6 @@ func showHelp(cmd *Command) error {
 		return errors.New("showHelp called on a nil Command")
 	}
 
-	usage, err := cmd.flagSet().Usage()
-	if err != nil {
-		return fmt.Errorf("could not write usage: %w", err)
-	}
-
 	// Note: The decision to not use text/template here is intentional, template calls
 	// reflect.Value.MethodByName() and/or reflect.Type.MethodByName() which disables dead
 	// code elimination in the compiler, meaning any application that uses cli for it's
@@ -540,7 +537,10 @@ func showHelp(cmd *Command) error {
 
 	s.WriteString(style.Title.Text("Options"))
 	s.WriteString(":\n\n")
-	s.WriteString(usage)
+
+	if err := writeFlags(cmd, s); err != nil {
+		return err
+	}
 
 	// Subcommand help
 	if len(cmd.subcommands) != 0 {
@@ -657,6 +657,35 @@ func writeSubcommands(cmd *Command, s *strings.Builder) error {
 	return nil
 }
 
+// writeFlags writes the flag usage block to the help text string builder.
+func writeFlags(cmd *Command, s *strings.Builder) error {
+	tw := style.Tabwriter(s)
+
+	for name, fl := range cmd.flags.All() {
+		var shorthand string
+		if fl.Short() != publicflag.NoShortHand {
+			shorthand = "-" + string(fl.Short())
+		} else {
+			shorthand = "N/A"
+		}
+
+		fmt.Fprintf(
+			tw,
+			"  %s\t--%s\t%s\t%s\n",
+			style.Bold.Text(shorthand),
+			style.Bold.Text(name),
+			fl.Type(),
+			fl.Usage(),
+		)
+	}
+
+	if err := tw.Flush(); err != nil {
+		return fmt.Errorf("could not write flag usage: %w", err)
+	}
+
+	return nil
+}
+
 // writeFooter writes the footer to the help text string builder.
 func writeFooter(cmd *Command, s *strings.Builder) {
 	s.WriteByte('\n')

internal/flag/set.go

@@ -1,7 +1,6 @@
 package flag
 
 import (
-	"bytes"
 	"errors"
 	"fmt"
 	"iter"
@@ -10,12 +9,8 @@ import (
 
 	"go.followtheprocess.codes/cli/flag"
 	"go.followtheprocess.codes/cli/internal/format"
-	"go.followtheprocess.codes/cli/internal/style"
 )
 
-// usageBufferSize is sufficient to hold most commands flag usage text.
-const usageBufferSize = 256
-
 // Set is a set of command line flags.
 type Set struct {
 	flags      map[string]Value // The actual stored flags, can lookup by name
@@ -207,51 +202,6 @@ func (s *Set) All() iter.Seq2[string, Value] {
 	}
 }
 
-// Usage returns a string containing the usage info of all flags in the set.
-func (s *Set) Usage() (string, error) {
-	buf := &bytes.Buffer{}
-	buf.Grow(usageBufferSize)
-
-	// Flags should be sorted alphabetically
-	names := make([]string, 0, len(s.flags))
-	for name := range s.flags {
-		names = append(names, name)
-	}
-
-	slices.Sort(names)
-
-	tw := style.Tabwriter(buf)
-
-	for _, name := range names {
-		f := s.flags[name]
-		if f == nil {
-			return "", fmt.Errorf("Value stored against key %s was nil", name) // Should never happen
-		}
-
-		var shorthand string
-		if f.Short() != flag.NoShortHand {
-			shorthand = "-" + string(f.Short())
-		} else {
-			shorthand = "N/A"
-		}
-
-		fmt.Fprintf(
-			tw,
-			"  %s\t--%s\t%s\t%s\n",
-			style.Bold.Text(shorthand),
-			style.Bold.Text(name),
-			f.Type(),
-			f.Usage(),
-		)
-	}
-
-	if err := tw.Flush(); err != nil {
-		return "", fmt.Errorf("could not format flags: %w", err)
-	}
-
-	return buf.String(), nil
-}
-
 // parseLongFlag parses a single long flag e.g. --delete. It is passed
 // the possible long flag and the rest of the argument list and returns
 // the remaining arguments after it's done parsing to the caller.

internal/flag/set_test.go

@@ -1,8 +1,6 @@
 package flag_test
 
 import (
-	goflag "flag"
-	"fmt"
 	"iter"
 	"maps"
 	"slices"
@@ -12,15 +10,9 @@ import (
 	publicflag "go.followtheprocess.codes/cli/flag"
 	"go.followtheprocess.codes/cli/internal/flag"
 	"go.followtheprocess.codes/cli/internal/format"
-	"go.followtheprocess.codes/snapshot"
 	"go.followtheprocess.codes/test"
 )
 
-var (
-	debug  = goflag.Bool("debug", false, "Print debug output during tests")
-	update = goflag.Bool("update", false, "Update golden files")
-)
-
 func TestParse(t *testing.T) {
 	tests := []struct {
 		name    string                            // The name of the test case
@@ -1305,114 +1297,6 @@ func TestAll(t *testing.T) {
 	}
 }
 
-func TestUsage(t *testing.T) {
-	tests := []struct {
-		newSet func(t *testing.T) *flag.Set // Function to build the flag set under test
-		name   string                       // Name of the test case
-	}{
-		{
-			name: "simple",
-			newSet: func(t *testing.T) *flag.Set {
-				help, err := flag.New(new(bool), "help", 'h', "Show help for test", flag.Config[bool]{})
-				test.Ok(t, err)
-
-				version, err := flag.New(new(bool), "version", 'V', "Show version info for test", flag.Config[bool]{})
-				test.Ok(t, err)
-
-				set := flag.NewSet()
-
-				err = flag.AddToSet(set, help)
-				test.Ok(t, err)
-
-				err = flag.AddToSet(set, version)
-				test.Ok(t, err)
-
-				return set
-			},
-		},
-		{
-			name: "no shorthand",
-			newSet: func(t *testing.T) *flag.Set {
-				help, err := flag.New(new(bool), "help", 'h', "Show help for test", flag.Config[bool]{})
-				test.Ok(t, err)
-
-				version, err := flag.New(new(bool), "version", 'V', "Show version info for test", flag.Config[bool]{})
-				test.Ok(t, err)
-
-				up, err := flag.New(new(bool), "update", publicflag.NoShortHand, "Update something", flag.Config[bool]{})
-				test.Ok(t, err)
-
-				set := flag.NewSet()
-
-				err = flag.AddToSet(set, help)
-				test.Ok(t, err)
-
-				err = flag.AddToSet(set, version)
-				test.Ok(t, err)
-
-				err = flag.AddToSet(set, up)
-				test.Ok(t, err)
-
-				return set
-			},
-		},
-		{
-			name: "full",
-			newSet: func(t *testing.T) *flag.Set {
-				help, err := flag.New(new(bool), "help", 'h', "Show help for test", flag.Config[bool]{})
-				test.Ok(t, err)
-
-				version, err := flag.New(new(bool), "version", 'V', "Show version info for test", flag.Config[bool]{})
-				test.Ok(t, err)
-
-				up, err := flag.New(new(bool), "update", publicflag.NoShortHand, "Update something", flag.Config[bool]{})
-				test.Ok(t, err)
-
-				count, err := flag.New(new(int), "count", 'c', "Count things", flag.Config[int]{})
-				test.Ok(t, err)
-
-				thing, err := flag.New(new(string), "thing", 't', "Name the thing", flag.Config[string]{})
-				test.Ok(t, err)
-
-				set := flag.NewSet()
-
-				err = flag.AddToSet(set, help)
-				test.Ok(t, err)
-
-				err = flag.AddToSet(set, version)
-				test.Ok(t, err)
-
-				err = flag.AddToSet(set, up)
-				test.Ok(t, err)
-
-				err = flag.AddToSet(set, count)
-				test.Ok(t, err)
-
-				err = flag.AddToSet(set, thing)
-				test.Ok(t, err)
-
-				return set
-			},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			snap := snapshot.New(t, snapshot.Update(*update))
-			set := tt.newSet(t)
-
-			got, err := set.Usage()
-			test.Ok(t, err)
-
-			if *debug {
-				fmt.Printf("DEBUG (%s)\n_____\n\n%s\n", tt.name, got)
-			}
-
-			snap.Snap(got)
-		})
-	}
-}
-
 func BenchmarkParse(b *testing.B) {
 	set := flag.NewSet()
 	f, err := flag.New(new(int), "count", 'c', "Count something", flag.Config[int]{})