From 6b0e46f5c73b937f7bacd3bda9e61c9f1021fab6 Mon Sep 17 00:00:00 2001 From: Bob Lail Date: Tue, 18 Aug 2026 17:09:38 -0700 Subject: [PATCH] feat: Add HasSubcommands() so menus don't require discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rendering a menu called Subcommands() on every child twice — once in Expand() (even at depth 0, where the result was discarded) and once to decide whether to render the ':' sigil. Both calls can trigger discovery, which may exec the child. Expand() now returns immediately at depth 0, and the sigil is rendered with the new HasSubcommands() predicate, which answers "may this command have subcommands?" without performing discovery. Commands built by external contracts can opt in via the SubcommandsReporter interface; otherwise HasSubcommands() falls back to calling Subcommands(). Co-Authored-By: Claude Fable 5 --- commands.go | 8 +++++++- commands_test.go | 9 +++++++++ directory_command.go | 9 +++++++++ executable_command.go | 9 +++++++++ fixtures_test.go | 31 +++++++++++++++++++++++++++++++ menu.go | 2 +- menu_test.go | 11 +++++++++++ predicates.go | 22 ++++++++++++++++++++++ predicates_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 predicates_test.go 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{}}})) +}