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

Workspace configuration explained? #363

Closed
andreyorst opened this issue Nov 29, 2019 · 15 comments
Closed

Workspace configuration explained? #363

andreyorst opened this issue Nov 29, 2019 · 15 comments
Labels

Comments

@andreyorst
Copy link

I've read #59 and #196 but still can't figure out how to set up clippy integration for RLS.

RLS documentation says:

clippy_preference (String, defaults to "opt-in") controls eagerness of clippy diagnostics when available. Valid values are (case-insensitive):

  • "off" Disable clippy lints.
  • "on" Display the same diagnostics as command-line clippy invoked with no arguments (clippy::all unless overridden).
  • "opt-in" Only display the lints explicitly enabled in the code. Start by adding #![warn(clippy::all)] to the root of each crate you want linted.

I've tested and eglot works with clippy just fine when #![warn(clippy::all)] is at the root of the crate. But I want to do that via file-local variable.

I've defined this function:

(defun aorst/setup-rls ()
  "Configure RLS via `eglot-workspace-configuration' variable."
  (setq eglot-workspace-configuration '((rls . (:clippy_preference "on")))))

But I can't figure out what should I put inside the '() list. I've tried different configurations, like:

  • (rls.clippy_preference . "on")
  • (rls . (clippy_preference "on"))

both with and without : in various places but nothing worked. (I've also tried looking through github and google, and seems that noone have configured clippy_preference for rls in eglot yet.)

There should be some logic behind this format and how it is being converted to JSON, but it isn't clear to me from current description:

Alist of (SECTION . VALUE) entries configuring the LSP server.
SECTION should be a keyword or a string, value can be anything
that can be converted to JSON.

I expect section to be somehow related to the RLS, and the field we need to set simultaneously. In #59 it is mentioned that Pyls is configured this way:

'((pyls . ((configurationSources . ["flake8"]))))

I've tried translating it by the same logic to '((rls . ((clippy_preference . ["on"])))) but it did not work.

What is the proper way to do this?

@xuchunyang
Copy link
Contributor

SECTION should be a keyword or a string

So :rls and "rls" are qualified.

value can be anything that can be converted to JSON

Lots of Emacs Lisp values can be converted to JSON, but it seems plist is preferred. So, for example, this should be good (json-encode will convert :clippy_preference into "clippy_preference" for us):

((:rls :clippy_preference "on"))
;; this is the same to the Emacs reader, but verbose
((:rls . (:clippy_preference "on")))
(defun aorst/setup-rls ()
  "Configure RLS via `eglot-workspace-configuration' variable."
  (setq eglot-workspace-configuration '((rls . (:clippy_preference "on")))))

I guess you need to ensure this function runs before eglot starts. You can use M-x eglot-signal-didChangeConfiguration to ask the lsp server update the configuration explicitly. The eglot's README suggest you use the Emacs directory variable instead.

https://github.com/joaotavora/eglot#per-project-server-configuration

though personally I simply change the global/default value since I don't use eglot for more than one type of lsp server.

@joaotavora
Copy link
Owner

What is the proper way to do this?

Try and miss, unfortunately. The format I chose for eglot-workspace-configuration was a bit unfortunate, because I chose an alist for the variable, but the VALUE (the thing that should be serializable to json), should really really be a plist (as @xuchunyang points out, it can also be an alist, but that's only in Emacs 26.x).

To make matters slightly more complicated, the format of .dir-locals is also an alist...

So if you use .dir-locals (you don't have to, but if you want per-project stuff it's the easiest way to go), then you have an alist of alists of VALUEs (which should be plists if you're using objects).

That said, the form of the JSON VALUE that the server is expected for value is completely up to it, so it might take an array, an object, a simple string... Hit and miss.

I guess you need to ensure this function runs before eglot starts.

Yes: @xuchunyang's advice is 100% correct here.

@joaotavora
Copy link
Owner

@xuchunyang you've mentioned frequently that Eglot needs better doc. You're right, and this is a point where it does. If you want to improve the README.md, I'm all for it.

@andreyorst
Copy link
Author

andreyorst commented Nov 30, 2019 via email

@andreyorst
Copy link
Author

For example:

M-: (setq eglot-workspace-configuration '((:rls . (:clippy_preference "on")))) RET
((:rls :clippy_preference "on"))
M-x eglot-signal-didChangeConfiguration RET
[eglot] Server reports (type=2): Unknown RLS configuration: `rls` 

So these configurations result in errors (from *Messages* buffer):

((:clippy_preference . "on"))
[eglot] Server reports (type=2): Unknown RLS configuration: `clippy_preference`
((rls :clippy_preference "on"))
[eglot] Server reports (type=2): Unknown RLS configuration: `rls`
((rls\.clippy_preference . "on"))
[eglot] Server reports (type=2): Unknown RLS configuration: `rls.clippy_preference`

I'm not sure wht else should I try. Othe Editor uses rust.clippy_preference="on" but it doesn't work with eglot either:

((:rust\.clippy_preference . "on"))
[eglot] Server reports (type=2): Unknown RLS configuration: `rust.clippy_preference`

@joaotavora
Copy link
Owner

Sorry I should have mentioned that I am calling eglot function to update
the configuration, and every time get wrong configuration option error
message. So the hook was not the case here. I don't want to use dir locals
because I use clippy for all rust projects, so dir local files are
additional work to set up every time.

In that case, you probably want to use rust-mode-hook to setup eglot-workspace-configuration (major)-mode-locally.

That still won't solve the syntax problems you are having. Can you capture a successful JSONRPC negotiation about this preference from another client (say VSCode?)

@andreyorst
Copy link
Author

I've installed VSCode, entered settings, and turned on clippy preference, then pressed copy setting as json, and this is what I've got:

"rust.clippy_preference": "on"

I've mentioned that another editor uses this format to set up clippy, but it doesn't seem to work in Eglot.

@andreyorst
Copy link
Author

andreyorst commented Dec 1, 2019

Oh, it seems

(setq eglot-workspace-configuration '(("rust" "clippy_preference" "on"))))

worked, but it doesn't seem to enable clippy for me.

@andreyorst
Copy link
Author

andreyorst commented Dec 1, 2019

I've changed it to '((:rust . (:clippy_preference "on")) and called eglot-signal-didChangeConfiguration, and now clippy works, but I also get this message:

[eglot] (warning) Server tried to unregister unsupported capability `textDocument/rangeFormatting'

However when turrning clippy file locally with #![warn(clippy::all)] works fine.

@joaotavora
Copy link
Owner

joaotavora commented Dec 1, 2019 via email

@andreyorst
Copy link
Author

yeah, so this is for another ticket. In this particular example, why '((rust . (:clippy_preference "on"))) get's converted to, "rust.clippy_preference": "on" but '(("rust.clippy_preference" . "on")) which is more like the documented format is not? What are genereal rules here? Sorry if I bother with these questions, maybe those aren't related to eglot at all and I just should learn how json works.

@joaotavora
Copy link
Owner

What are genereal rules here?

I don't know.

Sorry if I bother with these questions, maybe those aren't related to eglot at all and I just should learn how json works.

Indeed, they are not related to Eglot. And they are not related to JSON directly, I think. They could be replated to the LSP protocol, but even that I don't think is true. I think this is just up to the particular LSP server that you are using, in this case rls.

But you are not bothering. Indeed, as you find answers to these, keep use posted here. We can extract something for the README.md later on.

@andreyorst
Copy link
Author

Indeed, as you find answers to these, keep use posted here.

I'm not sure if I will search for something related to the format for some time, because I already have everything I need now. If I will set up another server that will require setting up workspace configuration and will struggle/find something related to the format, I will open new issue, or submit a PR.

I think this issue can be closed as solved since I've got clippy working.

We can extract something for the README.md later on.

And to variable documentation as well. I think most of people check it first.


As for another request with dynamic registration should I open another issue?

joaotavora added a commit that referenced this issue Dec 9, 2019
Per #363.

* eglot.el (eglot-register-capability)
(eglot-unregister-capability): Actually implement instead of warning.
(eglot-client-capabilities): Allow dynamicRegistration in many
capabilities.
(eglot-lsp-server): Add dynamic-capabilities slot.
(eglot--connect): use only initial-capabilities slot.
(eglot--capability-keyword): New helper.
(eglot--server-capable): use eglot--dynamic-capabilities.
@andreyorst
Copy link
Author

I think this issue can be closed as solved since I've got clippy working.

@Wilfred
Copy link

Wilfred commented Oct 15, 2023

Note that enabling clippy in the newer rust-analyzer server is an initialization option, not a setting:

(add-to-list 'eglot-server-programs
             '((rust-ts-mode rust-mode) .
               ("rust-analyzer" :initializationOptions (:checkOnSave (:command "clippy")))))

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

No branches or pull requests

4 participants