joaotavora / eglot

A client for Language Server Protocol servers
GNU General Public License v3.0
2.25k stars 201 forks source link

Working with rustic and eglot #565

Closed samhedin closed 3 years ago

samhedin commented 3 years ago

Hi! I am looking at making some rustic functionality that currently only works with LSP-mode work with eglot as well. There are 2 things I'm trying to accomplish but I haven't been able to figure out.

One is for the PR https://github.com/brotzeit/rustic/pull/177 - which relies on getting diagnostics for the current buffer. At the moment get them from eglot--unreported-diagnostics along with some flymake functions, but I would prefer to make it not require flymake. Is there a nice way to do this?

The other is getting the information shown with with display-local-help, which eglot seems to call eglot-hover-eldoc-function. With LSP-mode this is accomplished by sending a textDocument/hover request. Is there an equivalent for eglot?

(-some->> (lsp--text-document-position-params)
                          (lsp--make-request "textDocument/hover")
                          (lsp--send-request)
                          (lsp:hover-contents)))
joaotavora commented 3 years ago

Hello Sam.

  1. You shouldn't be accessing any Elisp variable that has an -- in it. flymake-diagnostics returns a list of Flymake diagnostics, but, obviously, it requires flymake. Eglot requires Flymake.

  2. Eglot sends the textDocument/hover RPC call for you. eglot-hover-eldoc-function is a member of eldoc-documentation-functions, it follows that protocol.

What don't you tell me, without code, what you're generally trying to accomplish?

samhedin commented 3 years ago

Hi!

  1. I agree that this is suboptimal. I am creating a function for rustic which will add any missing imports in the current rust buffer to the project's Cargo.toml. If the user has the following rust code in the main file.

    use package_not_in_Cargo_toml;

    eglot will show the error Unresolved import 'package_not_in_Cargo.toml'. no 'package_not_in_Cargo.toml' external crate.. The user should now be able to call rustic-cargo-add-missing-imports, which will add the package to Cargo.toml. What I need from Eglot to accomplish this is a list of all missing imports in the current buffer. At the moment, this is accomplished by inspecting eglot--unreported-diagnostics, which contains, among others, the missing imports. Is there a better way to accomplish this, that does not rely on using a supposed-to-be private variable?

  2. This is for https://github.com/brotzeit/rustic#inline-documentation which displays exhaustive documentation for rust. If you hover over a function/macro and call rustic-doc-search, rustic-doc should search for whatever you have your cursor over. In order to do this accurately, rustic-doc wants to make use of the full name of the function/macro. One way to do this is to inspect the information returned by what seems to be a textDocument/hover request. If you hover over println in

    fn main() {
    println!("hello world");
    }

    and call eldoc-doc-buffer you get some useful information about println.

    
    println: std

[macro_export]

macro_rules! println


Prints to the standard output, with a newline. [...]


The text shows that `println` belongs to `std`. `rustic-doc` can take the first line of this text and use it to perform a reasonably accurate search - `std::println` instead of just `println`. So the question - how do I get this information programmatically when using eglot?
This feature exists because while the documentation returned by the language server is great, it is also quite limited in comparison to what is available - the goal of `rustic-doc` is to let users easily read all available documentation.
I hope that clears it up, otherwise, I can give more details. :heart:
nemethf commented 3 years ago

Nowadays I read Eglot issues as a curious bystander, so I can be totally wrong here, but I think these kind of features should be implemented on the server side.

As an example, LSP clients can send code action requests with a given location and the server usually replies with possible CodeActions. For example, when the point is somewhere at "use package_not_in_Cargo_toml;" a code action can contain a WorkspaceEdit that (when the user chooses to execute it) inserts the missing import. Alternatively, the user may ask code actions of a given kind (e.g., source.organizeImports) for the entire file.

joaotavora commented 3 years ago

The user should now be able to call rustic-cargo-add-missing-imports, which will add the package to Cargo.toml. What I need from Eglot to accomplish this is

That is so interesting that you're trying to do this. Eglot is primarily an LSP client, and it support these things called "code actions" that are part of the LSP protocol. So when an import is missing, in any language, or when a language-specific thing needs to be inserted in the buffer the the server should provide that code action. It seems this "rustic" mode is a rust-specific mode that uses some LSP?

@nemeth's advice is very good advice.

That's not to say that Eglot cannot in the future function as an API for communication with LSP servers, but it's not its primary purpose. I'd suggest that you use the jsonrpc.el library directly for that: you can read its brief documentation in the Emacs Lisp manual and learn how to request diagnostics or hover information from the server from the LSP protocol page. This is a preferred approach to messing with internal Eglot variables, which might go away/be renamed suddently and break the program you're trying to build on top of them.

Nowadays I read Eglot issues as a curious bystander, so I can be totally wrong here, but I think these kind of features should be implemented on the server side.

Yep. It seems again an again that either LSP is failing totally or people are actively resisting its premise (and its promise).

samhedin commented 3 years ago

@nemethf

I think these kind of features should be implemented on the server side.

I agree that this is more appropriate. If I remember correctly, I looked into at least the 2nd feature and people seemed content to have short documentation available through the LSP server and complete documentation available online/as html files. However, I much prefer browsing org-files than html files and that's how that feature was born. It is possible that I should have spent my time trying to implement something on the server instead of what I have here, but I imagine that would have been more work.

@joaotavora

That is so interesting that you're trying to do this. Eglot is primarily an LSP client, and it support these things called "code actions" that are part of the LSP protocol. So when an import is missing, in any language, or when a language-specific thing needs to be inserted in the buffer the the server should provide that code action.

Right. I do not believe that this feature is implemented in the Rust LSP server rust-analyzer. Again, upstreaming these features would be preferable but more work - maybe I should look at it though.

It seems this "rustic" mode is a rust-specific mode that uses some LSP?

Yes!

I'd suggest that you use the jsonrpc.el library directly for that

Thank you, I will look into it :+1: