I'm defining some flags like this:
follow_sync_gwCmd.PersistentFlags().String("url", "", "Sync Gateway URL")
follow_sync_gwCmd.MarkPersistentFlagRequired("url")
and even explicitly calling ParseFlags (out of desperation):
Run: func(cmd *cobra.Command, args []string) {
if err := cmd.ParseFlags(args); err != nil {
log.Printf("err: %v", err)
}
...
},
and calling:
$ go run main.go follow_sync_gw
with no --url flag, but cobra isn't throwing errors.
I was expecting that if I set MarkPersistentFlagRequired but failed to pass the flag on the command line then the command would fail with an error and a usage example.
I am not sure that is implemented as it uses, pflag
So there is no direct way unless you do it in PreRun function with an if or switch but I could be wrong but that is how I have handled it in the past.
Really its just a bad name since it was never implemented. It will cause bash completions and things to always suggest that flag, but we aren't enforcing it. You doing it in a PreRunE is a good idea.
I didn't know about the PreRun function, I'll look into that. Yeah, the name is misleading .. so does that mean calling MarkPersistentFlagRequired has no effect whatsoever?
@tleyden use PreRunE :+1:
At this time its only effect is on the bash completions :-(
I will try to write a PreRunE which enforces it....
MarkFlagRequired and MarkPersistentFlagRequired should enforce the flag IMO. Or the function name should be different at least. I just wasted an hour wondering why it's not working :/
just a reminder, if it is required it is an parameter, or argument. If it is and option or flag it is NOT required. That is the very nature of flags and options. Every command line library supporting "required" or mandatory options/flags I would considered broken by design.
@crux like the heroku toolbelt which which requires the name of the application you're dealing with to be passed to the -a flag?
From my point of view it it makes sense to use arguments if there are only a few (1-3) but as soon as you have more mandatory arguments, it makes sense to use flags. Otherwise it is hard to keep the overview.
If you need it I wrote something for this purpose. It could be done better but this does the work. You just need to mark the flags as required and then:
import (
"errors"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
)
func CheckRequiredFlags(flags *pflag.FlagSet) error {
requiredError := false
flagName := ""
flags.VisitAll(func(flag *pflag.Flag) {
requiredAnnotation := flag.Annotations[cobra.BashCompOneRequiredFlag]
if len(requiredAnnotation) == 0 {
return
}
flagRequired := requiredAnnotation[0] == "true"
if flagRequired && !flag.Changed {
requiredError = true
flagName = flag.Name
}
})
if requiredError {
return errors.New("Required flag `" + flagName + "` has not been set")
}
return nil
}
Then add these in PreRunE:
...
PreRunE: func(cmd *cobra.Command, args []string) error {
return CheckRequiredFlags(cmd.Flags())
},
...
Is @0xC0D3D00D's solution still the way to go with this? I understand his snippet and it's pretty clever, but it seems that it should be a better way to mark flags as required (even when some could argue that it shouldn't be a flag in the first instance).
@agonzalezro, At Kubernetes @dixudx @brendandburns made my snippet even better. You can find it in this merge request.
On the other side, I think the current design of flags in cobra is impractical in many cases. I like the verb and noun concepts but the flags need to be required in some cases. For example:
$ hypervisor create vm --name=MyVM --ram=2Gi --cpus 4 --network=bridge --os=linux
$ http get http://example.com --auth=http-basic --user=john --password=smith
The latter is a little harder to implement. I'm sure there are more cases which this feature is necessary for them.
I submitted a PR #502 to enforce the required flag. PTAL.
Hi guys, can this issue be closed since #502 has been merged.
Fixed in #502
I added a usage example here
Don't know if this is really fixed. When using the examples in the readme for creating a required flag but with PersistentFlags() rather than Flags(), the program continues to run even though the "required" flag wasn't set. Don't know if this is per design, or if only non-persistent flags are suppose to be able to be required.
Anyhow, a workaround, hope it help someone who ends up here:
var requestSingleURL = &cobra.Command{
Use: "singleURL",
Short: "Test a single URL",
Long: `See if a single URL is up and running`,
Args: cobra.MinimumNArgs(0),
Run: func(cmd *cobra.Command, args []string) {
fflags := cmd.Flags() // fflags is a *flag.FlagSet
if fflags.Changed("path") == false { // check if the flag "path" is set
fmt.Println("no path specified") // If not, we'll let the user know
return // and return
}
testIfOnline(args[0])
},
}
Thanks
@Spaddex thank you for the code snippet - I was running into a similar issue when trying to use required persistent flags. I ended up using the following approach (harnessing cobra.MarkFlagRequired):
var foo string
func init() {
pf := rootCmd.PersistentFlags()
pf.StringVarP(&foo, "foo", "f", "", "example flag")
cobra.MarkFlagRequired(pf, "foo")
}
@spaddex thank you for the code snippet - I was running into a similar issue when trying to use required persistent flags. I ended up using the following approach (harnessing
cobra.MarkFlagRequired):var foo string func init() { pf := rootCmd.PersistentFlags() pf.StringVarP(&foo, "foo", "f", "", "example flag") cobra.MarkFlagRequired(pf, "foo") }
It's unfortunate that enforcing required flags natively is solely dependent on shell completion scripts. This just passes the buck to the user, who now has to inject the script into their shell in order for the app to run as it's designed.
It's unfortunate that enforcing required flags natively is solely dependent on shell completion scripts. This just passes the buck to the user, who now has to inject the script into their shell in order for the app to run as it's designed.
@copejon I'm not sure I understand what you mean. What scenario is causing you problems?
Most helpful comment
MarkFlagRequiredandMarkPersistentFlagRequiredshould enforce the flag IMO. Or the function name should be different at least. I just wasted an hour wondering why it's not working :/