feat: Add a proper --help/-h flag with exit code 0 - #180
Merged
Merged
Conversation
Adds printCheckUsage() to display custom per-flag help text instead of Go's default auto-generated format, and handles flag.ErrHelp in runCheck() to return ExitSuccess with the custom help text rather than ExitUsage. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Exercises the compiled archguard-e2e binary end-to-end for --help, -h, help, check --help/-h, and index --help/-h, confirming exit code 0 and usage text through the real CLI entry point rather than only in-process. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Extract check/index flag construction into registerCheckFlags/newCheckFlagSet and newIndexFlagSet so Execute can detect and print check/index --help before git.GetRepoRoot(), config loading, and provider construction run, without duplicating flag definitions. Also trims isTopLevelHelpRequest's doc comment to fit CLAUDE.md's 2-line comment cap, and documents both the help early-exit paths and the runIndexCommand/runIndex invariant in CLAUDE.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Help detection remains incorrect when value-taking flags precede --help.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds successful top-level and subcommand help handling, including operation without repository configuration.
Changes:
- Adds custom help output and early flag handling.
- Refactors shared flag registration.
- Adds unit/e2e tests and documentation.
File summaries
| File | Reviewed changes | Findings |
|---|---|---|
test/e2e_test.go |
Adds binary-level help coverage. | None |
internal/cli/cli.go |
Implements help detection and shared flag setup. | Moderate (3 votes): separate flag values can prevent help detection. Nit (1 vote): use “flag package” wording. |
internal/cli/cli_test.go |
Adds help behavior and exit-code tests. | None |
CLAUDE.md |
Documents CLI help and dispatch behavior. | None |
Review details
Suppressed comments (1)
internal/cli/cli.go:921
- The Go package is named
flag, notflag.Package; please use “flag package” in this comment.
break // flag.Package stops parsing flags at the first positional arg
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
golangci-lint's errcheck flagged the unchecked Fprintln/Fprintf calls in printCheckUsage/printIndexUsage, added in this branch. Match the existing _, _ = fmt.Fprintf(...) convention used elsewhere in this file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
subcommandHelpRequest's manual loop broke at the first token not starting with "-", so a value-taking flag's value (e.g. "json" in --format json) was misread as a positional arg and any --help after it went undetected. Parse with the real FlagSet instead, which already knows each flag's arity. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
archguard --help,-h, andhelpnow print top-level usage and exit0— and work even outside a git repo, since help is detected before any git/config resolution.archguard check --help/-handarchguard index --help/-hprint a custom, per-flag usage listing (built from each subcommand's own flag descriptions viaflag.FlagSet.VisitAll, not Go's defaultflag.PrintDefaults()output) and exit0.2(ExitUsage).check/index --helpwork even without anarchguard.yaml, valid config, or API key set — a gap surfaced during final review (the initial implementation only handled help after config/provider setup, so it would fail with exit1/3in exactly the unconfigured state where a user is most likely to reach for--help). Flag definitions forcheckandindexare now built by shared helpers (registerCheckFlags,newIndexFlagSet) so the early-help path and the normal parsing path can never drift out of sync.Deviations from the plan
bd0b088) rather than left for a separate issue, since it directly affects whether the feature is genuinely usable in the state where--helpmatters most.printIndexUsage's flags-section is currently dead code (sinceindexhas zero flags today), andarchguard init --helpstill runs a realinitinstead of printing help (the issue never mentionedinit). Both are small, independent follow-ups.Test plan
go test ./...— all packages passgo build -o archguard ./cmd/archguard— builds cleanarchguard --help,check --help,index --helpall exit0with readable usage and no config/API-key warning noisearchguard check --not-a-real-flagandarchguard typo(unknown command) still exit2, unaffectedTestIsTopLevelHelpRequest,TestExecute_TopLevelHelpExitsSuccess,TestRunCheck_HelpFlagExitsSuccessWithCustomUsage,TestRunIndexCommand_HelpFlagExitsSuccess,TestSubcommandHelpRequestTestE2E_SubcommandHelpWorksWithoutConfig(unconfigured-repo case)golangci-lint run— could not verify in this environment. It panics locally on every branch, including unmodifiedmain, due to a Go 1.27 vs. golangci-lint-v2.12.2 toolchain mismatch (confirmed the panic stack is entirely insidego/types/golangci-lintinternals, unrelated to this diff). Docker wasn't available locally as a fallback. Please confirm this is clean via CI.Closes #177
🤖 Generated with Claude Code