Skip to content

Treat empty DefValue as zero value in defaultIsZeroValue - #517

Open
AdamMagued wants to merge 2 commits into
spf13:masterfrom
AdamMagued:fix-zeroed-defvalue-usage
Open

AdamMagued wants to merge 2 commits into
spf13:masterfrom
AdamMagued:fix-zeroed-defvalue-usage

Conversation

@AdamMagued

Copy link
Copy Markdown

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 only f.DefValue == "[]".

Because defaultIsZeroValue() returned false, FlagUsagesWrapped formatted the default value using fmt.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. *stringValue checks f.DefValue == "", isNoOptBoolValue checks f.DefValue == "", and the fallback default: case checks case "": return true. However, the slice/array cases only checked f.DefValue == "[]", omitting the empty string check.

Solution

Check if f.DefValue == "" { return true } at the beginning of defaultIsZeroValue(). This unifies zero-value handling across all flag types when DefValue is cleared or empty, ensuring (default ) is never printed.

Verification

  • Added TestZeroedDefValueIsZeroValue in flag_test.go verifying that slice and other flag types report defaultIsZeroValue() == true when DefValue is empty.
  • Added TestPrintDefaultsZeroedDefValue in flag_test.go reproducing the exact issue reported in StringSlice displays default even if "zeroed" DefValue #313 and ensuring (default ) is not printed in PrintDefaults().
  • Verified all tests pass across the entire package with go test -v ./....

Fixes #313

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.
@CLAassistant

CLAassistant commented Oct 1, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@tomasaschan

tomasaschan commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

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.

@AdamMagued

Copy link
Copy Markdown
Author

Thanks for checking this @tomasaschan.

To clarify how this interacts with slice parsing:
defaultIsZeroValue() is an unexported helper called exclusively inside FlagUsagesWrapped when formatting flag usage and help text. Its sole responsibility is deciding whether to append (default %s) to the usage line for a flag.

It does not participate in argument parsing, flag.Value.Set(...), CSV splitting, or slice value construction when flags are supplied on the command line. Command-line parsing of empty string parameters (such as --flag "") routes entirely through the slice value's Set(val) method (such as stringSliceValue.Set), which is untouched by this change.

This modification only ensures that if a caller explicitly clears flag.DefValue = "" to suppress default value display in usage output (as requested in #313), FlagUsagesWrapped recognizes it as zeroed rather than emitting a trailing (default ) suffix.

@tomasaschan

Copy link
Copy Markdown
Collaborator

My point is, what happens to a slice valued flag if/when the user sets DefVal = ""? Does it always result in defaulting to a zero value?

@AdamMagued

Copy link
Copy Markdown
Author

Setting DefValue = "" affects only the generated usage text; it has no effect on the underlying slice value or how command-line arguments are parsed:

  1. Runtime Slice Value: flag.DefValue is purely a formatted display string used during help text generation (FlagUsagesWrapped / PrintDefaults). The actual slice value is stored in flag.Value (e.g. *stringSliceValue). Mutating flag.DefValue = "" does not reset or alter the underlying slice in memory. If the flag is omitted on the command line, the slice retains whatever default elements were originally passed when declaring the flag.

  2. Command-Line Parsing: Command-line flag parsing routes arguments directly to flag.Value.Set(val). It does not inspect DefValue. Supplying arguments like --flag a,b or --flag "" behaves identically whether DefValue is set, cleared, or left as default.

  3. Usage Text Output: In FlagUsagesWrapped, (default %s) is appended unless defaultIsZeroValue() returns true. Prior to this change, if a caller explicitly cleared flag.DefValue = "" to suppress default display (such as in Cobra or custom help formatters), slice flags would still evaluate as non-zero because their type switch only matched DefValue == "[]", producing an extraneous (default ). With if f.DefValue == "" { return true }, setting DefValue = "" cleanly suppresses the default suffix across all flag types, matching the existing behavior for stringValue and custom Value implementations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

StringSlice displays default even if "zeroed" DefValue

3 participants