Dispatch to specific strconv formatters rather than %v for printing flag/arg types (#215)

* No longer use '%v' formatting, dispatch to faster, more specific strconv formatters

* Use the strings.Builder unsafe trick

* Include test coverage gaps

Author: FollowTheProcess

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.
 

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

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

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".

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