From 391790ba4215203427091536cad4161007e0805b Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Mon, 18 May 2026 15:42:24 -0500 Subject: [PATCH 1/3] Add CommandGroup metadata for grouped help rendering Callers want to organize commands into named groups in their own help output (for example, separating "Getting started" commands from "Server operations" in a CLI's top-level listing). mflags had no way to carry that information through. Adds a CommandGroupProvider interface, a WithCommandGroup option, a Group field on ChildEntry and CommandDoc, and exports GetDirectChildren so callers can iterate the command tree and read group metadata. Also exposes Dispatcher.Name() so callers rendering their own help can use the same program name without re-plumbing it. Strictly additive plumbing. mflags' own built-in help rendering still ignores Group; rendering decisions stay with the caller. --- dispatcher.go | 48 +++++++++++++++++++++++++------ dispatcher_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++++++ help_doc.go | 4 ++- 3 files changed, 113 insertions(+), 9 deletions(-) diff --git a/dispatcher.go b/dispatcher.go index 4857567..ffdcb78 100644 --- a/dispatcher.go +++ b/dispatcher.go @@ -70,6 +70,14 @@ type RequiredFeatureProvider interface { RequiredFeature() string } +// CommandGroupProvider is an interface for commands that belong to a named group. +// Groups are used by consumers to organize commands in help output (e.g. separating +// operator commands from deployer commands). +type CommandGroupProvider interface { + // CommandGroup returns the group name for this command, or "" for ungrouped. + CommandGroup() string +} + // OutputFormatter is an interface for commands that can specify their output format type OutputFormatter interface { // OutputFormat returns the output format for this command @@ -91,6 +99,7 @@ type funcCommand struct { usage string examples []Example outputFormat OutputFormat + group string } // CommandOption is a functional option for configuring a command @@ -110,6 +119,13 @@ func WithExamples(examples ...Example) CommandOption { } } +// WithCommandGroup sets the group name for the command. +func WithCommandGroup(group string) CommandOption { + return func(c *funcCommand) { + c.group = group + } +} + // WithOutputFormat sets the output format for the command func WithOutputFormat(format OutputFormat) CommandOption { return func(c *funcCommand) { @@ -157,6 +173,11 @@ func (c *funcCommand) Examples() []Example { return c.examples } +// CommandGroup returns the group name for this command. +func (c *funcCommand) CommandGroup() string { + return c.group +} + // OutputFormat returns the output format for this command func (c *funcCommand) OutputFormat() OutputFormat { return c.outputFormat @@ -180,6 +201,11 @@ type Dispatcher struct { name string } +// Name returns the dispatcher's program name. +func (d *Dispatcher) Name() string { + return d.name +} + // NewDispatcher creates a new command dispatcher func NewDispatcher(name string) *Dispatcher { return &Dispatcher{ @@ -504,7 +530,7 @@ func (d *Dispatcher) showHelp() error { fmt.Printf("Usage: %s [arguments]\n\n", d.name) fmt.Println("Available commands:") - children := d.getDirectChildren("") + children := d.GetDirectChildren("") // Find max length for alignment maxLen := 0 @@ -515,7 +541,7 @@ func (d *Dispatcher) showHelp() error { } for _, child := range children { - grandchildren := d.getDirectChildren(child.Path) + grandchildren := d.GetDirectChildren(child.Path) suffix := "" if len(grandchildren) > 0 { suffix = " " + faint(subCommandsLabel(len(grandchildren))) @@ -599,7 +625,7 @@ func (d *Dispatcher) showCommandHelp(entry *CommandEntry) error { } // Show sub-commands if any exist (including implicit namespaces) - children := d.getDirectChildren(entry.Path) + children := d.GetDirectChildren(entry.Path) if len(children) > 0 { fmt.Println("\nSub-commands:") @@ -613,7 +639,7 @@ func (d *Dispatcher) showCommandHelp(entry *CommandEntry) error { // Print sub-commands with usage for _, child := range children { - grandchildren := d.getDirectChildren(child.Path) + grandchildren := d.GetDirectChildren(child.Path) suffix := "" if len(grandchildren) > 0 { suffix = " " + faint(subCommandsLabel(len(grandchildren))) @@ -662,12 +688,13 @@ type ChildEntry struct { Path string // The full path (parentPath + " " + Name, or just Name for top-level) Usage string // Usage text (from registered command, or empty for namespaces) IsEntry bool // True if this is a registered command, false if just a namespace + Group string // Group name from CommandGroupProvider, or empty for ungrouped } -// getDirectChildren returns the direct children of a path, including both +// GetDirectChildren returns the direct children of a path, including both // registered commands and implicit namespaces. If parentPath is empty, returns // top-level entries. -func (d *Dispatcher) getDirectChildren(parentPath string) []ChildEntry { +func (d *Dispatcher) GetDirectChildren(parentPath string) []ChildEntry { children := make(map[string]*ChildEntry) for path, entry := range d.commands { @@ -695,11 +722,16 @@ func (d *Dispatcher) getDirectChildren(parentPath string) []ChildEntry { if len(parts) == 1 { // Direct child command + group := "" + if gp, ok := entry.Command.(CommandGroupProvider); ok { + group = gp.CommandGroup() + } children[childName] = &ChildEntry{ Name: childName, Path: childPath, Usage: entry.Usage, IsEntry: true, + Group: group, } } else { // Deeper command — childName is a namespace (unless already registered) @@ -743,7 +775,7 @@ func (d *Dispatcher) isNamespace(path string) bool { func (d *Dispatcher) showNamespaceHelp(namespacePath string) error { fmt.Printf("Usage: %s %s [arguments]\n", d.name, namespacePath) - children := d.getDirectChildren(namespacePath) + children := d.GetDirectChildren(namespacePath) if len(children) > 0 { fmt.Println("\nAvailable commands:") @@ -756,7 +788,7 @@ func (d *Dispatcher) showNamespaceHelp(namespacePath string) error { } for _, child := range children { - grandchildren := d.getDirectChildren(child.Path) + grandchildren := d.GetDirectChildren(child.Path) suffix := "" if len(grandchildren) > 0 { suffix = " " + faint(subCommandsLabel(len(grandchildren))) diff --git a/dispatcher_test.go b/dispatcher_test.go index 5b387fb..05f4614 100644 --- a/dispatcher_test.go +++ b/dispatcher_test.go @@ -1699,3 +1699,73 @@ func TestDispatcherErrShowHelp(t *testing.T) { }) } +func TestGetDirectChildrenPopulatesGroup(t *testing.T) { + d := NewDispatcher("myapp") + + d.Dispatch("deploy", NewCommand(NewFlagSet("deploy"), nil, WithUsage("Deploy an app"))) + d.Dispatch("server", NewCommand(NewFlagSet("server"), nil, WithUsage("Start server"), WithCommandGroup("Operator"))) + d.Dispatch("debug", NewCommand(NewFlagSet("debug"), nil, WithUsage("Debug tools"), WithCommandGroup("Operator"))) + + children := d.GetDirectChildren("") + + groups := make(map[string]string) + for _, child := range children { + groups[child.Name] = child.Group + } + + assert.Equal(t, "", groups["deploy"]) + assert.Equal(t, "Operator", groups["server"]) + assert.Equal(t, "Operator", groups["debug"]) +} + +func TestGetDirectChildrenGroupForNamespaceOverriddenByCommand(t *testing.T) { + d := NewDispatcher("myapp") + + // Register a subcommand first (creates implicit namespace for "server") + d.Dispatch("server config", NewCommand(NewFlagSet("server config"), nil, WithUsage("Server config"))) + // Then register the parent command with a group + d.Dispatch("server", NewCommand(NewFlagSet("server"), nil, WithUsage("Start server"), WithCommandGroup("Operator"))) + + children := d.GetDirectChildren("") + + var serverEntry ChildEntry + for _, child := range children { + if child.Name == "server" { + serverEntry = child + break + } + } + + assert.True(t, serverEntry.IsEntry) + assert.Equal(t, "Operator", serverEntry.Group) +} + +func TestHelpDocIncludesGroup(t *testing.T) { + d := NewDispatcher("myapp") + + d.Dispatch("deploy", NewCommand(NewFlagSet("deploy"), nil, WithUsage("Deploy an app"))) + d.Dispatch("server", NewCommand(NewFlagSet("server"), nil, WithUsage("Start server"), WithCommandGroup("Operator"))) + + doc := d.HelpDoc() + + groups := make(map[string]string) + for _, cmd := range doc.Commands { + groups[cmd.Path] = cmd.Group + } + + assert.Equal(t, "", groups["deploy"]) + assert.Equal(t, "Operator", groups["server"]) +} + +func TestHelpJSONIncludesGroup(t *testing.T) { + d := NewDispatcher("myapp") + + d.Dispatch("server", NewCommand(NewFlagSet("server"), nil, WithUsage("Start server"), WithCommandGroup("Operator"))) + + data, err := d.HelpJSON() + assert.NoError(t, err) + + json := string(data) + assert.Contains(t, json, `"group": "Operator"`) +} + diff --git a/help_doc.go b/help_doc.go index f8cf6de..71c4f4f 100644 --- a/help_doc.go +++ b/help_doc.go @@ -19,6 +19,7 @@ type CommandDoc struct { Path string `json:"path"` Usage string `json:"usage"` Description string `json:"description,omitempty"` + Group string `json:"group,omitempty"` RequiredFeature string `json:"requiredFeature,omitempty"` Flags []FlagDoc `json:"flags"` FlagGroups []string `json:"flagGroups"` @@ -135,13 +136,14 @@ func (d *Dispatcher) HelpJSON() ([]byte, error) { // buildCommandDocs recursively builds CommandDoc entries for direct children of parentPath. func (d *Dispatcher) buildCommandDocs(parentPath string) []CommandDoc { - children := d.getDirectChildren(parentPath) + children := d.GetDirectChildren(parentPath) docs := make([]CommandDoc, 0, len(children)) for _, child := range children { cmd := CommandDoc{ Path: child.Path, Usage: child.Usage, + Group: child.Group, Flags: []FlagDoc{}, FlagGroups: []string{}, PositionalArgs: []PositionalDoc{}, From 626d988976a7a45e44f3a95f7967bfc87631ad6e Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Mon, 18 May 2026 16:04:20 -0500 Subject: [PATCH 2/3] Address CodeRabbit review Normalize parentPath in GetDirectChildren to match the convention of other public path-based methods. Tests for group propagation now check key presence explicitly instead of relying on map zero values. --- dispatcher.go | 1 + dispatcher_test.go | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/dispatcher.go b/dispatcher.go index ffdcb78..06f2991 100644 --- a/dispatcher.go +++ b/dispatcher.go @@ -695,6 +695,7 @@ type ChildEntry struct { // registered commands and implicit namespaces. If parentPath is empty, returns // top-level entries. func (d *Dispatcher) GetDirectChildren(parentPath string) []ChildEntry { + parentPath = normalizeCommandPath(parentPath) children := make(map[string]*ChildEntry) for path, entry := range d.commands { diff --git a/dispatcher_test.go b/dispatcher_test.go index 05f4614..cb18919 100644 --- a/dispatcher_test.go +++ b/dispatcher_test.go @@ -1713,8 +1713,11 @@ func TestGetDirectChildrenPopulatesGroup(t *testing.T) { groups[child.Name] = child.Group } + assert.Contains(t, groups, "deploy") assert.Equal(t, "", groups["deploy"]) + assert.Contains(t, groups, "server") assert.Equal(t, "Operator", groups["server"]) + assert.Contains(t, groups, "debug") assert.Equal(t, "Operator", groups["debug"]) } @@ -1753,7 +1756,9 @@ func TestHelpDocIncludesGroup(t *testing.T) { groups[cmd.Path] = cmd.Group } + assert.Contains(t, groups, "deploy") assert.Equal(t, "", groups["deploy"]) + assert.Contains(t, groups, "server") assert.Equal(t, "Operator", groups["server"]) } From 50a3de18904e350831f587abd28a904380d5e894 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Mon, 18 May 2026 16:53:15 -0500 Subject: [PATCH 3/3] Implement hidden:"yes" struct tag for flag help omission Discovered while bumping mflags in mirendev/runtime: a deprecated server flag had been tagged hidden:"yes" with the intent of keeping it parseable but out of --help output. mflags didn't recognize the tag, so it had been silently doing nothing on main, and our newer strict tag validation made the latent omission visible. Adds a Hidden bool to Flag, threads it through FromStruct (accepting "yes", "true", or "1"), drops hidden flags from WriteFlagHelp, and surfaces it in FlagDoc/JSON output so callers rendering their own help can honor it too. --- fromstruct.go | 13 +++++++++++++ help.go | 3 +++ help_doc.go | 2 ++ mflags.go | 1 + mflags_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 67 insertions(+) diff --git a/fromstruct.go b/fromstruct.go index 2d51791..378a4b0 100644 --- a/fromstruct.go +++ b/fromstruct.go @@ -61,6 +61,7 @@ var knownTags = map[string]bool{ "default": true, "env": true, "required": true, + "hidden": true, "usage": true, "description": true, "choice": true, @@ -70,6 +71,16 @@ var knownTags = map[string]bool{ "group": true, } +// isHiddenTag reports whether a hidden:"..." struct tag value is truthy. +// Accepts the common boolean-ish forms callers tend to emit. +func isHiddenTag(v string) bool { + switch v { + case "yes", "true", "1": + return true + } + return false +} + // validateStructTags checks that every struct tag on exported fields is one // that FromStruct actually reads. It returns an error listing all unrecognized // tags so the caller can fix them all in one pass. @@ -368,6 +379,7 @@ func (f *FlagSet) FromStruct(v any, opts ...FromStructOption) error { defaultValue := field.Tag.Get("default") envVar := field.Tag.Get("env") required := field.Tag.Get("required") == "true" + hidden := isHiddenTag(field.Tag.Get("hidden")) usage := field.Tag.Get("usage") if usage == "" { @@ -554,6 +566,7 @@ func (f *FlagSet) FromStruct(v any, opts ...FromStructOption) error { if flag, ok := f.flags[longName]; ok { flag.EnvVar = envVar flag.Required = required + flag.Hidden = hidden if envVar != "" { if envVal, ok := os.LookupEnv(envVar); ok { if err := flag.Value.Set(envVal); err != nil { diff --git a/help.go b/help.go index f3a0620..d514d68 100644 --- a/help.go +++ b/help.go @@ -49,6 +49,9 @@ func (f *FlagSet) WriteFlagHelp() { var defaultFlags []*Flag f.VisitAll(func(flag *Flag) { + if flag.Hidden { + return + } if flag.Group == "" { defaultFlags = append(defaultFlags, flag) } else { diff --git a/help_doc.go b/help_doc.go index 71c4f4f..6111707 100644 --- a/help_doc.go +++ b/help_doc.go @@ -47,6 +47,7 @@ type FlagDoc struct { Choices []string `json:"choices,omitempty"` EnvVar string `json:"envVar,omitempty"` Required bool `json:"required,omitempty"` + Hidden bool `json:"hidden,omitempty"` } // PositionalDoc describes a positional argument in the help document. @@ -88,6 +89,7 @@ func (f *FlagSet) HelpDoc() *FlagSetDoc { Choices: []string{}, EnvVar: flag.EnvVar, Required: flag.Required, + Hidden: flag.Hidden, } if flag.Short != 0 { fd.Short = string(flag.Short) diff --git a/mflags.go b/mflags.go index 30b270f..1dcc726 100644 --- a/mflags.go +++ b/mflags.go @@ -57,6 +57,7 @@ type Flag struct { Group string // group name for help rendering; empty = default "Options:" EnvVar string // environment variable name (from env:"..." struct tag) Required bool // whether this flag must be provided + Hidden bool // omit from help output (from hidden:"yes" struct tag) HasValue bool // true if value was set by env var or CLI arg } diff --git a/mflags_test.go b/mflags_test.go index 1a1eb13..4ebc805 100644 --- a/mflags_test.go +++ b/mflags_test.go @@ -3947,3 +3947,51 @@ func TestRequiredBoolFlag(t *testing.T) { assert.True(t, config.Accept) }) } + +// --- hidden tag tests --- + +func TestFromStructHiddenFlag(t *testing.T) { + type Config struct { + Visible string `long:"visible" description:"shown in help"` + LegacyYes string `long:"legacy-yes" description:"deprecated" hidden:"yes"` + LegacyTrue string `long:"legacy-true" description:"deprecated" hidden:"true"` + } + + config := &Config{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + + t.Run("flag is still parseable", func(t *testing.T) { + err := fs.Parse([]string{"--legacy-yes", "old"}) + assert.NoError(t, err) + assert.Equal(t, "old", config.LegacyYes) + }) + + t.Run("hidden flag omitted from help output", func(t *testing.T) { + var buf bytes.Buffer + stdout := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + fs.WriteFlagHelp() + _ = w.Close() + os.Stdout = stdout + _, _ = io.Copy(&buf, r) + + output := buf.String() + assert.Contains(t, output, "--visible") + assert.NotContains(t, output, "--legacy-yes") + assert.NotContains(t, output, "--legacy-true") + }) + + t.Run("hidden propagates to FlagDoc", func(t *testing.T) { + doc := fs.HelpDoc() + flagsByName := make(map[string]FlagDoc) + for _, f := range doc.Flags { + flagsByName[f.Name] = f + } + assert.False(t, flagsByName["visible"].Hidden) + assert.True(t, flagsByName["legacy-yes"].Hidden) + assert.True(t, flagsByName["legacy-true"].Hidden) + }) +}