From f3bd6dc2ae50a2d1bc908571eb8b4383057a81f1 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 26 Oct 2026 02:50:54 +0000 Subject: [PATCH] Install shared reporting flags --- command.go | 3 + docs/roadmap/implementation.md | 8 ++- flag_aliases.go | 6 +- flag_aliases_test.go | 89 ++++++++++++++++++++++++++- help_view.go | 1 + internal/cobra/parse.go | 15 ++++- internal/cobra/parse_test.go | 56 +++++++++++++++++ internal/commandmeta/metadata.go | 27 +++++++- internal/commandmeta/metadata_test.go | 28 +++++++++ parser.go | 3 +- 10 files changed, 226 insertions(+), 10 deletions(-) create mode 100644 internal/commandmeta/metadata_test.go diff --git a/command.go b/command.go index d2904e5..21965a2 100644 --- a/command.go +++ b/command.go @@ -471,6 +471,9 @@ func validateBindings[I any](flags []Flag[I], arguments []Argument[I]) error { if binding.meta.Name == "help" { return fmt.Errorf("flag name %q is reserved for help", binding.meta.Name) } + if commandmeta.IsReportingFlag(binding.meta.Name) { + return fmt.Errorf("flag name %q is reserved for reporting", binding.meta.Name) + } if _, exists := flagNames[binding.meta.Name]; exists { return fmt.Errorf("duplicate flag name %q", binding.meta.Name) } diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 985f5aa..f0cc33c 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Invocation reporting implementation plan -Status: ready for implementation. No implementation stages are complete. +Status: Stages 1–5 complete. Stage 6 is next. ## Objective and authority @@ -204,7 +204,7 @@ reporter/deferred/output handles. Include failures during terminal delivery. ## Stage 5: Install shared reporting flag metadata -Status: pending. +Status: complete. Primary files: [command.go](../../command.go), [parser.go](../../parser.go), [flag_aliases.go](../../flag_aliases.go), @@ -231,6 +231,10 @@ Validation: protect installation collisions and atomicity, aliases to each core flag, canonical-spelling collisions, unchanged typed binding indexes, and flag availability in root/group/built-in/discovery help. +Focused metadata and Cobra adapter tests pass. Root-package registration and +alias tests are included, but their execution is deferred until the planned API +migration restores root-package buildability. + ## Stage 6: Preserve occurrence order through the normal parser Status: pending. diff --git a/flag_aliases.go b/flag_aliases.go index 5d51377..560f7cd 100644 --- a/flag_aliases.go +++ b/flag_aliases.go @@ -127,10 +127,14 @@ func indexCanonicalNodes(roots map[string]*canonicalNode) map[string]*canonicalN } func helpFlagSpellings() map[string]capturedFlagSpelling { - flags := make(map[string]capturedFlagSpelling, 2) + flags := make(map[string]capturedFlagSpelling, 2+len(commandmeta.ReportingFlags())) for _, spelling := range []capturedFlagSpelling{{name: "help"}, {name: "h", short: true}} { flags[flagSpellingKey(spelling)] = spelling } + for _, metadata := range commandmeta.ReportingFlags() { + spelling := capturedFlagSpelling{name: metadata.Name} + flags[flagSpellingKey(spelling)] = spelling + } return flags } diff --git a/flag_aliases_test.go b/flag_aliases_test.go index 1e3c6d9..14f40e9 100644 --- a/flag_aliases_test.go +++ b/flag_aliases_test.go @@ -4,6 +4,7 @@ import ( "strings" "testing" + commandmeta "gitea.maximumdirect.net/eric/CommandKit/internal/commandmeta" "gitea.maximumdirect.net/eric/CommandKit/internal/commandpath" ) @@ -74,12 +75,16 @@ func TestFlagAliasChainsUseScopedOverridesAndCanonicalEndpoints(t *testing.T) { } } -func TestFlagAliasDiscoveryContextsUseOnlyGlobalHelpAliases(t *testing.T) { +func TestFlagAliasDiscoveryContextsUseGlobalReportingAliases(t *testing.T) { show := flagAliasCommand("show") app, err := New(Identity{Name: "Example"}, Group("Tools", "tools", show), Alias("shortcut show", "Tools show"), FlagAlias("--hlep", "--help"), + FlagAlias("--silent", "--quiet"), + FlagAlias("--chatty", "--verbose"), + FlagAlias("--trace", "--debug"), + FlagAlias("--show-progress", "--progress"), FlagAliasFor("Tools show", "--local-help", "--help"), ) if err != nil { @@ -97,11 +102,93 @@ func TestFlagAliasDiscoveryContextsUseOnlyGlobalHelpAliases(t *testing.T) { if got := discovery[globalHelp].target.token(); got != "--help" { t.Fatalf("discovery --hlep target = %s, want --help", got) } + for label, aliases := range map[string]resolvedFlagAliases{ + "root": app.flagAliases[commandpath.Path(nil).Key()], + "group": app.flagAliases[commandpath.Path{"Tools"}.Key()], + "command": app.flagAliases[commandpath.Path{"Tools", "show"}.Key()], + "discovery": app.discoveryFlags[commandpath.Path{"shortcut"}.Key()], + } { + for source, target := range map[string]string{ + "silent": "quiet", + "chatty": "verbose", + "trace": "debug", + "show-progress": "progress", + } { + key := flagSpellingKey(capturedFlagSpelling{name: source}) + if got := aliases.bySpelling[key].target.token(); got != "--"+target { + t.Errorf("%s --%s target = %s, want --%s", label, source, got, target) + } + } + } if _, inherited := discovery[flagSpellingKey(capturedFlagSpelling{name: "local-help"})]; inherited { t.Fatal("discovery context inherited a command-scoped alias") } } +func TestReportingFlagsAreReservedAndAcceptApplicationShortAliases(t *testing.T) { + for _, name := range []string{"quiet", "verbose", "debug", "progress"} { + command := flagAliasCommand("reserved", stringFlag(name, "")) + if len(command.record.issues) == 0 { + t.Errorf("capturing --%s did not retain a declaration error", name) + } + app, err := New(Identity{Name: "Example"}, flagAliasCommand("safe")) + if err != nil { + t.Fatal(err) + } + snapshot := app.routeSnapshot() + if err := app.Register(command, Alias("reserved-shortcut", "reserved")); err == nil || !strings.Contains(err.Error(), "reserved for reporting") { + t.Errorf("registering --%s returned %v, want a retained reporting reservation error", name, err) + } + if app.routeSnapshot() != snapshot || app.commands["reserved"] != nil || app.commands["safe"] == nil || len(app.routingRules) != 0 { + t.Fatalf("failed registration of --%s changed the installed commands", name) + } + } + + declarations := []Declaration{flagAliasCommand("run")} + for short, target := range map[string]string{"q": "quiet", "v": "verbose", "d": "debug"} { + declarations = append(declarations, FlagAlias("-"+short, "--"+target)) + } + if _, err := New(Identity{Name: "Example"}, declarations...); err != nil { + t.Fatalf("application short aliases for reporting flags were rejected: %v", err) + } +} + +func TestReportingFlagSpellingsConflictWithAliasSources(t *testing.T) { + for _, name := range []string{"quiet", "verbose", "debug", "progress"} { + if _, err := New(Identity{Name: "Example"}, FlagAlias("--"+name, "--help")); err == nil || !strings.Contains(err.Error(), "conflicts with a canonical spelling") { + t.Errorf("alias source --%s returned %v, want a canonical spelling collision", name, err) + } + } + if _, err := New(Identity{Name: "Example"}, Group("tools", "", flagAliasCommand("show")), FlagAliasFor("tools", "--quiet", "--help")); err == nil || !strings.Contains(err.Error(), "conflicts with a canonical spelling") { + t.Errorf("group-scoped canonical reporting spelling returned %v, want a collision", err) + } +} + +func TestRoutingSnapshotCarriesReportingMetadataOutsideConsumerBindings(t *testing.T) { + app, err := New(Identity{Name: "Example"}, + flagAliasCommand("run", stringFlag("format", "f")), + Group("tools", "", flagAliasCommand("show")), + ) + if err != nil { + t.Fatal(err) + } + var check func(*commandmeta.Command) + check = func(node *commandmeta.Command) { + if len(node.CoreFlags) != 4 { + t.Errorf("%s core reporting flags = %#v", node.Name, node.CoreFlags) + } + for _, flag := range node.Flags { + if commandmeta.IsReportingFlag(flag.Name) { + t.Errorf("%s reporting flag %q was added to consumer bindings", node.Name, flag.Name) + } + } + for _, child := range node.Children { + check(child) + } + } + check(app.routeSnapshot().metadata) +} + func TestFlagAliasDuplicatesAndScopesAreDeterministic(t *testing.T) { command := flagAliasCommand("inspect", stringFlag("color", "c"), stringFlag("format", "f")) duplicate := FlagAlias("--colour", "--color") diff --git a/help_view.go b/help_view.go index 6011f25..67278d5 100644 --- a/help_view.go +++ b/help_view.go @@ -12,6 +12,7 @@ func (a *App) helpView(route routeResult, snapshot *routingSnapshot) commandmeta EnteredPath: clonePath(route.enteredPath), CanonicalPath: clonePath(route.canonicalPath), Discovery: route.kind == routeDiscovery, + CoreFlags: commandmeta.ReportingFlags(), } if snapshot == nil { return view diff --git a/internal/cobra/parse.go b/internal/cobra/parse.go index d1f8e6a..9cdb59b 100644 --- a/internal/cobra/parse.go +++ b/internal/cobra/parse.go @@ -77,8 +77,11 @@ func (session *Session) Parse(argv []string) commandmeta.Result { } remaining := append(commandpath.CloneTokens(unconsumed), tail...) - interpreted, spellings, interpretationFailure := interpretFlagTokens(remaining, selectedNode.Flags, session.flagAliases) - for _, flag := range selectedNode.Flags { + localFlags := make([]commandmeta.Flag, 0, len(selectedNode.Flags)+len(selectedNode.CoreFlags)) + localFlags = append(localFlags, selectedNode.Flags...) + localFlags = append(localFlags, selectedNode.CoreFlags...) + interpreted, spellings, interpretationFailure := interpretFlagTokens(remaining, localFlags, session.flagAliases) + for _, flag := range localFlags { if value := selected.Flags().Lookup(flag.Name); value != nil { value.Value.(*rawValue).queueSpellings(spellings[flag.Name]) } @@ -209,6 +212,12 @@ func appendHelpAliases(content string, commandAliases []commandmeta.CommandAlias func renderDiscoveryHelp(rootName string, view commandmeta.HelpView) string { lines := []string{"Usage:", " " + displayCommandPath(rootName, view.EnteredPath) + " [command]"} + if len(view.CoreFlags) > 0 { + lines = append(lines, "", "Reporting Flags:") + for _, flag := range view.CoreFlags { + lines = append(lines, " --"+flag.Name+" — "+flag.Usage+" (default: "+flag.Default+")") + } + } if len(view.CommandAliases) > 0 { lines = append(lines, "", "Available Aliases:") for _, alias := range view.CommandAliases { @@ -309,7 +318,7 @@ func buildTree(root *commandmeta.Command, options commandmeta.ParseOptions) (*sp helpFlag := backend.Flags().Lookup("help") helpFlag.NoOptDefVal = "true" helpFlag.Usage = "help for this command (default: false)" - for _, declarationFlag := range declaration.Flags { + for _, declarationFlag := range append(append([]commandmeta.Flag(nil), declaration.Flags...), declaration.CoreFlags...) { value := &rawValue{boolean: declarationFlag.Boolean} backend.Flags().VarP(value, declarationFlag.Name, declarationFlag.Short, flagUsage(declarationFlag)) backendFlag := backend.Flags().Lookup(declarationFlag.Name) diff --git a/internal/cobra/parse_test.go b/internal/cobra/parse_test.go index 152bab7..526abdd 100644 --- a/internal/cobra/parse_test.go +++ b/internal/cobra/parse_test.go @@ -77,6 +77,62 @@ func TestParseInterspersedOperandsBooleanFlagsAndTerminator(t *testing.T) { } } +func TestReportingFlagsParseWithoutChangingConsumerFlagIndexes(t *testing.T) { + tree := &Command{Name: "app", CoreFlags: commandmeta.ReportingFlags(), Children: []*Command{ + {Name: "config", CoreFlags: commandmeta.ReportingFlags(), Children: []*Command{ + {Name: "show", Runnable: true, CoreFlags: commandmeta.ReportingFlags(), Flags: []Flag{{Name: "format"}}}, + }}, + }} + result := Parse(tree, []string{"config", "show", "--quiet", "-v", "--format=yaml"}, ParseOptions{ + FlagAliases: []commandmeta.FlagAlias{{Source: "-v", Target: "--verbose"}}, + }) + if result.Failure != nil { + t.Fatalf("parse failed: %s", result.Failure.Message) + } + if !reflect.DeepEqual(result.FlagValues, [][]string{{"yaml"}}) { + t.Fatalf("consumer binding values = %#v", result.FlagValues) + } + if !reflect.DeepEqual(result.FlagSpellings, [][]string{{"--format"}}) { + t.Fatalf("consumer binding spellings = %#v", result.FlagSpellings) + } +} + +func TestReportingFlagsAppearInCanonicalAndDiscoveryHelp(t *testing.T) { + flags := commandmeta.ReportingFlags() + root := &Command{Name: "app", CoreFlags: flags, Children: []*Command{ + {Name: "group", CoreFlags: commandmeta.ReportingFlags(), Children: []*Command{ + {Name: "run", Runnable: true, CoreFlags: commandmeta.ReportingFlags()}, + }}, + {Name: "version", Runnable: true, CoreFlags: commandmeta.ReportingFlags()}, + }} + session := Prepare(root, ParseOptions{}) + for _, path := range [][]string{nil, {"group"}, {"group", "run"}, {"version"}} { + help, failure := session.RenderHelp(path) + if failure != nil { + t.Fatalf("help for %v failed: %s", path, failure.Message) + } + for _, name := range []string{"quiet", "verbose", "debug", "progress"} { + if !strings.Contains(help, "--"+name) { + t.Errorf("help for %v omits --%s: %q", path, name, help) + } + } + } + discovery, failure := session.RenderHelpView(commandmeta.HelpView{ + EnteredPath: []string{"shortcut"}, Discovery: true, CoreFlags: commandmeta.ReportingFlags(), + }) + if failure != nil { + t.Fatal(failure.Message) + } + for _, name := range []string{"quiet", "verbose", "debug", "progress"} { + if !strings.Contains(discovery, "--"+name) { + t.Errorf("discovery help omits --%s: %q", name, discovery) + } + } + if strings.Contains(discovery, "Available Commands:") || len(session.Metadata().Children) != 2 { + t.Fatalf("discovery help manufactured canonical command metadata: %q", discovery) + } +} + func TestParseHelpIsLocalAndFalseDoesNotRequestHelp(t *testing.T) { tree := &Command{Name: "app", Children: []*Command{ {Name: "run", Runnable: true, Flags: []Flag{{Name: "required", Required: true}}}, diff --git a/internal/commandmeta/metadata.go b/internal/commandmeta/metadata.go index bea0dde..9220cff 100644 --- a/internal/commandmeta/metadata.go +++ b/internal/commandmeta/metadata.go @@ -18,8 +18,10 @@ type Command struct { Description string Runnable bool Flags []Flag - Arguments []Argument - Children []*Command + // CoreFlags are parsed by CommandKit but do not occupy consumer binding indexes. + CoreFlags []Flag + Arguments []Argument + Children []*Command } // Argument describes one positional input for usage rendering. @@ -41,6 +43,26 @@ type Flag struct { Required bool } +var reportingFlags = [...]Flag{ + {Name: "quiet", Usage: "suppress substantive output and optional messages; keep failures visible", Default: "false", Boolean: true}, + {Name: "verbose", Usage: "show warning and informational messages", Default: "false", Boolean: true}, + {Name: "debug", Usage: "show debugging messages", Default: "false", Boolean: true}, + {Name: "progress", Usage: "show progress messages", Default: "false", Boolean: true}, +} + +// ReportingFlags returns the flags shared by CommandKit's invocation reporter. +func ReportingFlags() []Flag { return append([]Flag(nil), reportingFlags[:]...) } + +// IsReportingFlag reports whether name is reserved for a shared reporting flag. +func IsReportingFlag(name string) bool { + for _, flag := range reportingFlags { + if flag.Name == name { + return true + } + } + return false +} + // FlagAlias maps one option spelling to a canonical flag spelling. type FlagAlias struct { Source string @@ -58,6 +80,7 @@ type HelpView struct { EnteredPath []string CanonicalPath []string Discovery bool + CoreFlags []Flag CommandAliases []CommandAlias FlagAliases []FlagAlias } diff --git a/internal/commandmeta/metadata_test.go b/internal/commandmeta/metadata_test.go new file mode 100644 index 0000000..ead103d --- /dev/null +++ b/internal/commandmeta/metadata_test.go @@ -0,0 +1,28 @@ +package commandmeta + +import "testing" + +func TestReportingFlagsAreBooleanAndReturnedIndependently(t *testing.T) { + flags := ReportingFlags() + want := []string{"quiet", "verbose", "debug", "progress"} + if len(flags) != len(want) { + t.Fatalf("reporting flags = %#v", flags) + } + for index, name := range want { + if flags[index].Name != name || !flags[index].Boolean || flags[index].Short != "" || flags[index].Default != "false" { + t.Errorf("reporting flag %d = %#v", index, flags[index]) + } + if !IsReportingFlag(name) { + t.Errorf("%q is not recognized as a reporting flag", name) + } + } + flags[0].Name = "changed" + if ReportingFlags()[0].Name != "quiet" { + t.Fatal("caller mutation changed shared reporting metadata") + } + for _, name := range []string{"help", "quietly", ""} { + if IsReportingFlag(name) { + t.Errorf("%q was incorrectly recognized as a reporting flag", name) + } + } +} diff --git a/parser.go b/parser.go index 418a71e..36da825 100644 --- a/parser.go +++ b/parser.go @@ -26,7 +26,7 @@ func (a *App) routeSnapshot() *routingSnapshot { } func (a *App) buildRoutingSnapshot() *routingSnapshot { - root := &commandmeta.Command{Name: a.identity.Name} + root := &commandmeta.Command{Name: a.identity.Name, CoreFlags: commandmeta.ReportingFlags()} snapshot := &routingSnapshot{ metadata: root, runners: make(map[commandmeta.Key]commandPayload), @@ -67,6 +67,7 @@ func bridgeCommand(command Command, snapshot *routingSnapshot, next *commandmeta Name: record.name, Summary: record.summary, Description: record.description, + CoreFlags: commandmeta.ReportingFlags(), } if record.payload != nil { snapshot.runners[declaration.Key] = record.payload