zbelial / lspce

LSP Client for Emacs implemented as a module using rust.
GNU General Public License v3.0
154 stars 11 forks source link

I am not getting any diagnostics with clangd #16

Closed pushqrdx closed 12 months ago

pushqrdx commented 12 months ago

Everything else seems to be working correctly but I am no getting any diagnostics even though fly-make is active and works with other LSP clients.

zbelial commented 12 months ago

Can you post a minimum config and a test project to reproduce the problem? Thanks.

I tested lspce with clangd (15.0.7) on Manjaro, it showed diagnostics correctly. But I don't write cpp code now, so maybe there are some corner cases I've not found.

Anyway, here is a picture showing flymake displaying diagnostics. 图片

pushqrdx commented 12 months ago

@zbelial Everything is OOB, I just add lspce to my load-path and enabled lspce-mode in an empty from scratch Emacs configuration like so:

(add-to-list 'load-path (concat user-emacs-directory "lsp"))
(require 'lspce)

And a simple cpp file:

int main(int argc, char** argv) {
    (void) argc;
    (void) argv;

    xxx NO FLYMAKE ERRORS xxx

    return 0;
}

Completion, jump to definition and other LSP features are working as expected but no diagnostics are shown.

image

I am on Windows, but this shouldn't be related as the server seems to be functioning properly.

zbelial commented 12 months ago

Got it, thanks. I'll prepare a windows env and test lspce ASAP.

zbelial commented 12 months ago

My windows is not ready now, but I have another question to ask and some things to share.

When you said lspce + flymake did not work, you meant after you opened the file but before editing it, or after you edited it?

I mean, lspce + flymake works in a different way than what eglot does. After you enable eglot in a buffer, if the file has some syntax errors, the server (eg. clangd) will send diagnotics notification to eglot. When eglot receives this diagnostics notification, it will handle them immediately.

But lspce does not work in this way. lspce receives diagnotics and cache them (this happens in the lspce's rust code), until the elisp code of lspce retrieves them (after saving buffers, after modifying buffers, and so on, all these are controlled by flymake).

So if you just enable lspce in a buffer but do not make any modification, lspce won't show any diagnostics. Only after you edit the buffer, it will retrieve diagnostics.

Lspce works in this way because in this way, Emacs won't have to handle those diagnostics notifications (receving them, deserializing them, all of these may slow down Emacs or even freeze it) all the time. And you may have heard that for some lsp servers, some of these diagnostics are thousands or tens of thousands of lines.

I know there is a flymake-start-on-flymake-mode, which means flymake will start to check code immediately after it's enabled. But in lspce, when lspce is enabled, it will enable flymake. But at the moment, lsp server may have not finished analyzing code, so no diagnostics ATM.

pushqrdx commented 12 months ago

When you said lspce + flymake did not work, you meant after you opened the file but before editing it, or after you edited it?

@zbelial no, even when I edit the buffer, completion, jump to definition, function signatures, even hover are working properly but flymake doesn't report any diagnostics

Lspce works in this way because in this way, Emacs won't have to handle those diagnostics notifications (receving them, deserializing them, all of these may slow down Emacs or even freeze it) all the time. And you may have heard that for some lsp servers, some of these diagnostics are thousands or tens of thousands of lines.

Also makes sense, if diagnostics are cached for performance, I don't have much of problem with that, I just want them to show up when I edit the buffer though xD

zbelial commented 12 months ago

Hi @pushqrdx , I just pushed a new commit, can you test it? Thanks.

pushqrdx commented 12 months ago

@zbelial thanks! diagnostics started to show up, albeit I am having another issue that maybe due to caching of diagnostics, sometimes flymake is reporting errors that no longer exist (and other times it's not reporting issues that were there), although maybe we should close this issue and open a new one for that?

zbelial commented 12 months ago

It's okay to keep this issue open, I'll dig it deeper.

Is it easy to reproduce? if yes, how to reproduce? Thanks.

pushqrdx commented 12 months ago

@zbelial Yes, it's extremely easy to reproduce, | represents where the cursor is after typing auto y = 22 now wait for the error squiggly to appear under return for the missing semicolon, then type the semicolon, the error won't go away even though it should.

int main(int argc, char** argv) {
    (void) argc;
    (void) argv;

    auto y = 22|

    return 0;
}

Now whenever you do another edit to the buffer let's say adding a space on the next line, the error under return should go away and a warning under y will appear for unused variable. Now, on the same line, type dd or whatever to produce another error then backspace, now the warning will disappear.

Here's a gif illustrating everything:

lspce


It seems like the cache is always one iteration behind.

zbelial commented 12 months ago

What's the value of your flymake-no-changes-timeout, I guess it's small enough, so when flyamke tries to retrieve diagnostics, clangd haven't finished analyzing. I set it to 3, and flymake seems work well. or you can add an idle timer to flymake like the following:

(add-hook 'flymake-mode-hook
          (lambda () (run-with-idle-timer flymake-no-changes-timeout t #'flymake-start)))
pushqrdx commented 12 months ago

@zbelial a flymake-no-changes-timeout value of 0.75 seems to work but also is kinda arbitrary and wouldn't probably work on bigger buffers. I wonder is there a way to force request diagnostic info from lspce and hence disable the cache? If the caching is optional, I think it would be excellent because we can decide on a per project/language server bases whether the caching is causing performance issues and toggle between eager/delayed modes.

Personally, I find value of 3.0 too long for my test, as I prefer it to be snappy.

zbelial commented 12 months ago

I wonder is there a way to force request diagnostic info from lspce and hence disable the cache?

This is impossible ATM.

lspce cannot do it because as a multithreaded Emacs module, it receives lsp servers' messages in a seperate thread, and this thread cannot call elisp (This is Emacs's limit).

Eglot/lsp-mode starts lsp server process directly, and read from/write to the process without any bridge like lspce. So when receiving notifications, they can handle them immediately.

Have you tried lsp-bridge (another lsp client for Emacs)? I don't use it, but maybe it works better in this aspect. Or you may switch to eglot/lsp-mode, lspce cannot do better IMO.

pushqrdx commented 12 months ago

@zbelial Understandable, and yes, I have been testing all lsp clients and so far, I like lspce the most. lsp-bridge is really buggy and IMO bloated. Also being fully asynchronous makes the completion experience very bad, if you type too fast you can literally see the completion window catching up after you stop typing.

What I was thinking was to force retrieve diagnostics from the native side of lspce like you presumably already do on document changes etc? So maybe instead of relying on flymake-no-changes-timeout or document save, I can retrieve diagnostics on each key press.

zbelial commented 12 months ago

I like lspce the most.

Thanks :).

What I was thinking was to force retrieve diagnostics from the native side of lspce like you presumably already do on document changes etc? So maybe instead of relying on flymake-no-changes-timeout or document save, I can retrieve diagnostics on each key press.

Lsp protocol does not work in the way you described. It works in this way ( the following description focuses on diagnostic):

  1. User modifies a file
  2. Lsp client sends a didChange notification (containing what has been modified) to lsp server.
  3. Lsp server analyzes file content (combined with its old content snapshot and what's inside didChange), after finishing (this may take tens of milliseconds or even seconds), it will send a publishDiagnostics notification to the client
  4. (lspce specifically) Lspce's rust code receives this publishDiagnostics notification and caches the content
  5. (lspce + flymake specifically) flymake calls lspce (its elisp code will call a api provided by the lspce module) to request diagnostics then. (this step may happen before step 4, in this case, lspce will retrieve outdated diagnostics)

Of course we can add a function to after-change-functions to request diagnostics after changes, but there still is a large chance that the diagnostics returned are outdated, since in step 3 above, lsp server need some time to analyze file content.

But I haven't tested using after-change-functions to force request diagnostics, so I cannot say it will work better or even worse.

PS.: not a native English speaker, I tried my best but I don't know I described accurately or not :)

pushqrdx commented 12 months ago

PS.: not a native English speaker, I tried my best but I don't know I described accurately or not :)

I've had no issues understanding what you said, thanks for clarifying how lsp/lspce diagnostic caching works.


Well, the value of 0.75 seems to be the magic number that works for me so far, I'll give it a test drive for a few days and see. If it's still giving me trouble, I'd have to open another issue xD. But I understand that given how things work now, it's probably unfixable without some serious hacks and/or a major re-architecture which might have some negative impact on performance.

zbelial commented 12 months ago

Ok, feel free to open issues if you have other troubles. I'll close this one.