cobra icon indicating copy to clipboard operation
cobra copied to clipboard

If NoOptDefVal is set, arguments of the form "--myflag myvalue" does not work

Open vasanthv16 opened this issue 5 years ago • 4 comments

If flag.NoOptDefVal is not set, the following two variations work

  1. myapp --myflag myvalue
  2. myapp --myflag=myvalue

But if flag.NoOptDefVal is set, (1) does not work. Only (2) works.

From what I see, the relevant code in command.go seems to be here: https://github.com/spf13/cobra/blob/67fc4837d267bc9bfd6e47f77783fcc3dffc68de/command.go#L487-L499

As you can see, if NoOptDefVal is set, the code is stripping off the argument to the flag. I did not understand why it is doing that. Doesn't NoOptDefVal mean that a default value is to be used when (and only when) no value is specified with the flag? Can someone please clarify this?

vasanthv16 avatar May 22 '19 22:05 vasanthv16

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

github-actions[bot] avatar Apr 06 '20 00:04 github-actions[bot]

Can we reopen this bug? Looking at the docs I think this feature should work like this... If we define a string flag called foo on a command (cmd) and set it's default value to the empty string and we set NoOptDefVal to "NOARGS".

	cmd.PersistentFlags().StringVarP(&fooVal, "foo", "f", "", "value for foo")
	cmd.Flag("foo").NoOptDefVal = "NOARGS"

I expect the code should handle the following cases like this:

  1. "cmd" -> foo is set to ""
  2. "cmd --foo" -> foo is set to "NOARGS"
  3. "cmd --foo bar" -> foo is set to "bar"

Using Cobra version 1.0.0 case 3 does not work, foo is set to "NOARGS". Looking at the code if NoOptDefVal is defined it is used regardless of an option, bar, being specified. Maybe this feature is only meant to work when specifying flag options using the equals syntax?

Looking at pflag.flag.go line 984 the if cases seem out of order. Current code:

	var value string
	if len(split) == 2 {
		// '--flag=arg'
		value = split[1]
	} else if flag.NoOptDefVal != "" {
		// '--flag' (arg was optional)
		value = flag.NoOptDefVal
	} else if len(a) > 0 {
		// '--flag arg'
		value = a[0]
		a = a[1:]
	} else {
		// '--flag' (arg was required)
		err = f.failf("flag needs an argument: %s", s)
		return
	}

If NoOptDefVal is set the code never looks to see if a option has been specified since it is the following case in the if/then/else. Rewriting this code to something like the code below fixes the problem but is probably inadequate and/or incomplete:

        var value string
        if len(split) == 2 {
                // '--flag=arg'
                value = split[1]
        } else if len(a) > 0 {
                if flag.NoOptDefVal != "" && strings.HasPrefix(a[0], "-") {
                        // '--flag'
                        value = flag.NoOptDefVal
                } else {
                        // '--flag arg'
                        value = a[0]
                }
                a = a[1:]
        } else {
                // '--flag' (arg was required)
                err = f.failf("flag needs an argument: %s", s)
                return
        }

Any chance on getting someone to look at this bug and maybe get a more robust solution?

Thank you.

ryanoonayr avatar Jul 08 '20 14:07 ryanoonayr

Repro'd with current codebase.

johnSchnake avatar Feb 17 '22 17:02 johnSchnake

Reproduced with v1.5.0

ohdle avatar Aug 02 '22 23:08 ohdle

Any updates on this issue?

cpach avatar Sep 27 '22 12:09 cpach

Piling on - any ETA for a fix? I think there's a related problem that this fix either needs to get remedied first or would get fixed along the way - namely that Cobra/pflag treats a flag as an argument when it shouldn't.

EG myapp -N -r If -N is defined as a StringVarP , it is given a value of "-r" instead of erroring out as missing its argument. As a user, my expectation is that any argument whose value starts with a - character would need to be in "" to disambiguate.

shueybubbles avatar Jun 02 '23 18:06 shueybubbles

I believe this is correct behavior. When setting the NoOptDefVal field for a flag then only the = form can be used.

The NoOptDefVal field is used to specify a value when the flag is on the command line without any value. For example cmd --foo. But when this is enabled and the user provides cmd --foo bar, how can cobra know if bar is a value for the flag or an argument to the command? It cannot.

So, when NoOptDefVal is set, then only the = form can be used for the flag.

marckhouzam avatar Jun 10 '23 21:06 marckhouzam

i agree there are constraints on when a generic flag processor could support this. Our specific scenario is to replicate the behavior of the old sqlcmd app from https://learn.microsoft.com/en-us/sql/tools/sqlcmd/sqlcmd-utility?view=sql-server-ver15#syntax, particularly for flags like -x and -r that take optional integer values, which may or may not be separated from their flag by a space. This app supports no positional arguments. If there are no positional arguments defined, the processor can infer that everything is either a flag or a parameter to a flag.

So if -n has a NoOptDefVal defined, and there are no positional arguments for the command, we have a few situations:

  1. -n -x foo == -n takes its default value, and -x has the value of "foo"
  2. -n val -x foo == -n' takes the value of "val" and -x` has the value of "foo"
  3. -n foo == -n takes the value of "foo"

shueybubbles avatar Jun 11 '23 14:06 shueybubbles

If there are no positional arguments defined

I don’t believe it’s possible for cobra to know this at the moment.

Supporting such a feature would probably mean having to define an optional behaviour that would be significantly different than how Cobra does things now. I am under the impression that this is not a significantly common case that would justify such complexity.

marckhouzam avatar Jun 11 '23 20:06 marckhouzam

I don’t believe it’s possible for cobra to know this at the moment.

Yes, you can specify that a command doesn't have any positional args. https://github.com/spf13/cobra/blob/0e3a0bfe91a52670ba78ec020f3657d48954a4f7/args.go#L42

jon4hz avatar Jun 12 '23 09:06 jon4hz

Yes, you can specify that a command doesn't have any positional args.

I wish we could use that but: https://github.com/spf13/cobra/issues/1969#issuecomment-1573972622

marckhouzam avatar Jun 12 '23 10:06 marckhouzam

I support the current implementation of NoOptDefVal, but what I don't like is the change in behaviour for a single field when NoOptDefVal is enabled. I.e. myApp -a aVal -b bVal -c=cVal -ddVal. The user would never know / remember to switch to c=cVal for one specific value. Therefore is it possible (without defining NoOptDefVal on all fields) to force the use of x=y on all flags in order to set a common behavior?

Looking at the code I see:

	} else if len(a) > 0 {
		// '--flag arg'
		value = a[0]
		a = a[1:]

This could be changed to something like

	} else if cobra.EnableNextArgFlagAssociation && len(a) > 0  {
		// '--flag arg' if permitted
		value = a[0]
		a = a[1:]

jfigge avatar Jun 21 '23 13:06 jfigge