Suggestion: flag types should implement the Getter interface from the standard library's flag package

[e-nikolov]

All standard library flags since Go 1.2 implement the Getter interface and I think that pflag should do it too.

https://github.com/golang/go/blob/master/src/flag/flag.go#L306

// Getter is an interface that allows the contents of a Value to be retrieved.
// It wraps the Value interface, rather than being part of it, because it
// appeared after Go 1 and its compatibility rules. All Value types provided
// by this package satisfy the Getter interface, except the type used by Func.
type Getter interface {
    Value
    Get() interface{}
}

Currently if a user of pflag needs to parse the flags and put them in a map[stirng]interface{} or Unmarshal them into a struct, they would have to manually write a switch case for all possible flag types and then call the Get method for the correct type, e.g.: https://github.com/knadh/koanf/blob/master/providers/posflag/posflag.go#L55

switch f.Value.Type() {
    case "int":
        i, _ := p.flagset.GetInt(key)
        val = int64(i)
    case "int8":
        i, _ := p.flagset.GetInt8(key)
        val = int64(i)
    case "int16":
        i, _ := p.flagset.GetInt16(key)
        val = int64(i)
    case "int32":
        i, _ := p.flagset.GetInt32(key)
        val = int64(i)
    case "int64":
        i, _ := p.flagset.GetInt64(key)
        val = int64(i)
    case "float32":
        val, _ = p.flagset.GetFloat32(key)
    case "float":
        val, _ = p.flagset.GetFloat64(key)
    case "bool":
        val, _ = p.flagset.GetBool(key)
    case "stringSlice":
        val, _ = p.flagset.GetStringSlice(key)
    case "intSlice":
        val, _ = p.flagset.GetIntSlice(key)
    case "stringToString":
        val, _ = p.flagset.GetStringToString(key)
    case "stringToInt":
        val, _ = p.flagset.GetStringToInt(key)
    case "stringToInt64":
        val, _ = p.flagset.GetStringToInt64(key)
    default:
        val = f.Value.String()
}

This is not very clean nor maintainable because if new flag types are added to pflag, that switch case would have to be updated. If all flags in pflag implemented the Getter interface, then the switch case could be replaced with something like flag.Value.Get().

[hoshsadiq]

How will Get() interface{} solve this issue exactly? You still need to do p.flagset.Get().(<type>), e.g. p.flagset.Get().(bool)

[e-nikolov]

It makes it easier to couple it with libraries that use reflection at runtime to do something with the flag values based on their types, because there will be a single known function that needs to be called (value.Get()). For example json.Marshal or yaml.Marshal already know what to do if they know the type of something. But in the current situation I first need to call f.Value.Type() to get the type in a string format and then do a type switch with all possible flag types to determine which function to call, e.g. GetFloat()/GetString()/GetBool()/etc and only after this I can pass it on to json.Marshal.

There are a lot of config parsing libraries that want to support pflag as a config provider that have various implementations of that type switch, and they are usually not covering all flag types because they have to manually add new checks every time pflag adds support for a new flag type.

If the flags themselves had a Get() interface{} method, they wouldn't need the type switch anymore, but could just call .Get() and then do their magic for sticking the flags into their config state.

[memreflect]

I certainly dislike the type-dependent functions/methods like GetBool, GetInt, etc. Unfortunately, the value returned by a Get method is useless until you know the type of that value, which only the package defining the flag will know usually.

The only exception that comes to mind where it is useful to an outside package is when that outside package simply stores the unmarshalled form returned by Get without doing anything else to it, meaning it does not need to know the type of value returned by Get. An example is FlagVal in koanf, which i believe is the point of this issue. Since the data type is irrelevant, a single Get method would be beneficial.