-
-
Notifications
You must be signed in to change notification settings - Fork 37
Change help output and long description to make it neater #164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…" only if the "ahoy <command> --help" structure is used Signed-off-by: Drew Robinson <[email protected]>
WalkthroughAdds pre-command help interception to display command help when -h/--help appears before "--". Refactors help templates: new CommandHelpTemplate, updated AppHelpTemplate, and adds a trimSpace template function. Updates flag usage strings. Adjusts tests to match new spacing in command listings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant A as CLI App
participant P as Arg Scanner
participant C as Command
participant H as Help Printer
U->>A: ahoy <command> [args]
A->>P: Scan args for -h/--help before "--"
alt Help flag before "--"
P-->>A: help requested
A->>H: Render CommandHelpTemplate
H-->>U: Show command help
A-->>U: Exit without running command
else No early help
A->>C: Execute command action
C-->>U: Command output
end
note over P,H: Arguments after "--" are not scanned for help
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
v2/flag.go (1)
22-33: Consistent Usage phrasing and capitalisation (minor polish).Great tidy-up. For consistency across flags and with other sentences in this file, consider adding terminal periods and capitalising “Bash”.
- Usage: "Show help", + Usage: "Show help.", @@ - Usage: "Print the Ahoy version", + Usage: "Print the Ahoy version.", @@ - Name: "generate-bash-completion", - Usage: "Generate bash completion script.", + Name: "generate-bash-completion", + Usage: "Generate Bash completion script.",v2/ahoy.go (3)
268-280: Pre-command help interception: solid; consider supporting combined short flags and add tests.The interception correctly respects “--” and short-circuits to ShowCommandHelp. Two optional tweaks:
- Combined short flags like -hv won’t match. If you want to catch -h within a combined group before “--”, extend the check cautiously.
- Add tests to lock this behaviour (see suggested Bats cases below).
Example tweak:
- for _, arg := range c.Args() { + for _, arg := range c.Args() { if arg == "--" { foundDoubleDash = true continue } - if !foundDoubleDash && (arg == "--help" || arg == "-h") { + if !foundDoubleDash && (arg == "--help" || arg == "-h" || + (strings.HasPrefix(arg, "-") && !strings.HasPrefix(arg, "--") && strings.Contains(arg[1:], "h"))) { cli.ShowCommandHelp(c, c.Command.Name) return } }Would you like me to open a follow-up PR adding Bats tests for:
- “ahoy -h” shows per-command help and does not execute the command.
- “ahoy -- -h” passes -h through to the underlying command?
528-533: Use interface{} instead of any for broader Go compatibility.The any alias requires Go ≥ 1.18. If the project supports older Go toolchains, prefer interface{} to avoid unexpected build failures.
- cli.HelpPrinterCustom = func(out io.Writer, templ string, data any, customFuncs map[string]any) { + cli.HelpPrinterCustom = func(out io.Writer, templ string, data interface{}, customFuncs map[string]interface{}) { funcMap := template.FuncMap{ "join": strings.Join, "replace": strings.ReplaceAll, "trimSpace": strings.TrimSpace, }If the repo is already on Go ≥ 1.18, feel free to ignore.
577-590: Prefer VisibleFlags over Flags to avoid exposing hidden flags.Using .Flags may print hidden flags inside command help. urfave/cli’s default template uses .VisibleFlags for this reason.
- cli.CommandHelpTemplate = `NAME: + cli.CommandHelpTemplate = `NAME: {{.HelpName}} - {{.Usage}}{{if .Description}} DESCRIPTION: {{trimSpace .Description}} {{end}} USAGE: - {{.HelpName}}{{if .Flags}} [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}} -{{if .Flags}} + {{.HelpName}}{{if .VisibleFlags}} [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}} +{{if .VisibleFlags}} OPTIONS: - {{range .Flags}}{{.}} - {{end}}{{end}} + {{range .VisibleFlags}}{{.}} + {{end}}{{end}} `This keeps behaviour aligned with urfave/cli’s defaults and avoids accidental leakage of hidden/internal options.
v2/tests/descriptions.bats (2)
14-16: Relax whitespace matching to reduce brittleness across environments.The output is tabwriter-based; exact spacing can vary. Using regex whitespace classes avoids fragile tests.
- [[ "$output" =~ "simple Simple command" ]] + [[ "$output" =~ simple[[:space:]]+Simple[[:space:]]command ]] @@ - [[ "$output" =~ "no-description Command without description" ]] + [[ "$output" =~ no-description[[:space:]]+Command[[:space:]]without[[:space:]]description ]] @@ - [[ "$output" =~ "echo Display a message" ]] + [[ "$output" =~ echo[[:space:]]+Display[[:space:]]a[[:space:]]message ]] @@ - [[ "$output" =~ "list List directory contents" ]] + [[ "$output" =~ list[[:space:]]+List[[:space:]]directory[[:space:]]contents ]] @@ - [[ "$output" =~ "ahoy-there, ahoy Say \"ahoy there!\"" ]] + [[ "$output" =~ ahoy-there,\ ahoy[[:space:]]+Say\ \"ahoy\ there!\" ]]Happy to apply similar adjustments to other expectations if desired.
Also applies to: 27-28, 35-35
72-79: Add tests for the new -h/--help interception semantics.To prevent regressions, add two cases covering “-h before --” vs “after --”.
+@test "Per-command -h shows help and does not execute" { + run ./ahoy -f testdata/simple.ahoy.yml echo -h + [ "$status" -eq 0 ] + [[ "$output" =~ ^NAME: ]] + [[ "$output" =~ ^USAGE: ]] + [[ ! "$output" =~ test\ message ]] # ensure underlying echo didn't run +} + +@test "-h after -- is passed through to command" { + run ./ahoy -f testdata/simple.ahoy.yml echo -- -h + [ "$status" -eq 0 ] + [[ "$output" =~ -h ]] +}If the simple.ahoy.yml “echo” command differs, I can tailor these to a command that reliably echoes its args.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
v2/ahoy.go(3 hunks)v2/flag.go(1 hunks)v2/tests/descriptions.bats(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
v2/flag.go (3)
v2/vendor/github.com/urfave/cli/flag_bool.go (1)
BoolFlag(10-18)v2/cli_parsing_test.go (3)
TestEnvironmentVariableFlags(136-154)TestFlagParsing(11-34)TestFlagNameAliases(156-182)v2/vendor/github.com/urfave/cli/app.go (1)
Name(28-104)
v2/tests/descriptions.bats (1)
v2/description_test.go (5)
TestDescriptionWithExistingCommands(127-198)TestDescriptionInCLICommands(67-125)TestDescriptionParsing(10-65)command(17-21)command(82-86)
v2/ahoy.go (3)
v2/vendor/github.com/urfave/cli/context.go (1)
Args(183-183)v2/vendor/github.com/urfave/cli/help.go (7)
ShowCommandHelp(189-215)ShowAppHelp(74-93)printHelpCustom(256-276)interface{}(256-256)interface{}(278-278)interface{}(86-86)interface{}(48-48)v2/vendor/github.com/urfave/cli/template.go (1)
CommandHelpTemplate(39-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (macos-latest)
🔇 Additional comments (1)
v2/ahoy.go (1)
552-575: New AppHelpTemplate reads cleaner.The condensed commands list and explicit “Use ‘… --help’” guidance make the UX clearer. No functional concerns from this template block.
Now shows long "description ..." only if the "ahoy --help" structure is used.
Extension of the implementation of #155
Summary by CodeRabbit
New Features
Documentation
Tests