feat : organize CLI help menu inot logical groups #90#91
Conversation
📝 WalkthroughWalkthroughThis PR changes the quoting/escaping style of generated ChangesCheck Command Quoting and Test Alignment
Custom CLI Help Output
Estimated code review effort: 2 (Simple) | ~12 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
YasserYG8
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/test_cli_and_git.py (2)
25-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLGTM, but consider deduplicating this helper across test files.
The
sys.executable -c ...check-command construction here is duplicated near-verbatim intest_engine_integration.py,test_mcp.py,test_report_and_json.py, andtest_staged_content.py. Extracting a shared helper (e.g. inconftest.py) would avoid the five copies drifting apart if the invocation strategy changes again.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cli_and_git.py` around lines 25 - 28, The sys.executable check-command assembly in _rules_yaml is duplicated across multiple test files and should be centralized to avoid drift. Extract the shared helper used to build the "-c" invocation into a common test utility such as conftest.py, then update _rules_yaml and the matching helpers in test_engine_integration.py, test_mcp.py, test_report_and_json.py, and test_staged_content.py to call that shared helper instead of inlining the command string.
522-537: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider also asserting the
-hshort flag path.
main()explicitly checksargs_list == ["-h"]as a distinct branch from["--help"], but the test only exercises[]and["--help"]. Addingassert cli.main(["-h"]) == 0would cover that branch too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cli_and_git.py` around lines 522 - 537, The CLI help test does not cover the explicit `main()` branch for the `-h` short flag, so extend `test_cli_custom_help` to also call `cli.main(["-h"])` and assert it returns 0, alongside the existing `[]` and `["--help"]` checks, to exercise the distinct help path in `main()`.src/becwright/cli.py (1)
505-530: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a small helper to build
--pattern "..."check strings.The double-quote-and-escape pattern for building
forbid/requirecheck commands is now manually repeated across the JS/TS/Go/Rust starter rules, the CLAUDE.md signal table, and the commit-msg signal table. A tiny helper (e.g.,_forbid_check(pattern, *, ignore_case=False)/_require_check(pattern)) that performs the quoting once would reduce the risk of a future pattern being mis-escaped when hand-edited.Also applies to: 571-576, 628-645
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/becwright/cli.py` around lines 505 - 530, The check-command string assembly for forbid/require patterns is duplicated in the rule builders, which makes escaping easy to get wrong when editing by hand. Add a small helper near the existing rule construction logic in cli.py, such as a dedicated function used by the JS/TS, Go, and Rust rule blocks (and the other signal table builders mentioned in the diff), to centralize quoting and escaping for --pattern values. Update the affected rule definitions to call that helper instead of inlining the escaped command strings so the behavior stays consistent across all rule tables.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/becwright/cli.py`:
- Around line 505-530: The check-command string assembly for forbid/require
patterns is duplicated in the rule builders, which makes escaping easy to get
wrong when editing by hand. Add a small helper near the existing rule
construction logic in cli.py, such as a dedicated function used by the JS/TS,
Go, and Rust rule blocks (and the other signal table builders mentioned in the
diff), to centralize quoting and escaping for --pattern values. Update the
affected rule definitions to call that helper instead of inlining the escaped
command strings so the behavior stays consistent across all rule tables.
In `@tests/test_cli_and_git.py`:
- Around line 25-28: The sys.executable check-command assembly in _rules_yaml is
duplicated across multiple test files and should be centralized to avoid drift.
Extract the shared helper used to build the "-c" invocation into a common test
utility such as conftest.py, then update _rules_yaml and the matching helpers in
test_engine_integration.py, test_mcp.py, test_report_and_json.py, and
test_staged_content.py to call that shared helper instead of inlining the
command string.
- Around line 522-537: The CLI help test does not cover the explicit `main()`
branch for the `-h` short flag, so extend `test_cli_custom_help` to also call
`cli.main(["-h"])` and assert it returns 0, alongside the existing `[]` and
`["--help"]` checks, to exercise the distinct help path in `main()`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ec688140-9ba2-4b4d-bbe4-e309405d6fb8
📒 Files selected for processing (7)
src/becwright/cli.pytests/test_cli_and_git.pytests/test_engine_integration.pytests/test_init.pytests/test_mcp.pytests/test_report_and_json.pytests/test_staged_content.py
2 participants
Description
This PR improves the readability and user experience of the
becwrightCLI tool. It organizes the global help output into logical subcommand groups (matching modern patterns seen in CLI tools like Docker or Git) and intercepts empty arguments to guide the user immediately.Key Changes
1. Command Categorization & Custom Help Layout
src/becwright/cli.py:_print_custom_help()method that groups all 15+ subcommands into 4 logical sections:check,init,why,validateinstall,uninstall,check-msgsearch,add,import,exportdoctor,list,mcp,demo,runmain()calls when no arguments are provided or when the top-level--help/-hflags are passed, rendering the categorized layout.becwright check --help) intact via standard argparse parsing.2. Unit Testing
tests/test_cli_and_git.py: Addedtest_cli_custom_helpto verify that executing the CLI with no arguments or--helpcorrectly prints the new grouped category headers and matches the expected layout.Verification Results
Successfully verified locally with the updated test suite:
Summary by CodeRabbit
New Features
-h/--help.Bug Fixes