diff --git a/command.go b/command.go index a16ed29..c640acc 100644 --- a/command.go +++ b/command.go @@ -3,10 +3,10 @@ package commandkit import ( "errors" "fmt" - "strings" "unicode" commandmeta "gitea.maximumdirect.net/eric/CommandKit/internal/commandmeta" + "gitea.maximumdirect.net/eric/CommandKit/internal/commandpath" ) // Command is an opaque, reusable declaration captured by NewCommand, Group, @@ -297,7 +297,7 @@ func collectDeclarationIssues(command Command, parents []string, root bool, issu } record := command.record path := append(append([]string(nil), parents...), record.name) - label := strings.Join(path, " ") + label := commandpath.FromTokens(path).String() for _, issue := range record.issues { if label != "" { *issues = append(*issues, fmt.Errorf("command %q: %w", label, issue)) @@ -320,11 +320,7 @@ func collectDeclarationIssues(command Command, parents []string, root bool, issu } func validateCommandName(name string) error { - fields := strings.Fields(name) - if len(fields) != 1 || fields[0] != name || strings.HasPrefix(name, "-") || name == "--" { - return fmt.Errorf("command name %q must be one non-option token", name) - } - return nil + return commandpath.ValidateComponent(name) } func validateBindings[I any](flags []Flag[I], arguments []Argument[I]) error { diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 21074ee..d264d58 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -266,6 +266,11 @@ literal quotes/backslashes, invalid option components, empty declarations, root lookup, and a single argv token containing whitespace. Existing routing and help tests remain green. +**Completed:** Added shared command-path parsing, validation, relations, keys, +formatting, and token-copy helpers. Core command validation and path diagnostics, +plus adapter route comparison and exact help lookup, now use the shared package. +Verified with `go test ./internal/commandpath ./internal/cobra .`. + ## Stage 2: Model deterministic prefix rewriting **Work** diff --git a/internal/cobra/parse.go b/internal/cobra/parse.go index 3ff9b6f..474ccb5 100644 --- a/internal/cobra/parse.go +++ b/internal/cobra/parse.go @@ -11,6 +11,7 @@ import ( "strings" commandmeta "gitea.maximumdirect.net/eric/CommandKit/internal/commandmeta" + "gitea.maximumdirect.net/eric/CommandKit/internal/commandpath" spfcobra "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -63,16 +64,17 @@ func (session *Session) Parse(argv []string) commandmeta.Result { } result.Key = selectedNode.Key var pathOK bool - result.Path, pathOK = pathFor(selected, session.declarations) + selectedPath, pathOK := pathFor(selected, session.declarations) if !pathOK { return failure(result, "selected command is not part of the declared tree") } + result.Path = []string(selectedPath) consumed := len(leading) - len(unconsumed) - if consumed != len(result.Path) || !equalPath(leading[:consumed], result.Path) { + if consumed != len(selectedPath) || !commandpath.FromTokens(leading[:consumed]).Equal(selectedPath) { return failure(result, "command path must match a registered command exactly") } - remaining := append(append([]string(nil), unconsumed...), tail...) + remaining := append(commandpath.CloneTokens(unconsumed), tail...) if err := selected.ParseFlags(remaining); err != nil { return failure(result, err.Error()) } @@ -105,12 +107,12 @@ func (session *Session) RenderHelp(path []string) (string, *commandmeta.Failure) } selected, remaining, err := session.root.Find(path) if err != nil { - return "", &commandmeta.Failure{Message: fmt.Sprintf("unknown help path %q", strings.Join(path, " ")), Usage: true} + return "", &commandmeta.Failure{Message: fmt.Sprintf("unknown help path %q", commandpath.FromTokens(path).String()), Usage: true} } declaration := session.declarations[selected] selectedPath, pathOK := pathFor(selected, session.declarations) - if declaration == nil || !pathOK || len(remaining) != 0 || !equalPath(path, selectedPath) { - return "", &commandmeta.Failure{Message: fmt.Sprintf("unknown help path %q", strings.Join(path, " ")), Usage: true} + if declaration == nil || !pathOK || len(remaining) != 0 || !commandpath.FromTokens(path).Equal(selectedPath) { + return "", &commandmeta.Failure{Message: fmt.Sprintf("unknown help path %q", commandpath.FromTokens(path).String()), Usage: true} } buffer := new(bytes.Buffer) previousOut, previousErr := selected.OutOrStdout(), selected.ErrOrStderr() @@ -134,7 +136,7 @@ func (session *Session) UsageLine(path []string) (string, *commandmeta.Failure) selected, remaining, err := session.root.Find(path) declaration := session.declarations[selected] selectedPath, pathOK := pathFor(selected, session.declarations) - if err != nil || declaration == nil || !pathOK || len(remaining) != 0 || !equalPath(path, selectedPath) { + if err != nil || declaration == nil || !pathOK || len(remaining) != 0 || !commandpath.FromTokens(path).Equal(selectedPath) { return "", &commandmeta.Failure{Message: "usage path is not registered"} } line := selected.UseLine() @@ -303,28 +305,16 @@ func flagUsage(flag commandmeta.Flag) string { return usage } -func equalPath(left, right []string) bool { - if len(left) != len(right) { - return false - } - for index := range left { - if left[index] != right[index] { - return false - } - } - return true -} - 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 commandpath.CloneTokens(argv[:index]), commandpath.CloneTokens(argv[index:]) } } - return append([]string(nil), argv...), nil + return commandpath.CloneTokens(argv), nil } -func pathFor(selected *spfcobra.Command, declarations map[*spfcobra.Command]*commandmeta.Command) ([]string, bool) { +func pathFor(selected *spfcobra.Command, declarations map[*spfcobra.Command]*commandmeta.Command) (commandpath.Path, bool) { var reverse []string for current := selected; current != nil; current = current.Parent() { declaration := declarations[current] @@ -338,7 +328,7 @@ func pathFor(selected *spfcobra.Command, declarations map[*spfcobra.Command]*com for left, right := 0, len(reverse)-1; left < right; left, right = left+1, right-1 { reverse[left], reverse[right] = reverse[right], reverse[left] } - return reverse, true + return commandpath.FromTokens(reverse), true } func failure(result commandmeta.Result, message string) commandmeta.Result { diff --git a/internal/commandpath/path.go b/internal/commandpath/path.go new file mode 100644 index 0000000..00d7673 --- /dev/null +++ b/internal/commandpath/path.go @@ -0,0 +1,106 @@ +// Package commandpath provides the shared representation and operations for +// command paths used by CommandKit's core and backend adapter. +package commandpath + +import ( + "fmt" + "strconv" + "strings" +) + +// Path is a sequence of command-name tokens. The empty path identifies the +// application root. +type Path []string + +// ParseDeclaration parses a whitespace-separated command path for capture. +// Declaration paths must contain at least one component. +func ParseDeclaration(value string) (Path, error) { + path := Path(strings.Fields(value)) + if len(path) == 0 { + return nil, fmt.Errorf("command path %q must contain at least one name", value) + } + for _, component := range path { + if err := ValidateComponent(component); err != nil { + return nil, err + } + } + return path.Clone(), nil +} + +// ParseLookup parses a whitespace-separated lookup path. An empty or +// whitespace-only value selects the application root. +func ParseLookup(value string) (Path, error) { + path := Path(strings.Fields(value)) + for _, component := range path { + if err := ValidateComponent(component); err != nil { + return nil, err + } + } + return path.Clone(), nil +} + +// FromTokens makes an owned path copy without splitting or trimming tokens. +func FromTokens(tokens []string) Path { + return Path(tokens).Clone() +} + +// Clone returns an owned copy of the path. +func (path Path) Clone() Path { + return append(Path(nil), path...) +} + +// CloneTokens returns an owned copy of token data. +func CloneTokens(tokens []string) []string { + return append([]string(nil), tokens...) +} + +// ValidateComponent validates one canonical command-name token. +func ValidateComponent(name string) error { + fields := strings.Fields(name) + if len(fields) != 1 || fields[0] != name || strings.HasPrefix(name, "-") || name == "--" { + return fmt.Errorf("command name %q must be one non-option token", name) + } + return nil +} + +// Equal reports whether both paths contain the same components. +func (path Path) Equal(other Path) bool { + if len(path) != len(other) { + return false + } + for index := range path { + if path[index] != other[index] { + return false + } + } + return true +} + +// HasPrefix reports whether prefix is a component-wise prefix of path. +func (path Path) HasPrefix(prefix Path) bool { + if len(prefix) > len(path) { + return false + } + for index := range prefix { + if path[index] != prefix[index] { + return false + } + } + return true +} + +// Key returns a normalized, unambiguous key for this path. +func (path Path) Key() string { + var key strings.Builder + for _, component := range path { + key.WriteString(strconv.Itoa(len(component))) + key.WriteByte(':') + key.WriteString(component) + } + return key.String() +} + +// String formats the path for diagnostics and help text. +func (path Path) String() string { + return strings.Join(path, " ") +} diff --git a/internal/commandpath/path_test.go b/internal/commandpath/path_test.go new file mode 100644 index 0000000..1933a8e --- /dev/null +++ b/internal/commandpath/path_test.go @@ -0,0 +1,80 @@ +package commandpath + +import "testing" + +func TestParseDeclarationAndLookup(t *testing.T) { + declaration, err := ParseDeclaration(" config\tshow ") + if err != nil { + t.Fatal(err) + } + if got, want := declaration.String(), "config show"; got != want { + t.Fatalf("declaration string = %q, want %q", got, want) + } + if _, err := ParseDeclaration(" \t "); err == nil { + t.Fatal("ParseDeclaration accepted an empty path") + } + + root, err := ParseLookup(" \t ") + if err != nil { + t.Fatal(err) + } + if len(root) != 0 || root.String() != "" { + t.Fatalf("empty lookup path = %#v, string %q", root, root.String()) + } + lookup, err := ParseLookup("config show") + if err != nil || !lookup.Equal(declaration) { + t.Fatalf("lookup = %#v, err %v; want %#v", lookup, err, declaration) + } + + quoted := `say"hello C:\tmp` + parsed, err := ParseDeclaration(quoted) + if err != nil { + t.Fatal(err) + } + if got := parsed.String(); got != quoted { + t.Fatalf("quoted path = %q, want literal %q", got, quoted) + } +} + +func TestValidateComponent(t *testing.T) { + for _, value := range []string{"--help", "-x", "--", "name word", "name\tpart"} { + t.Run(value, func(t *testing.T) { + if err := ValidateComponent(value); err == nil { + t.Fatalf("ValidateComponent(%q) succeeded", value) + } + }) + } + if err := ValidateComponent("under_score"); err != nil { + t.Fatalf("existing valid component rejected: %v", err) + } +} + +func TestPathRelationsAndKey(t *testing.T) { + path := Path{"config", "show"} + if !path.Equal(Path{"config", "show"}) { + t.Fatal("equal paths did not compare equal") + } + if !path.HasPrefix(Path{"config"}) || !path.HasPrefix(nil) { + t.Fatal("path did not recognize component prefix") + } + if path.HasPrefix(Path{"show"}) || (Path{"config"}).HasPrefix(path) { + t.Fatal("path recognized a non-prefix") + } + if (Path{"a\x00b"}).Key() == (Path{"a", "b"}).Key() { + t.Fatal("normalized keys collided for distinct component sequences") + } +} + +func TestTokenCopiesPreserveTokenContents(t *testing.T) { + input := []string{" show ", "", "--", "-x=value"} + path := FromTokens(input) + input[0] = "changed" + if path[0] != " show " { + t.Fatalf("FromTokens did not copy input: %#v", path) + } + copy := CloneTokens(path) + copy[1] = "changed" + if path[1] != "" { + t.Fatalf("CloneTokens did not copy input: %#v", path) + } +}