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

Expand note to use Ruff with other language server in Kate #12806

Merged
merged 5 commits into from
Aug 20, 2024

Conversation

tfardet
Copy link
Contributor

@tfardet tfardet commented Aug 11, 2024

Summary

Provide instructions to use Ruff together with other servers in the Kate editor.
Because Kate does not support running multiple servers for the same language, one needs to use the python-lsp-server (pylsp) tool.

Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

Thank you for working on this!

I'm a bit hesitant on including this section with all the details as provided here. We had a section for python-lsp-ruff before the stable release of the Ruff language server (https://github.com/astral-sh/ruff/blob/7a7c601d5ed294a3c868b5e83f757105e0a189b8/docs/integrations.md#language-server-protocol-unofficial) but we removed it in favor of promoting the native language server. Another important difference with python-lsp-ruff is that it uses the ruff executable which means that the server settings won't be considered if provided.

That said, what we could do is to expand on the important note section stating that if someone would like to use Ruff along with another language server, the python-lsp can be used as a language server along with the python-lsp-ruff plugin. We should also state that the Ruff plugin used here would be through the executable. You might want to paraphrase this.

What do you think about this?

@dhruvmanila dhruvmanila added the documentation Improvements or additions to documentation label Aug 12, 2024
@tfardet
Copy link
Contributor Author

tfardet commented Aug 12, 2024

Hi @dhruvmanila could you elaborate on the settings part?
As far as I can tell, you can provide a toml file via the pylsp-ruff "config" option. Do you mean that the ruff executable and the language server have different options, or did I misunderstand?

Otherwise, just let me tell you why I did it that way:

  • I don't think many people are knowledgeable about tools like ruff, jedi, or pylsp, so providing a config file they can simply copy-paste and have things work (well, almost, since they need to specify the path) is desirable
  • Quite frankly, I don't think anyone would want to use the current configuration instructions for Kate since they make it lose basic goto functionalities
  • It also took me quite a while to figure this out, so I thought I would just save others the trouble by providing everything 😉
  • using "details", it doesn't really take much space and I would not expect these instructions to be much of a maintenance burden

That being said, I understand if you do not want to risk that burden, or having people asking about how to use pylsp-ruff in your issue tracker.
In that case let me know if adding a disclaimer at the top of the details to tell people they should ask pylsp-ruff people if anything doesn't work would be enough, otherwise I'll just remove the details and paraphrase what you said, as requested.

@dhruvmanila
Copy link
Member

As far as I can tell, you can provide a toml file via the pylsp-ruff "config" option. Do you mean that the ruff executable and the language server have different options, or did I misunderstand?

I mean the server settings as mentioned here: https://docs.astral.sh/ruff/editors/settings. Some of them controls the behavior of the server itself (ruff.lint.enable, etc.) while others overrides the config values (ruff.lint.select, etc.).

I think your reasoning makes sense but I'd rather prefer to re-use existing resources from other documentations. There have been some confusion in the past with how the server settings are suppose to work like #12778 and #12514 where the users were confused that the server settings weren't being considered when running the linter / formatter.

I think having a general guide on how to setup the python-lsp-server in Kate would be useful to more users while having it here would only benefit the Ruff users.
I think a general user of python-lsp-server would find it more useful to refer to a guide on setting it up in Kate rather than a Ruff user. It's not about maintenance burden but thinking on how much benefit does this provide to Ruff users. I hope this reasoning makes sense.

That said, I think it would be more useful to expand the note by informing the users that they could use python-lsp-server along with python-lsp-ruff to use Ruff with an existing language server along with relevant references like python-lsp-ruff configuration options.

@tfardet
Copy link
Contributor Author

tfardet commented Aug 14, 2024

I mean the server settings as mentioned here: https://docs.astral.sh/ruff/editors/settings. Some of them controls the behavior of the server itself (ruff.lint.enable, etc.) while others overrides the config values (ruff.lint.select, etc.).

I think your reasoning makes sense but I'd rather prefer to re-use existing resources from other documentations. There have been some confusion in the past with how the server settings are suppose to work like #12778 and #12514 where the users were confused that the server settings weren't being considered when running the linter / formatter.

OK, I think this should probably be mentioned in the note, then.
Since you have much more experience with this confusion issue, would you have an idea about how to phrase that?

I think having a general guide on how to setup the python-lsp-server in Kate would be useful to more users while having it here would only benefit the Ruff users. I think a general user of python-lsp-server would find it more useful to refer to a guide on setting it up in Kate rather than a Ruff user. It's not about maintenance burden but thinking on how much benefit does this provide to Ruff users. I hope this reasoning makes sense.

As I mentioned, I don't think people know much about these tools and would expect them to have heard (like me) that ruff is super fast and so they would want to use it and would just arrive on the ruff docs without prior knowledge.

However, I completely understand your reasoning and definitely agree that it would be better to have the detailed doc where it belongs (somewhere in the pylsp ecosystem) and just link to it in the ruff documentation.

That said, I think it would be more useful to expand the note by informing the users that they could use python-lsp-server along with python-lsp-ruff to use Ruff with an existing language server along with relevant references like python-lsp-ruff configuration options.

I'll propose something along those lines, then, and maybe I'll make another PR once I have added the detailed configuration somewhere in the pylsp documentation.

@dhruvmanila dhruvmanila requested review from dhruvmanila and removed request for dhruvmanila August 16, 2024 05:46
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

I expanded the "important" note for the Kate setup guide with a workaround for using Ruff with another language server. Feel free to provide any thoughts on that. I'll merge this for now but happy to take any follow-ups if you have any in mind. Thanks for the suggestion and opening this PR.

@dhruvmanila dhruvmanila changed the title Instructions to use Ruff with other servers in Kate Expand note to use Ruff with other language server in Kate Aug 20, 2024
@dhruvmanila dhruvmanila enabled auto-merge (squash) August 20, 2024 06:16
@dhruvmanila dhruvmanila merged commit fc811f5 into astral-sh:main Aug 20, 2024
19 checks passed
@tfardet
Copy link
Contributor Author

tfardet commented Aug 20, 2024

OK, that seems reasonable! I'll try to propose something to python-lsp people, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants