From 5420672dcc9609d091fe126172a5ee591284b287 Mon Sep 17 00:00:00 2001 From: Bart de Boer Date: Mon, 25 May 2020 17:02:42 +0200 Subject: [PATCH] Include EnablePersistentHookOverride option to control if PersistentPostRun* functions override their parents --- README.md | 4 +++- cobra.go | 4 ++++ command.go | 34 ++++++++++++++++++++++++++-------- command_test.go | 25 ++++++++++++++----------- 4 files changed, 47 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index a932dd7407..fb088f6583 100644 --- a/README.md +++ b/README.md @@ -601,7 +601,9 @@ It is possible to run functions before or after the main `Run` function of your - `PostRun` - `PersistentPostRun` -An example of two commands which use all of these features is below. When the subcommand is executed, it will run the root command's `PersistentPreRun` but not the root command's `PersistentPostRun`: +By default `Persistent*Run` functions declared within children will override their parents. Setting `cobra.DisablePersistentHookOverride` to true changes this behavior to also run the parent `Persistent*Run` functions. + +An example of two commands is below. When the subcommand is executed, it will run the root command's `PersistentPreRun` but not the root command's `PersistentPostRun`: ```go package main diff --git a/cobra.go b/cobra.go index d01becc8fa..042e0c6e8a 100644 --- a/cobra.go +++ b/cobra.go @@ -48,6 +48,10 @@ var EnablePrefixMatching = false // To disable sorting, set it to false. var EnableCommandSorting = true +// EnablePersistentHookOverride controls if PersistentPreRun* and PersistentPostRun* hooks +// should override their parents. When set to true only the final hooks are executed (default). +var EnablePersistentHookOverride = true + // MousetrapHelpText enables an information splash screen on Windows // if the CLI is started from explorer.exe. // To disable the mousetrap, just set this variable to blank string (""). diff --git a/command.go b/command.go index 5b81f61dab..cec234a331 100644 --- a/command.go +++ b/command.go @@ -816,17 +816,28 @@ func (c *Command) execute(a []string) (err error) { return err } + // By default PersistentPostRun* functions override their parent functions. + // Disabling EnablePersistentHookOverride runs all PersistentPostRun* functions from root to child + chain := []*Command{} for p := c; p != nil; p = p.Parent() { - if p.PersistentPreRunE != nil { - if err := p.PersistentPreRunE(c, argWoFlags); err != nil { + if p.PersistentPreRunE != nil || p.PersistentPreRun != nil { + chain = append(chain, p) + if EnablePersistentHookOverride { + break + } + } + } + for i := len(chain) - 1; i >= 0; i-- { + cc := chain[i] + if cc.PersistentPreRunE != nil { + if err := cc.PersistentPreRunE(c, argWoFlags); err != nil { return err } - break - } else if p.PersistentPreRun != nil { - p.PersistentPreRun(c, argWoFlags) - break + } else if cc.PersistentPreRun != nil { + cc.PersistentPreRun(c, argWoFlags) } } + if c.PreRunE != nil { if err := c.PreRunE(c, argWoFlags); err != nil { return err @@ -852,15 +863,22 @@ func (c *Command) execute(a []string) (err error) { } else if c.PostRun != nil { c.PostRun(c, argWoFlags) } + + // By default PersistentPostRun* functions override their parent functions. + // Disabling EnablePersistentHookOverride runs all PersistentPostRun* functions from child to root for p := c; p != nil; p = p.Parent() { if p.PersistentPostRunE != nil { if err := p.PersistentPostRunE(c, argWoFlags); err != nil { return err } - break + if EnablePersistentHookOverride { + break + } } else if p.PersistentPostRun != nil { p.PersistentPostRun(c, argWoFlags) - break + if EnablePersistentHookOverride { + break + } } } diff --git a/command_test.go b/command_test.go index 16cc41b4c6..fc6f8bd924 100644 --- a/command_test.go +++ b/command_test.go @@ -1379,11 +1379,10 @@ func TestPersistentHooks(t *testing.T) { t.Errorf("Unexpected error: %v", err) } - // TODO: currently PersistenPreRun* defined in parent does not - // run if the matchin child subcommand has PersistenPreRun. - // If the behavior changes (https://github.com/spf13/cobra/issues/252) - // this test must be fixed. - if parentPersPreArgs != "" { + if !EnablePersistentHookOverride && parentPersPreArgs != "one two" { + t.Errorf("Expected parentPersPreArgs %q, got %q", "one two", parentPersPreArgs) + } + if EnablePersistentHookOverride && parentPersPreArgs != "" { t.Errorf("Expected blank parentPersPreArgs, got %q", parentPersPreArgs) } if parentPreArgs != "" { @@ -1395,14 +1394,12 @@ func TestPersistentHooks(t *testing.T) { if parentPostArgs != "" { t.Errorf("Expected blank parentPostArgs, got %q", parentPostArgs) } - // TODO: currently PersistenPostRun* defined in parent does not - // run if the matchin child subcommand has PersistenPostRun. - // If the behavior changes (https://github.com/spf13/cobra/issues/252) - // this test must be fixed. - if parentPersPostArgs != "" { + if !EnablePersistentHookOverride && parentPersPostArgs != "one two" { + t.Errorf("Expected parentPersPostArgs %q, got %q", "one two", parentPersPostArgs) + } + if EnablePersistentHookOverride && parentPersPostArgs != "" { t.Errorf("Expected blank parentPersPostArgs, got %q", parentPersPostArgs) } - if childPersPreArgs != "one two" { t.Errorf("Expected childPersPreArgs %q, got %q", "one two", childPersPreArgs) } @@ -1420,6 +1417,12 @@ func TestPersistentHooks(t *testing.T) { } } +func TestPersistentHooksNotOverriding(t *testing.T) { + EnablePersistentHookOverride = false + TestPersistentHooks(t) + EnablePersistentHookOverride = true +} + // Related to https://github.com/spf13/cobra/issues/521. func TestGlobalNormFuncPropagation(t *testing.T) { normFunc := func(f *pflag.FlagSet, name string) pflag.NormalizedName {