Add a Sorted iterator on flag set for help text and keep All unordered (#210)

Author: FollowTheProcess

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())

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.

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]{})