Skip to content

Auto-disabling flake8 doesn't seem possible #46

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

Closed
projectgus opened this issue Oct 10, 2023 · 3 comments · Fixed by #47
Closed

Auto-disabling flake8 doesn't seem possible #46

projectgus opened this issue Oct 10, 2023 · 3 comments · Fixed by #47

Comments

@projectgus
Copy link
Contributor

projectgus commented Oct 10, 2023

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:

  1. LSP client sends initialize
  2. pylsp loads all plugins, including python-lsp-ruff and flake8_lint.
  3. pylsp runs the ruff plugin pylsp_settings hook. It will merge 'flake8': {'enabled': False} into the plugins settings (along with the same for other other auto-disabled plugins.)
  4. pylsp will also run the flake8_lint plugin pylsp_settings hook, 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!)
  5. At this point, pylsp -v will log a Disabled plugins: line that includes pylsp.plugins.flake8_lint.
  6. LSP client sends 'workspace/didChangeConfiguration' with 'params': {'settings': {'pylsp': {'plugins': [...] 'flake8': {'enabled': True} [...]
  7. At this point, pylsp -v will log a Disabled plugins: line that no longer includes pylsp.plugins.flake8_lint.
  8. 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

  1. Making sure there's no flake8 binary on the PATH of the venv works, as then the plugin enables but never produces any results.
  2. 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.)
  3. 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.

@jhossbach
Copy link
Member

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.

@projectgus
Copy link
Contributor Author

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. :)

@jhossbach
Copy link
Member

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

@jhossbach jhossbach reopened this Oct 12, 2023
projectgus added a commit to projectgus/python-lsp-ruff that referenced this issue Oct 13, 2023
flake8 plugin is disabled by default. If it's been enabled in the pylsp
configuration then it's not possible for another plugin to disable it.

The other four plugins (pycodestyle, pyflakes, mccabe, pyls_isort) are all
enabled by default so the ruff plugin can disable them, provided they're not
explicitly enabled in the configuration.

Closes python-lsp#46
jhossbach pushed a commit that referenced this issue Oct 18, 2023
flake8 plugin is disabled by default. If it's been enabled in the pylsp
configuration then it's not possible for another plugin to disable it.

The other four plugins (pycodestyle, pyflakes, mccabe, pyls_isort) are all
enabled by default so the ruff plugin can disable them, provided they're not
explicitly enabled in the configuration.

Closes #46
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 a pull request may close this issue.

2 participants