Replace per-command custom help with a global styled renderer#229
Replace per-command custom help with a global styled renderer#229
Conversation
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/cli/help.go">
<violation number="1" location="internal/cli/help.go:287">
P2: The LEARN MORE section is empty for leaf commands directly under root (e.g., `basecamp search --help`, `basecamp todo --help`). The second condition excludes commands whose parent is root, so neither branch writes any content after the header. Removing the `cmd.Parent() != cmd.Root()` guard would show `basecamp --help` for these commands, which is the sensible fallback.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
This PR replaces per-command custom help renderers with a single global styled help renderer for all non-root commands, relying on Cobra’s command tree and metadata (Use/Short/Long/Example/flags) to generate consistent help output.
Changes:
- Adds
renderCommandHelpand updatesrootHelpFuncto render styled help for all subcommands (and keep agent-mode JSON). - Removes the custom-help registry (
RegisterCustomHelp/CustomHelp) and deletes the bespoke projects help renderer. - Moves
projectsexamples into Cobra’sExamplefield and updates tests/e2e assertions to match the new styled output.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/cli/help.go | Implements global styled help dispatch and adds renderCommandHelp for all non-root commands. |
| internal/cli/root.go | Switches root command to use the new rootHelpFunc() signature. |
| internal/cli/help_test.go | Updates/extends tests to assert styled help for subcommands and flag rendering behavior. |
| internal/commands/commands.go | Removes the custom help function registry types and map. |
| internal/commands/projects.go | Removes bespoke projects help; adds Example so the generic renderer can display examples. |
| e2e/cards_columns_steps.bats | Updates expectations from Cobra’s default “Available Commands” to styled “COMMANDS”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79529a1b63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed all review feedback in
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/cli/help.go">
<violation number="1" location="internal/cli/help.go:304">
P2: `filterInheritedFlags` filters root-level flags for all commands, but per the design intent (and the inline comment referencing 'leaf help'), group commands should show the full inherited set. Add an early return for group commands so they get unfiltered inherited flags.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
One renderCommandHelp in help.go now handles all non-root commands, reading structure from cobra's command tree instead of hardcoding per-command. Deletes the RegisterCustomHelp registry and the bespoke renderProjectsHelp. Inherited flags are curated: parent-defined persistent flags always appear (--project, --campfire, --content-type, etc.), root globals are filtered to a salient set (--account, --json, --md, --project, --quiet).
Group commands without RunE now show "<command> [flags]" in USAGE instead of bare "[flags]". Root-level leaf commands (todo, done, etc.) now show "basecamp --help" in LEARN MORE instead of an empty section.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
renderCommandHelpinhelp.go— a single function that renders styled help for every non-root command, reading structure from cobra's command treeRegisterCustomHelp/CustomHelpregistry,renderProjectsHelp, and thedefaultHelpfallback parameter fromrootHelpFunc--account,--json,--md,--project,--quiet)Examplefield so the generic renderer picks them upMotivation
The projects command had a hand-crafted help renderer to avoid cobra's default dumping 20+ inherited flags inline. But the
customHelpmap approach doesn't scale — every new command group would need its own bespoke renderer. This replaces it with one renderer that applies everywhere, matching the styled-section layout (USAGE,COMMANDS,FLAGS,INHERITED FLAGS,EXAMPLES,LEARN MORE) already used for root help.filterInheritedFlagsdetermines flag provenance by pointer identity: if a flag is the same object as the one on root's PersistentFlags, it's a root global subject to the salient allowlist. A parent that redefines the same name (e.g.--projecton messages) produces a different pointer and always passes through.Test plan
make checkpasses (fmt, vet, lint, unit tests, e2e, surface snapshot, provenance)basecamp projects --help,basecamp projects list --help,basecamp campfire post --help,basecamp timesheet report --helpbasecamp projects --agent --help | jq .