-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Unified command for linting and formatting #8232
Comments
Roughly, one proposed interface would be the generalization of On Further configuration of each tool would be done from the respective I believe, eventually, it would be ideal for This approach would require significant consideration of our existing check diagnostic and violation design, as it is very lint focused at the moment. Consideration should be made for the possibility of future tools that would raise diagnostics, such as a type checker. It's an open question whether a dedicated |
If you ask me about how to join into one command, my answer (and I think most of users) would be put formatter as a lint rule. @zanieb I'm pretty sure you also considered this, but I didn't find why you ignored this approach. Can you show what are blockers for this design? solving those blockers maybe less expensive for team (and users)? For now, I think |
We discussed this internally. Building the formatter as a linter raised a couple of design questions:
Overall, formatting just felt different enough. |
Isn't that the job of the linting mechanism that works based on whether the formatting check would find anything.. How does this work for I001 (isort)?
I feel it makes sense to keep formatting also as a separate command for these use cases
Make it two different rules? You can even (if not configured explicit) enable the preview rule in preview and disable the normal formatting rule...
|
Thanks, I didn't realize these issues, here are my proposed solutions for them:
A. Use
For formatting, editors can keep current behavior, also for linting and autofixes I don't think
Add |
We are also of the opinion that import sorting is not well suited to be a lint rule. We are still trying to figure out the best way to resolve that.
We already have this as I was originally a strong proponent of using a lint code for the formatter. Here are some notes:
The problem with this approach is that there are many special cases where the formatter does not behave like a lint rule. While it's feasible to just push the formatter into the linter, we want to do better. |
I have no strong opinion, just fyi. I always used https://pypi.org/project/flake8-isort/ and https://pypi.org/project/flake8-black/. Basically the lining check for those plugins is "would isort or black make any changes?". To me that was good enough, never considered adding suppression comment (failure means, just run the formatter). |
FWIW, the way I use ruff is wrapped in 2 commands: The
I use this in CI for example. The
I use this when I want to cleanup my code, e.g. before commit. I like that I say this because a top-level |
I also have a long proposal adding a I think regardless there needs to be a way for |
@zanieb I believe (Maybe there is a better term then both |
@zanieb Did you discuss whether this unified command will be implemented in the near future? I am asking because I am considering merging python-lsp/python-lsp-ruff#57 as an intermediary solution. |
@jhossbach I'd recommend moving forward with an intermediary solution as there's still quite a bit of work to be done on this issue. |
FWIW Hatch did it: https://hatch.pypa.io/latest/cli/reference/#hatch-fmt |
This would be a good QoL fix for us. Is there an open PR we could jump on this fix this, or is there stuff to be hashed out in the discussion still? |
That's true for any subset of linters, not just format VS lint. When you're doing such a large refactor, picking the linters you want to run with a separate config file/CLI args is a common way out as an escape hatch.
I mean |
Because Ruff isn't the only formatter/style checker, and forcing the formatter with the linter only hurts adoption. Even for someone using both, tooling integration may demand to keep those somewhat separate. |
That just indicates that the formatter should be opt-in exactly like every other rule in ruff right now. Not that it needs to be a completely separate subcommand. |
Another datapoint here: I want to configure ruff to run in the gitlab CI. Regardless of the discussion related to the structure of commands, it would help me in this if |
In .NET we have a single command for the linter, type checker and formatter with |
Please design in such a way to accommodate a future feature |
It should be possible to check if files would be formatted and if there are any lint violations in a single Ruff invocation.
It should be possible to apply linter fixes and formatting changes in a single Ruff invocation.
The design of this interface is still being discussed internally. This issue will be updated as development continues.
Previous discussion at #7310 (reply in thread), #7310 (comment), and #7232
The text was updated successfully, but these errors were encountered: