From 5420672dcc9609d091fe126172a5ee591284b287 Mon Sep 17 00:00:00 2001
From: Bart de Boer <bart.deboer@gmail.com>
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 {