Adds Persistent{Pre,Post}Run hook chaining

[poy]

PersistentPreRun and PersistentPostRun are chained together so that each child PersistentPreRun is ran, and the PersistentPostRun are ran in reverse order. For example:

Commands: root -> subcommand-a -> subcommand-b

root - PersistentPreRun subcommand-a - PersistentPreRun subcommand-b - PersistentPreRun subcommand-b - Run subcommand-b - PersistentPostRun subcommand-a - PersistentPostRun root - PersistentPostRun

fixes #252

[CLAassistant]

All committers have signed the CLA.

[marckhouzam]

Thanks @poy for the contribution.

We recently discussed this topic in #1198 and with my limited experience, I got the impression that the current behaviour allowed for the most flexibility. It allows to either override an ancestor's PersistentPreRun function by defining one in a subcmd, and it allows to extend ancestor's PersistentPreRun by calling them explicitly from a subcmd.

I'm trying to compare the value of this PR's new approach with the current one. Could you explain a little more how your change will change current behaviour, and why it is better?

I'm also curious if this change is breaking? Existing programs that have PersistentPreRuns will behave differently with this change? This was mentioned in https://github.com/spf13/cobra/issues/252#issuecomment-622561414.

[poy]

This PR will allow a user to create a hierarchy of PrePersistentRun and PostPersistentRun. Meaning if the root and sub command both have a hook, then both will be invoked. The pre-hooks get invoked in order of root -> subcommand-1 -> subcommand-2 -> ... -> subcommand-N. This allows the earlier subcommands to setup anything necessary for the laster commands. The post-hooks are invoked in the opposite order: subcommand-N -> ... -> subcommand-2 -> subcommand-1 -> root. This reverse order allows for easier cleanup (similar to how defer was implemented).

This is not a breaking change (now :) as this behavior only takes place when TraverseChildrenHooks is set to true on the root node. It will otherwise utilize the old behavior of subcommands overriding their parent's hooks.

[poy]

Is there any chance of this being merged?

[infalmo]

@marckhouzam A gentle reminder to review (and merge) this PR when free :smile:

[marckhouzam]

@marckhouzam A gentle reminder to review (and merge) this PR when free 😄

I'm not a maintainer 😉 I just try to help when I can.

[marckhouzam]

This PR will allow a user to create a hierarchy of PrePersistentRun and PostPersistentRun.

@poy Could you instead call cmd.Parent().PrePersistentRun() from the subcommand?

[poy]

Yes. However, I think given the name "Persistent", it shouldn't be required. PersistentFlags for example don't have to be added in this way.

[github-actions[bot]]

This PR is being marked as stale due to a long period of inactivity

[poy]

Would still love for this to be considered.

[f0m41h4u7]

Hey all. I am also facing this issue with persistent pre- and post-runs currently, and it leads to some annoying code repeats... So I'm really looking forward to this fix being merged some day.

[niamster]

@marckhouzam A gentle reminder to review (and merge) this PR when free 😄

I'm not a maintainer 😉 I just try to help when I can.

@marckhouzam what is required to have this PR merged (now that you are a maintainer 😄) ?

[niamster]

This PR will allow a user to create a hierarchy of PrePersistentRun and PostPersistentRun.

@poy Could you instead call cmd.Parent().PrePersistentRun() from the subcommand?

It might be very easy to forget adding that if new command creation is not automated and there are many commands added by multiple people, potentially by multiple teams.

[polothy]

Think this was fixed by https://github.com/spf13/cobra/pull/2044 and released in 1.8.0

[niamster]

Think this was fixed by #2044 and released in 1.8.0

Great news! I think it's worth closing all issues and non-merged PRs now 🎉

[marckhouzam]

Thank you for your efforts @niamster . I'm going to close this as #2044 addresses this.