Skip to content

fix: allow empty StringToString values#486

Closed
happysnaker wants to merge 1 commit into
spf13:masterfrom
happysnaker:fix-empty-string-to-string-values
Closed

fix: allow empty StringToString values#486
happysnaker wants to merge 1 commit into
spf13:masterfrom
happysnaker:fix-empty-string-to-string-values

Conversation

@happysnaker

Copy link
Copy Markdown

Summary

  • allow an explicit --flag= / flag.Set("") for StringToString to clear the map to an empty map instead of returning a formatting error
  • preserve the existing default value when the flag is omitted
  • keep the resulting value non-nil so an explicit empty override is distinguishable from an untouched default

Why

Issue #312 asks for a way to override a non-empty default map with an explicit empty value.

This change follows the same semantics already accepted for typed slice flags in #222 / #485:

  • omitted flag => keep the default value
  • explicit empty value => non-nil empty collection

That makes StringToString behave like a real override channel instead of forcing callers to choose between "keep defaults" and "replace with at least one entry".

Scope / compatibility

  • this is intentionally limited to StringToString
  • it does not change StringArray / StringSlice / other collection types here
  • the previously rejected input was --flag= itself, so this uses syntax that currently errors rather than reinterpreting a previously valid non-empty value

Tests

Added regression coverage for:

  • clearing a non-empty default map with --s2s=
  • keeping explicit empty results non-nil
  • following an empty override with later map entries

Note: I did not run go test / go build locally for this branch.

@happysnaker

Copy link
Copy Markdown
Author

Maintainer context: this intentionally scopes the empty-value semantics to StringToString only, matching the existing user ask in #312 (explicit empty override of a non-empty default) without changing the broader StringSlice / StringArray compatibility discussion from #415. Omitted flag still preserves defaults; explicit --flag= now becomes a non-nil empty map.

@tomasaschan

Copy link
Copy Markdown
Collaborator

Maintainer context: [...]

So it seems you read this comment and then promptly ignored it? :)

Changing the behavior of empty slices vs slices with a single empty value (vs potentially other, also conflicting, behavior that I haven't thought of) is explicitly not in scope for 1.x. Contributions to the 1.x codebase to change this behavior will not be accepted. Figuring out exactly which changes are backward compatible or not, without potentially breaking users' expectations four levels down the dependency tree, is just more complicated than it's worth.

I recently created an experimentation bed for v2 in #482; I will make sure that whatever implementation for slice-valued flags we end up there solve this better.

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.

2 participants