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

rootCmd MarkPersistentFlagRequired does not raise error #747

Open
niski84 opened this issue Sep 17, 2018 · 7 comments
Open

rootCmd MarkPersistentFlagRequired does not raise error #747

niski84 opened this issue Sep 17, 2018 · 7 comments
Labels
area/flags-args Changes to functionality around command line flags and args kind/bug A bug in cobra; unintended behavior triage/needs-triage Needs to be placed into a milestone or be closed by maintainers

Comments

@niski84
Copy link

niski84 commented Sep 17, 2018

Using spf13/Cobra for cli flag parsing.

root command has a field marked required:


rootCmd.PersistentFlags().StringVarP(&configFilePath, "config", "c","", "REQUIRED: config file")
    rootCmd.MarkPersistentFlagRequired("config")    
    rootCmd.MarkFlagRequired("config")

However, cobra does not raise an error if it's the root command.

If I add a subcommand and add a required field, .MarkFlagRequired raises an error as expected if the argument is not provided on the command line.

@kjaskiewiczz
Copy link

I'm facing the same issue.
Before the upgrade, I've been using revision a1f051b and it was working (at least this issue was not present there).

@mattthym
Copy link

I came across this issue in one of my CLI tools built with cobra in which I have a command for which I have defined some flags which I marked as required, which works fine as long as I define the flag with Flags().StringP(...) but does not work if I use Flags().StringVarP(...) so I think it does not relate to the flags being persistent.

Code to replicate the issue:

package cmd

import (
	"fmt"

	"github.com/spf13/cobra"
)

var testCmd = &cobra.Command{
	Use: "test",
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Println(Foo)
	},
}

var Foo string

func init() {
	rootCmd.AddCommand(testCmd)

	testCmd.Flags().StringVarP(&Foo, "foo", "f", "", "A help for foo")
	testCmd.MarkFlagRequired(Foo)
}

The problem is, that the command executes without printing any error, unlike how it does when defining flags with Flags().StringP().

This works just fine:

package cmd

import (
	"fmt"

	"github.com/spf13/cobra"
)

var testCmd = &cobra.Command{
	Use: "test",
	RunE: func(cmd *cobra.Command, args []string) error {
		Foo, err := cmd.Flags().GetString("foo")
		if err != nil {
			return err
		}
		fmt.Println(Foo)
		return nil
	},
}

func init() {
	rootCmd.AddCommand(testCmd)

	testCmd.Flags().StringP("foo", "f", "", "A help for foo")
	testCmd.MarkFlagRequired("foo")
}

@ashishgalagali
Copy link

#206 (comment)

@github-actions
Copy link

This issue is being marked as stale due to a long period of inactivity

@dghant1024
Copy link

dghant1024 commented Apr 9, 2021

I'm really surprised this code below doesn't work either:

         rootCmd.PersistentFlags().StringP("needed-file", "i", "", "needed file path. Defaults (required)")
	 if err := rootCmd.MarkPersistentFlagRequired("needed-file"); err != nil {
		fmt.Println("HELLO people")
	}

Error not raise if in rootCmd. Any thoughts are this are welcomed.

@johnSchnake johnSchnake added kind/bug A bug in cobra; unintended behavior area/flags-args Changes to functionality around command line flags and args triage/needs-triage Needs to be placed into a milestone or be closed by maintainers labels Aug 15, 2022
@alexrs
Copy link

alexrs commented Nov 22, 2022

I'm having the same problem with:

package cmd

import (
	"fmt"

	"github.com/spf13/cobra"
)

var (
	// Used for flags.
	Region string
	Test   string

	rootCmd = &cobra.Command{
		Use: "cli-test",
	}
)

// Execute executes the root command.
func Execute() {
	fmt.Println(Test)
}

func init() {
	rootCmd.PersistentFlags().StringVarP(&Region, "region", "r", "", "region (required)")
	rootCmd.PersistentFlags().StringVarP(&Test, "test", "t", "", "Test flag")
	rootCmd.MarkPersistentFlagRequired("region")
}

If I call my CLI without -r, it does not return any errors.

@elulcao
Copy link

elulcao commented Jan 16, 2023

I'm facing the same issue mentioned here, however I feel this is not a problem with Cobra and root commands but an interpretation maybe.
In my case the default value, as well examples here, is "" an empty string which is not nil nor empty so it seems that this empty value is correct for the persistent and required flag.

rootcmd.PersistentFlags().String("config-file", "", "The config file that contains the information to add the entry")
rootcmd.MarkPersistentFlagRequired("config-file")

If we add some extra traces we will see that a config file wasn't found:

2023-01-16T11:04:00.810-0600 [ERROR] mycmd: error adding an entry: open : no such file or directory

Previous output is actually including the empty string like adding an open <empty string here>: no such file, this make me think that Cobra is OK but missing documentation issue. It would be better if Cobra doc mention that MarkPersistentFlagRequired() interprets empty string as valid value and hence Cobra User should add some extra code to check situations where empty values are misinterpreted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/flags-args Changes to functionality around command line flags and args kind/bug A bug in cobra; unintended behavior triage/needs-triage Needs to be placed into a milestone or be closed by maintainers
Projects
None yet
Development

No branches or pull requests

8 participants