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

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

Open
vasanthv16 opened this issue May 22, 2019 · 12 comments
Labels
area/flags-args Changes to functionality around command line flags and args kind/bug A bug in cobra; unintended behavior

Comments

@vasanthv16
Copy link

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:

cobra/command.go

Lines 487 to 499 in 67fc483

case strings.HasPrefix(s, "--") && !strings.Contains(s, "=") && !hasNoOptDefVal(s[2:], flags):
// If '--flag arg' then
// delete arg from args.
fallthrough // (do the same as below)
case strings.HasPrefix(s, "-") && !strings.Contains(s, "=") && len(s) == 2 && !shortHasNoOptDefVal(s[1:], flags):
// If '-f arg' then
// delete 'arg' from args or break the loop if len(args) <= 1.
if len(args) <= 1 {
break Loop
} else {
args = args[1:]
continue
}

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?

@github-actions
Copy link

github-actions bot commented Apr 6, 2020

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

@ryanoonayr
Copy link

ryanoonayr commented Jul 8, 2020

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.

@johnSchnake
Copy link
Collaborator

Repro'd with current codebase.

@johnSchnake johnSchnake added area/flags-args Changes to functionality around command line flags and args kind/bug A bug in cobra; unintended behavior needs PR and removed kind/stale labels Feb 17, 2022
@jpmcb jpmcb removed the needs PR label Feb 23, 2022
@ohdle
Copy link

ohdle commented Aug 2, 2022

Reproduced with v1.5.0

@cpach
Copy link

cpach commented Sep 27, 2022

Any updates on this issue?

@shueybubbles
Copy link

shueybubbles commented Jun 2, 2023

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.

@marckhouzam
Copy link
Collaborator

marckhouzam commented Jun 10, 2023

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.

@shueybubbles
Copy link

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"

@marckhouzam
Copy link
Collaborator

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.

@jon4hz
Copy link

jon4hz commented Jun 12, 2023

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.

cobra/args.go

Line 42 in 0e3a0bf

func NoArgs(cmd *Command, args []string) error {

@marckhouzam
Copy link
Collaborator

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

I wish we could use that but: #1969 (comment)

@jfigge
Copy link

jfigge commented Jun 21, 2023

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

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
Projects
None yet
Development

No branches or pull requests

10 participants