-
Notifications
You must be signed in to change notification settings - Fork 45
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
Use Cobra built-in mutually exclusive functionality #1627
Comments
In order to make use of the |
Thanks for the hint. Will investigate that once I tackle this issue. |
No problem. It's a pretty easy fix to implement once that is in place. I do have the updated few files to resolve the issue, however, I was hitting problems with the |
So based on the above, you may need to look at setting the In your
This is a very rough idea, and will need more ideal refinement. Without tracing back through your logger package to see if that can be made use of more than simply returning the formatted error, this was the simplest idea I could come up with. From what I am able to ascertain from the cobra documentation, there is no specific function that will handle the error message for mutually exclusive flags being used, and I'm not particularly confident that the above will work as intended. |
Frankly, I dont think the default error message from Cobra is terrible. It surely is not bad enough that I would implement any specific error logging for it. |
Indeed, however for consistency it would be nice to customise it in a way that you would like. Even if it's something so simple as to update the date format & app prefix as you have done for your logger implementation. |
I agree with @Galzzly ; and actually I didn't see an issue with the code without using MarkFlagsMutuallyExclusive @jdknives if you think it's fine as-is; you can go ahead and review / merge #1629 @Galzzly i invited you to my fork of skywire ; if you can think of a way to integrate the logging feel free to push to the branch of my PR if you want to. |
@0pcom - I'm taking a look at this now. I didn't get a chance over the weekend or yesterday. |
I've gone through the Cobra code to try to understand where the log output for mutually exclusive flags can be set. In short, outside of what you do already or the default mutually exclusive message, there currently does not appear to be a way to do this. As a part of the execution of a command, the flags will be parsed and an error returned (which can be customised via the I will be raising an issue against Cobra for this, as I believe that this would be a useful addition to have. With this in mind, though, for the time being - if you prefer to have the output per your logger, then it may be more worthwhile to keep your existing code as is unless you are happy with the default output from Cobra. Apologies that I was not able to help too much with this. |
Feature description
We currently manually detect mutually exclusive flags inside Skywire. Apparently there is a built-in function for that in Cobra, which we could use.
Documentation on the function https://pkg.go.dev/github.com/spf13/cobra#Command.MarkFlagsMutuallyExclusive
Example of case where we detect incompatibility manually:
skywire/cmd/skywire-cli/commands/config/gen.go
Lines 183 to 189 in 222be51
The text was updated successfully, but these errors were encountered: