Address some simple todos (#73)

TODO.md

@@ -22,6 +22,8 @@ Things I want to do to make this library as good as it can be and a better, simp
 - [ ] Try this on some of my CLI tools to work out the bugs in real world programs
 - [ ] Test that shuffles the order of options and ensures the command we get is the same every time
 - [x] Use [vhs] to make a nice demo gif
+- [ ] Implement a count flag type so that `-vvv` sets the `verbose` flag to 3
+- [ ] Make `ExtraArgs` return `([]string, bool)`
 
 ## Ideas
 

internal/flag/flag.go

@@ -461,10 +461,7 @@ func validateFlagShort(short rune) error {
 
 // errParse is a helper to quickly return a consistent error in the face of flag
 // value parsing errors.
-//
-// TODO: A proper error type and use errors.Is/As during testing rather
-// than comparing string contents.
-func errParse[T any](name, str string, typ *T, err error) error {
+func errParse[T Flaggable](name, str string, typ *T, err error) error {
 	return fmt.Errorf(
 		"flag %q received invalid value %q (expected %T), detail: %w",
 		name,

internal/flag/set.go

@@ -33,7 +33,6 @@ func AddToSet[T Flaggable](set *Set, flag Flag[T]) error {
 	if set == nil {
 		return errors.New("cannot add flag to a nil set")
 	}
-	// TODO: Would this be better as a method on Flag[T]?
 	_, exists := set.flags[flag.name]
 	if exists {
 		return fmt.Errorf("flag %q already defined", flag.name)
@@ -140,8 +139,6 @@ func (s *Set) ExtraArgs() []string {
 }
 
 // Parse parses flags and their values from the command line.
-//
-// TODO: Currently thinks arg terminator "--" is just an empty flag.
 func (s *Set) Parse(args []string) (err error) {
 	if s == nil {
 		return errors.New("Parse called on a nil set")

internal/flag/set_test.go

@@ -17,8 +17,6 @@ var (
 	update = goflag.Bool("update", false, "Update golden files")
 )
 
-// TODO: Test cases where we use NoShorthand but pass shorthands, should return unrecognised flag error
-
 func TestParse(t *testing.T) {
 	tests := []struct {
 		name    string                            // The name of the test case
@@ -737,6 +735,65 @@ func TestParse(t *testing.T) {
 			args:    []string{"args", "-c=1", "more", "args"},
 			wantErr: false,
 		},
+		{
+			name: "no shorthand use long",
+			newSet: func(t *testing.T) *flag.Set {
+				t.Helper()
+				set := flag.NewSet()
+				f, err := flag.New(new(int), "count", flag.NoShortHand, 0, "Count something")
+				test.Ok(t, err)
+
+				err = flag.AddToSet(set, f)
+				test.Ok(t, err)
+
+				return set
+			},
+			test: func(t *testing.T, set *flag.Set) {
+				t.Helper()
+				// Get by name
+				entry, exists := set.Get("count")
+				test.True(t, exists)
+
+				test.Equal(t, entry.Value.Type(), "int")
+				test.Equal(t, entry.Value.String(), "1")
+
+				// Get by short
+				entry, exists = set.GetShort('c')
+				test.False(t, exists) // Short shouldn't exist
+			},
+			args:    []string{"--count", "1"},
+			wantErr: false,
+		},
+		{
+			name: "no shorthand use short",
+			newSet: func(t *testing.T) *flag.Set {
+				t.Helper()
+				set := flag.NewSet()
+				f, err := flag.New(new(int), "count", flag.NoShortHand, 0, "Count something")
+				test.Ok(t, err)
+
+				err = flag.AddToSet(set, f)
+				test.Ok(t, err)
+
+				return set
+			},
+			test: func(t *testing.T, set *flag.Set) {
+				t.Helper()
+				// Get by name
+				entry, exists := set.Get("count")
+				test.True(t, exists)
+
+				test.Equal(t, entry.Value.Type(), "int")
+				test.Equal(t, entry.Value.String(), "0")
+
+				// Get by short
+				entry, exists = set.GetShort('c')
+				test.False(t, exists) // Short shouldn't exist
+			},
+			args:    []string{"-c", "1"},
+			wantErr: true,
+			errMsg:  "unrecognised shorthand flag: -c",
+		},
 	}
 
 	for _, tt := range tests {

option.go

@@ -354,8 +354,7 @@ func Flag[T Flaggable](p *T, name string, short rune, value T, usage string) Opt
 
 		// Experimental flags
 		if err := flag.AddToSet(cfg.flags, f); err != nil {
-			// TODO: This error message is just for me debugging for now, make it more user friendly
-			return fmt.Errorf("xflags.Add: %w", err)
+			return fmt.Errorf("could not add flag %s to command: %w", name, err)
 		}
 
 		return nil