Skip to content

Trim the key of a map flag when TrimSpace is set - #2431

Open
youdie006 wants to merge 1 commit into
urfave:mainfrom
youdie006:map-flag-trims-key
Open

youdie006 wants to merge 1 commit into
urfave:mainfrom
youdie006:map-flag-trims-key

Conversation

@youdie006

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • bug

What this PR does / why we need it:

StringConfig.TrimSpace reaches only half of each pair in a map flag. MapBase.Set routes the value through the configured parser at flag_map_impl.go:84 but writes the key into the dict raw at :87, so the key never sees the config:

--m " a = 1 "   TrimSpace: true    before  map[" a ":"1"]     after  map["a":"1"]
--m " a = 1 "   TrimSpace: false   before  map[" a ":" 1 "]   after  map[" a ":" 1 "]
--f "a, b"      TrimSpace: true    StringSliceFlag gives ["a","b"], both trimmed

cmd.StringMap("m")["b"] misses on --m "a=1, b=2" because the stored key is " b".

SliceBase has routed every element through the parser since #2171; MapBase is the one place a StringConfig-carrying string reaches storage without passing the config.

Which issue(s) this PR fixes:

None filed. Follows #2171 and #2425, which fixed the value side of the same seam.

Release Notes

Map flags now apply `StringConfig.TrimSpace` to the key as well as the value.
Verification

Base 9b981a0.

row flag_map_impl.go md5 subtests run result
this PR 7031d44c5d 3 ok
base f33bcad660 3 fails trim_enabled
revert the guarded trim c3186d07eb 3 fails trim_enabled
trim the key unconditionally caa059492c 3 fails trim_disabled

The two mutants fail on different subtests, so the test pins both that the key is trimmed under TrimSpace: true and that it is left alone under TrimSpace: false.

make vet, make test, make v3diff and make generate all clean; gofmt -l . empty; no exported API change and no godoc drift. make check-binary-size and the gfmrun examples were not run here.

One thing worth your call: reading the config in Create with any(c).(StringConfig) mirrors the reflect.TypeOf(t).Kind() approach SliceBase already uses, but it is still a type assertion inside a generic. If you would rather thread it differently, the test and the behaviour survive either shape. StringMapFlag is currently the only map flag the library ships, so this does not affect any other element type.


Disclosure: written with the help of Claude (an AI assistant). Every number above is from runs on my machine.

MapBase.Set routes only the value through the configured value parser and
writes the key into the dict raw, so StringConfig.TrimSpace reached one
half of each pair. SliceBase has had this since urfave#2171.
@youdie006
youdie006 requested a review from a team as a code owner September 15, 2026 01:12
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.

1 participant