BindPFlag don't work properly with pflag.StringSlice

[b0d0nne11]

An open and close bracket are added to the first and last item of a StringSlice when it's updated by a flag.

Here's some test code that demonstrates the issue:

package main

import (
    "fmt"

    jww "github.com/spf13/jwalterweatherman"
    "github.com/spf13/pflag"
    "github.com/spf13/viper"
)

func main() {
    jww.SetStdoutThreshold(jww.LevelDebug)

    viper.SetConfigName("config")
    viper.AddConfigPath(".")
    err := viper.ReadInConfig()
    if err != nil {
        panic(fmt.Errorf("fatal error: %s \n", err))
    }

    pflag.StringSlice("foo", []string{}, "Test flag for StringSlice")
    err = viper.BindPFlag("foo", pflag.Lookup("foo"))
    if err != nil {
        panic(fmt.Errorf("fatal error: %s \n", err))
    }
    pflag.Parse()

    for _, value := range viper.GetStringSlice("foo") {
        fmt.Println(value)
    }
}

And here's the output:

(04:14:41) brendan @ ~/workspace/golang/src/github.com/b0d0nne11/test 
→ go run test.go 
INFO: 2015/09/03 Trying to resolve absolute path to .
INFO: 2015/09/03 adding /home/brendan/workspace/golang/src/github.com/b0d0nne11/test to paths to search
INFO: 2015/09/03 Attempting to read in config file
INFO: 2015/09/03 Searching for config in  [/home/brendan/workspace/golang/src/github.com/b0d0nne11/test]
DEBUG: 2015/09/03 Searching for config in  /home/brendan/workspace/golang/src/github.com/b0d0nne11/test
DEBUG: 2015/09/03 Checking for /home/brendan/workspace/golang/src/github.com/b0d0nne11/test/config.json
DEBUG: 2015/09/03 Checking for /home/brendan/workspace/golang/src/github.com/b0d0nne11/test/config.toml
DEBUG: 2015/09/03 Checking for /home/brendan/workspace/golang/src/github.com/b0d0nne11/test/config.yaml
DEBUG: 2015/09/03 Found:  /home/brendan/workspace/golang/src/github.com/b0d0nne11/test/config.yaml
DEBUG: 2015/09/03 ToStringSliceE called on type: []interface {}
DEBUG: 2015/09/03 ToStringE called on type: string
DEBUG: 2015/09/03 ToStringE called on type: string
bar
baz

(04:14:52) brendan @ ~/workspace/golang/src/github.com/b0d0nne11/test 
→ go run test.go --foo='bar baz'
INFO: 2015/09/03 Trying to resolve absolute path to .
INFO: 2015/09/03 adding /home/brendan/workspace/golang/src/github.com/b0d0nne11/test to paths to search
INFO: 2015/09/03 Attempting to read in config file
INFO: 2015/09/03 Searching for config in  [/home/brendan/workspace/golang/src/github.com/b0d0nne11/test]
DEBUG: 2015/09/03 Searching for config in  /home/brendan/workspace/golang/src/github.com/b0d0nne11/test
DEBUG: 2015/09/03 Checking for /home/brendan/workspace/golang/src/github.com/b0d0nne11/test/config.json
DEBUG: 2015/09/03 Checking for /home/brendan/workspace/golang/src/github.com/b0d0nne11/test/config.toml
DEBUG: 2015/09/03 Checking for /home/brendan/workspace/golang/src/github.com/b0d0nne11/test/config.yaml
DEBUG: 2015/09/03 Found:  /home/brendan/workspace/golang/src/github.com/b0d0nne11/test/config.yaml
DEBUG: 2015/09/03 ToStringE called on type: string
DEBUG: 2015/09/03 ToStringSliceE called on type: string
[bar
baz]

[alexferl]

Just hit this too. For now I just replaced that flag to a comma-delimited string and just strings.split(), kinda dirty but it works.

[matcornic]

Got that problem too.

[Morlinest]

