From c231a416656c0163c55e50893eea690b1a10c7f2 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 20 Oct 2026 00:25:33 +0000 Subject: [PATCH] Compose discovery help through group aliases --- docs/roadmap/remediation.md | 11 ++++- help_view.go | 36 +++++++++++----- route.go | 13 +++--- route_test.go | 82 +++++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 19 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 378f388..238f4bb 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -146,7 +146,7 @@ The complete offline root suite passed on Go 1.26.8. ## R-4: Resolve discovery-help contexts through group shortcuts -Status: open. Priority: P2. Category: incomplete alias composition. +Status: complete. Priority: P2. Category: incomplete alias composition. **Evidence:** Discovery contexts are indexed by declared source prefixes in [discoverFlagContexts](../../flag_aliases.go#L157), while @@ -176,6 +176,15 @@ show `pk admin` in usage, and list the routes actually reachable there. Cover the global help alias and a more-specific route override; do not add canonical groups or inherit descendant flags to make discovery work. +**Completed:** Discovery selection now checks the resolved prefix and retains +the entered route; help merges applicable entered/resolved routes and resolves +each destination through actual shortcut precedence. Regressions first failed +for the missing composed context and omitted inherited routes. Both help forms, +global help aliases, more-specific overrides, entered-only prefixes, expanded +group paths, bare-prefix usage, and descendant-flag exclusion now pass. No +canonical nodes were added. The full offline root suite and added path cases +passed on Go 1.26.8. + ## R-5: Include alias routes in help for groups without canonical children Status: open. Priority: P2. Category: help discovery gap. diff --git a/help_view.go b/help_view.go index 82a17d5..d934ebf 100644 --- a/help_view.go +++ b/help_view.go @@ -17,7 +17,7 @@ func (a *App) helpView(route routeResult, snapshot *routingSnapshot) commandmeta return view } if view.Discovery { - view.CommandAliases = discoveryHelpAliases(snapshot.commandAliases, route.discoveryPath) + view.CommandAliases = discoveryHelpAliases(snapshot, route) view.FlagAliases = snapshot.discoveryFlags[route.discoveryPath.Key()].neutralRecords() return view } @@ -30,17 +30,31 @@ func clonePath(path commandpath.Path) []string { return append([]string(nil), path...) } -func discoveryHelpAliases(aliases []commandmeta.CommandAlias, prefix commandpath.Path) []commandmeta.CommandAlias { - var result []commandmeta.CommandAlias - for _, alias := range aliases { - source := commandpath.FromTokens(alias.Source) - if len(source) <= len(prefix) || !source.HasPrefix(prefix) { - continue +func discoveryHelpAliases(snapshot *routingSnapshot, route routeResult) []commandmeta.CommandAlias { + bySource := make(map[string]commandmeta.CommandAlias) + for _, prefix := range []commandpath.Path{route.enteredPath, route.discoveryPath} { + for _, alias := range snapshot.commandAliases { + source := commandpath.FromTokens(alias.Source) + if len(source) <= len(prefix) || !source.HasPrefix(prefix) { + continue + } + suffix := source[len(prefix):] + entered := append(route.enteredPath.Clone(), suffix...) + target := snapshot.aliases.Resolve(entered) + if traceCanonicalPath(snapshot.canonical, target).exact == nil { + continue + } + bySource[suffix.Key()] = commandmeta.CommandAlias{Source: clonePath(suffix), Target: clonePath(target)} } - result = append(result, commandmeta.CommandAlias{ - Source: clonePath(source[len(prefix):]), - Target: append([]string(nil), alias.Target...), - }) + } + keys := make([]string, 0, len(bySource)) + for key := range bySource { + keys = append(keys, key) + } + sort.Strings(keys) + result := make([]commandmeta.CommandAlias, 0, len(keys)) + for _, key := range keys { + result = append(result, bySource[key]) } return result } diff --git a/route.go b/route.go index 35e545c..7bb9417 100644 --- a/route.go +++ b/route.go @@ -40,10 +40,10 @@ func resolveEnteredRoute(args []string, snapshot *routingSnapshot) routeResult { case len(path) == 0: result.kind = routeCanonical default: - if discovery, ok := findDiscoveryPath(snapshot.discoveryFlags, path); ok && len(discovery) == len(path) { + if discovery, ok := findDiscoveryPath(snapshot.discoveryFlags, resolved, path); ok { result.kind = routeDiscovery result.discoveryPath = discovery - result.enteredPath = discovery.Clone() + result.enteredPath = path.Clone() result.args = commandpath.CloneTokens(trailing) } else if trace.operandOwner != nil { result.kind = routeCanonical @@ -89,11 +89,10 @@ func enteredEndpoint(path commandpath.Path, origins [][]int, canonicalLength int return path[:inputLength].Clone() } -func findDiscoveryPath(contexts map[string]resolvedFlagAliases, path commandpath.Path) (commandpath.Path, bool) { - for length := len(path); length > 0; length-- { - prefix := path[:length] - if _, exists := contexts[prefix.Key()]; exists { - return prefix.Clone(), true +func findDiscoveryPath(contexts map[string]resolvedFlagAliases, paths ...commandpath.Path) (commandpath.Path, bool) { + for _, path := range paths { + if _, exists := contexts[path.Key()]; len(path) > 0 && exists { + return path.Clone(), true } } return nil, false diff --git a/route_test.go b/route_test.go index ddde153..6cdaebe 100644 --- a/route_test.go +++ b/route_test.go @@ -132,6 +132,88 @@ func TestExplicitHelpRequiresAnExactResolvedTarget(t *testing.T) { } } +func TestDiscoveryHelpComposesGroupShortcutsAndOverrides(t *testing.T) { + for _, override := range []bool{false, true} { + t.Run(fmt.Sprint(override), func(t *testing.T) { + initialized := false + inspect := NewCommand(CommandSpec[struct{}]{ + Name: "inspect", NewInput: func() struct{} { initialized = true; return struct{}{} }, + Flags: []Flag[struct{}]{ValueFlag(FlagMeta{Name: "format"}, (*string)(nil), + func(value string) (string, error) { return value, nil }, func(*struct{}, Supplied[string]) {})}, + Run: func(Invocation, struct{}) Outcome { return Success() }, + }) + declarations := []Declaration{Group("box", "Box"), inspect, simpleRunnable("alternate"), + Alias("pk", "box"), Alias("box admin pick", "inspect"), Alias("box admin other", "inspect"), + FlagAlias("--hlep", "--help"), FlagAliasFor("inspect", "--fmt", "--format")} + wantPick := "pick → inspect" + if override { + declarations = append(declarations, Alias("pk admin pick", "alternate"), Alias("pk admin nested choose", "inspect")) + wantPick = "pick → alternate" + } + app, err := New(Identity{Name: "Demo"}, declarations...) + if err != nil { + t.Fatal(err) + } + for _, args := range [][]string{{"pk", "admin", "--help"}, {"help", "pk", "admin"}, {"pk", "admin", "--hlep"}} { + var output, diagnostics bytes.Buffer + outcome := app.Execute(context.Background(), args, Streams{Out: &output, Err: &diagnostics}) + if outcome.Kind != OutcomeSuccess { + t.Fatalf("Execute(%q) = %v, diagnostics %q", args, outcome.Kind, diagnostics.String()) + } + for _, fragment := range []string{"Demo pk admin [command]", wantPick, "other → inspect"} { + if !strings.Contains(output.String(), fragment) { + t.Errorf("help %q omits %q", output.String(), fragment) + } + } + if override && strings.Contains(output.String(), "pick → inspect") { + t.Errorf("help displays overridden target: %q", output.String()) + } + if strings.Contains(output.String(), "--fmt") { + t.Errorf("discovery inherited a descendant flag: %q", output.String()) + } + } + if initialized { + t.Fatal("discovery initialized application input") + } + if override { + var output bytes.Buffer + outcome := app.Execute(context.Background(), []string{"help", "pk", "admin", "nested"}, Streams{Out: &output, Err: io.Discard}) + if outcome.Kind != OutcomeSuccess || !strings.Contains(output.String(), "choose → inspect") { + t.Fatalf("entered-only discovery outcome = %v, help %q", outcome.Kind, output.String()) + } + } + var diagnostics bytes.Buffer + outcome := app.Execute(context.Background(), []string{"pk", "admin"}, Streams{Out: io.Discard, Err: &diagnostics}) + if outcome.Kind != OutcomeUsage || !strings.Contains(diagnostics.String(), "pk admin") { + t.Fatalf("bare discovery = %v, diagnostics %q", outcome.Kind, diagnostics.String()) + } + outcome = app.Execute(context.Background(), []string{"pk", "admin", "--fmt=yaml", "--hlep"}, Streams{Out: io.Discard, Err: io.Discard}) + if outcome.Kind != OutcomeUsage { + t.Fatal("discovery accepted descendant flag") + } + outcome = app.Execute(context.Background(), []string{"pk", "admin", "other"}, Streams{Out: io.Discard, Err: io.Discard}) + if outcome.Kind != OutcomeSuccess || !initialized { + t.Fatal("complete composed route is unavailable") + } + }) + } +} + +func TestDiscoveryHelpThroughExpandedGroupPath(t *testing.T) { + app, err := New(Identity{Name: "Demo"}, Group("outer", "Outer", Group("box", "Box")), + simpleRunnable("inspect"), Alias("pk", "outer box"), Alias("outer box admin pick", "inspect")) + if err != nil { + t.Fatal(err) + } + for _, args := range [][]string{{"pk", "admin", "--help"}, {"help", "pk", "admin"}} { + var output bytes.Buffer + outcome := app.Execute(context.Background(), args, Streams{Out: &output, Err: io.Discard}) + if outcome.Kind != OutcomeSuccess || !strings.Contains(output.String(), "Demo pk admin [command]") || !strings.Contains(output.String(), "pick → inspect") { + t.Fatalf("expanded discovery outcome = %v, help %q", outcome.Kind, output.String()) + } + } +} + func routeTestApp(t *testing.T, extra ...Declaration) *App { t.Helper() collections := simpleRunnable("collections")