Refactor: commands show help when no flag is set#7
Conversation
WalkthroughAdds runtime version flag handling and pre-flight config-file existence checks to CLI commands (root, requests, certinfo); refactors flag value retrieval to local variables; updates command help/README text; integrates govulncheck into the development environment and adds automated version/help smoke tests; minor formatting change in an internal handler. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI Command
participant Viper as Config/Flags
Note over CLI,Viper: Common pre-flight checks (new)
User->>CLI: run command --version
CLI->>Viper: viper.GetBool("version")
Viper-->>CLI: true
CLI-->>User: print version and exit
User->>CLI: run command without required config
CLI->>Viper: viper.ConfigFileUsed()
Viper-->>CLI: (empty or path)
alt no config file
CLI-->>User: show help and exit
else config file present
CLI->>CLI: LoadConfig() and continue execution
CLI-->>User: command output
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)cmd/requests.go (1)
🔇 Additional comments (5)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
cmd/root.go (1)
88-89: Consider removing commented code or documenting the intent.The commented-out flag registrations suggest these flags are no longer needed for the
requestscommand. If this is intentional and permanent, consider removing the commented lines. If there's a plan to re-enable them, consider adding a TODO comment explaining why they're temporarily disabled.Apply this diff to remove the commented code:
addCaBundleFlag(requestsCmd) - // addCertBundleFlag(requestsCmd) - // addKeyFileFlag(requestsCmd)cmd/certinfo.go (2)
52-55: Version handling works correctly but consider extracting to a shared function.The version handling logic is duplicated across
rootCmd(root.go:55-60),requestsCmd(requests.go:31-36), andcertinfoCmd(certinfo.go:52-55). Consider extracting this to a shared helper function to follow the DRY principle.Example refactor:
// In cmd/root.go or a new cmd/helpers.go func handleVersionFlag(cmd *cobra.Command) bool { if viper.GetBool("version") { fmt.Print(version) return true } return false } // In each command's Run function: if handleVersionFlag(cmd) { return }
57-61: Consider a more explicit flag check for clarity.The string concatenation length check is clever but may be confusing for maintainability. Consider using a more explicit approach:
- // display the help if none of the main flags is set - if len(caBundleValue+certBundleValue+keyFileValue+tlsEndpoint) == 0 { - _ = cmd.Help() - return - } + // display the help if none of the main flags is set + if caBundleValue == "" && certBundleValue == "" && keyFileValue == "" && tlsEndpoint == "" { + _ = cmd.Help() + return + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
devenv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
README.md(4 hunks)cmd/certinfo.go(3 hunks)cmd/requests.go(2 hunks)cmd/root.go(3 hunks)devenv.nix(4 hunks)internal/certinfo/certinfo_handlers.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-15T17:16:55.051Z
Learnt from: xenOs76
Repo: xenOs76/https-wrench PR: 5
File: devenv.nix:569-569
Timestamp: 2025-11-15T17:16:55.051Z
Learning: In the xenOs76/https-wrench repository, the devenv.nix test environment is local-only without GitHub CI integration, so automatic dependency updates in enterTest are acceptable for the maintainer's workflow.
Applied to files:
devenv.nix
📚 Learning: 2025-11-08T16:00:48.187Z
Learnt from: xenOs76
Repo: xenOs76/https-wrench PR: 3
File: internal/requests/requests.go:164-182
Timestamp: 2025-11-08T16:00:48.187Z
Learning: In the https-wrench tool (xenOs76/https-wrench repository), do not enforce minimum TLS version constraints in the HTTP client configuration. The tool is designed to inspect and test TLS settings on various servers, including legacy ones, and needs the flexibility to connect to servers with older TLS versions.
Applied to files:
README.md
🧬 Code graph analysis (1)
internal/certinfo/certinfo_handlers.go (2)
internal/style/style_handlers.go (1)
LgSprintf(21-26)internal/style/style.go (1)
CertKeyP4(61-63)
🔇 Additional comments (14)
internal/certinfo/certinfo_handlers.go (1)
171-174: LGTM! Formatting improvement.The multi-line formatting improves readability without changing functionality.
README.md (1)
27-42: LGTM! Documentation correctly updated.The documentation changes accurately reflect the new command structure and usage patterns introduced in this PR.
Also applies to: 49-49, 105-105, 143-143, 159-159
cmd/requests.go (2)
31-36: LGTM! Clean version handling.The version flag implementation follows a clear early-return pattern.
10-10: LGTM! Imports and descriptions updated appropriately.The new imports support the version and config validation features, and the updated descriptions are clearer.
Also applies to: 14-14, 26-28
cmd/root.go (3)
50-52: LGTM! Clearer command descriptions.The updated descriptions better communicate the tool's purpose.
62-64: LGTM! Simplified help display logic.Showing help when no config file is provided aligns with the PR objective and improves user experience.
68-73: LGTM! Better error handling pattern.Returning the error instead of calling
os.Exit(1)is better practice as it allows the caller to handle errors appropriately.cmd/certinfo.go (3)
24-45: LGTM! Excellent documentation improvement.The extended help text with examples significantly improves the user experience by providing clear usage patterns.
47-50: LGTM! Improved flag value handling.Using local variables instead of repeated
viper.GetString()calls improves code readability and reduces coupling.
71-71: LGTM! Consistent use of local variables.The refactored code consistently uses the local variables defined earlier, improving readability.
Also applies to: 75-75, 83-83
devenv.nix (4)
27-27: LGTM! Good addition for security.Adding
govulncheckhelps identify known vulnerabilities in dependencies.
152-159: LGTM! Integrated vulnerability checking into dependency workflow.Running
govulncheckafter updating dependencies ensures vulnerabilities are caught early in the development process.
210-238: LGTM! Comprehensive test coverage for new functionality.The new test commands effectively verify version and help output for all commands. The consistent pattern and use of
grepfor validation is appropriate.
601-619: LGTM! Test sequence updated appropriately.The test sequence now includes the new version and help verification tests, ensuring the refactored command behavior is validated.
Based on learnings
1 participant
Summary by CodeRabbit
Documentation
Improvements
Tests