Skip to content
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

[feat] Add setting for enabling/disabling Preview rules #54

Merged
merged 2 commits into from
Nov 25, 2023

Conversation

magnuslarsen
Copy link
Contributor

Added a setting for enabling/disabling ruff Preview rules

This enables it for both lint and format actions, though format is not something this plugin supports for now. It doesn not seem that ruff supports different --preview flags for linting / formatting as of now.

Should likely be revisited if #53 is being pursued :-)

@jhossbach
Copy link
Member

What is the advantage of having preview as an option for the LSP? If you are working in a project, shouldn't this option be dictated by the project settings in the pyproject.toml rather than the LSP?

@magnuslarsen
Copy link
Contributor Author

That is true if you work in a project :-) Unfortunately not all of my tasks are in a repo where a configuration file would be present

I also like to be able to toggle the preview mode and check out new rules / autofixes, which is a lot easier to do directly in the LSP server (toggle in vim, then restarting the LSP), than it is the edit a project configuration file, restarting the LSP server, and then having to deal with git at some point

I guess it is also the same category of extendSelect and such, which overrides per-project behavior (which I like, because I can choose when ruff should do more of my work :-))

@jhossbach
Copy link
Member

Sorry for not getting back to this for a while. While it is definitely nice to expose the option in the config, I feel like overwriting the project setting on preview would lead to gotchas. If e.g. the pyproject.toml contains extendSelect = ['E'], running the Fix all code action would lead to a call like ruff --preview --extend-select='E' ..., which would mean that rules starting with E that are still in preview mode would also be fixed.

Can you test if this is the case with your PR?

@jhossbach jhossbach added this to the v2.0.0 milestone Nov 16, 2023
@magnuslarsen
Copy link
Contributor Author

magnuslarsen commented Nov 18, 2023

Here is my findings, using a temp directory with only a pyproject file, and a test script:

mlar@cettisanger /tmp/ruff-preview-test> ruff --version
ruff 0.1.6

mlar@cettisanger /tmp/ruff-preview-test> cat pyproject.toml
[tool.ruff]
extend-select = ["E"]

mlar@cettisanger /tmp/ruff-preview-test> cat test.py
# E201 (preview, autofix)
print( "E201")
# E703 (not preview, autofix)
print("E703");

mlar@cettisanger /tmp/ruff-preview-test> ruff check test.py
test.py:4:14: E703 [*] Statement ends with an unnecessary semicolon
Found 1 error.
[*] 1 fixable with the `--fix` option.

mlar@cettisanger /tmp/ruff-preview-test [1]> ruff check test.py --preview
test.py:2:7: E201 [*] Whitespace after '('
test.py:4:14: E703 [*] Statement ends with an unnecessary semicolon
Found 2 errors.
[*] 2 fixable with the `--fix` option.

So yes, setting --preview will also auto-fix preview rules

Though I find that somewhat weird; it would make sense that preview rules (and fixes) would inherently be unsafe (but that is a question for upstream Ruff. EDIT: astral-sh/ruff#8754)

Still, to argument my case, if you explicitly enable preview in the LSP-server, you should yourself know what you are getting into. But I will gladly add a note about the caveat to the README

@jhossbach
Copy link
Member

LGTM. Merging.

@jhossbach jhossbach merged commit 5d7338d into python-lsp:main Nov 25, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants