Skip to content

fix: support StringMap values from map value sources (#1634) - #2473

Closed
manish-9245 wants to merge 1 commit into
urfave:mainfrom
manish-9245:fix/1634-stringmap-altsrc
Closed

manish-9245 wants to merge 1 commit into
urfave:mainfrom
manish-9245:fix/1634-stringmap-altsrc

Conversation

@manish-9245

@manish-9245 manish-9245 commented Oct 16, 2026 •

Copy link
Copy Markdown

What type of PR is this?

  • bug

What this PR does / why we need it:

Map value sources stringified map values with Go syntax (fmt.Sprintf("%+v"), e.g. map[env:prod]), which StringMap.Set rejects with a "missing separator =" error. The Lookup in value_source.go now serializes map values through the same cli.NewStringMap(...).Serialize() path used for flag values, and keeps the scalar and slice behavior unchanged. A regression test covers a StringMap flag loaded from a map source.

Which issue(s) this PR fixes:

Fixes #1634

Special notes for your reviewer:

The commands listed above are the ones that were actually run.

Testing

go build ./..., go test ./..., go test -run 'TestMapValueSource|TestStringMapFlagFromMapSource' -v ., gofmt -l (clean), go vet . (clean).

Release Notes

Fix StringMap flags failing to load values from map-based value sources.

Map value source Lookup stringified map values with Go-syntax
fmt.Sprintf("%+v"), e.g. map[env:prod], which MapBase.Set rejects
with missing separator "=". Serialize map values through the same
NewStringMap(...).Serialize() path used for flag values so they
round-trip, while preserving scalar and slice formatting.

Fixes urfave#1634.
@manish-9245
manish-9245 requested a review from a team as a code owner October 16, 2026 07:01

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@Juneezee

Copy link
Copy Markdown
Member

Thanks for the PR, but this doesn't fix #1634.

#1634 is about altsrc, i.e. loading flags from config files. In v3 that lives in urfave/cli-altsrc, which does not use cli.NewMapValueSource. It has its own ValueSource.Lookup, which still formats map values with fmt.Sprintf:

https://github.com/urfave/cli-altsrc/blob/b0da8e071c8ebf613ae8b21b568cf2bbc060dfb2/altsrc.go#L116-L137

For example, with this branch and cli-altsrc main, a StringMapFlag read from a YAML file still fails:

labels:
  env: prod
&cli.StringMapFlag{
	Name:    "labels",
	Sources: cli.NewValueSourceChain(yaml.YAML("labels", altsrc.StringSourcer("config.yaml"))),
}
could not parse "map[env:prod]" as map[string]string value from yaml file "config.yaml" at key "labels" for flag labels: item "map[env:prod]" is missing separator "="

Merging this would still close #1634 because of "Fixes #1634". The same fix is already open for cli-altsrc in urfave/cli-altsrc#50, so let's continue there. Closing this one.

@Juneezee Juneezee closed this Oct 16, 2026
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.

Add StringMap flag for altsrc

2 participants