Skip to content

fix: honor custom IsBoolFlag compatibility#487

Open
happysnaker wants to merge 1 commit into
spf13:masterfrom
happysnaker:fix-custom-isbool-compatibility
Open

fix: honor custom IsBoolFlag compatibility#487
happysnaker wants to merge 1 commit into
spf13:masterfrom
happysnaker:fix-custom-isbool-compatibility

Conversation

@happysnaker

Copy link
Copy Markdown

Summary

  • treat custom Value implementations with IsBoolFlag() == true as no-opt bool flags consistently
  • automatically assign NoOptDefVal="true" for those custom bool-like values
  • fix default-value and usage/help rendering for custom no-opt bool flags

Problem

pflag already exposes IsBoolFlag() for bool-like values, but several code paths still special-case only the built-in bool type. That leaves custom bool-like values in an inconsistent state:

  • --flag / -f can fail unless callers manually set NoOptDefVal
  • usage/help text can render those flags as if they require a value unconditionally
  • empty / false defaults can still show up as non-zero defaults in help output

Changes

  • add a shared helper for detecting bool-like no-opt values via IsBoolFlag()
  • auto-fill NoOptDefVal in AddFlag() for custom bool-like values
  • use the same detection in defaultIsZeroValue(), UnquoteUsage(), and FlagUsagesWrapped()
  • add regression coverage for parsing and help/default rendering

Fixes #281
Fixes #360

@happysnaker

Copy link
Copy Markdown
Author

Maintainer context for review:

  • this intentionally treats IsBoolFlag() == true as the single compatibility signal across parsing and help rendering, instead of continuing to special-case only the built-in bool type
  • pflag already honors this signal when importing Go flag.Flags via AddGoFlag; this patch extends that behavior to direct Var / VarPF registration too
  • the scope is kept narrow to custom no-opt bool values, so other custom Value implementations keep their current parsing/usage behavior
  • this is meant to close the loop on both the parser-compat issue in Add missing compatibility for flag.boolFlag.IsBoolFlag() #281 and the default/help-rendering issue in Value's that implement IsBoolFlag() always show a default value #360 with one focused change set

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.

Value's that implement IsBoolFlag() always show a default value Add missing compatibility for flag.boolFlag.IsBoolFlag()

1 participant