Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: how to make flags required #206

Closed
tleyden opened this issue Dec 6, 2015 · 21 comments
Closed

Question: how to make flags required #206

tleyden opened this issue Dec 6, 2015 · 21 comments

Comments

@tleyden
Copy link

tleyden commented Dec 6, 2015

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.

@apriendeau
Copy link
Contributor

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.

@eparis
Copy link
Collaborator

eparis commented Dec 9, 2015

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.

@tleyden
Copy link
Author

tleyden commented Dec 9, 2015

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?

@apriendeau
Copy link
Contributor

@tleyden use PreRunE 👍

@eparis
Copy link
Collaborator

eparis commented Dec 9, 2015

At this time its only effect is on the bash completions :-(

I will try to write a PreRunE which enforces it....

@ApsOps
Copy link

ApsOps commented Sep 20, 2016

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 :/

@crux
Copy link

crux commented Mar 30, 2017

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.

@davidcpell
Copy link

@crux like the heroku toolbelt which which requires the name of the application you're dealing with to be passed to the -a flag?

@thomaspeitz
Copy link

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.

@0xC0D3D00D
Copy link

0xC0D3D00D commented Jun 24, 2017

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())
	},
...

concaf added a commit to concaf/kedge that referenced this issue Jul 12, 2017
Before this commit, running `kedge` subcommands without passing
any files would do nothing.

$ kedge deploy
$

$ kedge generate
$

$ kedge undeploy
$

This commit adds a check that input files are passed using -f or
--files flags, and if not, throws an error.

$ kedge generate
Error: Unable to validate input files: No files were passed. Please pass file(s) using '-f' or '--files'

The flags have also been marked as required, however, that takes
no effect, and there is an open issue on the spf13/cobra package
tracking the same - spf13/cobra#206
concaf added a commit to concaf/kedge that referenced this issue Jul 12, 2017
Before this commit, running `kedge` subcommands without passing
any files would do nothing.

$ kedge deploy
$

$ kedge generate
$

$ kedge undeploy
$

This commit adds a check that input files are passed using -f or
--files flags, and if not, throws an error.

$ kedge generate
Error: Unable to validate input files: No files were passed. Please pass file(s) using '-f' or '--files'

The flags have also been marked as required, however, that takes
no effect, and there is an open issue on the spf13/cobra package
tracking the same - spf13/cobra#206

Fixes kedgeproject#96
concaf added a commit to concaf/kedge that referenced this issue Jul 12, 2017
Before this commit, running `kedge` subcommands without passing
any files would do nothing.

$ kedge deploy
$

$ kedge generate
$

$ kedge undeploy
$

This commit adds a check that input files are passed using -f or
--files flags, and if not, throws an error.

$ kedge generate
Error: Unable to validate input files: No files were passed. Please pass file(s) using '-f' or '--files'

The flags have also been marked as required, however, that takes
no effect, and there is an open issue on the spf13/cobra package
tracking the same - spf13/cobra#206

Fixes kedgeproject#96
concaf added a commit to concaf/kedge that referenced this issue Jul 14, 2017
Before this commit, running `kedge` subcommands without passing
any files would do nothing.

$ kedge deploy
$

$ kedge generate
$

$ kedge undeploy
$

This commit adds a check that input files are passed using -f or
--files flags, and if not, throws an error.

$ kedge generate
Error: Unable to validate input files: No files were passed. Please pass file(s) using '-f' or '--files'

The flags have also been marked as required, however, that takes
no effect, and there is an open issue on the spf13/cobra package
tracking the same - spf13/cobra#206

Fixes kedgeproject#96
@agonzalezro
Copy link

agonzalezro commented Aug 10, 2017

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).

@0xC0D3D00D
Copy link

0xC0D3D00D commented Aug 16, 2017

@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:

  • Commands which need a lot of required parameters to run
$ hypervisor create vm --name=MyVM --ram=2Gi --cpus 4 --network=bridge --os=linux
  • Commands which a flag required as a result of using another flag:
$ 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.

@dixudx
Copy link
Contributor

dixudx commented Aug 16, 2017

I submitted a PR #502 to enforce the required flag. PTAL.

@dixudx
Copy link
Contributor

dixudx commented Oct 10, 2017

Hi guys, can this issue be closed since #502 has been merged.

@n10v
Copy link
Collaborator

n10v commented Oct 10, 2017

Fixed in #502

@n10v n10v closed this as completed Oct 10, 2017
@czerasz
Copy link

czerasz commented Jan 4, 2018

I added a usage example here

@spaddex
Copy link

spaddex commented May 27, 2018

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])
    },
}

hannahhenderson added a commit to CircleCI-Public/circleci-cli that referenced this issue Jul 26, 2018
Seems like this doesn't _actually_ work. Sadness
spf13/cobra#206
hannahhenderson added a commit to CircleCI-Public/circleci-cli that referenced this issue Jul 30, 2018
Seems like this doesn't _actually_ work. Sadness
spf13/cobra#206
@euclid1990
Copy link

Thanks

@jabclab
Copy link

jabclab commented Mar 12, 2019

@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")
}

@copejon
Copy link

copejon commented Oct 26, 2020

@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.

@marckhouzam
Copy link
Collaborator

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests