Run Persistent*Run from root to sub

[galexrt]

Call all Persitent*Run defined from sub to root

Signed-off-by: Alexander Trost <[email protected]>

Resolves #252

I haven't updated the docs with the "new" behavior yet, because I want to see if this is wanted.

[CLAassistant]

All committers have signed the CLA.

[au-phiware]

https://github.com/spf13/cobra/pull/714/files#diff-9c42b524cb14ca001c61267826cbefb1R787

I think it makes more sense that the post hooks mirror the order of execution of the pre hooks. E.g.

Run root's PersistentPreRun
Run parent's PersistentPreRun
Run child's PersistentPreRun
Run child's PreRun
Run child's Run
Run child's PostRun
Run child's PersistentPostRun
Run parent's PersistentPostRun
Run root's PersistentPostRun

https://github.com/spf13/cobra/pull/714/files#diff-9c42b524cb14ca001c61267826cbefb1R791

Is the intended behaviour to run all post run hooks? I.e. some setup is done in a pre hook and then the corresponding post hook does cleanup? We could wind up with unexpected behaviour if one of those inner post hooks return an error. Perhaps this is a separate issue, since the same could be said of the pre hooks too.

[au-phiware]

Also, this is a breaking change, please don't merge. Perhaps it could be enabled with a TraverseRunHooks flag on the Command struct?

FWIW, I have implemented the similar behaviour with this function:

// TraverseRunHooks modifies c's PersistentPreRun* and PersistentPostRun*
// functions (when present) so that they will search c's command chain and
// invoke the corresponding hook of the first parent that provides a hook.
// When used on every command in the chain the invocation of hooks will be
// propagated up the chain to the root command.
//
// In the case of PersistentPreRun* hooks the parent hook is invoked before the
// child hook.  In the case of PersistentPostRun* the child hook is invoked
// first.
//
// Use it in place of &cobra.Command{}, e.g.
//     command := TraverseRunHooks(&cobra.Command{
//     	PersistentPreRun: func ...,
//     })
func TraverseRunHooks(c *cobra.Command) *cobra.Command {
	preRunE := c.PersistentPreRunE
	preRun := c.PersistentPreRun
	if preRunE != nil || preRun != nil {
		c.PersistentPreRun = nil
		c.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
			for p := c.Parent(); p != nil; p = p.Parent() {
				if p.PersistentPreRunE != nil {
					if err := p.PersistentPreRunE(cmd, args); err != nil {
						return err
					}
					break
				} else if p.PersistentPreRun != nil {
					p.PersistentPreRun(cmd, args)
					break
				}
			}

			if preRunE != nil {
				return preRunE(cmd, args)
			}

			preRun(cmd, args)

			return nil
		}
	}

	postRunE := c.PersistentPostRunE
	postRun := c.PersistentPostRun
	if postRunE != nil || postRun != nil {
		c.PersistentPostRun = nil
		c.PersistentPostRunE = func(cmd *cobra.Command, args []string) error {
			if postRunE != nil {
				if err := postRunE(cmd, args); err != nil {
					return err
				}
			} else if postRun != nil {
				postRun(cmd, args)
			}

			for p := c.Parent(); p != nil; p = p.Parent() {
				if p.PersistentPostRunE != nil {
					if err := p.PersistentPostRunE(cmd, args); err != nil {
						return err
					}
					break
				} else if p.PersistentPostRun != nil {
					p.PersistentPostRun(cmd, args)
					break
				}
			}

			return nil
		}
	}

	return c
}

[hamdiallam]

Can this be merged into a new release.

Otherwise, it's not really Persistent.

[ghostsquad]

Can this behavior be enabled via a flag and merged? This seems quite useful.

[steven200796]

Could this be merged in, it seems a 4 year old pending diff is similar
#220

[WGH-]

Strictly speaking, cobra is at 0.0.3 now, so from semver perspective it's OK to break the API in 0.0.4.

[zuoRambo]

Vote for TraverseRunHooks @au-phiware

[rickbau5]

Could this get merged in? Looking forward to this getting fixed

[ghostsquad]

If backwards compatibility is still a concern, a flag/option to enable this behavior would likely be acceptable by the community at large, unless of course most people think it's surprising that this isn't the default behavior. Not sure how one might verify that hypothesis though.

[github-actions]

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

[galexrt]

Closing the PR. I have not interest in keeping the PR updated and / or have moved on to simply use workarounds for this use case.