Properly handle string slice values#244
Conversation
viper.go
Outdated
| val = cast.ToInt(flag.ValueString()) | ||
| case "bool": | ||
| val = cast.ToBool(flag.ValueString()) | ||
| case "stringSlice": |
There was a problem hiding this comment.
As I mentioned in other PR comment. I'm not sure if this is necessary, so I hope you know why this might be correct more than I do. Not only do I forget why I put it there in my diff, but it just plain looks wrong haha :)
| if len(s) == 0 { | ||
| return []string{} | ||
| } | ||
| res, _ := readAsCSV(s) |
There was a problem hiding this comment.
Is the call to readAsCSV necessary?
I assume you'd use it instead of string split to to handle this case: go run main.go -t 'one,"two three"'
But when I run this command on my old version of viper (7fb2782) I get this:
(string) (len=15) "[one,two three]"
([]string) (len=2 cap=2) {
(string) (len=8) "[one,two",
(string) (len=6) "three]"
}
([]string) (len=2 cap=2) {
(string) (len=3) "one",
(string) (len=9) "two three"
}
Which means pflags is already doing this. Is there no way we can get the correct value out of pflags directly?
This is just a question, like the previous comment. Thanks for taking the time to work on this!
There was a problem hiding this comment.
Thanks for the feedback. pflag does in fact handle stringSlice correctly, and it knows the actual underlying Go value, but the best I can tell, it would take changes to pflag and viper APIs to be able to get to the underlying Go value. This PR solves the immediate problem without changing the APIs.
Having said that, I did test adding a Get() interface{} func to the pflag.Value and viper.FlagValue interfaces. Get() returns the underlying Go value instead of string that we have to parse. It works, but I'm unclear on the ramifications and don't feel comfortable submitting a PR at this point.
|
Possible duplicate with #200 , will check. |
|
Any progress/mouvement regarding this ? afaict #200 wasn't definitively enough to fix this issue... |
This commit attempts to properly retrieve string slice values. To support that effort, I've copied over the readAsCSV() func from pflag, which adds a dependency on encoding/csv.
This commit attempts to properly retrieve string slice values. To support that effort, I've copied over the readAsCSV() func from pflag, which adds a dependency on encoding/csv.
Fixes #200 (hopefully)
Cc: @aarondl