Refactor how flag usage is generated

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 {
@@ -583,14 +583,12 @@ func writeArgumentsSection(cmd *Command, s *strings.Builder) error {
 	tw := style.Tabwriter(s)
 
 	for _, arg := range cmd.args {
-		switch arg.Default() {
-		case "":
-			// It's required
-			fmt.Fprintf(tw, "  %s\t%s\t%s\t[required]\n", style.Bold.Text(arg.Name()), arg.Type(), arg.Usage())
-		default:
-			// It has a default
-			fmt.Fprintf(tw, "  %s\t%s\t%s\t[default %s]\n", style.Bold.Text(arg.Name()), arg.Type(), arg.Usage(), arg.Default())
+		line := fmt.Sprintf("  %s\t%s\t%s\t[required]", style.Bold.Text(arg.Name()), arg.Type(), arg.Usage())
+		if arg.Default() != "" {
+			line = fmt.Sprintf("  %s\t%s\t%s\t[default %s]", style.Bold.Text(arg.Name()), arg.Type(), arg.Usage(), arg.Default())
 		}
+
+		fmt.Fprintln(tw, line)
 	}
 
 	if err := tw.Flush(); err != nil {
@@ -657,6 +655,46 @@ 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"
+		}
+
+		line := fmt.Sprintf(
+			"  %s\t--%s\t%s\t%s\t", style.Bold.Text(shorthand),
+			style.Bold.Text(name),
+			fl.Type(),
+			fl.Usage(),
+		)
+
+		if fl.Default() != "" {
+			line = fmt.Sprintf(
+				"  %s\t--%s\t%s\t%s\t[default: %s]",
+				style.Bold.Text(shorthand),
+				style.Bold.Text(name),
+				fl.Type(),
+				fl.Usage(),
+				fl.Default(),
+			)
+		}
+
+		fmt.Fprintln(tw, line)
+	}
+
+	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/flag.go

@@ -55,19 +55,6 @@ func New[T flag.Flaggable](p *T, name string, short rune, usage string, config C
 		short: short,
 	}
 
-	// TODO(@FollowTheProcess): This needs to live in command.go and we should iterate over the flagset
-	// adding the values to the tabwriter as we go rather than relying on the flagset.Usage() method
-	// to provide *all* the usage
-
-	// If the default value is not the zero value for the type, it is treated as
-	// significant and shown to the user
-	flag.usage += "\t"
-	if !isZeroIsh(*p) {
-		// \t so that defaults get aligned by tabwriter when the command
-		// dumps the flags
-		flag.usage += fmt.Sprintf("[default: %s]", flag.String())
-	}
-
 	return flag, nil
 }
 
@@ -87,6 +74,21 @@ func (f Flag[T]) Usage() string {
 	return f.usage
 }
 
+// Default returns the default value for the flag, as a string.
+//
+// If the flag's default is unset (i.e. the zero value for its type),
+// an empty string is returned.
+func (f Flag[T]) Default() string {
+	// Special case a --help flag, because if we didn't, when you call --help
+	// it would show up with a default of true because you've passed it
+	// so it's value is true here
+	if isZeroIsh(*f.value) || f.name == "help" {
+		return ""
+	}
+
+	return f.String()
+}
+
 // NoArgValue returns a string representation of value the flag should hold
 // when it is given no arguments on the command line. For example a boolean flag
 // --delete, when passed without arguments implies --delete true.

internal/flag/flag_test.go

@@ -30,6 +30,7 @@ func TestFlaggableTypes(t *testing.T) {
 		test.Equal(t, i, 42)
 		test.Equal(t, intFlag.Type(), "int")
 		test.Equal(t, intFlag.String(), "42")
+		test.Equal(t, intFlag.Default(), "42")
 	})
 
 	t.Run("int invalid", func(t *testing.T) {
@@ -54,6 +55,7 @@ func TestFlaggableTypes(t *testing.T) {
 		test.Equal(t, i, int8(42))
 		test.Equal(t, intFlag.Type(), "int8")
 		test.Equal(t, intFlag.String(), "42")
+		test.Equal(t, intFlag.Default(), "42")
 	})
 
 	t.Run("int8 invalid", func(t *testing.T) {
@@ -78,6 +80,7 @@ func TestFlaggableTypes(t *testing.T) {
 		test.Equal(t, i, int16(42))
 		test.Equal(t, intFlag.Type(), "int16")
 		test.Equal(t, intFlag.String(), "42")
+		test.Equal(t, intFlag.Default(), "42")
 	})
 
 	t.Run("int16 invalid", func(t *testing.T) {
@@ -383,6 +386,20 @@ func TestFlaggableTypes(t *testing.T) {
 		test.Equal(t, boolFlag.String(), format.True)
 	})
 
+	t.Run("bool help", func(t *testing.T) {
+		var h bool
+
+		boolFlag, err := flag.New(&h, "help", 'h', "Show help", flag.Config[bool]{})
+		test.Ok(t, err)
+
+		err = boolFlag.Set(format.True)
+		test.Ok(t, err)
+		test.Equal(t, h, true)
+		test.Equal(t, boolFlag.Type(), "bool")
+		test.Equal(t, boolFlag.String(), format.True)
+		test.Equal(t, boolFlag.Default(), "") // Special case
+	})
+
 	t.Run("bool invalid", func(t *testing.T) {
 		var b bool
 

internal/flag/set.go

@@ -1,20 +1,16 @@
 package flag
 
 import (
-	"bytes"
 	"errors"
 	"fmt"
+	"iter"
 	"slices"
 	"strings"
 
 	"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
@@ -187,49 +183,23 @@ func (s *Set) Parse(args []string) (err error) {
 	return nil
 }
 
-// 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
+// All returns an iterator through the flags in the flagset
+// in alphabetical order by name.
+func (s *Set) All() iter.Seq2[string, Value] {
+	return func(yield func(string, Value) bool) {
+		names := make([]string, 0, len(s.flags))
+		for name := range s.flags {
+			names = append(names, name)
 		}
 
-		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(),
-		)
-	}
+		slices.Sort(names)
 
-	if err := tw.Flush(); err != nil {
-		return "", fmt.Errorf("could not format flags: %w", err)
+		for _, name := range names {
+			if !yield(name, s.flags[name]) {
+				return
+			}
+		}
 	}
-
-	return buf.String(), nil
 }
 
 // parseLongFlag parses a single long flag e.g. --delete. It is passed