Repository navigation
fix(ruff): match format routing to first argument - #3933
Tiancheng-Xu wants to merge 1 commit into
Conversation
|
|
There was a problem hiding this comment.
🟢 Approval recommended
The change directly addresses the reported routing bug with a minimal, well-scoped fix and includes a regression test covering the problematic argv shape.
Pull request overview
This PR fixes Ruff subcommand routing in RTK’s Python Ruff wrapper so that the format filter is only selected when format is the first CLI argument, preventing accidental selection when the token appears later in argv (e.g., help format). It aligns the routing decision with the intended invocation shape and adds a targeted regression test.
Changes:
- Replace
args.iter().any(|a| a == "format")with a first-argument check viais_format_invocation(args). - Introduce
is_format_invocation()helper to make the routing rule explicit and testable. - Add a focused unit test ensuring
format --checkmatches whilehelp formatdoes not.
File summaries
| File | Description |
|---|---|
| src/cmds/python/ruff_cmd.rs | Tightens format routing to first-arg-only and adds a regression test for the routing decision. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: tiancheng-Xu <[email protected]>
1f5dd82 to
f5655e9
Compare
Protocol-zero-0
left a comment
Protocol-zero-0
left a comment
There was a problem hiding this comment.
The help format fix works, but matching only argv[0] introduces a regression for valid Ruff global options before the subcommand.
I compared parent f7495466 and head f5655e9a with real Ruff 0.17.0 on Linux. For a directory containing 80 Python files with x= [1,2,3]:
ruff --isolated format --check bulk
rtk ruff --isolated format --check bulk
rtk ruff --config pyproject.toml format --check bulkRaw Ruff and the parent retain all diagnostics and the final 80 files would be reformatted summary (10,190 stdout bytes). This head emits 2,001 bytes, stopping partway through a diagnostic and losing the summary. All exit codes remain 1. The equivalent ordinary format --check bulk invocation retains the format route. The same loss occurs with --output-format=concise.
Could the routing identify the actual subcommand after supported global options, including --isolated and the value of --config, while still keeping help format out of the format filter? A regression covering a leading global option and output longer than the passthrough cap would catch the loss; the current helper-only test does not.
Validation: all 11 Ruff unit tests passed. I exercised 11 real CLI scenarios against raw/base/head, plus three concise-output comparisons: help, format/check successes and failures, version, and leading globals. help format now correctly follows the intended cap (4,884 to 2,001 bytes). Ruff 0.17.0's ordinary format output already falls back to raw output in both versions; I am not attributing that existing parser gap to this change. No source edits or full-suite/cross-platform validation were performed for this review.
|
Thanks for the detailed report and reproduction. I’m updating the routing to account for supported global options before the subcommand and adding regression coverage for the truncated-output case, while preserving the |
|
Thanks for the update. I will verify the global-option-before-subcommand path, truncated-output regression, and preserved |
|
Thanks for this fix. #3968, merged on 2026-10-31, rewrote Ruff argument routing so the format filter applies only when format is the effective subcommand, and it pins |
Summary
formatis the first argument.format --checkandhelp format.中文说明
修复 Ruff 参数路由：只有首个参数是
format时才进入 format 过滤；后续参数中的format不再意外绕过 passthrough 截断。Testing
cargo fmt --allcargo clippy --all-targetscargo test --all(3258 unit tests passed, 8 ignored; all integration tests passed)ruff_format_routing_requires_first_argumentFixes #3928