-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[Bug] SetArgs doesn't reset flag value #2079
Comments
Thanks for the report @levisyin . Can you explain in what scenario this is a problem? |
Hi sir, we use this in test case. As |
Ok, I understand. This does seem limiting. I have to think about this a bit as there may be backwards-compatibility issues in changing the behaviour of |
How about changing the |
@levisyin I actually hit this problem today when writing unit tests. What I did was reset the relevant flags myself in the tests. I recommend you implement this solution in your own tests. So, although I agree it can be annoying, since it can be fixed by the project using cobra, I will close this issue as "won't fix". |
@marckhouzam Hi sir, I met another issue with this bug. It's too difficult to implement the solution in my own tests as following. func SetArgs(cmd *cobra.Command, args []string) {
if cmd.Flags().Parsed() {
cmd.Flags().Visit(func(pf *pflag.Flag) {
if err := pf.Value.Set(pf.DefValue); err != nil {
panic(fmt.Errorf("reset argument[%s] value error %v", pf.Name, err))
}
})
}
cmd.SetArgs(args)
}
So, can we add a new function type Value interface {
String() string
Set(string) error
Reset() error
Type() string
} Every implementations implement
If this LGTY, I will draft a pr to implement this~ |
@marckhouzam Hi sir, can we reopen this issue, I indeed couldn't find a better way for implementing the solution in my own project tests, could you please give me some suggestions😀 |
@levisyin However, two thoughts on this:
|
@marckhouzam Hi sir, thanks for the reminder. For thought 1, after I reviewed again, the builtin implements of For thought 2, since this is aimed to be a |
See following test code. Call
SetArgs
won't reset flag value,Execute()
also won't reset flag value too.The text was updated successfully, but these errors were encountered: