diff --git a/README.md b/README.md index 914c759..e1bba50 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,7 @@ cli.Flag(&force, "force", 'f', "Force deletion", cli.Env[bool]("MYTOOL_FORCE")) > [!NOTE] > `cli.Env` requires an explicit type parameter because Go cannot infer it from the string argument alone. -> The compiler enforces that the type matches the flag — `cli.Env[string](...)` on a `bool` flag is a compile error. +> The compiler enforces that the type matches the flag, `cli.Env[string](...)` on a `bool` flag is a compile error. When `MYTOOL_FORCE=true` is set in the environment, `--force` is implied. Passing `--force=false` on the command line always wins. diff --git a/internal/flag/flag.go b/internal/flag/flag.go index 11fa996..ad040fd 100644 --- a/internal/flag/flag.go +++ b/internal/flag/flag.go @@ -682,7 +682,7 @@ func validateFlagShort(short rune) error { // // "ish" means that empty slices will return true despite their official zero // value being nil. The primary use is to determine whether a default value is -// worth displaying to the user in the help text — an empty slice is probably +// worth displaying to the user in the help text, an empty slice is probably // not. // //nolint:cyclop // Not much else we can do here diff --git a/internal/flag/set_test.go b/internal/flag/set_test.go index 2ee1cac..b230b3c 100644 --- a/internal/flag/set_test.go +++ b/internal/flag/set_test.go @@ -984,7 +984,7 @@ func TestParse(t *testing.T) { test: func(t *testing.T, set *flag.Set) { f, exists := set.Get("verbosity") test.True(t, exists) - // Env var contributes 2, CLI contributes 1 more — total 3 + // Env var contributes 2, CLI contributes 1 more, total 3 test.Equal(t, f.String(), "3") }, args: []string{"--verbosity"}, @@ -1633,9 +1633,6 @@ func TestParse(t *testing.T) { { name: "slice env var splits on every comma (no escape mechanism)", newSet: func(t *testing.T) *flag.Set { - // Any comma in an env var value is interpreted as a separator — - // there is no way to embed a literal comma in a slice item. - // Users needing commas should pass values via --flag one,two. t.Setenv("MYTOOL_ITEMS", "a,b,c") var val []string @@ -1845,7 +1842,7 @@ func TestParse(t *testing.T) { { name: "combined short flags where early flag needs value captures rest", newSet: func(t *testing.T) *flag.Set { - // -fgh — f is non-bool so consumes the rest of the cluster as its value + // -fgh. f is non-bool so consumes the rest of the cluster as its value // i.e. f gets "gh", and g/h are not parsed as flags. set := flag.NewSet() diff --git a/internal/flag/value.go b/internal/flag/value.go index 97067b5..9e2c568 100644 --- a/internal/flag/value.go +++ b/internal/flag/value.go @@ -33,7 +33,7 @@ type Value interface { // IsSlice reports whether the flag holds a slice value that accumulates // repeated calls to Set (e.g. []string, []int). Note that []byte and net.IP - // are NOT slice flags in this sense — they are parsed atomically. + // are NOT slice flags in this sense, they are parsed atomically. IsSlice() bool // Set sets the stored value of a flag by parsing the string "str". diff --git a/internal/format/format.go b/internal/format/format.go index 754a89f..c1d3fc4 100644 --- a/internal/format/format.go +++ b/internal/format/format.go @@ -4,10 +4,8 @@ package format import ( - "fmt" - "reflect" "strconv" - "strings" + "unsafe" "go.followtheprocess.codes/cli/internal/constraints" ) @@ -17,6 +15,17 @@ const ( floatFmt = 'g' floatPrecision = -1 slice = "[]" + + // Capacity hints used to pre-size []byte buffers in the slice formatters. + // The "brackets" pair is the leading '[' and trailing ']'. + // + // The per-element hints are good enough default cases to minimise the + // buffer growing and re-allocating. + bracketsCap = 2 + intElemHint = 4 // "-12, " + floatElemHint = 8 // "-1.234, " + boolElemHint = 7 // "false, " + stringElemHint = 4 // surrounding quotes plus ", " ) const ( @@ -99,39 +108,145 @@ func Float64(f float64) string { // Slice([]int{1, 2, 3, 4}) // "[1, 2, 3, 4]" // Slice([]string{"one", "two", "three"}) // `["one", "two", "three"]` func Slice[T any](s []T) string { - length := len(s) + if len(s) == 0 { + return slice + } - if length == 0 { - // If it's empty or nil, avoid doing the work below - // and just return "[]" + switch v := any(s).(type) { + case []string: + return formatStringSlice(v) + case []bool: + return formatBoolSlice(v) + case []int: + return formatSignedSlice(v) + case []int8: + return formatSignedSlice(v) + case []int16: + return formatSignedSlice(v) + case []int32: + return formatSignedSlice(v) + case []int64: + return formatSignedSlice(v) + case []uint: + return formatUnsignedSlice(v) + case []uint16: + return formatUnsignedSlice(v) + case []uint32: + return formatUnsignedSlice(v) + case []uint64: + return formatUnsignedSlice(v) + case []float32: + return formatFloat32Slice(v) + case []float64: + return formatFloat64Slice(v) + default: return slice } +} + +// toString casts b to a string by reinterpreting the bytes. +// +// This is the same trick [strings.Builder.String] uses to avoid the +// allocation of doing `string(b)`. The caveat is b MUST not be mutated +// after passing to this function. +// +// This is fine in our case here as the []byte buffer is created in the function body +// and never escapes. +func toString(b []byte) string { + return unsafe.String(unsafe.SliceData(b), len(b)) +} + +func formatSignedSlice[T constraints.Signed](s []T) string { + buf := make([]byte, 0, bracketsCap+len(s)*intElemHint) + buf = append(buf, '[') + buf = strconv.AppendInt(buf, int64(s[0]), base10) + + for _, e := range s[1:] { + buf = append(buf, ", "...) + buf = strconv.AppendInt(buf, int64(e), base10) + } + + buf = append(buf, ']') + + return toString(buf) +} + +func formatUnsignedSlice[T constraints.Unsigned](s []T) string { + buf := make([]byte, 0, bracketsCap+len(s)*intElemHint) + buf = append(buf, '[') + buf = strconv.AppendUint(buf, uint64(s[0]), base10) + + for _, e := range s[1:] { + buf = append(buf, ", "...) + buf = strconv.AppendUint(buf, uint64(e), base10) + } - builder := &strings.Builder{} - builder.WriteByte('[') + buf = append(buf, ']') - typ := reflect.TypeFor[T]().Kind() + return toString(buf) +} + +func formatFloat32Slice(s []float32) string { + buf := make([]byte, 0, bracketsCap+len(s)*floatElemHint) + buf = append(buf, '[') + buf = strconv.AppendFloat(buf, float64(s[0]), floatFmt, floatPrecision, bits32) - first := fmt.Sprintf("%v", s[0]) - if typ == reflect.String { - first = strconv.Quote(first) + for _, e := range s[1:] { + buf = append(buf, ", "...) + buf = strconv.AppendFloat(buf, float64(e), floatFmt, floatPrecision, bits32) } - builder.WriteString(first) + buf = append(buf, ']') + + return toString(buf) +} + +func formatFloat64Slice(s []float64) string { + buf := make([]byte, 0, bracketsCap+len(s)*floatElemHint) + buf = append(buf, '[') + buf = strconv.AppendFloat(buf, s[0], floatFmt, floatPrecision, bits64) - for _, element := range s[1:] { - builder.WriteString(", ") + for _, e := range s[1:] { + buf = append(buf, ", "...) + buf = strconv.AppendFloat(buf, e, floatFmt, floatPrecision, bits64) + } + + buf = append(buf, ']') + + return toString(buf) +} + +func formatStringSlice(s []string) string { + capacity := bracketsCap + for _, e := range s { + capacity += len(e) + stringElemHint + } + + buf := make([]byte, 0, capacity) + buf = append(buf, '[') + buf = strconv.AppendQuote(buf, s[0]) + + for _, e := range s[1:] { + buf = append(buf, ", "...) + buf = strconv.AppendQuote(buf, e) + } + + buf = append(buf, ']') + + return toString(buf) +} - str := fmt.Sprintf("%v", element) - if typ == reflect.String { - // If it's a string, quote it - str = strconv.Quote(str) - } +func formatBoolSlice(s []bool) string { + buf := make([]byte, 0, bracketsCap+len(s)*boolElemHint) + buf = append(buf, '[') + buf = strconv.AppendBool(buf, s[0]) - builder.WriteString(str) + for _, e := range s[1:] { + buf = append(buf, ", "...) + buf = strconv.AppendBool(buf, e) } - builder.WriteByte(']') + buf = append(buf, ']') - return builder.String() + return toString(buf) } diff --git a/internal/format/format_test.go b/internal/format/format_test.go index 4b23456..9ad97e7 100644 --- a/internal/format/format_test.go +++ b/internal/format/format_test.go @@ -63,17 +63,179 @@ func TestFloat64(t *testing.T) { } func TestSlice(t *testing.T) { - oneString := []string{"one"} - twoStrings := []string{"one", "two"} - strings := []string{"one", "two", "three"} - ints := []int{1, 2, 3} - floats := []float64{1.0, 2.0, 3.0} - bools := []bool{true, true, false} - - test.Equal(t, Slice(oneString), `["one"]`) - test.Equal(t, Slice(twoStrings), `["one", "two"]`) - test.Equal(t, Slice(strings), `["one", "two", "three"]`, test.Context("strings")) - test.Equal(t, Slice(ints), "[1, 2, 3]", test.Context("ints")) - test.Equal(t, Slice(floats), "[1, 2, 3]", test.Context("floats")) - test.Equal(t, Slice(bools), "[true, true, false]", test.Context("bools")) + tests := []struct { + got func() string + name string + want string + }{ + { + name: "nil string slice", + got: func() string { return Slice([]string(nil)) }, + want: "[]", + }, + { + name: "empty int slice", + got: func() string { return Slice([]int{}) }, + want: "[]", + }, + { + name: "one string", + got: func() string { return Slice([]string{"one"}) }, + want: `["one"]`, + }, + { + name: "two strings", + got: func() string { return Slice([]string{"one", "two"}) }, + want: `["one", "two"]`, + }, + { + name: "three strings", + got: func() string { return Slice([]string{"one", "two", "three"}) }, + want: `["one", "two", "three"]`, + }, + { + name: "strings with escapes", + got: func() string { return Slice([]string{"hi\nthere", "tab\there", `quote"here`}) }, + want: `["hi\nthere", "tab\there", "quote\"here"]`, + }, + { + name: "empty string element", + got: func() string { return Slice([]string{""}) }, + want: `[""]`, + }, + { + name: "ints", + got: func() string { return Slice([]int{1, 2, 3}) }, + want: "[1, 2, 3]", + }, + { + name: "negative ints", + got: func() string { return Slice([]int{-1, 0, 1}) }, + want: "[-1, 0, 1]", + }, + { + name: "int8s", + got: func() string { return Slice([]int8{-128, 0, 127}) }, + want: "[-128, 0, 127]", + }, + { + name: "int16s", + got: func() string { return Slice([]int16{-32768, 0, 32767}) }, + want: "[-32768, 0, 32767]", + }, + { + name: "int32s", + got: func() string { return Slice([]int32{-2147483648, 0, 2147483647}) }, + want: "[-2147483648, 0, 2147483647]", + }, + { + name: "int64s", + got: func() string { return Slice([]int64{-1 << 62, 0, 1 << 62}) }, + want: "[-4611686018427387904, 0, 4611686018427387904]", + }, + { + name: "uints", + got: func() string { return Slice([]uint{1, 2, 3}) }, + want: "[1, 2, 3]", + }, + { + name: "uint16s", + got: func() string { return Slice([]uint16{0, 1, 65535}) }, + want: "[0, 1, 65535]", + }, + { + name: "uint32s", + got: func() string { return Slice([]uint32{0, 1, 4294967295}) }, + want: "[0, 1, 4294967295]", + }, + { + name: "uint64s", + got: func() string { return Slice([]uint64{0, 1, 1 << 63}) }, + want: "[0, 1, 9223372036854775808]", + }, + { + name: "floats", + got: func() string { return Slice([]float64{1.0, 2.0, 3.0}) }, + want: "[1, 2, 3]", + }, + { + name: "floats with decimals", + got: func() string { return Slice([]float64{1.5, -2.25, 3.125}) }, + want: "[1.5, -2.25, 3.125]", + }, + { + name: "float32s", + got: func() string { return Slice([]float32{1.5, -2.25, 3.125}) }, + want: "[1.5, -2.25, 3.125]", + }, + { + name: "bools", + got: func() string { return Slice([]bool{true, true, false}) }, + want: "[true, true, false]", + }, + { + name: "unsupported type falls through to empty", + got: func() string { return Slice([]uintptr{1, 2, 3}) }, + want: "[]", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + test.Equal(t, tt.got(), tt.want) + }) + } +} + +func BenchmarkSlice(b *testing.B) { + ints := []int{1, 2, 3, 4, 5, 6, 7, 8} + int64s := []int64{1, 2, 3, 4, 5, 6, 7, 8} + uints := []uint{1, 2, 3, 4, 5, 6, 7, 8} + floats := []float64{1.5, 2.5, 3.5, 4.5, 5.5, 6.5, 7.5, 8.5} + strs := []string{"alpha", "beta", "gamma", "delta", "epsilon", "zeta", "eta", "theta"} + bools := []bool{true, false, true, false, true, false, true, false} + + b.Run("ints", func(b *testing.B) { + for b.Loop() { + _ = Slice(ints) + } + }) + + b.Run("int64s", func(b *testing.B) { + for b.Loop() { + _ = Slice(int64s) + } + }) + + b.Run("uints", func(b *testing.B) { + for b.Loop() { + _ = Slice(uints) + } + }) + + b.Run("floats", func(b *testing.B) { + for b.Loop() { + _ = Slice(floats) + } + }) + + b.Run("strings", func(b *testing.B) { + for b.Loop() { + _ = Slice(strs) + } + }) + + b.Run("bools", func(b *testing.B) { + for b.Loop() { + _ = Slice(bools) + } + }) + + b.Run("empty", func(b *testing.B) { + var s []int + + for b.Loop() { + _ = Slice(s) + } + }) } diff --git a/mise.toml b/mise.toml index 1bbe768..4d73b1a 100644 --- a/mise.toml +++ b/mise.toml @@ -41,11 +41,7 @@ sources = ["**/*.go"] [tasks.lint] description = "Run the linters and auto-fix if possible" depends = ["fmt"] -run = [ - "golangci-lint run --fix", - "typos", - "nilaway ./...", -] +run = ["golangci-lint run --fix", "typos", "nilaway ./..."] sources = ["**/*.go", ".golangci.yml"] [tasks.docs] @@ -86,14 +82,8 @@ run = "fd . -e go | xargs wc -l | sort -nr | head" [tasks.clean] description = "Remove build artifacts and other clutter" -run = [ - "go clean ./...", - "rm -rf *.out", -] +run = ["go clean ./...", "rm -rf *.out"] [tasks.update] description = "Updates dependencies in go.mod and go.sum" -run = [ - "go get -u ./...", - "go mod tidy", -] +run = ["go get -u ./...", "go mod tidy"]