pflag
pflag copied to clipboard
spf13
Add capability to restrict flag values to a set of allowed values
I have a use-case where I need to perform validation on flag values. In other words, I need to check that the value passed in by the user belongs to some set of valid values. I end up writing a lot of code that looks like this:
package main
import (
"fmt"
"os"
flag "github.com/spf13/pflag"
)
var color string
func main() {
addFlags()
flag.Parse()
if err := validateFlags(); err != nil {
fmt.Fprintf(os.Stderr, "Error: %v", err)
os.Exit(1)
}
fmt.Printf("Selected color is %s\n", color)
}
func addFlags() {
flag.StringVar(&color, "color", "", "The color for the example")
}
func validateFlags() error {
validColors := []string{"red", "blue", "yellow"}
for _, validColor := range validColors {
if color == validColor {
// color is valid
return nil
}
}
// if we're here, color is invalid
return fmt.Errorf("Value '%s' is invalid for flag 'color'. Valid values "+
"come from the set %v", color, validColors)
}
I'd like to be able to do something like the following:
flag.StringVarRestricted(
&color, // pointer to variable
"color", // flag name
"", // default value
"The color for the example", // description
[]string{"red", "blue", "yellow"}) // Allowed values
I wouldn't mind taking a shot at implementing something like this if it sounds acceptable.
Just dropping a comment noting that this is the most thumbs-upped issue in this repo, and I'd love to see it come to fruition 🙂
Right now I do some reflection-gymnastics based on pre-defined types to validate and define flags, so if this were added in this project it'd obviate the need for that custom logic and make my codebase that much easier to maintain. I see #237 was opened, but looks like it didn't get much traction?
Python's argparse supports flags with predefined choices. I want to replicate the same behaviour in Cobra, and think it would be a great addition.
This is would be so useful. Is there anything keeping this from getting merged? 🙏
I think this feature would be awesome, I would like to suggest a change, instead a slice of valid values could be a bool function that validates the value 😃
@felipe-avelar that is a better approach as it is more flexiable, so using the initial example it would become:
flag.StringVarValidated(
&color, // pointer to variable
"color", // flag name
"", // default value
"The color for the example", // description
func (s string) bool { ... }(), // Function to evaluate valid values
Alternative POV: If you provide a slice of valid values, then help output can tell the user what they are, and shell autocomplete integration can leverage that information.
Of course, not all cases can be handled by a slice, so this is probably a case of "yes, and". For example, could have the validation function, and an (optional) slice of "suggested" values?
@fastcat You make a great point.
@pmenglund I like your suggestion and think it should be a feature either way..
I will try to find time soon to implement this on my fork, which also has other features, and works with cobra (instructions in the readme).
Is this project dead? How come this issue is still open and stale? Where are the repo admins? cc: @spf13
@thalesfsp last I check this is an open source project, so you either avoid looking a gift horse in the mouth and accept the free software with its limitations, or you implement the missing feature you want on your own (or pay someone to do it for you).
@thalesfsp last I check this is an open source project, so you either avoid looking a gift horse in the mouth and accept the free software with its limitations, or you implement the missing feature you want on your own (or pay someone to do it for you).
Why is this snarky reply so popular? The question is completely valid given the neglect this library sees and the outright hostility everyone encounters when touching the subject on the cobra repos.
A fork has been floated on so many issues with this repo and everyone keeps shooting it down and complaining when people mention it, so please don't act like that's a real solution for the average user. If you have any experience in the industry, you know that's a straw argument intended to silence people.
It is popular because so many put demands on people who devote their free time to maintaining open source projects, with no compensation but the "fame". My guess is that @spf13 has a demanding day-job, and probably a family, so any sliver of spare time is probably spent on something to unwind.
So what are the alternatives? You can whine or you can do something to fix it. Whining gets you snarky comments from people who have maintained open source projects.
That said, I should not have posted the snarky comment. Instead I should have offered advice, but my patience quota had run out.
Now it has been refilled, so here goes:
Go makes it easy very to fork this repo and use a replace directive in your go.mod to swap it out for your own fork, where you merged #237. That is a solution the average user can use. Not optimal but workable.
Since @cornfeedhobo has forked it, @thalesfsp can just add this to go.mod and get all benefits from the fork:
replace (
github.com/spf13/pflag => github.com/cornfeedhobo/pflag
)
@pmenglund thanks for such a thoughtful reply. Re-reading my comment and I can see I was more incensed than I meant to be. Thanks for reading through that, and my apologies.
I think this is achievable with custom flags (type), as flag is only an interface:
Sample code:
package main
import (
"fmt"
"strings"
flag "github.com/spf13/pflag"
)
type enum struct {
Allowed []string
Value string
}
// newEnum give a list of allowed flag parameters, where the second argument is the default
func newEnum(allowed []string, d string) *enum {
return &enum{
Allowed: allowed,
Value: d,
}
}
func (a enum) String() string {
return a.Value
}
func (a *enum) Set(p string) error {
isIncluded := func(opts []string, val string) bool {
for _, opt := range opts {
if val == opt {
return true
}
}
return false
}
if !isIncluded(a.Allowed, p) {
return fmt.Errorf("%s is not included in %s", p, strings.Join(a.Allowed, ","))
}
a.Value = p
return nil
}
func (a *enum) Type() string {
return "string"
}
// Main program starts here
func main() {
color := newEnum([]string{"red", "green", "blue"}, "green")
flag.VarP(color, "color", "c", "the color you want")
flag.Parse()
fmt.Printf("You have selected %s\n", color)
}
@frankywahl is the best answer here. Trivial to extend and presumably exactly what the author intended.
While enumflag is not a generic add-on for spf13, it covers the use case of enumeration flags (and slices). Maybe that might be of some help in such cases?
The addition of Func/FuncP in #350 (disclaimer: i'm the PR author) would also fix the issue in a more generic (and indirect) manner in the case of only one or two flags:
package main
import (
"fmt"
"os"
"sort"
"strings"
flag "github.com/spf13/pflag"
)
func main() {
color := ""
flag.Func("color", "use `COLOR` for the example, or \"help\"",
func(s string) error {
// NOTE: keep sorted!
validColors := []string{"blue", "red", "yellow"}
if s == "help" {
// special case: --color=help
fmt.Fprintln(flag.CommandLine.Output(), "Valid colors:", strings.Join(validColors, ", "))
os.Exit(2) // "invalid argument" error if `return flag.ErrHelp` is used :(
}
// sort.SearchStrings can be used since validColors is already sorted.
i := sort.SearchStrings(validColors, s)
if i < len(validColors) && validColors[i] == s {
color = s
return nil
}
return fmt.Errorf("invalid color %q; use --color=help to list valid colors", s)
},
)
flag.Parse()
if color == "" {
flag.Lookup("color").Value.Set("") // stay DRY
} else {
fmt.Printf("Selected color is %s\n", color)
}
}
For many such flags, however, a custom Var implementation as suggested by @frankywahl is a much better solution, though i would sort enum.Allowed before returning from newEnum to allow faster searches with sort.SearchStrings.
Of course, that is merely the most straightforward implementation; there may be better alternatives to consider.
Naturally you'll have people asking if one can extend this to yet another Var implementation with a one-to-one mapping from a restricted value to a custom function, or a way to restrict a range of int values without storing all possible int values in enum.Allowed, or some other request to add more code to pflag just so their own code does not need to do it.
I would not be surprised if an author/maintainer/contributor of pflag is especially wary of adding such Var implementations since pflag already has significantly more Var implementations than Go's standard flag package, even excluding the P variants:
bash$ topfuncs() { go doc $1 | grep '^func.*name.*usage' | grep -v '^func[^(]*P' ; }
bash$ wc -l <(topfuncs flag) <(topfuncs github.com/spf13/pflag)
19 /tmp//sh-np.azCogP
65 /tmp//sh-np.MqaD5O
84 total
That said, i agree an implementation of Var that allows you to restrict flag values is general enough and popular enough to be a useful addition with the caveat that enum is a bad name as various proposals exist to add typed enums to Go, meaning enum might become a keyword or a built-in at some point in the future.
Hi, an ther one solution i choosed can be to add a PersistentPreRunE qualifier to your root command to operate on all general flags, or on the command linked flags concerned by. Then you can check for the variable that has catched the value and operate a regex or a switch/case or what ever you want. This way should add a "clean" solution (i mean... readable and splitable easy in a MVC design too) to contourn the lake of time interest from the owner of this nice repo. If you need an example, please ask, i will modify my comment to show you that (but i think you got the point there).
