Command.Run should return error about cobra HOT 13 CLOSED

Perhaps an alternative Run command, e.g:

RunWithError: func(cmd *cobra.Command, args []string) error {

(not the best name to be sure)

from cobra.

👍 this is very welcomed. I cringe everytime I have to do error processing on my inner functions. :D

from cobra.

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.

from cobra.

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?

from cobra.

For the most part, everything I do called

if err := cmd.Execute(); if err != nil {
   os.Exit(1)
}
os.Exit(0)

And inside run we've got glog.Fatal() (which does os.Exit(1) on unhandl-able errors)

from cobra.

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.

from cobra.

Agreed, I'm using a fork for the time being. I prefer to have a single os.Exit on a single error exit point.
https://github.com/pgruenbacher/cobra

from cobra.

So I don't think we can/should change Run. Too many people are using it I'd expect. Maybe we should add a RunE or something like that? Same thing as run but which returns an error which gets populated back up through Execute() ?

from cobra.

https://gist.github.com/eparis/be6ca4c767e69fcaaea8 ?? Maybe something like that?

from cobra.

@eparis agree. RunE also matches the naming convention I've seen elsewhere in situation like this.

from cobra.

@eparis @bep I agree. That works if we want to use that but I think it would be better in the longer term to make that the default. Possibly a deprecation period?

from cobra.

I think that the only thing is that patch would need to have is these: PostRunE, PersistentPostRunE, PreRunE and PersistentPreRunE to achieved the full desired effect.

from cobra.

@bep @eparis I think this should be good to close with the fix from Alex, right?

from cobra.