-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Support built-in LSP server #11926
Support built-in LSP server #11926
Conversation
0027d89
to
53fab84
Compare
Impressive work! I'll need a bit of time to review this more carefully, but at a glance it looks pretty good!
I didn't know you were an Emacs user as well! M-x forever! 🚀 |
53fab84
to
6d36dc0
Compare
@@ -0,0 +1,47 @@ | |||
= LSP (Language Server Protocol) | |||
|
|||
NOTE: The built-in language server was introduced in RuboCop 1.53. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to mention that we'll consider this experimental feature for a while.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! I updated it.
|
||
== Run as a Language Server | ||
|
||
Run `rubocop --lsp` command from LSP client. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if a name more consistent with the existing commands (e..g --start-lsp-server
) is not idea as well. We can keep --lsp
as a shorter alias.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm concerned that LSP client is the trigger that reconnects or shuts down the LSP server. For Elgot, use M-x elgot-reconnection
or M-x elgot-shutdown
to reconnect or shutdown. In other words, the rubocop
command that the user recognizes in the LSP setting may be enough with --lsp
option.
How about just providing --lsp
option to start with? So --start-lsp-server
will be added later when really needed for user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. If that's the only flag the users will ever need we can stick with it.
21adcf1
to
a4abf41
Compare
Preparing to resolve rubocop#11686 This commit imports built-in LSP server of https://github.com/standardrb/standard into RuboCop core. The `standard` gem's original license (MIT) is written at the beginning of the imported files.
4e1423d
to
d0e2fd9
Compare
|
||
https://microsoft.github.io/language-server-protocol/[The Language Server Protocol] is the modern standard for providing cross-editor support for various programming languages. | ||
|
||
This feature enables extremely fast interactions through the LSP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in this section we should also mention which LSP features does our implementation support, so it's clearer for the end users. We might also mention the advantages of preferring LSP for editor integration over --rubocop-server
and just shelling out a RuboCop process.
This might encourage more people to try out the LSP support.
d0e2fd9
to
933df72
Compare
Resolves rubocop#11686 This commit supports built-in language server. The integrated language server feature is described in the docs/modules/ROOT/pages/usage/lsp.adoc. ## LSP option ```console % bundle exec rubocop --help (snip) LSP Option: --lsp Start a language server listening on STDIN. ``` The language server `--lsp` is disabled by default because it is not yet stable as an integrated feature. Run `rubocop --lsp` command from LSP client. When the language server is started, the command displays the language server's PID: ```console $ ps aux | grep rubocop user 17414 0.0 0.2 5557716 144376 ?? Ss 4:48PM 0:02.13 /Users/user/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/bin/rubocop --lsp ``` The language server supports `textDocument/formatting` method and is autocorrectable. The autocorrection is safe. ## Design note Same concept as standardrb/standard#475. See there for details. ## Usage Example (Eglot) I'm running the LSP on Emacs with Eglot. It behaves roughly as expected. https://github.com/joaotavora/eglot Add the following to your Emacs configuration file (e.g. `~/.emacs.d/init.el`): ```lisp (require 'eglot) (add-to-list 'eglot-server-programs '(ruby-mode . ("bundle" "exec" "rubocop" "--lsp"))) (add-hook 'ruby-mode-hook 'eglot-ensure) ``` Below is an example of additional setting for autocorrecting on save: ```lisp (add-hook 'ruby-mode-hook (lambda () (add-hook 'before-save-hook 'eglot-format-buffer nil 'local))) ``` In theory it should be available for other LSP clients as well.
933df72
to
45850ed
Compare
The xref:usage/lsp.adoc[Server Mode] is primarily used to speed up RuboCop runs in the terminal. | ||
Therefore, if you want real-time feedback from RuboCop in your editor or IDE, | ||
opting to use this language server instead of the server mode will not only provide a fast and efficient solution, | ||
but also offer a straightforward setup for integration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added this text to the doc in response to the review. What do you think?
Overall the code looks good to me, plus I guess it's already battle tested to some extent by Standard.rb. |
@bbatsov I'm developing vscode-rubocop: It is based on vscode-standard-ruby. (Thank you @searls and @kddnewton. Mentioned in LICENSE.) After RuboCop 1.53.0 is released, I plan to transfer it to |
This PR has two parts committed. That is, the import LSP server from Standard and support RuboCop built-in LSP. Please keep these commits.
Part 1: Import LSP server from Standard
Preparing to resolve #11686
This commit imports built-in LSP server of https://github.com/standardrb/standard into RuboCop core.
The
standard
gem's original license (MIT) is written at the beginning of the imported files.@searls @will
Part 2: Support built-in LSP
Resolves #11686
This commit supports built-in language server.
The integrated language server feature is described in the docs/modules/ROOT/pages/usage/lsp.adoc.
LSP option
The language server
--lsp
is disabled by default because it is not yetstable as an integrated feature.
Run
rubocop --lsp
command from LSP client.When the language server is started, the command displays the language server's PID:
The language server supports
textDocument/formatting
method and is autocorrectable. The autocorrection is safe.Design note
Same concept as standardrb/standard#475. See there for details.
Usage Example (Eglot)
I'm running the LSP on Emacs with Eglot. It behaves roughly as expected.
https://github.com/joaotavora/eglot
Add the following to your Emacs configuration file (e.g.
~/.emacs.d/init.el
):Below is an example of additional setting for autocorrecting on save:
In theory it should be available for other LSP clients as well.
NOTE: I've been using this language server locally with Emacs (Eglot) for a while and it's performing well.
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.md
if the new code introduces user-observable changes. See changelog entry format for details.