Repository navigation
Treat empty DefValue as zero value in defaultIsZeroValue - #517
AdamMagued wants to merge 2 commits into
Conversation
When DefValue is set to an empty string (e.g. to suppress the default indicator in usage output), defaultIsZeroValue did not recognize it as a zero value for slice/array flags and other types whose explicit type cases checked only for literal zero representations like "[]" or "0". As a result, usage strings displayed an awkward "(default )" suffix. Treating DefValue == "" as a zero value upfront ensures consistent handling across all flag types, matching the existing behavior of stringValue and custom Value implementations in the default branch.
|
There have been several issues about how parsing the empty string as a parameter to a slice valued flag doesn't result in an empty slice. I don't know off the top of my head how those code paths might interact with this one, so I'd have to check that before I can pass any judgment on this proposed change. |
|
Thanks for checking this @tomasaschan. To clarify how this interacts with slice parsing: It does not participate in argument parsing, This modification only ensures that if a caller explicitly clears |
|
My point is, what happens to a slice valued flag if/when the user sets |
|
Setting
|
Problem
When setting
flag.DefValue = ""(e.g. to suppress the default value in usage/help strings for flags with multiline descriptions),defaultIsZeroValue()did not recognize an empty string as a zero value for slice/array types (*stringSliceValue,*intSliceValue,*stringArrayValue). Their type switch cases explicitly checked onlyf.DefValue == "[]".Because
defaultIsZeroValue()returned false,FlagUsagesWrappedformatted the default value usingfmt.Sprintf(" (default %s)", flag.DefValue), resulting in an awkward(default )suffix in the generated usage text.Root Cause
In
flag.go,defaultIsZeroValue()delegates to a type switch.*stringValuechecksf.DefValue == "",isNoOptBoolValuechecksf.DefValue == "", and the fallbackdefault:case checkscase "": return true. However, the slice/array cases only checkedf.DefValue == "[]", omitting the empty string check.Solution
Check
if f.DefValue == "" { return true }at the beginning ofdefaultIsZeroValue(). This unifies zero-value handling across all flag types whenDefValueis cleared or empty, ensuring(default )is never printed.Verification
TestZeroedDefValueIsZeroValueinflag_test.goverifying that slice and other flag types reportdefaultIsZeroValue() == truewhenDefValueis empty.TestPrintDefaultsZeroedDefValueinflag_test.goreproducing the exact issue reported in StringSlice displays default even if "zeroed" DefValue #313 and ensuring(default )is not printed inPrintDefaults().go test -v ./....Fixes #313