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

Problem when an argument is empty #909

Closed
provCristianMaluenda opened this issue Jul 16, 2019 · 12 comments
Closed

Problem when an argument is empty #909

provCristianMaluenda opened this issue Jul 16, 2019 · 12 comments
Assignees
Labels
kind/support Questions, supporting users, etc.

Comments

@provCristianMaluenda
Copy link

Hi,
I am facing this issue. When an argument is empty, username in this case:
command --username $USERNAME --pwd $PWD
Cobra assigns --pwd to --username.
If both are required, the error says required flag(s) "pwd" not set.
The message is confused, and I don't understand why this happens.


Library versions:

github.com/spf13/cobra v0.0.3
github.com/spf13/pflag v1.0.3
github.com/spf13/viper v1.3.1

Regards,
Cristian.

@jharshman jharshman added the kind/support Questions, supporting users, etc. label Jul 16, 2019
@jharshman jharshman self-assigned this Jul 16, 2019
@jharshman
Copy link
Collaborator

The message is confused, and I don't understand why this happens.

I'm not sure what you mean by this. The message required flag(s) "pwd" not set is correct because --username is being set to --pwd so it's requirement of being required is filled.

However, to address your primary issue, you can use "=" to ensure that the following flag is not read accidentally in the case of an empty value.

i.e:

command --username=$USERNAME --pwd=$PWD

@umarcor
Copy link
Contributor

umarcor commented Jul 16, 2019

Alternatively, I think you can 'fix' it by using quotes around the variable, "$USERNAME", which is good practice regardless of cobra.

@provCristianMaluenda
Copy link
Author

Thanks for the quick replay.
I tried using "=" and quote in the env var, but the validation doesn't work. In this case, Cobra considers the argument as empty ( "" ).
I think I can try to implement my own type.

@umarcor
Copy link
Contributor

umarcor commented Jul 17, 2019

Do you mean that it fails because it does not accept an empty argument? Or that you cannot handle it because you need to tell apart a default empty and a user-provided empty?

@provCristianMaluenda
Copy link
Author

It doesn't work because the flag exists but it is an empty string. So, the required validation doesn't work in this case. I have two options:

  • Add a preRun function to validate the arguments
  • Create a custom type.

@umarcor
Copy link
Contributor

umarcor commented Jul 17, 2019

I cannot reproduce. See:

package main

import (
  "fmt"
  "os"
  "github.com/spf13/cobra"
)

func main() {
  if err := rootCmd.Execute(); err != nil {
    fmt.Println(err)
    os.Exit(1)
  }
}

// rootCmd represents the base command when called without any subcommands
var rootCmd = &cobra.Command{
  Use:   "test",
  Short: "A brief description of your application",
  Long: `A longer description...`,
  Run: func(cmd *cobra.Command, args []string) {
    fmt.Println(args)
  },
}

var Username string
var Password string

func init() {
  rootCmd.Flags().StringVarP(&Username, "username", "u", "default", "username (required)")
  rootCmd.Flags().StringVarP(&Password, "password", "p", "default", "password (required)")

  rootCmd.MarkFlagRequired("username")
  rootCmd.MarkFlagRequired("password")
}
root@91266ab2b8e1:/src/cobra/test# go run main.go    
Error: required flag(s) "password", "username" not set

root@91266ab2b8e1:/src/cobra/test# go run main.go -u user -p pass
[]

root@91266ab2b8e1:/src/cobra/test# go run main.go -u user -p pass aaa
[aaa]

root@91266ab2b8e1:/src/cobra/test# go run main.go -u -p pass aaa
Error: required flag(s) "password" not set

root@91266ab2b8e1:/src/cobra/test# go run main.go -u "" -p pass aaa
[aaa]

root@91266ab2b8e1:/src/cobra/test# go run main.go -u= -p pass aaa
[aaa]

root@91266ab2b8e1:/src/cobra/test# go run main.go -u= -p= aaa
[aaa]

root@91266ab2b8e1:/src/cobra/test# go run main.go -u= -p "" aaa
[aaa]

root@91266ab2b8e1:/src/cobra/test# go run main.go -u "" -p "" aaa
[aaa]

@jharshman
Copy link
Collaborator

@umarcor which version of cobra did you try to reproduce this with?
I notice that @provCristianMaluenda is using an older version. I haven't looked into it yet, but maybe a bug fix occurred between 0.0.3 and current 0.0.5

@provCristianMaluenda
Copy link
Author

provCristianMaluenda commented Jul 17, 2019

Thank @umarcor for your example.
This is the behavior that I think it is not desired.

root@91266ab2b8e1:/src/cobra/test# go run main.go -u= -p= aaa
[aaa]

root@91266ab2b8e1:/src/cobra/test# go run main.go -u= -p "" aaa
[aaa]

root@91266ab2b8e1:/src/cobra/test# go run main.go -u "" -p "" aaa
[aaa]

For me, if I set an argument -p "" I would expect an Error: required flag(s) "password" not set. But, instead of an error, it continues the execution. So, in this case, the required is useless, and I need to validate the input in my code anyway.

@jharshman
Copy link
Collaborator

This is expected behavior.
https://godoc.org/github.com/spf13/cobra#Command.MarkFlagRequired

It guarantees the presence of the flag, not the argument.

@provCristianMaluenda
Copy link
Author

You are right. My bad!
Thank guys for your help!

@jharshman
Copy link
Collaborator

No problem.

@umarcor
Copy link
Contributor

umarcor commented Jul 18, 2019

@umarcor which version of cobra did you try to reproduce this with?

I used the latest master.

For me, if I set an argument -p "" I would expect an Error: required flag(s) "password" not set. But, instead of an error, it continues the execution. So, in this case, the required is useless, and I need to validate the input in my code anyway.

I think that this feature request makes sense, but not as the default behaviour for MarkFlagRequired. Either MarkFlagRequired could accept a RequireNonEmpty boolean argument, or another function could be added (say MarkRequiredNonEmpty). However, this deserves some care, as it is related to spf13/viper#276 (also spf13/viper#657).

Meanwhile, you can add a check in the initConfig function, just as it is done with cfgFile. I don't know how does this work with non-root commands, tho.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/support Questions, supporting users, etc.
Projects
None yet
Development

No branches or pull requests

3 participants