I was debuging this problem too (few hours searching how to use multiple times same flag with viper, then few hours to find what is wrong) and have found that you can use (get) only int, bool or string type with viper if you bind flag to it. In code there is few places, where it goes wrong and also there is few re-casting to the same value and same type (if int cast to int...). If you want to work with flags slices, do it without viper, like (i am using cobra, serverCmd is cobra.Command):

var fieldsSetup  []string

func init(){
...
serverCmd.Flags().StringSliceVarP(&fieldsSetup, "field", "f", []string{}, "bla bla bla")
...
}

func main(){
...
for _, fieldSetup := range fieldsSetup {
//parse flag slice
}
...
}

Viper only call ValueString() string method on each flag to get value, than find()/get() value is casting only to int/bool/string value anything he gets from flag. And then few more cast methods to get your value... If you call viper.GetStringSlice("foo"), you get slice with your binded value as string at first position of slice

[matcornic]

Yes, thank you @Morlinest for your answer. I use Cobra as well, and this was working nicely (same strategy as yours). I faced that issue when I tried to bind values from a configuration file, as it is possible with Viper.
I had the right behavior when getting the default value from the config file, but when I wanted to override the value by a flag in the command line, types was not the right one anymore (a weird list of list of string)

I think I'll follow this post and use a simple list of comma separate string in the meantime, as suggested by @AdmiralObvious

[Morlinest]

Yes, using list of "x" separate string is good. I am already using it (using ":" as separator) but also needs more flags of same type (using another separator like "|" would be mess :D).

Some sugestions to get it work (dont know meaning of all this code and where its used and dont have time to work on it myself right now):

• Add new method to interface in flags.go ("ValueStringSlice() []string"/"Value() interface{}" ?)

type FlagValue interface {
    HasChanged() bool
    Name() string
    ValueString() string
    ValueType() string
}

• Change find method in "viper.go" so it can return more than int/bool/string, best to just return original value or at least add more types like "stringSlice":

...
    flag, exists := v.pflags[key]
    if exists && flag.HasChanged() {
        jww.TRACE.Println(key, "found in override (via pflag):", flag.ValueString())
        switch flag.ValueType() {
        case "int", "int8", "int16", "int32", "int64":
            return cast.ToInt(flag.ValueString())
        case "bool":
            return cast.ToBool(flag.ValueString())
        default:
            return flag.ValueString()
        }
    }
...

• Viper use "*pflag.Flag" as default to hold flags. If i remember, i saw StringSlice() method and also returns "stringSlice" as type from ValueType() method. Maybe viper should return real value by viper.Get() method instead of changing its type and recasting it few times on the way?

[akutz]

Regarding separators, I suggest the creation of the environment variable $VIFS, as Viper's version of Bash's $IFS. Defaulting to a ,, the variable can be set to specify Viper's internal field separator for when parsing strings as slices.

FWIW, does #110 and its merged PR #111 help at all? I ran into this issue for the exact same reason as well, that PFlag does funky things when emitting arrays as strings (prefixing and suffixing the contents with a bracket) such that later attempts to read the data made it impossible.

[Morlinest]

Thanks, for my purpose your patch does not help. I am not using default values nor structs to hold pure flag/config data (want to use viper as "store"...) or to work with them. In this situation, there will be still same issue because viper can get flag value only as string and then cast it to int/bool/string and if you set []string as default value, the result will be same (slice of size 1 with value as "joined string" with brackets inside it). Also i think, there should be simple and easy way how to get value from flag/viper, without using cast or marshal and another structs.

You are right about implementation of stringslice in "string_slice.go" (github.com/spf13/pflag) - func (s *stringSliceValue) String() string { return "[" + strings.Join(*s.value, ",") + "]" } dont help to get data (maybe only for print purpose).

[akutz]

Hi @Morlinest,

What if you use a default value simply to init the default type? It's a band-aid, but maybe it could help in the short-term? An empty string slice for example.

[Morlinest]

Hi @akutz, dont think so as i said:

In this situation, there will be still same issue because viper can get flag value only as string and then cast it to int/bool/string and if you set []string as default value, the result will be same (slice of size 1 with value as "joined string" with brackets inside it).

Using type from default value is last part of Get() method (every viper.Get* calls it), getting flag value is first. So u will get string and at end of method it will be cast to slice.