feat(cli): groups can declare inheritable flags#11
Conversation
Adds a context-carried stack of flag registrars so a Group can declare flags that every descendant leaf inherits automatically: - `Group.Flags func(*flag.FlagSet)` — optional group-level flag closure. - `WithAncestorFlags` / `ApplyAncestorFlags` — ctx-carried slice Groups append to and leaves replay on their own FlagSet. - `DeclareString` / `DeclareBool` — Lookup-guarded wrappers ancestors use to avoid stdlib flag-redeclaration panics when a leaf already declared the same name. Leaves call ApplyAncestorFlags AFTER declaring their own flags, so "leaf wins" falls out of the guard. - `Flagger` interface — opt-in for leaves that want an auto-generated `Flags:` block in --help listing own + ancestor flags sorted. - `renderFlagsAsText` — shared flag-enumeration helper. - `Group.Help()` now emits a `Flags:` block when Flags is set, so `ana <group> --help` surfaces inheritable flags at the group level. Unblocks the connector-tree phase where dialect Groups will own common flags (--name, --account, --warehouse, ...) that auth-mode leaves reuse.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds context-backed stacking of ancestor flag registrars and propagates group-declared flags to descendant leaf commands; introduces guarded flag declaration helpers, a Flagger interface, help rendering of accumulated flags, and tests covering propagation, rendering, and precedence. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Group
participant Context
participant Leaf
participant FlagSet
User->>Group: invoke command
Group->>Context: WithAncestorFlags(g.Flags) (push registrar)
Group->>Leaf: dispatch named child
Leaf->>FlagSet: create temporary FlagSet
Leaf->>Context: ApplyAncestorFlags(ctx, FlagSet) (invoke registrars in order)
Leaf->>FlagSet: declare leaf flags (DeclareString/Bool guards)
alt help requested
Leaf->>FlagSet: parse/format flags
Leaf-->>User: help + renderFlagsAsText(FlagSet)
else run
Leaf->>FlagSet: parse args
Leaf-->>User: execute
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/cli/root.go`:
- Around line 10-35: The call in WithAncestorFlags performs an unnecessary type
conversion when appending reg; remove the redundant conversion and append reg
directly (i.e., replace flagRegistrar(reg) with reg) so the function still
builds next from prior and reg; check symbols: WithAncestorFlags, flagRegistrar,
ancestorFlagsKey, prior, next to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4d94c5b8-9aef-45b7-b9a4-e00c9948edf6
⛔ Files ignored due to path filters (1)
internal/cli/CLAUDE.mdis excluded by!**/*.md
📒 Files selected for processing (3)
internal/cli/cli.gointernal/cli/cli_test.gointernal/cli/root.go
flagRegistrar is a type alias for func(*flag.FlagSet), so appending reg (which already has that type) needs no conversion. Caught by golangci-lint unconvert via coderabbit review.
Summary
Groups can now declare flags that descend to every leaf via a ctx-carried registrar stack:
Group.Flags func(*flag.FlagSet)— optional group-level closure.WithAncestorFlags/ApplyAncestorFlags— ctx stack append / replay.DeclareString/DeclareBool— Lookup-guarded wrappers for ancestor closures (stdlib panics on duplicate declarations). Leaves declare first, ancestors no-op on conflicts, so "leaf wins."Flaggerinterface — opt-in: leaves that implementFlags(fs)get an autoFlags:block in--help, enumerating own + ancestor flags viarenderFlagsAsText.Group.Help()surfaces group-level flags soana <group> --helpshows them too.Unblocks the connector-tree phase where dialect Groups will own common flags (
--name,--account,--warehouse) reused across auth-mode leaves.Independent of
fix/global-flags-help-visibility; can merge in either order.Summary by CodeRabbit
New Features
Bug Fixes
Tests