-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
rework CLI (3/3): Use clap subcommands #2190
Conversation
2481dcb
to
3a6032a
Compare
d001e51
to
0141dee
Compare
This commit greatly simplifies the implementation of the CLI, as well as the user expierence (since --help no longer lists all options even though many of them are in fact incompatible). To preserve backwards-compatability as much as possible aliases have been added for the new subcommands, so for example the following two commands are equivalent: ruff explain E402 --format json ruff --explain E402 --format json However for this to work the legacy-format double-dash command has to come first, i.e. the following no longer works: ruff --format json --explain E402 Since ruff previously had an implicitly default subcommand, this is preserved for backwards compatibility, i.e. the following two commands are equivalent: ruff . ruff check . Previously ruff didn't complain about several argument combinations that should have never been allowed, e.g: ruff --explain RUF001 --line-length 33 previously worked but now rightfully fails since the explain command doesn't support a `--line-length` option.
0141dee
to
7418215
Compare
For what it's worth I think now that the PR is keeping As I tried to explain in
I think all of these are very niche cases. I think for the majority of users this change shouldn't break anything but rather have them benefit from a more intuitive CLI (where the synopsis of subcommands is clear and supplying an option that doesn't do anything results in an error rather than being silently ignored). |
I'm hoping to merge this today, but it might happen over the weekend. |
(I want to do some testing myself since it's a significant change.) |
Just FYI I have some followup changes coming up that we probably want to merge before making a new release. In particular I think we should rename the In particular my idea is:
(opened #2288) |
Followup PR for #2155.
This greatly simplifies the implementation, as well as improves the UX of the CLI (by no longer listing all options at once even though many of them are in fact incompatible).
To preserve backwards compatibility as much as possible aliases have been added (so e.g.
ruff --explain <rule>
still works) and the implicit default subcommand is also preserved (so e.g.ruff .
is equivalent toruff check .
).New
ruff --help
output:New
ruff help check
outputResolves #455.