diff --git a/commands.go b/commands.go index 4643b10..ab2b0a9 100644 --- a/commands.go +++ b/commands.go @@ -59,8 +59,14 @@ func WithoutExpandedModules() ExpandOption { func expand(c Commands, depth int, includeExpandedModules bool) (Commands, []error) { return parallelMap(c, func(cmd Command) ([]Command, []error) { + // At depth 0, nothing is expanded; don't ask for subcommands at all + // because Subcommands() may trigger discovery. + if depth == 0 { + return []Command{cmd}, []error{} + } + // If this command has subcommands, recursively flatten them... - if subcmds, err := cmd.Subcommands(); err == nil && len(subcmds) > 0 && depth != 0 { + if subcmds, err := cmd.Subcommands(); err == nil && len(subcmds) > 0 { cmds := []Command{} errs := []error{} diff --git a/commands_test.go b/commands_test.go index c3cf173..dce5c94 100644 --- a/commands_test.go +++ b/commands_test.go @@ -121,6 +121,15 @@ func TestExpand(t *testing.T) { } } +func TestExpandWithDepthZeroDoesNotCallSubcommands(t *testing.T) { + stub := &stubParent{name: "stub"} + + cmds, errs := Commands{stub}.Expand(WithDepth(0)) + assert.Empty(t, errs) + assert.Equal(t, Commands{stub}, cmds) + assert.False(t, stub.subcommandsCalled, "Expand should not call Subcommands() when depth is 0") +} + func namesOf(cmds Commands) string { var result []string for _, cmd := range cmds { diff --git a/directory_command.go b/directory_command.go index 82fc7dc..da5cb6d 100644 --- a/directory_command.go +++ b/directory_command.go @@ -32,6 +32,15 @@ func (m *directoryCommand) Help() (string, error) { func (m *directoryCommand) DefaultSubcommand() Command { return nil } +// HasSubcommands returns true if the directory has subcommands or — +// when discovery has not been performed — may have subcommands. +func (m *directoryCommand) HasSubcommands() bool { + if m.cmds != nil { + return len(m.cmds) > 0 + } + return true +} + func (m *directoryCommand) Subcommands() (Commands, error) { if m.cmds == nil && m.discoverer != nil { m.cmds, _ = m.discoverer.DiscoverIn(filepath.Dir(m.path), m) diff --git a/executable_command.go b/executable_command.go index 9d5d792..0e74755 100644 --- a/executable_command.go +++ b/executable_command.go @@ -119,6 +119,15 @@ func (cmd *executableCommand) DefaultSubcommand() Command { return nil } +// HasSubcommands returns true if the command has subcommands or — +// when discovery has not been performed — may have subcommands. +func (cmd *executableCommand) HasSubcommands() bool { + if cmd.cmds != nil { + return len(cmd.cmds) > 0 + } + return cmd.discoverer != nil +} + // Subcommands returns the list of subcommands for this command. // Returns an empty slice for leaf commands. func (cmd *executableCommand) Subcommands() (Commands, error) { diff --git a/fixtures_test.go b/fixtures_test.go index 5e76c6a..4650c01 100644 --- a/fixtures_test.go +++ b/fixtures_test.go @@ -3,6 +3,8 @@ package exoskeleton import ( "path/filepath" "runtime" + + "github.com/square/exoskeleton/v2/pkg/shellcomp" ) var fixtures string @@ -11,3 +13,32 @@ func init() { _, testfile, _, _ := runtime.Caller(0) fixtures = filepath.Join(testfile, "..", "fixtures") } + +// stubParent is a Command that reports that it may have subcommands +// and records whether Subcommands() was ever called. +type stubParent struct { + name string + parent Command + subcommandsCalled bool +} + +func (c *stubParent) Path() string { return "" } +func (c *stubParent) Name() string { return c.name } +func (c *stubParent) Parent() Command { return c.parent } +func (c *stubParent) Aliases() []string { return nil } +func (c *stubParent) Summary() (string, error) { return "A stub", nil } +func (c *stubParent) Help() (string, error) { panic("Unused") } +func (c *stubParent) HasSubcommands() bool { return true } + +func (c *stubParent) Exec(*Entrypoint, []string, []string) error { panic("Unused") } + +func (c *stubParent) Complete(*Entrypoint, []string, []string) ([]string, shellcomp.Directive, error) { + panic("Unused") +} + +func (c *stubParent) DefaultSubcommand() Command { return nil } + +func (c *stubParent) Subcommands() (Commands, error) { + c.subcommandsCalled = true + return Commands{}, nil +} diff --git a/menu.go b/menu.go index b814e04..a96ac44 100644 --- a/menu.go +++ b/menu.go @@ -123,7 +123,7 @@ func buildMenu(cmd Command, opts *MenuOptions) (*Menu, []error) { allItems, ferrs := parallelMap(c, func(subcmd Command) ([]*MenuItem, []error) { name := UsageRelativeTo(subcmd, cmd) - if subcmds, _ := subcmd.Subcommands(); len(subcmds) > 0 { + if HasSubcommands(subcmd) { name += ":" } diff --git a/menu_test.go b/menu_test.go index 7901a60..4423e4a 100644 --- a/menu_test.go +++ b/menu_test.go @@ -32,6 +32,17 @@ func TestMenuForTrailer(t *testing.T) { assert.Contains(t, menu, "Run \033[96mentrypoint help module \033[0m to print information on a specific command.") } +func TestBuildMenuRendersSigilWithoutDiscoveringSubcommands(t *testing.T) { + entrypoint := &Entrypoint{name: "entrypoint"} + stub := &stubParent{name: "stub", parent: entrypoint} + entrypoint.cmds = Commands{stub} + + menu, errs := buildMenu(entrypoint, &MenuOptions{}) + assert.Empty(t, errs) + assert.Equal(t, "stub:", menu.Sections[0].MenuItems[0].Name) + assert.False(t, stub.subcommandsCalled, "buildMenu should not discover subcommands to render the sigil") +} + func TestMenuForSections(t *testing.T) { entrypoint, err := New([]string{fixtures}) if err != nil { diff --git a/predicates.go b/predicates.go index 2eb7291..9156260 100644 --- a/predicates.go +++ b/predicates.go @@ -11,3 +11,25 @@ func IsNull(command Command) bool { _, ok := command.(nullCommand) return ok } + +// SubcommandsReporter is implemented by Commands that can report whether +// they may have subcommands without performing discovery. +type SubcommandsReporter interface { + // HasSubcommands returns true if the Command has subcommands or — + // when discovery has not been performed — may have subcommands. + HasSubcommands() bool +} + +// HasSubcommands returns true if the given Command has subcommands or — +// when the Command defers discovery — may have subcommands. +// +// Unlike calling Subcommands() and checking its length, HasSubcommands +// never triggers discovery. Use it when a cheap, possibly-approximate +// answer is preferable to an exact, possibly-expensive one. +func HasSubcommands(command Command) bool { + if r, ok := command.(SubcommandsReporter); ok { + return r.HasSubcommands() + } + cmds, err := command.Subcommands() + return err == nil && len(cmds) > 0 +} diff --git a/predicates_test.go b/predicates_test.go new file mode 100644 index 0000000..4752d99 --- /dev/null +++ b/predicates_test.go @@ -0,0 +1,42 @@ +package exoskeleton + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestHasSubcommandsIsFalseForLeafExecutables(t *testing.T) { + cmd := &executableCommand{name: "leaf"} + assert.False(t, HasSubcommands(cmd)) +} + +func TestHasSubcommandsIsFalseForShellScripts(t *testing.T) { + cmd := &shellScriptCommand{executableCommand: executableCommand{name: "script"}} + assert.False(t, HasSubcommands(cmd)) +} + +func TestHasSubcommandsIsTrueForUndiscoveredExecutables(t *testing.T) { + // cache is nil: if HasSubcommands performed discovery, it would panic. + cmd := &executableCommand{name: "parent", discoverer: &discoverer{}} + assert.True(t, HasSubcommands(cmd)) +} + +func TestHasSubcommandsIsExactAfterDiscovery(t *testing.T) { + cmd := &executableCommand{name: "parent", discoverer: &discoverer{}, cmds: Commands{}} + assert.False(t, HasSubcommands(cmd)) + + cmd.cmds = Commands{&executableCommand{name: "child"}} + assert.True(t, HasSubcommands(cmd)) +} + +func TestHasSubcommandsIsTrueForUndiscoveredDirectories(t *testing.T) { + // path is empty: if HasSubcommands performed discovery, it would find nothing. + cmd := &directoryCommand{discoverer: &discoverer{}} + assert.True(t, HasSubcommands(cmd)) +} + +func TestHasSubcommandsFallsBackToSubcommands(t *testing.T) { + assert.False(t, HasSubcommands(nullCommand{})) + assert.True(t, HasSubcommands(&builtinCommand{subcommands: Commands{nullCommand{}}})) +}