Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Getting the value of a StringToString pflag #874

Merged
merged 7 commits into from May 9, 2020

Conversation

terev
Copy link
Contributor

@terev terev commented Mar 28, 2020

As viper parses pflag values as strings, string to string values weren't being handled correctly . Fixes #608 .

@CLAassistant
Copy link

CLAassistant commented Mar 28, 2020

CLA assistant check
All committers have signed the CLA.

@danmx
Copy link

danmx commented Apr 30, 2020

@sagikazarmark could you take a look at this?

Copy link
Collaborator

@sagikazarmark sagikazarmark left a comment

Choose a reason for hiding this comment

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

@terev thanks a lot! I had one comment, but other than that, 👍 !

viper.go Outdated
@@ -1177,6 +1181,24 @@ func readAsCSV(val string) ([]string, error) {
return csvReader.Read()
}

func parseStringToStringFlagValue(val string) map[string]interface{} {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about copying the same function from pflag and keeping the implementation as close and possible?

https://github.com/spf13/pflag/blob/master/string_to_string.go#L79

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sagikazarmark i've made this change. i had to tweak the signature and return types slightly to work with GetStringMap that uses cast.ToStringMap

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure thing, thanks!

@sagikazarmark sagikazarmark added this to the 1.7.0 milestone May 5, 2020
@sagikazarmark sagikazarmark added the kind/bug Something isn't working label May 5, 2020
Copy link
Collaborator

@sagikazarmark sagikazarmark left a comment

Choose a reason for hiding this comment

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

Thanks @terev

@sagikazarmark sagikazarmark merged commit 3856c05 into spf13:master May 9, 2020
brainexe pushed a commit to brainexe/viper that referenced this pull request Nov 12, 2020
* add parsing for stringToString flags

* add logic to return flags default if not val set, add a test

* extract parsing into single func

* add a few more cases

* return nil if unable to parse instead of panicing

* return map[string]interface in order to work with cast.ToStringMap

* mostly copy pflags implementation of the conversion to a stringtostring
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

StringToString pflag values are not handled correctly by viper
4 participants