From 2dc8802089a7e5fea56c4dd796c18fad6a5d4f05 Mon Sep 17 00:00:00 2001 From: Tom Fleet Date: Thu, 23 Apr 2026 12:15:25 +0100 Subject: [PATCH] Add a `Sorted` iterator on flag set for help text and keep `All` unordered --- command.go | 2 +- internal/flag/set.go | 11 ++++-- internal/flag/set_test.go | 75 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/command.go b/command.go index b8adc2e..2e1e4a5 100644 --- a/command.go +++ b/command.go @@ -653,7 +653,7 @@ func writeSubcommands(cmd *Command, s *strings.Builder) error { func writeFlags(cmd *Command, s *strings.Builder) error { tw := style.Tabwriter(s) - for name, fl := range cmd.flags.All() { + for name, fl := range cmd.flags.Sorted() { var shorthand string if fl.Short() != publicflag.NoShortHand { shorthand = "-" + string(fl.Short()) diff --git a/internal/flag/set.go b/internal/flag/set.go index bffdb0d..688d5e5 100644 --- a/internal/flag/set.go +++ b/internal/flag/set.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "iter" + "maps" "os" "slices" "strings" @@ -200,9 +201,9 @@ func (s *Set) Parse(args []string) error { return nil } -// All returns an iterator through the flags in the flagset +// Sorted returns an iterator through the flags in the flagset // in alphabetical order by name. -func (s *Set) All() iter.Seq2[string, Value] { +func (s *Set) Sorted() iter.Seq2[string, Value] { return func(yield func(string, Value) bool) { names := make([]string, 0, len(s.flags)) for name := range s.flags { @@ -219,6 +220,12 @@ func (s *Set) All() iter.Seq2[string, Value] { } } +// All returns an iterator through the flags in the flagset +// in alphabetical order by name. +func (s *Set) All() iter.Seq2[string, Value] { + return maps.All(s.flags) +} + // applyEnvVars looks up each configured environment variable and applies its value // to the corresponding flag. It is called at the start of Parse so that CLI args // parsed afterward naturally override these values. diff --git a/internal/flag/set_test.go b/internal/flag/set_test.go index 0d09e1a..3dd3a7a 100644 --- a/internal/flag/set_test.go +++ b/internal/flag/set_test.go @@ -2307,7 +2307,7 @@ func TestHelpVersion(t *testing.T) { } } -func TestAll(t *testing.T) { +func TestSorted(t *testing.T) { tests := []struct { newSet func(t *testing.T) *flag.Set test func(t *testing.T, set *flag.Set) @@ -2320,7 +2320,7 @@ func TestAll(t *testing.T) { }, test: func(t *testing.T, set *flag.Set) { // Iterator should yield no values - got := maps.Collect(set.All()) + got := maps.Collect(set.Sorted()) test.Equal(t, len(got), 0) }, }, @@ -2353,8 +2353,7 @@ func TestAll(t *testing.T) { return set }, test: func(t *testing.T, set *flag.Set) { - // Iterator should yield no values - next, stop := iter.Pull2(set.All()) + next, stop := iter.Pull2(set.Sorted()) defer stop() // Should now be in alphabetical order @@ -2400,6 +2399,74 @@ func TestAll(t *testing.T) { } } +func TestAll(t *testing.T) { + tests := []struct { + newSet func(t *testing.T) *flag.Set + test func(t *testing.T, set *flag.Set) + name string + }{ + { + name: "empty", + newSet: func(t *testing.T) *flag.Set { + return flag.NewSet() + }, + 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 { + set := flag.NewSet() + + verbose, err := flag.New(new(bool), "verbose", 'v', "Show verbose info", flag.Config[bool]{}) + test.Ok(t, err) + + debug, err := flag.New(new(bool), "debug", 'd', "Show debug info", flag.Config[bool]{}) + test.Ok(t, err) + + thing, err := flag.New(new(string), "thing", 't', "A thing", flag.Config[string]{}) + test.Ok(t, err) + + number, err := flag.New(new(int), "number", 'n', "Number of times", flag.Config[int]{}) + test.Ok(t, err) + + 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) { + // Should get everything back, but order is not deterministic + all := maps.Collect(set.All()) + + want := []string{"verbose", "debug", "thing", "number", "duration"} + slices.Sort(want) + + got := slices.Collect(maps.Keys(all)) + slices.Sort(got) + + test.EqualFunc(t, got, want, slices.Equal) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + set := tt.newSet(t) + tt.test(t, set) + }) + } +} + func BenchmarkParse(b *testing.B) { set := flag.NewSet() f, err := flag.New(new(int), "count", 'c', "Count something", flag.Config[int]{})