From 5dfedbfd623a4db78f951cc87c86e4b42672a79f Mon Sep 17 00:00:00 2001 From: Tom Fleet Date: Mon, 29 Jul 2024 21:17:32 +0100 Subject: [PATCH] Make flag shorthand a rune so bad values are a compile time error (#46) * Make flag shorthand a rune so bad values are a compile time error * Include NoShortHand in tests * Add note about NoShortHand * Add doc comment on validateFlagShort * Move parsing functions out of parse.go * Add another valid short test case --- command_test.go | 25 ++++---- examples/simple/cli.go | 6 +- internal/flag/flag.go | 113 ++++++++++++++++++++++++++++++++----- internal/flag/flag_test.go | 109 +++++++++++++++++++---------------- internal/flag/parse.go | 89 ----------------------------- option.go | 28 ++++++--- 6 files changed, 195 insertions(+), 175 deletions(-) delete mode 100644 internal/flag/parse.go diff --git a/command_test.go b/command_test.go index f8ab51b..324e0c9 100644 --- a/command_test.go +++ b/command_test.go @@ -3,7 +3,7 @@ package cli_test import ( "bytes" "errors" - "flag" + goflag "flag" "fmt" "os" "path/filepath" @@ -11,12 +11,13 @@ import ( "testing" "github.com/FollowTheProcess/cli" + "github.com/FollowTheProcess/cli/internal/flag" "github.com/FollowTheProcess/test" ) var ( - debug = flag.Bool("debug", false, "Print debug output during tests") - update = flag.Bool("update", false, "Update golden files") + debug = goflag.Bool("debug", false, "Print debug output during tests") + update = goflag.Bool("update", false, "Update golden files") ) func TestExecute(t *testing.T) { @@ -72,7 +73,7 @@ func TestExecute(t *testing.T) { fmt.Fprintf(cmd.Stdout(), "My arguments were: %v\nForce was: %v\n", args, force) return nil }), - cli.Flag(&force, "force", "f", false, "Force something"), + cli.Flag(&force, "force", 'f', false, "Force something"), } cmd, err := cli.New("test", slices.Concat(options, tt.options)...) @@ -173,8 +174,8 @@ func TestSubCommandExecute(t *testing.T) { return nil }), - cli.Flag(&force, "force", "f", false, "Force for sub1"), - cli.Flag(&something, "something", "s", "", "Something for sub1"), + cli.Flag(&force, "force", 'f', false, "Force for sub1"), + cli.Flag(&something, "something", 's', "", "Something for sub1"), ) test.Ok(t, err) @@ -191,8 +192,8 @@ func TestSubCommandExecute(t *testing.T) { ) return nil }), - cli.Flag(&deleteMe, "delete", "d", false, "Delete for sub2"), - cli.Flag(&number, "number", "n", -1, "Number for sub2"), + cli.Flag(&deleteMe, "delete", 'd', false, "Delete for sub2"), + cli.Flag(&number, "number", 'n', -1, "Number for sub2"), ) test.Ok(t, err) @@ -289,8 +290,8 @@ func TestHelp(t *testing.T) { cli.Short("A cool CLI to do things"), cli.Long("A longer, probably multiline description"), cli.SubCommands(sub1, sub2), - cli.Flag(new(bool), "delete", "d", false, "Delete something"), - cli.Flag(new(int), "count", "", -1, "Count something"), + cli.Flag(new(bool), "delete", 'd', false, "Delete something"), + cli.Flag(new(int), "count", flag.NoShortHand, -1, "Count something"), }, golden: "subcommands-flags.txt", wantErr: false, @@ -467,8 +468,8 @@ func TestOptionValidation(t *testing.T) { { name: "flag already exists", options: []cli.Option{ - cli.Flag(new(int), "count", "c", 0, "Count something"), - cli.Flag(new(int), "count", "c", 0, "Count something (again)"), + cli.Flag(new(int), "count", 'c', 0, "Count something"), + cli.Flag(new(int), "count", 'c', 0, "Count something (again)"), }, errMsg: `flag "count" already defined`, }, diff --git a/examples/simple/cli.go b/examples/simple/cli.go index bfe99e8..d9c43f9 100644 --- a/examples/simple/cli.go +++ b/examples/simple/cli.go @@ -41,9 +41,9 @@ func buildSayCommand() (*cli.Command, error) { cli.Example("Say a well known phrase", "demo say hello world"), cli.Example("Now louder", "demo say hello world --shout"), cli.Run(runSay(&options)), - cli.Flag(&options.shout, "shout", "s", false, "Say the message louder"), - cli.Flag(&options.count, "count", "c", 0, "Count the things"), - cli.Flag(&options.thing, "thing", "t", "", "Name of the thing"), + cli.Flag(&options.shout, "shout", 's', false, "Say the message louder"), + cli.Flag(&options.count, "count", 'c', 0, "Count the things"), + cli.Flag(&options.thing, "thing", 't', "", "Name of the thing"), ) if err != nil { return nil, err diff --git a/internal/flag/flag.go b/internal/flag/flag.go index b29f0af..c7f715b 100644 --- a/internal/flag/flag.go +++ b/internal/flag/flag.go @@ -35,6 +35,10 @@ const ( bits64 // 64 bit integer ) +// NoShortHand should be passed as the "short" argument to [New] if the desired flag +// should be the long hand version only e.g. --count, not -c/--count. +const NoShortHand = rune(-1) + var _ pflag.Value = Flag[string]{} // This will fail if we violate pflag.Value. // Flaggable is a type constraint that defines any type capable of being parsed as a command line flag. @@ -50,7 +54,7 @@ type Flag[T Flaggable] struct { value *T // The actual stored value name string // The name of the flag as appears on the command line, e.g. "force" for a --force flag usage string // One line description of the flag, e.g. "Force deletion without confirmation" - short string // Optional shorthand version of the flag, e.g. "f" for a -f flag + short rune // Optional shorthand version of the flag, e.g. "f" for a -f flag } // New constructs and returns a new [Flag]. @@ -61,8 +65,8 @@ type Flag[T Flaggable] struct { // If you want the flag to be longhand only, pass "" for short. // // var force bool -// flag.New(&force, "force", "f", false, "Force deletion without confirmation") -func New[T Flaggable](p *T, name string, short string, value T, usage string) (Flag[T], error) { +// flag.New(&force, "force", 'f', false, "Force deletion without confirmation") +func New[T Flaggable](p *T, name string, short rune, value T, usage string) (Flag[T], error) { if err := validateFlagName(name); err != nil { return Flag[T]{}, fmt.Errorf("invalid flag name: %w", err) } @@ -336,6 +340,18 @@ func (f Flag[T]) Set(str string) error { //nolint:gocyclo // No other way of doi } } +// signed is the same as constraints.Signed but we don't have to depend +// on golang/x/exp. +type signed interface { + ~int | ~int8 | ~int16 | ~int32 | ~int64 +} + +// unsigned is the same as constraints.Unsigned but we don't hve to depend +// on golang/x/exp. +type unsigned interface { + ~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64 | ~uintptr +} + // cast converts a *T1 to a *T2, we use it here when we know (via generics and compile time checks) // that e.g. the Flag.value is a string, but we can't directly do Flag.value = "value" because // we can't assign a string to a generic 'T', but we *know* that the value *is* a string because when @@ -397,19 +413,90 @@ func validateFlagName(name string) error { return nil } -func validateFlagShort(short string) error { - // len(short) > 1 means an error, shorthand must be a single character - if length := utf8.RuneCountInString(short); length > 1 { - return fmt.Errorf("must be a single ASCII letter, got %q which has %d letters", short, length) +// validateFlagShort ensures a flag's shorthand is valid, returning an error if it's not. +// +// Flag shorthands must be a single ASCII letter, we use rune as the type here rather than string because +// it enforces only a single character, so all we have to do is make sure it's a valid ASCII character. +func validateFlagShort(short rune) error { + // If it's the marker for long hand only, this is fine + if short == NoShortHand { + return nil + } + // Shorthand must be a valid ASCII letter + if short == utf8.RuneError || short > unicode.MaxASCII || !unicode.IsLetter(short) { + return fmt.Errorf("invalid character, must be a single ASCII letter, got %q", string(short)) } - if short != "" { - // Shorthand must be a valid ASCII letter - char, _ := utf8.DecodeRuneInString(short) - if char == utf8.RuneError || char > unicode.MaxASCII || !unicode.IsLetter(char) { - return fmt.Errorf("invalid character, must be a single ASCII letter, got %q", string(char)) + return nil +} + +// errParse is a helper to quickly return a consistent error in the face of flag +// value parsing errors. +func errParse[T any](name, str string, typ *T, err error) error { + return fmt.Errorf( + "flag %s received invalid value %q (expected %T), detail: %w", + name, + str, + *typ, + err, + ) +} + +// parseInt is a generic helper to parse all signed integers, given a bit size. +// +// It returns the parsed value or an error. +func parseInt[T signed](bits int) func(str string) (T, error) { + return func(str string) (T, error) { + val, err := strconv.ParseInt(str, 0, bits) + if err != nil { + return 0, err } + + return T(val), nil } +} - return nil +// parseUint is a generic helper to parse all signed integers, given a bit size. +// +// It returns the parsed value or an error. +func parseUint[T unsigned](bits int) func(str string) (T, error) { + return func(str string) (T, error) { + val, err := strconv.ParseUint(str, 0, bits) + if err != nil { + return 0, err + } + + return T(val), nil + } +} + +// parseFloat is a generic helper to parse floating point numbers, given a bit size. +// +// It returns the parsed value or an error. +func parseFloat[T ~float32 | ~float64](bits int) func(str string) (T, error) { + return func(str string) (T, error) { + val, err := strconv.ParseFloat(str, bits) + if err != nil { + return 0, err + } + + return T(val), nil + } +} + +// formatInt is a generic helper to return a string representation of any signed integer. +func formatInt[T signed](in T) string { + return strconv.FormatInt(int64(in), 10) +} + +// formatUint is a generic helper to return a string representation of any unsigned integer. +func formatUint[T unsigned](in T) string { + return strconv.FormatUint(uint64(in), 10) +} + +// formatFloat is a generic helper to return a string representation of any floating point digit. +func formatFloat[T ~float32 | ~float64](bits int) func(T) string { + return func(in T) string { + return strconv.FormatFloat(float64(in), 'g', -1, bits) + } } diff --git a/internal/flag/flag_test.go b/internal/flag/flag_test.go index 6bb166a..0af46d7 100644 --- a/internal/flag/flag_test.go +++ b/internal/flag/flag_test.go @@ -16,7 +16,7 @@ func TestFlagValue(t *testing.T) { // so strap in for a bunch of copy pasta t.Run("int valid", func(t *testing.T) { var i int - intFlag, err := flag.New(&i, "int", "i", 0, "Set an int value") + intFlag, err := flag.New(&i, "int", 'i', 0, "Set an int value") test.Ok(t, err) err = intFlag.Set("42") @@ -28,7 +28,7 @@ func TestFlagValue(t *testing.T) { t.Run("int invalid", func(t *testing.T) { var i int - intFlag, err := flag.New(&i, "int", "i", 0, "Set an int value") + intFlag, err := flag.New(&i, "int", 'i', 0, "Set an int value") test.Ok(t, err) err = intFlag.Set("word") @@ -42,7 +42,7 @@ func TestFlagValue(t *testing.T) { t.Run("int8 valid", func(t *testing.T) { var i int8 - intFlag, err := flag.New(&i, "int", "i", 0, "Set an int8 value") + intFlag, err := flag.New(&i, "int", 'i', 0, "Set an int8 value") test.Ok(t, err) err = intFlag.Set("42") @@ -54,7 +54,7 @@ func TestFlagValue(t *testing.T) { t.Run("int8 invalid", func(t *testing.T) { var i int8 - intFlag, err := flag.New(&i, "int", "i", 0, "Set an int8 value") + intFlag, err := flag.New(&i, "int", 'i', 0, "Set an int8 value") test.Ok(t, err) err = intFlag.Set("word") @@ -68,7 +68,7 @@ func TestFlagValue(t *testing.T) { t.Run("int16 valid", func(t *testing.T) { var i int16 - intFlag, err := flag.New(&i, "int", "i", 0, "Set an int16 value") + intFlag, err := flag.New(&i, "int", 'i', 0, "Set an int16 value") test.Ok(t, err) err = intFlag.Set("42") @@ -80,7 +80,7 @@ func TestFlagValue(t *testing.T) { t.Run("int16 invalid", func(t *testing.T) { var i int16 - intFlag, err := flag.New(&i, "int", "i", 0, "Set an int16 value") + intFlag, err := flag.New(&i, "int", 'i', 0, "Set an int16 value") test.Ok(t, err) err = intFlag.Set("word") @@ -94,7 +94,7 @@ func TestFlagValue(t *testing.T) { t.Run("int32 valid", func(t *testing.T) { var i int32 - intFlag, err := flag.New(&i, "int", "i", 0, "Set an int32 value") + intFlag, err := flag.New(&i, "int", 'i', 0, "Set an int32 value") test.Ok(t, err) err = intFlag.Set("42") @@ -106,7 +106,7 @@ func TestFlagValue(t *testing.T) { t.Run("int32 invalid", func(t *testing.T) { var i int32 - intFlag, err := flag.New(&i, "int", "i", 0, "Set an int32 value") + intFlag, err := flag.New(&i, "int", 'i', 0, "Set an int32 value") test.Ok(t, err) err = intFlag.Set("word") @@ -120,7 +120,7 @@ func TestFlagValue(t *testing.T) { t.Run("int64 valid", func(t *testing.T) { var i int64 - intFlag, err := flag.New(&i, "int", "i", 0, "Set an int64 value") + intFlag, err := flag.New(&i, "int", 'i', 0, "Set an int64 value") test.Ok(t, err) err = intFlag.Set("42") @@ -132,7 +132,7 @@ func TestFlagValue(t *testing.T) { t.Run("int64 invalid", func(t *testing.T) { var i int64 - intFlag, err := flag.New(&i, "int", "i", 0, "Set an int64 value") + intFlag, err := flag.New(&i, "int", 'i', 0, "Set an int64 value") test.Ok(t, err) err = intFlag.Set("word") @@ -146,7 +146,7 @@ func TestFlagValue(t *testing.T) { t.Run("uint valid", func(t *testing.T) { var i uint - intFlag, err := flag.New(&i, "uint", "i", 0, "Set a uint value") + intFlag, err := flag.New(&i, "uint", 'i', 0, "Set a uint value") test.Ok(t, err) err = intFlag.Set("42") @@ -158,7 +158,7 @@ func TestFlagValue(t *testing.T) { t.Run("uint invalid", func(t *testing.T) { var i uint - intFlag, err := flag.New(&i, "uint", "i", 0, "Set a uint value") + intFlag, err := flag.New(&i, "uint", 'i', 0, "Set a uint value") test.Ok(t, err) err = intFlag.Set("word") @@ -172,7 +172,7 @@ func TestFlagValue(t *testing.T) { t.Run("uint8 valid", func(t *testing.T) { var i uint8 - intFlag, err := flag.New(&i, "uint", "i", 0, "Set a uint8 value") + intFlag, err := flag.New(&i, "uint", 'i', 0, "Set a uint8 value") test.Ok(t, err) err = intFlag.Set("42") @@ -184,7 +184,7 @@ func TestFlagValue(t *testing.T) { t.Run("uint8 invalid", func(t *testing.T) { var i uint8 - intFlag, err := flag.New(&i, "uint", "i", 0, "Set a uint8 value") + intFlag, err := flag.New(&i, "uint", 'i', 0, "Set a uint8 value") test.Ok(t, err) err = intFlag.Set("word") @@ -198,7 +198,7 @@ func TestFlagValue(t *testing.T) { t.Run("uint16 valid", func(t *testing.T) { var i uint16 - intFlag, err := flag.New(&i, "uint", "i", 0, "Set a uint16 value") + intFlag, err := flag.New(&i, "uint", 'i', 0, "Set a uint16 value") test.Ok(t, err) err = intFlag.Set("42") @@ -210,7 +210,7 @@ func TestFlagValue(t *testing.T) { t.Run("uint16 invalid", func(t *testing.T) { var i uint16 - intFlag, err := flag.New(&i, "uint", "i", 0, "Set a uint16 value") + intFlag, err := flag.New(&i, "uint", 'i', 0, "Set a uint16 value") test.Ok(t, err) err = intFlag.Set("word") @@ -224,7 +224,7 @@ func TestFlagValue(t *testing.T) { t.Run("uint32 valid", func(t *testing.T) { var i uint32 - intFlag, err := flag.New(&i, "uint", "i", 0, "Set a uint32 value") + intFlag, err := flag.New(&i, "uint", 'i', 0, "Set a uint32 value") test.Ok(t, err) err = intFlag.Set("42") @@ -236,7 +236,7 @@ func TestFlagValue(t *testing.T) { t.Run("uint32 invalid", func(t *testing.T) { var i uint32 - intFlag, err := flag.New(&i, "uint", "i", 0, "Set a uint32 value") + intFlag, err := flag.New(&i, "uint", 'i', 0, "Set a uint32 value") test.Ok(t, err) err = intFlag.Set("word") @@ -250,7 +250,7 @@ func TestFlagValue(t *testing.T) { t.Run("uint64 valid", func(t *testing.T) { var i uint64 - intFlag, err := flag.New(&i, "uint", "i", 0, "Set a uint64 value") + intFlag, err := flag.New(&i, "uint", 'i', 0, "Set a uint64 value") test.Ok(t, err) err = intFlag.Set("42") @@ -262,7 +262,7 @@ func TestFlagValue(t *testing.T) { t.Run("uint64 invalid", func(t *testing.T) { var i uint64 - intFlag, err := flag.New(&i, "uint", "i", 0, "Set a uint64 value") + intFlag, err := flag.New(&i, "uint", 'i', 0, "Set a uint64 value") test.Ok(t, err) err = intFlag.Set("word") @@ -276,7 +276,7 @@ func TestFlagValue(t *testing.T) { t.Run("uintptr valid", func(t *testing.T) { var i uintptr - intFlag, err := flag.New(&i, "uintptr", "i", 0, "Set a uintptr value") + intFlag, err := flag.New(&i, "uintptr", 'i', 0, "Set a uintptr value") test.Ok(t, err) err = intFlag.Set("42") @@ -288,7 +288,7 @@ func TestFlagValue(t *testing.T) { t.Run("uintptr invalid", func(t *testing.T) { var i uintptr - intFlag, err := flag.New(&i, "uintptr", "i", 0, "Set a uintptr value") + intFlag, err := flag.New(&i, "uintptr", 'i', 0, "Set a uintptr value") test.Ok(t, err) err = intFlag.Set("word") @@ -302,7 +302,7 @@ func TestFlagValue(t *testing.T) { t.Run("float32 valid", func(t *testing.T) { var f float32 - floatFlag, err := flag.New(&f, "float", "f", 0, "Set a float32 value") + floatFlag, err := flag.New(&f, "float", 'f', 0, "Set a float32 value") test.Ok(t, err) err = floatFlag.Set("3.14159") @@ -314,7 +314,7 @@ func TestFlagValue(t *testing.T) { t.Run("float32 invalid", func(t *testing.T) { var f float32 - floatFlag, err := flag.New(&f, "float", "f", 0, "Set a float32 value") + floatFlag, err := flag.New(&f, "float", 'f', 0, "Set a float32 value") test.Ok(t, err) err = floatFlag.Set("word") @@ -328,7 +328,7 @@ func TestFlagValue(t *testing.T) { t.Run("float64 valid", func(t *testing.T) { var f float64 - floatFlag, err := flag.New(&f, "float", "f", 0, "Set a float64 value") + floatFlag, err := flag.New(&f, "float", 'f', 0, "Set a float64 value") test.Ok(t, err) err = floatFlag.Set("3.14159") @@ -340,7 +340,7 @@ func TestFlagValue(t *testing.T) { t.Run("float64 invalid", func(t *testing.T) { var f float64 - floatFlag, err := flag.New(&f, "float", "f", 0, "Set a float64 value") + floatFlag, err := flag.New(&f, "float", 'f', 0, "Set a float64 value") test.Ok(t, err) err = floatFlag.Set("word") @@ -354,7 +354,7 @@ func TestFlagValue(t *testing.T) { t.Run("bool valid", func(t *testing.T) { var b bool - boolFlag, err := flag.New(&b, "bool", "b", false, "Set a bool value") + boolFlag, err := flag.New(&b, "bool", 'b', false, "Set a bool value") test.Ok(t, err) err = boolFlag.Set("true") @@ -366,7 +366,7 @@ func TestFlagValue(t *testing.T) { t.Run("bool invalid", func(t *testing.T) { var b bool - boolFlag, err := flag.New(&b, "bool", "b", false, "Set a bool value") + boolFlag, err := flag.New(&b, "bool", 'b', false, "Set a bool value") test.Ok(t, err) err = boolFlag.Set("word") @@ -382,7 +382,7 @@ func TestFlagValue(t *testing.T) { // getting an error here t.Run("string", func(t *testing.T) { var str string - strFlag, err := flag.New(&str, "string", "s", "", "Set a string value") + strFlag, err := flag.New(&str, "string", 's', "", "Set a string value") test.Ok(t, err) err = strFlag.Set("newvalue") @@ -394,7 +394,7 @@ func TestFlagValue(t *testing.T) { t.Run("byte slice valid", func(t *testing.T) { var byt []byte - byteFlag, err := flag.New(&byt, "byte", "b", []byte(""), "Set a byte slice value") + byteFlag, err := flag.New(&byt, "byte", 'b', []byte(""), "Set a byte slice value") test.Ok(t, err) err = byteFlag.Set("5e") @@ -406,7 +406,7 @@ func TestFlagValue(t *testing.T) { t.Run("byte slice invalid", func(t *testing.T) { var byt []byte - byteFlag, err := flag.New(&byt, "byte", "b", []byte(""), "Set a byte slice value") + byteFlag, err := flag.New(&byt, "byte", 'b', []byte(""), "Set a byte slice value") test.Ok(t, err) err = byteFlag.Set("0xF") @@ -420,7 +420,7 @@ func TestFlagValue(t *testing.T) { t.Run("time.Time valid", func(t *testing.T) { var tyme time.Time - timeFlag, err := flag.New(&tyme, "time", "t", time.Now(), "Set a time value") + timeFlag, err := flag.New(&tyme, "time", 't', time.Now(), "Set a time value") test.Ok(t, err) err = timeFlag.Set("2024-07-17T07:38:05Z") @@ -435,7 +435,7 @@ func TestFlagValue(t *testing.T) { t.Run("time.Time invalid", func(t *testing.T) { var tyme time.Time - timeFlag, err := flag.New(&tyme, "time", "t", time.Now(), "Set a time value") + timeFlag, err := flag.New(&tyme, "time", 't', time.Now(), "Set a time value") test.Ok(t, err) err = timeFlag.Set("not a time") @@ -452,7 +452,7 @@ func TestFlagValue(t *testing.T) { durationFlag, err := flag.New( &duration, "duration", - "d", + 'd', time.Duration(0), "Set a duration value", ) @@ -473,7 +473,7 @@ func TestFlagValue(t *testing.T) { durationFlag, err := flag.New( &duration, "duration", - "d", + 'd', time.Duration(0), "Set a duration value", ) @@ -490,7 +490,7 @@ func TestFlagValue(t *testing.T) { t.Run("ip valid", func(t *testing.T) { var ip net.IP - ipFlag, err := flag.New(&ip, "ip", "i", nil, "Set an IP address") + ipFlag, err := flag.New(&ip, "ip", 'i', nil, "Set an IP address") test.Ok(t, err) err = ipFlag.Set("192.0.2.1") @@ -502,7 +502,7 @@ func TestFlagValue(t *testing.T) { t.Run("ip invalid", func(t *testing.T) { var ip net.IP - ipFlag, err := flag.New(&ip, "ip", "i", nil, "Set an IP address") + ipFlag, err := flag.New(&ip, "ip", 'i', nil, "Set an IP address") test.Ok(t, err) err = ipFlag.Set("not an ip") @@ -515,14 +515,32 @@ func TestFlagValue(t *testing.T) { }) } -func TestFlagNameValidation(t *testing.T) { +func TestFlagValidation(t *testing.T) { tests := []struct { name string // Name of the test case flagName string // Input flag name - short string // Flag shorthand errMsg string // If we wanted an error, what should it say + short rune // Flag shorthand wantErr bool // Whether we want an error }{ + { + name: "short is uppercase", + flagName: "delete", + short: 'D', + wantErr: false, + }, + { + name: "valid short", + flagName: "delete", + short: 'd', + wantErr: false, + }, + { + name: "no shorthand", + flagName: "delete", + short: flag.NoShortHand, + wantErr: false, + }, { name: "empty name", flagName: "", @@ -571,24 +589,17 @@ func TestFlagNameValidation(t *testing.T) { wantErr: true, errMsg: `invalid flag name: non ascii character: "語"`, }, - { - name: "short too long", - flagName: "delete", - short: "word", - wantErr: true, - errMsg: `invalid shorthand for flag "delete": must be a single ASCII letter, got "word" which has 4 letters`, - }, { name: "short is digit", flagName: "delete", - short: "7", + short: '7', wantErr: true, errMsg: `invalid shorthand for flag "delete": invalid character, must be a single ASCII letter, got "7"`, }, { name: "short is non ascii", flagName: "delete", - short: "本", + short: '本', wantErr: true, errMsg: `invalid shorthand for flag "delete": invalid character, must be a single ASCII letter, got "本"`, }, @@ -611,7 +622,7 @@ func TestFlagNilSafety(t *testing.T) { // Should be impossible to make a nil pointer dereference when using .New var bang *bool - flag, err := flag.New(bang, "bang", "b", false, "Nil go bang?") + flag, err := flag.New(bang, "bang", 'b', false, "Nil go bang?") test.Ok(t, err) test.False(t, flag.Get()) diff --git a/internal/flag/parse.go b/internal/flag/parse.go deleted file mode 100644 index d9a6de7..0000000 --- a/internal/flag/parse.go +++ /dev/null @@ -1,89 +0,0 @@ -package flag - -import ( - "fmt" - "strconv" -) - -// signed is the same as constraints.Signed but we don't have to depend -// on golang/x/exp. -type signed interface { - ~int | ~int8 | ~int16 | ~int32 | ~int64 -} - -// unsigned is the same as constraints.Unsigned but we don't hve to depend -// on golang/x/exp. -type unsigned interface { - ~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64 | ~uintptr -} - -// errParse is a helper to quickly return a consistent error in the face of flag -// value parsing errors. -func errParse[T any](name, str string, typ *T, err error) error { - return fmt.Errorf( - "flag %s received invalid value %q (expected %T), detail: %w", - name, - str, - *typ, - err, - ) -} - -// parseInt is a generic helper to parse all signed integers, given a bit size. -// -// It returns the parsed value or an error. -func parseInt[T signed](bits int) func(str string) (T, error) { - return func(str string) (T, error) { - val, err := strconv.ParseInt(str, 0, bits) - if err != nil { - return 0, err - } - - return T(val), nil - } -} - -// parseUint is a generic helper to parse all signed integers, given a bit size. -// -// It returns the parsed value or an error. -func parseUint[T unsigned](bits int) func(str string) (T, error) { - return func(str string) (T, error) { - val, err := strconv.ParseUint(str, 0, bits) - if err != nil { - return 0, err - } - - return T(val), nil - } -} - -// parseFloat is a generic helper to parse floating point numbers, given a bit size. -// -// It returns the parsed value or an error. -func parseFloat[T ~float32 | ~float64](bits int) func(str string) (T, error) { - return func(str string) (T, error) { - val, err := strconv.ParseFloat(str, bits) - if err != nil { - return 0, err - } - - return T(val), nil - } -} - -// formatInt is a generic helper to return a string representation of any signed integer. -func formatInt[T signed](in T) string { - return strconv.FormatInt(int64(in), 10) -} - -// formatUint is a generic helper to return a string representation of any unsigned integer. -func formatUint[T unsigned](in T) string { - return strconv.FormatUint(uint64(in), 10) -} - -// formatFloat is a generic helper to return a string representation of any floating point digit. -func formatFloat[T ~float32 | ~float64](bits int) func(T) string { - return func(in T) string { - return strconv.FormatFloat(float64(in), 'g', -1, bits) - } -} diff --git a/option.go b/option.go index c4cb4e7..e16e3b3 100644 --- a/option.go +++ b/option.go @@ -327,36 +327,46 @@ func Allow(validator ArgValidator) Option { // The variable is set when the flag is parsed during command execution. The value provided // in the call to [Flag] is used as the default value. // -// To add a long flag only (e.g. --delete with no -d option), simply pass "" for short. +// To add a long flag only (e.g. --delete with no -d option), pass [flag.NoShortHand] for "short". // // // Add a force flag // var force bool -// cli.New("rm", cli.Flag(&force, "force", "f", false, "Force deletion without confirmation")) -func Flag[T flag.Flaggable](p *T, name string, short string, value T, usage string) Option { +// cli.New("rm", cli.Flag(&force, "force", 'f', false, "Force deletion without confirmation")) +func Flag[T flag.Flaggable](p *T, name string, short rune, value T, usage string) Option { + // TODO: Currently flag is internal, which means users couldn't pass flag.NoShortHand if they wanted to + // once flag is ready with our own parsing stuff, make it part of the public package and remove spf13/pflag f := func(cfg *config) error { if cfg.flags.Lookup(name) != nil { return fmt.Errorf("flag %q already defined", name) } - // Short is now either "" or a single letter - flag, err := flag.New(p, name, short, value, usage) + f, err := flag.New(p, name, short, value, usage) if err != nil { return err } var defVal string - if flag.Type() == "bool" { + if f.Type() == "bool" { defVal = "true" } + // If we've been told we don't want a shorthand, set it to "" which tells pflag + // not to give it one, otherwise use what we've been given + var shortHand string + if short == flag.NoShortHand { + shortHand = "" + } else { + shortHand = string(short) + } + // Annoyingly pflag does the same checks we've done above but will panic on error, hopefully // the above checks will come in handy when I implement my own flag parsing cfg.flags.AddFlag(&pflag.Flag{ Name: name, - Shorthand: short, + Shorthand: shortHand, Usage: usage, - Value: flag, - DefValue: flag.String(), + Value: f, + DefValue: f.String(), NoOptDefVal: defVal, }) return nil -- knotmirror 0.0.1