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

Add configuration via lsp guide to README #78

Merged

Conversation

LaurenceWarne
Copy link

Hi, this PR is in response to #32, I've added a description for how the plugin can be configured (to the best of my understanding 🙂) using lsp, and given an example similar to that given in https://github.com/python-lsp/python-lsp-ruff#configuration.

Thanks!

@Richardk2n
Copy link
Member

This is editor dependent, is it not?
I am not sure if we are responsible to provide documentation for that. In my opinion, the editor should do this.
If we do, we would need to cover a few relevant editors.
The preferred style (sadly badly documented here) is using pyproject.toml.
Maybe I should expand on that a little more, but I am not too keen on encouraging the use of esoteric command line editors.

@LaurenceWarne
Copy link
Author

LaurenceWarne commented Apr 24, 2024

I am not sure if we are responsible to provide documentation for that.

Yes it will be editor depedendent, I guess I was mainly trying to address that AFAICS the README config section doesn't mention that configuration from LSP is supported at all.

Maybe I should expand on that a little more, but I am not too keen on encouraging the use of esoteric command line editors.

I'm happy to drop the example. How about doing something similar to https://github.com/python-lsp/python-lsp-server/blob/develop/CONFIGURATION.md by sticking the configuration options in a table with the relevant configuration keys? e.g.:

pyproject.toml key LSP configuration key Type Description Default
live_mode pylsp.plugins.pylsp_mypy boolean This writes to a tempfile every time a check is done. Turning off live_mode means you must save your changes for mypy diagnostics to update correctly. True

etc

@nramirezuy
Copy link

@LaurenceWarne do you know how to handle overrides?

 pylsp_mypy = {
    enabled = true,
    overrides = {"--python-executable", "python"},
 }

This ends up with this:
image

But if I configure it through pyproject.toml works as expected. 🤔

@nramirezuy
Copy link

Ok, just figured out on my own:

 pylsp_mypy = {
    enabled = true,
    overrides = {"--python-executable", "python", true},
 }

You have to add the true at the end, but some reason this true value isn't required when set from pyproject.toml.

@Richardk2n is this expected behavior?

@Richardk2n
Copy link
Member

@LaurenceWarne This seems sensible to me.

@nramirezuy The true is the placeholder for the default command line arguments, that we pass, so depending on what you insert into the overrides it is required.
What exactly do you mean by set from pyprojectl.toml?

@LaurenceWarne LaurenceWarne force-pushed the add-lsp-config-to-documentation branch from c30b69d to c72e536 Compare August 13, 2024 18:09
@LaurenceWarne
Copy link
Author

This seems sensible to me.

👍 I've made edits to the PR

Copy link
Member

@Richardk2n Richardk2n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make the short summary in every description bold.

README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
@LaurenceWarne LaurenceWarne force-pushed the add-lsp-config-to-documentation branch from c72e536 to 03b30dc Compare August 17, 2024 15:53
@LaurenceWarne
Copy link
Author

I think I've addressed everything 👍

README.rst Outdated Show resolved Hide resolved
Alter README configuration documentation to be a table similar to
https://github.com/python-lsp/python-lsp-server/blob/develop/CONFIGURATION.md
with the addition that it includes the names of LSP configuration keys.
@LaurenceWarne LaurenceWarne force-pushed the add-lsp-config-to-documentation branch from 03b30dc to 7b64c74 Compare August 17, 2024 16:41
@Richardk2n Richardk2n merged commit dee6a6c into python-lsp:master Aug 17, 2024
11 checks passed
@LaurenceWarne LaurenceWarne deleted the add-lsp-config-to-documentation branch August 17, 2024 17:03
@nramirezuy
Copy link

nramirezuy commented Nov 15, 2024

@nramirezuy The true is the placeholder for the default command line arguments, that we pass, so depending on what you insert into the overrides it is required. What exactly do you mean by set from pyprojectl.toml?

@Richardk2n sorry I never came back to you, I meant setting the executable from pyproject.toml with this option.

So my configuration ended up like this, which I think should be the default, since it works with pyenv and venv.

I'm not looking to start a discussion on a closed ticket, but I'm happy to be @ in a new ticket about the subject.

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.

3 participants