-
Notifications
You must be signed in to change notification settings - Fork 2k
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
extend stringToString pflag binding to stringToInt pflag #1435
Conversation
👋 Thanks for contributing to Viper! You are awesome! 🎉 A maintainer will take a look at your pull request shortly. 👀 In the meantime: We are working on Viper v2 and we would love to hear your thoughts about what you like or don't like about Viper, so we can improve or fix those issues. ⏰ If you have a couple minutes, please take some time and share your thoughts: https://forms.gle/R6faU74qPRPAzchZ9 📣 If you've already given us your feedback, you can still help by spreading the news, https://twitter.com/sagikazarmark/status/1306904078967074816 Thank you! ❤️ |
@sagikazarmark can u look into this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for sending a PR! Can you please take a look at the comment I left?
viper.go
Outdated
@@ -1373,7 +1373,7 @@ func readAsCSV(val string) ([]string, error) { | |||
|
|||
// mostly copied from pflag's implementation of this operation here https://github.com/spf13/pflag/blob/master/string_to_string.go#L79 | |||
// alterations are: errors are swallowed, map[string]interface{} is returned in order to enable cast.ToStringMap | |||
func stringToStringConv(val string) interface{} { | |||
func csvKeyValueConv(val string) interface{} { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't stringToInterfaceCong
be a better name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure let me do that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. But moved to another MR. Some issue happened in this MR
New MR: #1462
@sagikazarmark I mistakenly closed this MR. I have created a new MR Your suggestion has been implemented there. |
Support for pflag's StringToString was added long back, but Viper Still couldn't bind to pflag StringToInt. Addid that support.