Skip to content
Open
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
8 changes: 7 additions & 1 deletion commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optimization: check this before calling cmd.Subcommands() 👍

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{}

Expand Down
9 changes: 9 additions & 0 deletions commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions directory_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions executable_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
31 changes: 31 additions & 0 deletions fixtures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package exoskeleton
import (
"path/filepath"
"runtime"

"github.com/square/exoskeleton/v2/pkg/shellcomp"
)

var fixtures string
Expand All @@ -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
}
2 changes: 1 addition & 1 deletion menu.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 += ":"
}

Expand Down
11 changes: 11 additions & 0 deletions menu_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ func TestMenuForTrailer(t *testing.T) {
assert.Contains(t, menu, "Run \033[96mentrypoint help module <command>\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 {
Expand Down
22 changes: 22 additions & 0 deletions predicates.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
42 changes: 42 additions & 0 deletions predicates_test.go
Original file line number Diff line number Diff line change
@@ -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{}}}))
}
Loading