-
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
Command.Run should return error #67
Comments
👍 this is very welcomed. I cringe everytime I have to do error processing on my inner functions. :D |
Yes, well - I don't see a way to change this without breaking a lot of user code ... To my surprise, the "other Go CLI framework", seems to be doing exactly the same. |
A little surprised to see so little interest in this issue. @eparis what do you to handle this problem? Handle all errors inside the commands? |
For the most part, everything I do called
And inside run we've got glog.Fatal() (which does os.Exit(1) on unhandl-able errors) |
Perhaps an alternative Run command, e.g: RunWithError: func(cmd *cobra.Command, args []string) error { (not the best name to be sure) |
I would agree with this but it would be a breaking news change. We should always populate the errors up. @eparis a good use case would be testing but also populating the errors up would allow for a single error handling for parent and child commands as well. |
Agreed, I'm using a fork for the time being. I prefer to have a single os.Exit on a single error exit point. |
So I don't think we can/should change |
https://gist.github.com/eparis/be6ca4c767e69fcaaea8 ?? Maybe something like that? |
@eparis agree. |
I think that the only thing is that patch would need to have is these: |
#67 creates RunE functions to allow for errors to populate to the top
The error handling in general confuses me.
The text was updated successfully, but these errors were encountered: