Auto-disabling flake8 doesn't seem possible
projectgus opened this issue · 3 comments
If the client configuration contains pylsp.plugins.flake8.enabled = true (which is necessary for flake8 to enable in pylsp), then this seems to always override the documented python-lsp-ruff behaviour of auto-disabling flake8.
I think this applies for all the linters that are auto-disabled by python-lsp-ruff, although in the other cases they are enabled by default so leaving enabled = true out of the client config will solve the problem.
What I observe with my LSP client (eglot) is the following sequence of events:
- LSP client sends
initialize - pylsp loads all plugins, including
python-lsp-ruffandflake8_lint. - pylsp runs the ruff plugin
pylsp_settingshook. It will merge'flake8': {'enabled': False}into the plugins settings (along with the same for other other auto-disabled plugins.) - pylsp will also run the flake8_lint plugin
pylsp_settingshook, which also sets'flake8': {'enabled': False}(not sure if these happen in a guaranteed order, but the outcome is the same - flake8 is definitely disabled!) - At this point,
pylsp -vwill log aDisabled plugins:line that includespylsp.plugins.flake8_lint. - LSP client sends
'workspace/didChangeConfiguration'with'params': {'settings': {'pylsp': {'plugins': [...] 'flake8': {'enabled': True} [...] - At this point,
pylsp -vwill log aDisabled plugins:line that no longer includespylsp.plugins.flake8_lint. - LSP sends linter results for both ruff and flake8.
This may be an eglot bug(?) but from what I've seen it seems likely most clients will send these events in this sequence.
Workarounds
- Making sure there's no
flake8binary on the PATH of the venv works, as then the plugin enables but never produces any results. - Writing per-project pylsp configs obviously will work, although I jump around a lot of Python codebases so it's undesirable for me (and I'm guessing python-lsp-ruff authors have the same preference, given the auto-disabling logic.)
- Depending on LSP client implementations, it may be possible to hook the client and have it do some checks before it sends
didChangeConfiguration. However this seems counter to the overall architecture of LSP, where the language-specific implementation is delegated to the server.
Idea for fix
The best idea I've come up with for a fix would actually be a python-lsp feature, not a python-lsp-ruff change. Although hopefully there's a simpler way to solve this in python-lsp-ruff, I'm pretty unfamiliar with LSP.
The idea is, pylsp could support a setting like pylsp.plugins.NAME.enabled = 'project' setting that will call a hook in the plugin that checks for the configuration files or keys used by that plugin's linter in the project, and only enables the plugin if some configuration is found. Most of this logic already exists in the plugins.
If this does seem like a good feature then I'm happy to raise it on the main pylsp repo, and potentially raise some PRs.
This behaviour is intended, since any configuration you pass to the LSP server should not be superseded by the configuration of any of the plugins.
Let me rewrite your problem to make sure I understand:
You have multiple projects that you switch frequently between, each using a different linting tool (e.g. flake8 and ruff), and you don't want to disable/enable the tools every time you switch projects?
If so, this is indeed a feature request for python-lsp-server rather than python-lsp-ruff.
In the case of ruff and flake8, as an alternative you could leave flake8 disabled and use ruff altogether since ruff aims to be a drop-in replacement and should provide the same linting messages (altough I am not sure if ruff uses the flake8 config)
Feel free to open a feature request here if this doesn't solve your issue.
Hey! Thanks for getting back to me so quickly.
Let me rewrite your problem to make sure I understand
Yes, you've got it 100%.
This behaviour is intended, since any configuration you pass to the LSP server should not be superseded by the configuration of any of the plugins.
Makes total sense, thanks.
In that case, if I submit a PR that removes flake8 from the list of auto-disabled plugins then would you consider merging it? It might save someone else from going doing the rabbit hole of "if this is documented then there must be a way for it to work!". I can also add a sentence to clarify "Plugins that are explicitly enabled in the pylsp configuration can't be auto-disabled."
could leave flake8 disabled and use ruff altogether since ruff aims to be a drop-in replacement and should provide the same linting messages (altough I am not sure if ruff uses the flake8 config)
Ruff doesn't load flake8's config files. However, it seems like more and more projects are standardising on Ruff, for good reasons. So the problem might solve itself that way. :)
I know the pain of going through the LSP logs, so I think it's a nice idea to remove flake8 (and all other plugins that are auto-disabled by default) from this list. Feel free to submit the PR and I'll merge it when ready.
Reopening to close this when merging