Skip to content

Commit

Permalink
Include EnablePersistentHookOverride option to control if PersistentP…
Browse files Browse the repository at this point in the history
…ostRun* functions should override their parents
  • Loading branch information
bartdeboer committed May 29, 2020
1 parent 94a87a7 commit 233ff01
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 20 deletions.
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.EnablePersistentHookOverride` to false 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
Expand Down
4 changes: 4 additions & 0 deletions cobra.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ("").
Expand Down
34 changes: 26 additions & 8 deletions command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
}
}

Expand Down
25 changes: 14 additions & 11 deletions command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand All @@ -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)
}
Expand All @@ -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 {
Expand Down

0 comments on commit 233ff01

Please sign in to comment.