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

added pre and post run hooks. #53

Closed
wants to merge 3 commits into from

Conversation

AlexanderThaller
Copy link

added two new hooks to the command type.

PreRun will run after parsing the flags and args but before starting Run.
PostRun will run after Run has completed.

@spf13
Copy link
Owner

spf13 commented Feb 10, 2015

I like the idea, but I can't figure out a good use case. What's the use case for this? I don't see why this isn't just part of Run?

@tdavis
Copy link

tdavis commented Feb 10, 2015

I'll just chime in and say that I'm writing my first cli with cobra and already need a prerun type thing. Each sub-command needs some global configuration stuff setup using the vars from persistent root flags. Since the root's Run method isn't called when sub-commands run, there's no place to do global setup after the persistent flags have been parsed but before the current command runs.

@spf13
Copy link
Owner

spf13 commented Feb 10, 2015

@tdavis Why not just make it the first line of Run?

I could possibly see the need for a Global PreRunner or a tree based PreRunner (like the persistent flags), but have a hard time seeing the need for it on a per command basis.

Overall I'm in favor of this idea, I just want to understand the need better.

@tdavis
Copy link

tdavis commented Feb 11, 2015

@spf13 I am; more specifically cmd.Parent().Run(), but it feels strange to manually invoke the command tree. I see even hugo does this with InitializeConfig() so who am I to argue your intuition? ;)

@AlexanderThaller
Copy link
Author

I needed it so I can setup some logging based on a flag I have defined. I wanted to do this with every sub command as well so it felt better to use a separate command and not duplicate code in every command Run function. Inheriting such a Pre/Post command to the sub commands would be better this was just to solve my problem and I saw that others had the same problem #47 so I made a pull request.

I can try to implement the inheriting if you still think that would be a good idea and update the pull request.

@spf13
Copy link
Owner

spf13 commented Feb 17, 2015 via email

@eparis
Copy link
Collaborator

eparis commented Apr 6, 2015

I've now stepped on enough code that this can not be merged. It seems spf13 was happy with the idea. If you still want to do it, please rebase and add some documentation (readme)

@eparis eparis mentioned this pull request Apr 30, 2015
@eparis
Copy link
Collaborator

eparis commented Apr 30, 2015

Closing, I rebased into #100

@eparis eparis closed this Apr 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants