diff --git a/command.go b/command.go index f8daf29..06e9c1b 100644 --- a/command.go +++ b/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') diff --git a/internal/flag/flag.go b/internal/flag/flag.go index 664df67..5cd0abd 100644 --- a/internal/flag/flag.go +++ b/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. diff --git a/internal/flag/flag_test.go b/internal/flag/flag_test.go index 5bad71b..a95c052 100644 --- a/internal/flag/flag_test.go +++ b/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 diff --git a/internal/flag/set.go b/internal/flag/set.go index e2da64f..1f2325d 100644 --- a/internal/flag/set.go +++ b/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 diff --git a/internal/flag/set_test.go b/internal/flag/set_test.go index 0e4b8af..b9def28 100644 --- a/internal/flag/set_test.go +++ b/internal/flag/set_test.go @@ -1,23 +1,18 @@ package flag_test import ( - goflag "flag" - "fmt" + "iter" + "maps" "slices" "testing" + "time" 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 @@ -1209,110 +1204,95 @@ func TestHelpVersion(t *testing.T) { } } -func TestUsage(t *testing.T) { +func TestAll(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 + newSet func(t *testing.T) *flag.Set + test func(t *testing.T, set *flag.Set) + name string }{ { - name: "simple", + name: "empty", 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 + return flag.NewSet() }, - }, - { - 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 + test: func(t *testing.T, set *flag.Set) { + // Iterator should yield no values + got := maps.Collect(set.All()) + test.Equal(t, len(got), 0) }, }, { 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) + verbose, err := flag.New(new(bool), "verbose", 'v', "Show verbose info", flag.Config[bool]{}) test.Ok(t, err) - err = flag.AddToSet(set, version) + debug, err := flag.New(new(bool), "debug", 'd', "Show debug info", flag.Config[bool]{}) test.Ok(t, err) - err = flag.AddToSet(set, up) + thing, err := flag.New(new(string), "thing", 't', "A thing", flag.Config[string]{}) test.Ok(t, err) - err = flag.AddToSet(set, count) + number, err := flag.New(new(int), "number", 'n', "Number of times", flag.Config[int]{}) test.Ok(t, err) - err = flag.AddToSet(set, thing) + duration, err := flag.New(new(time.Duration), "duration", 'D', "The time to do something for", flag.Config[time.Duration]{}) test.Ok(t, err) + test.Ok(t, flag.AddToSet(set, verbose)) + test.Ok(t, flag.AddToSet(set, debug)) + test.Ok(t, flag.AddToSet(set, thing)) + test.Ok(t, flag.AddToSet(set, number)) + test.Ok(t, flag.AddToSet(set, duration)) + return set }, + test: func(t *testing.T, set *flag.Set) { + // Iterator should yield no values + next, stop := iter.Pull2(set.All()) + defer stop() + + // Should now be in alphabetical order + name, fl, ok := next() + test.True(t, ok) + test.Equal(t, name, "debug") + test.Equal(t, fl.Name(), "debug") + + name, fl, ok = next() + test.True(t, ok) + test.Equal(t, name, "duration") + test.Equal(t, fl.Name(), "duration") + + name, fl, ok = next() + test.True(t, ok) + test.Equal(t, name, "number") + test.Equal(t, fl.Name(), "number") + + name, fl, ok = next() + test.True(t, ok) + test.Equal(t, name, "thing") + test.Equal(t, fl.Name(), "thing") + + name, fl, ok = next() + test.True(t, ok) + test.Equal(t, name, "verbose") + test.Equal(t, fl.Name(), "verbose") + + // Thats it + name, fl, ok = next() + test.False(t, ok) + test.Equal(t, name, "") + test.Equal(t, fl, nil) + }, }, } 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) + tt.test(t, set) }) } } diff --git a/internal/flag/testdata/snapshots/TestUsage/full.snap.txt b/internal/flag/testdata/snapshots/TestUsage/full.snap.txt deleted file mode 100644 index 6c5a413..0000000 --- a/internal/flag/testdata/snapshots/TestUsage/full.snap.txt +++ /dev/null @@ -1,5 +0,0 @@ - -c --count int Count things - -h --help bool Show help for test - -t --thing string Name the thing - N/A --update bool Update something - -V --version bool Show version info for test diff --git a/internal/flag/testdata/snapshots/TestUsage/no_shorthand.snap.txt b/internal/flag/testdata/snapshots/TestUsage/no_shorthand.snap.txt deleted file mode 100644 index 39df5f3..0000000 --- a/internal/flag/testdata/snapshots/TestUsage/no_shorthand.snap.txt +++ /dev/null @@ -1,3 +0,0 @@ - -h --help bool Show help for test - N/A --update bool Update something - -V --version bool Show version info for test diff --git a/internal/flag/testdata/snapshots/TestUsage/simple.snap.txt b/internal/flag/testdata/snapshots/TestUsage/simple.snap.txt deleted file mode 100644 index 6b3aec2..0000000 --- a/internal/flag/testdata/snapshots/TestUsage/simple.snap.txt +++ /dev/null @@ -1,2 +0,0 @@ - -h --help bool Show help for test - -V --version bool Show version info for test diff --git a/internal/flag/value.go b/internal/flag/value.go index bb0135b..97c3255 100644 --- a/internal/flag/value.go +++ b/internal/flag/value.go @@ -14,6 +14,12 @@ type Value interface { // String returns the stored value of a flag as a string. String() string + // Default return the default value of a flag as a string. + // + // If the flag's default is the zero value for it's type, + // an empty string is returned. + Default() string + // NoArgValue returns astring representation of the value of the flag when no // args are passed (e.g --bool implies --bool true). NoArgValue() string