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

Feature: allow running persistent run hooks of all parents #2044

Merged
merged 1 commit into from
Oct 22, 2023
Merged
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
11 changes: 8 additions & 3 deletions cobra.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@ var initializers []func()
var finalizers []func()

const (
defaultPrefixMatching = false
defaultCommandSorting = true
defaultCaseInsensitive = false
defaultPrefixMatching = false
defaultCommandSorting = true
defaultCaseInsensitive = false
defaultTraverseRunHooks = false
)

// EnablePrefixMatching allows setting automatic prefix matching. Automatic prefix matching can be a dangerous thing
Expand All @@ -60,6 +61,10 @@ var EnableCommandSorting = defaultCommandSorting
// EnableCaseInsensitive allows case-insensitive commands names. (case sensitive by default)
var EnableCaseInsensitive = defaultCaseInsensitive

// EnableTraverseRunHooks executes persistent pre-run and post-run hooks from all parents.
// By default this is disabled, which means only the first run hook to be found is executed.
var EnableTraverseRunHooks = defaultTraverseRunHooks

// 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
28 changes: 24 additions & 4 deletions command.go
Original file line number Diff line number Diff line change
Expand Up @@ -934,15 +934,31 @@
return err
}

parents := make([]*Command, 0, 5)
for p := c; p != nil; p = p.Parent() {
if EnableTraverseRunHooks {
// When EnableTraverseRunHooks is set:
// - Execute all persistent pre-runs from the root parent till this command.
// - Execute all persistent post-runs from this command till the root parent.
parents = append([]*Command{p}, parents...)
} else {
// Otherwise, execute only the first found persistent hook.
parents = append(parents, p)
}
}
for _, p := range parents {
if p.PersistentPreRunE != nil {
if err := p.PersistentPreRunE(c, argWoFlags); err != nil {
return err
}
break
if !EnableTraverseRunHooks {
break
}
} else if p.PersistentPreRun != nil {
p.PersistentPreRun(c, argWoFlags)
break
if !EnableTraverseRunHooks {
break
}
}
}
if c.PreRunE != nil {
Expand Down Expand Up @@ -979,10 +995,14 @@
if err := p.PersistentPostRunE(c, argWoFlags); err != nil {
return err
}
break
if !EnableTraverseRunHooks {
break
}
} else if p.PersistentPostRun != nil {
p.PersistentPostRun(c, argWoFlags)
break
if !EnableTraverseRunHooks {
break
}
}
}

Expand Down Expand Up @@ -1437,7 +1457,7 @@
if x.HasFlags() {
x.flags.VisitAll(func(f *flag.Flag) {
if x.HasPersistentFlags() && x.persistentFlag(f.Name) != nil {
c.Println(" -"+f.Shorthand+",", "--"+f.Name, "["+f.DefValue+"]", "", f.Value, " [LP]")

Check failure on line 1460 in command.go

View workflow job for this annotation

GitHub Actions / golangci-lint

string ` -` has 4 occurrences, make it a constant (goconst)
} else {
c.Println(" -"+f.Shorthand+",", "--"+f.Name, "["+f.DefValue+"]", "", f.Value, " [L]")
}
Expand Down
106 changes: 47 additions & 59 deletions command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1530,57 +1530,73 @@ func TestHooks(t *testing.T) {
}

func TestPersistentHooks(t *testing.T) {
var (
parentPersPreArgs string
parentPreArgs string
parentRunArgs string
parentPostArgs string
parentPersPostArgs string
)
EnableTraverseRunHooks = true
testPersistentHooks(t, []string{
"parent PersistentPreRun",
"child PersistentPreRun",
"child PreRun",
"child Run",
"child PostRun",
"child PersistentPostRun",
"parent PersistentPostRun",
})

var (
childPersPreArgs string
childPreArgs string
childRunArgs string
childPostArgs string
childPersPostArgs string
)
EnableTraverseRunHooks = false
testPersistentHooks(t, []string{
"child PersistentPreRun",
"child PreRun",
"child Run",
"child PostRun",
"child PersistentPostRun",
})
}

func testPersistentHooks(t *testing.T, expectedHookRunOrder []string) {
var hookRunOrder []string

validateHook := func(args []string, hookName string) {
hookRunOrder = append(hookRunOrder, hookName)
got := strings.Join(args, " ")
if onetwo != got {
t.Errorf("Expected %s %q, got %q", hookName, onetwo, got)
}
}

parentCmd := &Command{
Use: "parent",
PersistentPreRun: func(_ *Command, args []string) {
parentPersPreArgs = strings.Join(args, " ")
validateHook(args, "parent PersistentPreRun")
},
PreRun: func(_ *Command, args []string) {
parentPreArgs = strings.Join(args, " ")
validateHook(args, "parent PreRun")
},
Run: func(_ *Command, args []string) {
parentRunArgs = strings.Join(args, " ")
validateHook(args, "parent Run")
},
PostRun: func(_ *Command, args []string) {
parentPostArgs = strings.Join(args, " ")
validateHook(args, "parent PostRun")
},
PersistentPostRun: func(_ *Command, args []string) {
parentPersPostArgs = strings.Join(args, " ")
validateHook(args, "parent PersistentPostRun")
},
}

childCmd := &Command{
Use: "child",
PersistentPreRun: func(_ *Command, args []string) {
childPersPreArgs = strings.Join(args, " ")
validateHook(args, "child PersistentPreRun")
},
PreRun: func(_ *Command, args []string) {
childPreArgs = strings.Join(args, " ")
validateHook(args, "child PreRun")
},
Run: func(_ *Command, args []string) {
childRunArgs = strings.Join(args, " ")
validateHook(args, "child Run")
},
PostRun: func(_ *Command, args []string) {
childPostArgs = strings.Join(args, " ")
validateHook(args, "child PostRun")
},
PersistentPostRun: func(_ *Command, args []string) {
childPersPostArgs = strings.Join(args, " ")
validateHook(args, "child PersistentPostRun")
},
}
parentCmd.AddCommand(childCmd)
Expand All @@ -1593,41 +1609,13 @@ func TestPersistentHooks(t *testing.T) {
t.Errorf("Unexpected error: %v", err)
}

for _, v := range []struct {
name string
got string
}{
// TODO: currently PersistentPreRun* defined in parent does not
// run if the matching child subcommand has PersistentPreRun.
// If the behavior changes (https://github.com/spf13/cobra/issues/252)
// this test must be fixed.
{"parentPersPreArgs", parentPersPreArgs},
{"parentPreArgs", parentPreArgs},
{"parentRunArgs", parentRunArgs},
{"parentPostArgs", parentPostArgs},
// TODO: currently PersistentPostRun* defined in parent does not
// run if the matching child subcommand has PersistentPostRun.
// If the behavior changes (https://github.com/spf13/cobra/issues/252)
// this test must be fixed.
{"parentPersPostArgs", parentPersPostArgs},
} {
if v.got != "" {
t.Errorf("Expected blank %s, got %q", v.name, v.got)
}
}

for _, v := range []struct {
name string
got string
}{
{"childPersPreArgs", childPersPreArgs},
{"childPreArgs", childPreArgs},
{"childRunArgs", childRunArgs},
{"childPostArgs", childPostArgs},
{"childPersPostArgs", childPersPostArgs},
} {
if v.got != onetwo {
t.Errorf("Expected %s %q, got %q", v.name, onetwo, v.got)
for idx, exp := range expectedHookRunOrder {
if len(hookRunOrder) > idx {
if act := hookRunOrder[idx]; act != exp {
t.Errorf("Expected %q at %d, got %q", exp, idx, act)
}
} else {
t.Errorf("Expected %q at %d, got nothing", exp, idx)
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions site/content/user_guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,10 @@ Inside subCmd PostRun with args: [arg1 arg2]
Inside subCmd PersistentPostRun with args: [arg1 arg2]
```

By default, only the first persistent hook found in the command chain is executed.
That is why in the above output, the `rootCmd PersistentPostRun` was not called for a child command.
Set `EnableTraverseRunHooks` global variable to `true` if you want to execute all parents' persistent hooks.

## Suggestions when "unknown command" happens

Cobra will print automatic suggestions when "unknown command" errors happen. This allows Cobra to behave similarly to the `git` command when a typo happens. For example:
Expand Down
Loading