LSP call stream emptiness check is incorrect (?)

[jakubDoka]

Summary

When testing lsp for my language, is noticed that spinner stays frozen and no diagnostics show up on screen until I move cursor which is different from what rust extension does. I thought I am missing something, so i looked into source code.

I found this:

                self.handle_language_server_message(call, id).await;
                // limit render calls for fast language server messages
                let last = self.editor.language_servers.incoming.is_empty();

                if last || self.last_render.elapsed() > LSP_DEADLINE {
                    self.render().await;
                    self.last_render = Instant::now();
                }

is_empty will return true if there are no streams present.
Since lsp_servers holds the stream until client with UnboundedSender is dropped, last will only be true when there is no server active.

LSP_DEADLINE is 16ms but My language server can update reliably faster. By adding thread::sleep to server code i managed to fix it, though It was not very easy to figure out.

My expectations were that upon progress end notification, editor would clear the spinners but it, only stops them (possible fix).

Reproduction Steps

No response

Helix log

No response

Platform

Linux

Terminal Emulator

Ubuntu 20.04.5 LTS

Helix Version

helix 22.12 (6c95411)

[the-mikedavis]

I also see this frequently with erlang_ls, I think because it sends many progress update messages and then the progress-done or publishDiagnostics messages. The important events don't trigger the rendering since they arrive within the 16ms window after a progress message. I can't seem to figure out the proper incantation to check if there are any events pending in any streams without consuming the events though 🤔. I've added this hack to my fork in the meantime: the-mikedavis@d2034ac

[jakubDoka]

Maybe good solution would be listing on which LSP events should helix rerender, inside the config.

[the-mikedavis]

I think it would be best to keep this automatic if possible. Debouncing the rendering should be an implementation detail IMO

Another approach we could take to this would be setting a timer to go off after the LSP deadline. Roughly:

if last || self.last_render.elapsed() > LSP_DEADLINE {
    self.render().await;
    self.last_render = Instant::now();
} else {
    self.render_timer.reset(Instant::now() + LSP_DEADLINE)
}

And then rendering at the end of that timer. But I'm not sure how cheap it is to reset timers really often.

[pascalkuthe]

Fixed by #7538