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

Include EnablePersistentHookOverride option to control Persistent*Run override behavior #1123

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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