From f206c531f1f43ec8dcab14124b694ed748d1abe2 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 7 Oct 2026 13:27:00 +0000 Subject: [PATCH] Add Cobra-backed command parsing --- command.go | 20 +++ docs/development.md | 5 +- docs/roadmap/implementation.md | 10 +- internal/cobra/parse.go | 260 +++++++++++++++++++++++++++++++++ internal/cobra/parse_test.go | 118 +++++++++++++++ parser.go | 41 ++++++ parser_test.go | 50 +++++++ 7 files changed, 500 insertions(+), 4 deletions(-) create mode 100644 internal/cobra/parse.go create mode 100644 internal/cobra/parse_test.go create mode 100644 parser.go create mode 100644 parser_test.go diff --git a/command.go b/command.go index c247a15..368fe4f 100644 --- a/command.go +++ b/command.go @@ -4,6 +4,8 @@ import ( "fmt" "strings" "unicode" + + internalcobra "gitea.maximumdirect.net/eric/CommandKit/internal/cobra" ) // Command is an opaque command declaration. Use NewCommand or Group to create @@ -95,6 +97,7 @@ type CommandSpec[I any] struct { type commandPayload interface { validate() error hasRun() bool + bridgeFlags() []internalcobra.Flag prepare([][]string, []string, bool) (func() any, error) run(Invocation, any) Outcome } @@ -111,6 +114,23 @@ func (c typedCommand[I]) hasRun() bool { return c.spec.Run != nil } +func (c typedCommand[I]) bridgeFlags() []internalcobra.Flag { + flags := make([]internalcobra.Flag, len(c.spec.Flags)) + for index, flag := range c.spec.Flags { + binding := flag.binding + flags[index] = internalcobra.Flag{ + Name: binding.meta.Name, + Short: binding.meta.Short, + Usage: binding.meta.Usage, + Default: binding.defaultText, + Boolean: binding.boolean, + Repeated: binding.repeated, + Required: binding.meta.Required, + } + } + return flags +} + func (c typedCommand[I]) prepare(flagValues [][]string, operands []string, enforceRequired bool) (func() any, error) { prepared, err := prepareInput(c.spec, flagValues, operands, enforceRequired) if err != nil { diff --git a/docs/development.md b/docs/development.md index cddf29f..0916d6e 100644 --- a/docs/development.md +++ b/docs/development.md @@ -2,8 +2,9 @@ CommandKit currently contains adopted repository policies, accepted architectural decisions, implementation plans, identity and outcome primitives, typed command -declarations, and an instance-owned registry. Command execution is not implemented -yet. The [README](../README.md) provides project orientation; the +declarations, an instance-owned registry, and a Cobra-backed routing and parsing +adapter. Public invocation coordination is not implemented yet. The +[README](../README.md) provides project orientation; the [documentation policy](policy/documentation.md#canonical-ownership) assigns canonical documentation owners. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 8e76923..f200bdf 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Initial CommandKit implementation plan -Status: implementation underway; Stages 1-3 complete and Stages 4-14 pending. +Status: implementation underway; Stages 1-4 complete and Stages 5-14 pending. Date: 2026-10-07. @@ -294,7 +294,7 @@ preparation parses supplied values without enforcing omitted required values. ## Stage 4: implement the internal Cobra adapter -**Status:** pending. **Prerequisite:** Stage 3. +**Status:** complete. **Prerequisite:** Stage 3. 1. Create `internal/cobra` with neutral tree/flag/operand declarations and fresh per-run binding factories. A bridge declaration contains path metadata, @@ -338,6 +338,12 @@ parser causes without scanning their object graphs. unknown flags, boolean spellings, last-wins/repetition, help with missing required inputs, and fresh consecutive parses. Confirm the root does not import Cobra/pflag. +**Completion evidence:** internal adapter and root-to-adapter integration tests +cover command routing, local flags, interspersed operands, `--`, boolean presence +and explicit false, repeated raw values, help parsing, group failures, and fresh +parse state. `go test ./...` passes; the root package's direct imports include +only the internal adapter and not Cobra or pflag. + ## Stage 5: implement invocation and diagnostic delivery **Status:** pending. **Prerequisite:** Stage 4. diff --git a/internal/cobra/parse.go b/internal/cobra/parse.go new file mode 100644 index 0000000..fcbb7e5 --- /dev/null +++ b/internal/cobra/parse.go @@ -0,0 +1,260 @@ +// Package cobra adapts CommandKit's neutral command declarations to Cobra and +// pflag. Backend types and failures remain inside this package. +package cobra + +import ( + "context" + "fmt" + "io" + "strconv" + "strings" + + spfcobra "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// Command describes one node in a per-parse command tree. +type Command struct { + Name string + Summary string + Description string + Runnable bool + Flags []Flag + Children []*Command +} + +// Flag describes one local option without exposing a pflag value. +type Flag struct { + Name string + Short string + Usage string + Default string + Boolean bool + Repeated bool + Required bool +} + +// Failure is a backend-owned parse failure translated to neutral message data. +type Failure struct { + Message string +} + +// Result contains the selected command and raw input collected by pflag. +type Result struct { + Path []string + HelpRequested bool + FlagValues [][]string + Operands []string + Failure *Failure +} + +// ParseOptions makes the parser's context and streams explicit. Nil writers +// are replaced with io.Discard. +type ParseOptions struct { + Context context.Context + In io.Reader + Out io.Writer + Err io.Writer +} + +// Parse constructs a fresh Cobra tree and parses one argv. It routes only the +// leading command words, stopping permanently at the first option or --. +func Parse(root *Command, argv []string, options ParseOptions) Result { + result := Result{} + if root == nil { + return failure(result, "command tree is nil") + } + options = normalizeOptions(options) + backendRoot, nodes := buildTree(root, options) + leading, tail := splitRoutingPrefix(argv) + selected, unconsumed, err := backendRoot.Find(leading) + if err != nil { + return failure(result, err.Error()) + } + if selected == nil { + return failure(result, "no command was selected") + } + selectedNode := declarationFor(nodes, selected) + if selectedNode == nil { + return failure(result, "selected command is not part of the declared tree") + } + result.Path = pathFor(root, selectedNode) + + remaining := append(append([]string(nil), unconsumed...), tail...) + if err := selected.ParseFlags(remaining); err != nil { + return failure(result, err.Error()) + } + operands := selected.Flags().Args() + + result.Operands = append([]string(nil), operands...) + result.FlagValues = make([][]string, len(selectedNode.Flags)) + for index, flag := range selectedNode.Flags { + if value := selected.Flags().Lookup(flag.Name); value != nil { + result.FlagValues[index] = value.Value.(*rawValue).valuesCopy() + } + } + if helpFlag := selected.Flags().Lookup("help"); helpFlag != nil { + helpRequested, err := helpFlag.Value.(*rawValue).lastBool() + if err != nil { + return failure(result, err.Error()) + } + result.HelpRequested = helpRequested + } + if !selectedNode.Runnable && len(result.Operands) > 0 { + return failure(result, "unknown command or unexpected group operands: "+strings.Join(result.Operands, " ")) + } + return result +} + +func normalizeOptions(options ParseOptions) ParseOptions { + if options.Context == nil { + options.Context = context.Background() + } + if options.In == nil { + options.In = strings.NewReader("") + } + if options.Out == nil { + options.Out = io.Discard + } + if options.Err == nil { + options.Err = io.Discard + } + return options +} + +func buildTree(root *Command, options ParseOptions) (*spfcobra.Command, map[*Command]*spfcobra.Command) { + nodes := make(map[*Command]*spfcobra.Command) + var build func(*Command) *spfcobra.Command + build = func(declaration *Command) *spfcobra.Command { + backend := &spfcobra.Command{ + Use: declaration.Name, + Short: declaration.Summary, + Long: declaration.Description, + Args: func(*spfcobra.Command, []string) error { return nil }, + SilenceErrors: true, + SilenceUsage: true, + DisableSuggestions: true, + } + backend.SetContext(options.Context) + backend.SetIn(options.In) + backend.SetOut(options.Out) + backend.SetErr(options.Err) + backend.SetHelpFunc(func(*spfcobra.Command, []string) {}) + backend.SetUsageFunc(func(*spfcobra.Command) error { return nil }) + backend.SetFlagErrorFunc(func(_ *spfcobra.Command, err error) error { return err }) + backend.Flags().SetInterspersed(true) + backend.Flags().SortFlags = false + backend.DisableAutoGenTag = true + helpValue := &rawValue{boolean: true} + backend.Flags().VarP(helpValue, "help", "h", "help for this command") + backend.Flags().Lookup("help").NoOptDefVal = "true" + for _, declarationFlag := range declaration.Flags { + value := &rawValue{boolean: declarationFlag.Boolean} + backend.Flags().VarP(value, declarationFlag.Name, declarationFlag.Short, declarationFlag.Usage) + backendFlag := backend.Flags().Lookup(declarationFlag.Name) + backendFlag.DefValue = declarationFlag.Default + if declarationFlag.Boolean { + backendFlag.NoOptDefVal = "true" + } + } + // Cobra's automatic help flag is suppressed because CommandKit declares + // and interprets the local help flag itself. + backend.SetHelpCommand(nil) + nodes[declaration] = backend + for _, child := range declaration.Children { + backend.AddCommand(build(child)) + } + return backend + } + return build(root), nodes +} + +func declarationFor(nodes map[*Command]*spfcobra.Command, selected *spfcobra.Command) *Command { + for declaration, backend := range nodes { + if backend == selected { + return declaration + } + } + return nil +} + +func splitRoutingPrefix(argv []string) ([]string, []string) { + for index, argument := range argv { + if argument == "--" || (strings.HasPrefix(argument, "-") && argument != "-") { + return append([]string(nil), argv[:index]...), append([]string(nil), argv[index:]...) + } + } + return append([]string(nil), argv...), nil +} + +func pathFor(root, selected *Command) []string { + var reverse []string + for current := selected; current != nil && current != root; { + reverse = append(reverse, current.Name) + current = parentOf(root, current) + } + path := make([]string, len(reverse)) + for index := range reverse { + path[len(reverse)-index-1] = reverse[index] + } + return path +} + +func parentOf(root, target *Command) *Command { + for _, child := range root.Children { + if child == target { + return root + } + if parent := parentOf(child, target); parent != nil { + return parent + } + } + return nil +} + +func failure(result Result, message string) Result { + result.Failure = &Failure{Message: message} + return result +} + +type rawValue struct { + values []string + boolean bool +} + +func (v *rawValue) Set(value string) error { + v.values = append(v.values, value) + return nil +} + +func (v *rawValue) String() string { + if len(v.values) > 0 { + return v.values[len(v.values)-1] + } + return "" +} + +func (v *rawValue) Type() string { + if v.boolean { + return "bool" + } + return "string" +} + +func (v *rawValue) IsBoolFlag() bool { return v.boolean } + +func (v *rawValue) valuesCopy() []string { return append([]string(nil), v.values...) } + +func (v *rawValue) lastBool() (bool, error) { + var value bool + for _, raw := range v.values { + parsed, err := strconv.ParseBool(raw) + if err != nil { + return false, fmt.Errorf("invalid boolean value %q for --help", raw) + } + value = parsed + } + return value, nil +} + +var _ pflag.Value = (*rawValue)(nil) diff --git a/internal/cobra/parse_test.go b/internal/cobra/parse_test.go new file mode 100644 index 0000000..d4432b6 --- /dev/null +++ b/internal/cobra/parse_test.go @@ -0,0 +1,118 @@ +package cobra + +import ( + "context" + "io" + "reflect" + "strings" + "testing" +) + +func TestParseRoutesBeforeOptionsAndKeepsFlagsLocal(t *testing.T) { + tree := &Command{Name: "app", Children: []*Command{ + {Name: "config", Children: []*Command{ + {Name: "show", Runnable: true, Flags: []Flag{{Name: "scope"}}}, + }}, + }} + + result := Parse(tree, []string{"config", "show", "--scope", "app"}, ParseOptions{}) + if result.Failure != nil { + t.Fatalf("parse failed: %s", result.Failure.Message) + } + if !reflect.DeepEqual(result.Path, []string{"config", "show"}) || !reflect.DeepEqual(result.FlagValues, [][]string{{"app"}}) { + t.Fatalf("selected result = %#v", result) + } + + result = Parse(tree, []string{"config", "--scope", "app", "show"}, ParseOptions{}) + if result.Failure == nil { + t.Fatal("accepted a child flag before the complete command path") + } +} + +func TestParseInterspersedOperandsBooleanFlagsAndTerminator(t *testing.T) { + tree := &Command{Name: "app", Children: []*Command{ + {Name: "restore", Runnable: true, Flags: []Flag{ + {Name: "enabled", Boolean: true}, + {Name: "tag", Repeated: true}, + }}, + }} + result := Parse(tree, []string{"restore", "before", "--enabled", "--tag", "a,b", "--tag=c", "--", "--literal"}, ParseOptions{}) + if result.Failure != nil { + t.Fatalf("parse failed: %s", result.Failure.Message) + } + if !reflect.DeepEqual(result.FlagValues, [][]string{{"true"}, {"a,b", "c"}}) { + t.Fatalf("flag values = %#v", result.FlagValues) + } + if !reflect.DeepEqual(result.Operands, []string{"before", "--literal"}) { + t.Fatalf("operands = %#v", result.Operands) + } + + result = Parse(tree, []string{"restore", "--enabled=false"}, ParseOptions{}) + if result.Failure != nil || !reflect.DeepEqual(result.FlagValues[0], []string{"false"}) { + t.Fatalf("explicit false = %#v", result) + } +} + +func TestParseHelpIsLocalAndFalseDoesNotRequestHelp(t *testing.T) { + tree := &Command{Name: "app", Children: []*Command{ + {Name: "run", Runnable: true, Flags: []Flag{{Name: "required", Required: true}}}, + }} + result := Parse(tree, []string{"run", "--help"}, ParseOptions{}) + if result.Failure != nil || !result.HelpRequested { + t.Fatalf("help parse = %#v", result) + } + result = Parse(tree, []string{"run", "--help=false"}, ParseOptions{}) + if result.Failure != nil || result.HelpRequested { + t.Fatalf("false help parse = %#v", result) + } + result = Parse(tree, []string{"run", "--help=1"}, ParseOptions{}) + if result.Failure != nil || !result.HelpRequested { + t.Fatalf("strconv boolean help parse = %#v", result) + } + result = Parse(tree, []string{"run", "--help=invalid", "--help"}, ParseOptions{}) + if result.Failure == nil { + t.Fatal("accepted an invalid earlier help occurrence") + } +} + +func TestParseRunnableParentAndGroupUnknownCommand(t *testing.T) { + tree := &Command{Name: "app", Children: []*Command{ + {Name: "deploy", Runnable: true, Flags: []Flag{{Name: "scope"}}, Children: []*Command{{Name: "now", Runnable: true}}}, + {Name: "config", Children: []*Command{{Name: "show", Runnable: true}}}, + }} + result := Parse(tree, []string{"deploy", "target"}, ParseOptions{}) + if result.Failure != nil || !reflect.DeepEqual(result.Path, []string{"deploy"}) || !reflect.DeepEqual(result.Operands, []string{"target"}) { + t.Fatalf("runnable parent result = %#v", result) + } + result = Parse(tree, []string{"deploy", "--scope", "local"}, ParseOptions{}) + if result.Failure != nil || !reflect.DeepEqual(result.FlagValues, [][]string{{"local"}}) { + t.Fatalf("runnable parent local flag = %#v", result) + } + result = Parse(tree, []string{"deploy", "now"}, ParseOptions{}) + if result.Failure != nil || !reflect.DeepEqual(result.Path, []string{"deploy", "now"}) { + t.Fatalf("nested runnable child = %#v", result) + } + result = Parse(tree, []string{"config", "mystery"}, ParseOptions{}) + if result.Failure == nil { + t.Fatal("accepted an unknown trailing command beneath a group") + } +} + +func TestParseReturnsNeutralFailuresAndFreshState(t *testing.T) { + tree := &Command{Name: "app", Children: []*Command{ + {Name: "run", Runnable: true, Flags: []Flag{{Name: "value"}}}, + }} + options := ParseOptions{Context: context.Background(), In: strings.NewReader("unused"), Out: io.Discard, Err: io.Discard} + first := Parse(tree, []string{"run", "--value=old"}, options) + second := Parse(tree, []string{"run"}, options) + if first.Failure != nil || second.Failure != nil { + t.Fatalf("parse results = %#v, %#v", first, second) + } + if !reflect.DeepEqual(first.FlagValues, [][]string{{"old"}}) || !reflect.DeepEqual(second.FlagValues, [][]string{nil}) { + t.Fatalf("state leaked between parses: %#v then %#v", first.FlagValues, second.FlagValues) + } + failed := Parse(tree, []string{"run", "--unknown"}, options) + if failed.Failure == nil || failed.Failure.Message == "" { + t.Fatalf("unknown flag failure = %#v", failed.Failure) + } +} diff --git a/parser.go b/parser.go new file mode 100644 index 0000000..7d0d7da --- /dev/null +++ b/parser.go @@ -0,0 +1,41 @@ +package commandkit + +import ( + "sort" + + internalcobra "gitea.maximumdirect.net/eric/CommandKit/internal/cobra" +) + +func (a *App) parse(args []string, options internalcobra.ParseOptions) internalcobra.Result { + if a == nil { + return internalcobra.Result{Failure: &internalcobra.Failure{Message: "cannot parse commands on a nil app"}} + } + root := &internalcobra.Command{Name: a.identity.Name} + names := make([]string, 0, len(a.commands)) + for name, node := range a.commands { + if !node.reserved { + names = append(names, name) + } + } + sort.Strings(names) + for _, name := range names { + root.Children = append(root.Children, bridgeCommand(a.commands[name].command)) + } + return internalcobra.Parse(root, args, options) +} + +func bridgeCommand(command Command) *internalcobra.Command { + declaration := &internalcobra.Command{ + Name: command.name, + Summary: command.summary, + Description: command.description, + } + if command.payload != nil { + declaration.Runnable = command.payload.hasRun() + declaration.Flags = command.payload.bridgeFlags() + } + for _, child := range command.children { + declaration.Children = append(declaration.Children, bridgeCommand(child)) + } + return declaration +} diff --git a/parser_test.go b/parser_test.go new file mode 100644 index 0000000..8f49598 --- /dev/null +++ b/parser_test.go @@ -0,0 +1,50 @@ +package commandkit + +import ( + "context" + "reflect" + "testing" + + internalcobra "gitea.maximumdirect.net/eric/CommandKit/internal/cobra" +) + +func TestAppParseUsesRegisteredBindingsWithTheAdapter(t *testing.T) { + type input struct { + scope Supplied[string] + id Supplied[string] + } + show, err := NewCommand(CommandSpec[input]{ + Name: "show", + Flags: []Flag[input]{ValueFlag(FlagMeta{Name: "scope"}, (*string)(nil), func(value string) (string, error) { + return value, nil + }, func(value *input, supplied Supplied[string]) { value.scope = supplied })}, + Arguments: []Argument[input]{ValueArgument(ArgumentMeta{Name: "id"}, true, func(value string) (string, error) { + return value, nil + }, func(value *input, supplied Supplied[string]) { value.id = supplied })}, + Run: func(Invocation, input) Outcome { return Success() }, + }) + if err != nil { + t.Fatal(err) + } + config, err := Group("config", "configuration", show) + if err != nil { + t.Fatal(err) + } + app, err := New(Identity{Name: "example"}, config) + if err != nil { + t.Fatal(err) + } + + options := internalcobra.ParseOptions{Context: context.Background()} + result := app.parse([]string{"config", "show", "one", "--scope", "app"}, options) + if result.Failure != nil { + t.Fatalf("parse failed: %s", result.Failure.Message) + } + if !reflect.DeepEqual(result.Path, []string{"config", "show"}) || !reflect.DeepEqual(result.FlagValues, [][]string{{"app"}}) || !reflect.DeepEqual(result.Operands, []string{"one"}) { + t.Fatalf("parse result = %#v", result) + } + result = app.parse([]string{"config", "--scope", "app", "show"}, options) + if result.Failure == nil { + t.Fatal("accepted a child flag before the complete command path") + } +}