From 7a4efddab8d44dacf47f8f5c31592bb1167a4815 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 19 Oct 2026 17:23:32 +0000 Subject: [PATCH] Add captured alias declaration types --- app.go | 17 ++-- builtins.go | 4 +- command.go | 111 ++++++++++++++++++++++++++ declaration_external_test.go | 31 +++++++ docs/roadmap/implementation.md | 8 +- integrations/ckprompt/command_test.go | 4 +- performance_dispatch_test.go | 12 ++- registry.go | 54 +++++++++++-- registry_test.go | 77 ++++++++++++++++++ 9 files changed, 297 insertions(+), 21 deletions(-) create mode 100644 declaration_external_test.go diff --git a/app.go b/app.go index 4959740..92e4723 100644 --- a/app.go +++ b/app.go @@ -9,9 +9,10 @@ import ( // App owns one command registry and its application identity. Each App has an // independent registry; command declarations may be reused across Apps. type App struct { - identity Identity - commands map[string]*commandNode - snapshot *routingSnapshot + identity Identity + commands map[string]*commandNode + routingRules []RoutingRule + snapshot *routingSnapshot } type routingSnapshot struct { @@ -23,18 +24,22 @@ type commandNode struct { command Command } -// New creates an application with the supplied identity and initial commands. +// New creates an application with the supplied identity and initial +// declarations. // The identity name must not be empty. New installs its generated help and // version commands and caller declarations as one batch; an invalid declaration // returns an error without exposing an App. The root help and version names are // reserved for generated commands. -func New(identity Identity, commands ...Command) (*App, error) { +func New(identity Identity, declarations ...Declaration) (*App, error) { if identity.Name == "" { return nil, fmt.Errorf("application identity name must not be empty") } app := &App{identity: identity, commands: make(map[string]*commandNode)} - all := append(app.builtinCommands(), commands...) + builtins := app.builtinCommands() + all := make([]Declaration, 0, len(builtins)+len(declarations)) + all = append(all, builtins...) + all = append(all, declarations...) if err := app.installBatch(all...); err != nil { return nil, err } diff --git a/builtins.go b/builtins.go index f597d5f..94edcd3 100644 --- a/builtins.go +++ b/builtins.go @@ -29,7 +29,7 @@ type versionDocument struct { Revision string `yaml:"revision"` } -func (a *App) builtinCommands() []Command { +func (a *App) builtinCommands() []Declaration { help := captureCommand(CommandSpec[helpInput]{ Name: "help", Summary: "Show help for commands.", @@ -61,7 +61,7 @@ func (a *App) builtinCommands() []Command { return renderVersionOutcome(identity, input.Format, invocation.Streams) }, }) - return []Command{help, version} + return []Declaration{help, version} } func parseVersionScope(value string) (string, error) { diff --git a/command.go b/command.go index c640acc..795972c 100644 --- a/command.go +++ b/command.go @@ -3,6 +3,7 @@ package commandkit import ( "errors" "fmt" + "strings" "unicode" commandmeta "gitea.maximumdirect.net/eric/CommandKit/internal/commandmeta" @@ -15,6 +16,45 @@ import ( // declaration before changing an App registry. type Command struct{ record *commandRecord } +// Declaration is a captured command or application routing rule accepted by +// New and App.Register. Only CommandKit declaration types implement it. +type Declaration interface { + commandkitDeclaration() +} + +func (Command) commandkitDeclaration() {} + +// RoutingRule is an opaque, reusable command or flag alias declaration. +type RoutingRule struct{ record *routingRuleRecord } + +func (RoutingRule) commandkitDeclaration() {} + +type routingRuleKind uint8 + +const ( + commandAliasRule routingRuleKind = iota + 1 + globalFlagAliasRule + scopedFlagAliasRule +) + +type capturedFlagSpelling struct { + name string + short bool +} + +type routingRuleRecord struct { + kind routingRuleKind + sourcePath commandpath.Path + targetPath commandpath.Path + scopePath commandpath.Path + sourceFlag capturedFlagSpelling + targetFlag capturedFlagSpelling + sourceText string + targetText string + scopeText string + issues []error +} + type commandRecord struct { name string summary string @@ -259,6 +299,77 @@ func WithError(command Command, err error) Command { // not an absence marker. func NoCommand() Command { return Command{record: &commandRecord{absent: true}} } +// Alias captures a command-path alias from path to target. Paths are +// whitespace-separated sequences of command names; construction issues are +// retained for New or App.Register to report. +func Alias(path, target string) RoutingRule { + record := &routingRuleRecord{ + kind: commandAliasRule, + sourceText: path, + targetText: target, + } + record.sourcePath = captureRoutingPath("source", path, &record.issues) + record.targetPath = captureRoutingPath("target", target, &record.issues) + return RoutingRule{record: record} +} + +// FlagAlias captures an application-wide flag spelling alias. Spellings must +// be one long flag name or one individual short spelling. +func FlagAlias(spelling, target string) RoutingRule { + record := &routingRuleRecord{ + kind: globalFlagAliasRule, + sourceText: spelling, + targetText: target, + } + record.sourceFlag = captureFlagSpelling("source", spelling, &record.issues) + record.targetFlag = captureFlagSpelling("target", target, &record.issues) + return RoutingRule{record: record} +} + +// FlagAliasFor captures a flag spelling alias scoped to a canonical command +// path. +func FlagAliasFor(commandPath, spelling, target string) RoutingRule { + record := &routingRuleRecord{ + kind: scopedFlagAliasRule, + sourceText: spelling, + targetText: target, + scopeText: commandPath, + } + record.scopePath = captureRoutingPath("command scope", commandPath, &record.issues) + record.sourceFlag = captureFlagSpelling("source", spelling, &record.issues) + record.targetFlag = captureFlagSpelling("target", target, &record.issues) + return RoutingRule{record: record} +} + +func captureRoutingPath(label, value string, issues *[]error) commandpath.Path { + path, err := commandpath.ParseDeclaration(value) + if err != nil { + *issues = append(*issues, fmt.Errorf("invalid %s path: %w", label, err)) + } + return path +} + +func captureFlagSpelling(label, value string, issues *[]error) capturedFlagSpelling { + spelling, err := parseFlagSpelling(value) + if err != nil { + *issues = append(*issues, fmt.Errorf("invalid %s flag spelling %q: %w", label, value, err)) + } + return spelling +} + +func parseFlagSpelling(value string) (capturedFlagSpelling, error) { + if strings.HasPrefix(value, "--") { + name := value[2:] + if validFlagName(name) { + return capturedFlagSpelling{name: name}, nil + } + } + if len(value) == 2 && value[0] == '-' && value[1] != '-' { + return capturedFlagSpelling{name: value[1:], short: true}, nil + } + return capturedFlagSpelling{}, fmt.Errorf("must be a long flag or one-byte short flag spelling") +} + func captureComposition(record *commandRecord) { if err := validateCommandName(record.name); err != nil { record.issues = append(record.issues, err) diff --git a/declaration_external_test.go b/declaration_external_test.go new file mode 100644 index 0000000..266a589 --- /dev/null +++ b/declaration_external_test.go @@ -0,0 +1,31 @@ +package commandkit_test + +import ( + "testing" + + commandkit "gitea.maximumdirect.net/eric/CommandKit" +) + +func TestPublicInstallationAcceptsMixedDeclarations(t *testing.T) { + command := commandkit.NewCommand(commandkit.CommandSpec[struct{}]{ + Name: "show", + Summary: "Show a value.", + Flags: []commandkit.Flag[struct{}]{ + commandkit.ValueFlag(commandkit.FlagMeta{Name: "format"}, (*string)(nil), func(value string) (string, error) { return value, nil }, func(*struct{}, commandkit.Supplied[string]) {}), + }, + Run: func(commandkit.Invocation, struct{}) commandkit.Outcome { return commandkit.Success() }, + }) + declarations := []commandkit.Declaration{ + command, + commandkit.Alias("inspect", "show"), + commandkit.FlagAlias("--fmt", "--format"), + commandkit.FlagAliasFor("show", "-f", "--format"), + } + app, err := commandkit.New(commandkit.Identity{Name: "Example"}, declarations...) + if err != nil { + t.Fatal(err) + } + if app == nil { + t.Fatal("New() returned a nil app") + } +} diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 92a3707..bc46bd4 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Application-owned aliases implementation plan -Status: ready for implementation; no stages completed. +Status: in progress; Stages 1–4 completed. ## Authority, baseline, and scope @@ -348,6 +348,12 @@ invalid paths/spellings are reported at installation; nil and zero declarations are handled as selected; canonical command-only calls continue to work. No factories, loaders, or handlers run during capture. +**Completed:** Added the opaque `Declaration` and `RoutingRule` types, command +and flag alias factories, eager normalized capture with retained issues, and +value/pointer normalization at `New` and `Register`. Built-in and consumer +installation slices now use declarations explicitly. Verified with root and +both integration module suites under the local workspace. + ## Stage 5: Install canonical commands and command aliases atomically **Work** diff --git a/integrations/ckprompt/command_test.go b/integrations/ckprompt/command_test.go index 879209c..7df0290 100644 --- a/integrations/ckprompt/command_test.go +++ b/integrations/ckprompt/command_test.go @@ -16,9 +16,9 @@ import ( "go.yaml.in/yaml/v4" ) -func newPromptTestApp(t *testing.T, commands ...commandkit.Command) *commandkit.App { +func newPromptTestApp(t *testing.T, declarations ...commandkit.Declaration) *commandkit.App { t.Helper() - app, err := commandkit.New(commandkit.Identity{Name: "example"}, commands...) + app, err := commandkit.New(commandkit.Identity{Name: "example"}, declarations...) if err != nil { t.Fatal(err) } diff --git a/performance_dispatch_test.go b/performance_dispatch_test.go index a6b0a17..fdf17ee 100644 --- a/performance_dispatch_test.go +++ b/performance_dispatch_test.go @@ -102,7 +102,7 @@ func BenchmarkPerformanceRegistration(b *testing.B) { if err != nil { b.Fatal(err) } - if err := app.Register(commands...); err != nil { + if err := app.Register(performanceDeclarations(commands)...); err != nil { b.Fatal(err) } } @@ -150,13 +150,21 @@ func performanceNestedCommand(tb testing.TB) (commandkit.Command, []string) { func performanceApp(tb testing.TB, commands []commandkit.Command) *commandkit.App { tb.Helper() - app, err := commandkit.New(commandkit.Identity{Name: "bench"}, commands...) + app, err := commandkit.New(commandkit.Identity{Name: "bench"}, performanceDeclarations(commands)...) if err != nil { tb.Fatal(err) } return app } +func performanceDeclarations(commands []commandkit.Command) []commandkit.Declaration { + declarations := make([]commandkit.Declaration, len(commands)) + for index, command := range commands { + declarations[index] = command + } + return declarations +} + func performanceCommandName(fixture string) string { if fixture == "wide" { return "wide00" diff --git a/registry.go b/registry.go index 8e1e114..eab2f3c 100644 --- a/registry.go +++ b/registry.go @@ -1,19 +1,46 @@ package commandkit -import "fmt" +import ( + "errors" + "fmt" +) -// Register adds a batch of root commands to the application. The complete batch +// Register adds a batch of declarations to the application. The complete batch // is validated before registry mutation; on error the registry and its derived // command metadata remain unchanged. A successful nonempty batch becomes -// visible to later invocations. Command declarations remain reusable. -func (a *App) Register(commands ...Command) error { - return a.installBatch(commands...) +// visible to later invocations. Declarations remain reusable. +func (a *App) Register(declarations ...Declaration) error { + return a.installBatch(declarations...) } -func (a *App) installBatch(commands ...Command) error { +func (a *App) installBatch(declarations ...Declaration) error { if a == nil { return fmt.Errorf("cannot register commands on a nil app") } + commands := make([]Command, 0, len(declarations)) + rules := make([]RoutingRule, 0) + for _, declaration := range declarations { + switch value := declaration.(type) { + case Command: + commands = append(commands, value) + case *Command: + if value == nil { + return fmt.Errorf("nil *Command declaration") + } + commands = append(commands, *value) + case RoutingRule: + rules = append(rules, value) + case *RoutingRule: + if value == nil { + return fmt.Errorf("nil *RoutingRule declaration") + } + rules = append(rules, *value) + case nil: + return fmt.Errorf("nil declaration") + default: + return fmt.Errorf("unsupported declaration type %T", declaration) + } + } pending := make(map[string]*commandNode, len(commands)) for _, command := range commands { @@ -32,7 +59,15 @@ func (a *App) installBatch(commands ...Command) error { } pending[name] = &commandNode{command: command} } - if len(pending) == 0 { + for _, rule := range rules { + if rule.record == nil { + return fmt.Errorf("invalid zero routing rule declaration") + } + if len(rule.record.issues) > 0 { + return fmt.Errorf("routing rule: %w", errors.Join(rule.record.issues...)) + } + } + if len(pending) == 0 && len(rules) == 0 { return nil } if a.commands == nil { @@ -41,6 +76,9 @@ func (a *App) installBatch(commands ...Command) error { for name, node := range pending { a.commands[name] = node } - a.snapshot = nil + a.routingRules = append(a.routingRules, rules...) + if len(pending) > 0 { + a.snapshot = nil + } return nil } diff --git a/registry_test.go b/registry_test.go index 797c53e..812b004 100644 --- a/registry_test.go +++ b/registry_test.go @@ -40,6 +40,83 @@ func TestCapturedDeclarationsRetainCallbacksWithoutCallingThem(t *testing.T) { } } +func TestRoutingRuleFactoriesCaptureNormalizedValues(t *testing.T) { + commandRule := Alias(" old\tpath ", "new path") + if got, want := commandRule.record.sourcePath.String(), "old path"; got != want { + t.Fatalf("source path = %q, want %q", got, want) + } + if got, want := commandRule.record.targetPath.String(), "new path"; got != want { + t.Fatalf("target path = %q, want %q", got, want) + } + globalRule := FlagAlias("--colour", "-c") + if globalRule.record.sourceFlag != (capturedFlagSpelling{name: "colour"}) { + t.Fatalf("source spelling = %#v", globalRule.record.sourceFlag) + } + if globalRule.record.targetFlag != (capturedFlagSpelling{name: "c", short: true}) { + t.Fatalf("target spelling = %#v", globalRule.record.targetFlag) + } + scopedRule := FlagAliasFor("group show", "-f", "--format") + if got, want := scopedRule.record.scopePath.String(), "group show"; got != want { + t.Fatalf("scope path = %q, want %q", got, want) + } + + command := NewCommand(CommandSpec[struct{}]{Name: "new", Summary: "new"}) + app, err := New(Identity{Name: "Example"}, &command, &commandRule, globalRule, scopedRule) + if err != nil { + t.Fatal(err) + } + if len(app.routingRules) != 3 { + t.Fatalf("captured routing rules = %d, want 3", len(app.routingRules)) + } + registeredCommand := NewCommand(CommandSpec[struct{}]{Name: "registered", Summary: "registered"}) + registeredRule := Alias("another", "registered") + if err := app.Register(®isteredCommand, ®isteredRule); err != nil { + t.Fatal(err) + } + if len(app.routingRules) != 4 || app.commands["registered"] == nil { + t.Fatalf("pointer declarations were not normalized during Register: rules=%d command=%v", len(app.routingRules), app.commands["registered"]) + } +} + +func TestInvalidRoutingRuleValuesFailAtInstallation(t *testing.T) { + for name, rule := range map[string]RoutingRule{ + "empty command source": Alias(" \t ", "target"), + "invalid command target": Alias("source", "--help"), + "flag assignment": FlagAlias("--format=yaml", "--format"), + "short cluster": FlagAlias("-xy", "--format"), + "invalid scope": FlagAliasFor("", "-f", "--format"), + } { + t.Run(name, func(t *testing.T) { + if _, err := New(Identity{Name: "Example"}, rule); err == nil { + t.Fatal("New() succeeded with an invalid routing rule") + } + }) + } +} + +func TestNilAndZeroDeclarationsFailWithoutMutation(t *testing.T) { + app, err := New(Identity{Name: "Example"}) + if err != nil { + t.Fatal(err) + } + var nilCommand *Command + var nilRule *RoutingRule + declarations := []Declaration{nil, nilCommand, nilRule, Command{}, RoutingRule{}} + for index, declaration := range declarations { + if _, err := New(Identity{Name: "Example"}, declaration); err == nil { + t.Errorf("New() accepted invalid declaration %d", index) + } + beforeCommands := len(app.commands) + beforeRules := len(app.routingRules) + if err := app.Register(declaration); err == nil { + t.Errorf("Register() accepted invalid declaration %d", index) + } + if len(app.commands) != beforeCommands || len(app.routingRules) != beforeRules { + t.Errorf("invalid declaration %d mutated the app", index) + } + } +} + func TestAppsHaveIndependentRegistries(t *testing.T) { first, err := New(Identity{Name: "First"}) if err != nil {