fidget.nvim icon indicating copy to clipboard operation
fidget.nvim copied to clipboard

Use `vim.lsp.util.get_progress_messages()` as alternative LSP progress source

Open fitrh opened this issue 3 years ago • 6 comments

vim.lsp.util.get_progress_messages() is a good alternative for getting progress reports, especially when the language server does not use $/progress like jdtls

fitrh avatar Feb 20 '22 15:02 fitrh

The idea of retrieving the messages from a function seems like the right way forward, except that I have no idea how it works, and there appears to be no documentation for it..

A cursory inspection of what I get from calling it while using hls yields little useful information:

-- :lua print(vim.inspect(vim.lsp.util.get_progress_message()))
{ {
    name = "hls",
    progress = true,
    title = "empty title"
  }, {
    name = "hls",
    progress = true,
    title = "empty title"
  } }

j-hui avatar Feb 20 '22 21:02 j-hui

Just to clarify, what nvim-jdtls does is use a custom handler to respond to notifications from jdtls and populate the client messages table, the same way the progress handler populates client.messages from $progress. All get_progress_messages does is read client.messages by iterating over all clients

mjlbach avatar Feb 20 '22 21:02 mjlbach

Hey @mjlbach , thanks for jumping in here (from Discourse)! That clarifies things; I was missing the fact that Neovim actually specifies its own progress handler to populate clients' messages fields.

So in fact, the entire way this plugin works may be broken, or at least incompatible with how Neovim wants progress updates to be handled. Fidget overrides Neovim's own handler (which I wasn't even aware existed; should have looked into this earlier), so the messages fields are never properly populated. But why does calling get_progress_messages() yield empty messages?

The existence of get_progress_messages() also begs the question: how is get_progress_messages() meant to be used? Why is it marked private? I understand it was introduced in neovim/neovim#13294 but I'm not finding what the intention of this was, other than to introduce the LspProgressUpdate autocommand to allow multiple progress subscribers.

In any case, I would like to use get_progress_messages() as the backend instead of my own progress handler, but I do want to first understand how it's meant to be used (and perhaps get an idea of how it might change in the future, and what other plugins are populating the clients' messages field without the $progress handler).

j-hui avatar Feb 21 '22 04:02 j-hui

So in fact, the entire way this plugin works may be broken, or at least incompatible with how Neovim wants progress updates to be handled. Fidget overrides Neovim's own handler (which I wasn't even aware existed; should have looked into this earlier), so the messages fields are never properly populated. But why does calling get_progress_messages() yield empty messages?

It's not the end of the world, one option would be wrapping instead of replacing the original handler (call both).

The existence of get_progress_messages() also begs the question: how is get_progress_messages() meant to be used? Why is it marked private? I understand it was introduced in https://github.com/neovim/neovim/pull/13294 but I'm not finding what the intention of this was, other than to introduce the LspProgressUpdate autocommand to allow multiple progress subscribers.

I think it was basically just to provide a cache for the progress messages that could be accessed via subscribing to the cache via an autocommand triggered by the LspProgressUpdate. This could always be improved to support plugins like fidget.nvim, there just hasn't been much interest/feedback from statusline authors.

mjlbach avatar Feb 21 '22 04:02 mjlbach

It's not the end of the world, one option would be wrapping instead of replacing the original handler (call both).

Ultimately, I'd like to stick with what Neovim does by default if possible, since subscribing to an autocommand seems like the right pattern here. Overwriting the handler may cause this plugin to be incompatible with others if multiple plugins are fighting over who gets to use the handler. There's no reason Fidget needs to have its own handler if the alternative plays more nicely with others.

I think it was basically just to provide a cache for the progress messages that could be accessed via subscribing to the cache via an autocommand triggered by the LspProgressUpdate. This could always be improved to support plugins like fidget.nvim, there just hasn't been much interest/feedback from statusline authors.

Ah, so this is was introduced primarily for and by statusline authors? If that's the case, then that seems like the right thing to use for this plugin too.

j-hui avatar Feb 21 '22 14:02 j-hui

So my plan was to swoop in here and save the day with a PR that uses the built-in progress handler and get_progress_messages(), but I'm slow and it's taking me longer than anticipated. So, for now, I'll just share my observations in the hope they are useful:

  • The built-in progress handler fires a LspProgressUpdate autocmd, which you're supposed to hook into, I think. (I'm using in my own statusline to implement a kind of spinner—WIP).
  • get_progress_messages() is stateful: If it gives you a list with a bunch of done = true or show_once = true messages, you won't see those in the next call to get_progress_messages().
  • get_progress_messages() is (currently) global: It processes messages from all active clients.

The last two points imply that with the current implementation of get_progress_messages(), you're only supposed to have a single handler for all messages. I was pretty happy when I realized I could do my spinner without having to touch get_progress_messages() in the end. Maybe that's why it's still private for now?

runiq avatar Apr 10 '22 11:04 runiq

hi, @runiq @j-hui , I am trying to implement my progress status using this API, integrate with lualine. But there is an error when using null-ls. Do you met such an issue?

linrongbin16 avatar Jan 22 '23 22:01 linrongbin16

@linrongbin16 Can you share the error message?

j-hui avatar Feb 17 '23 02:02 j-hui

emmm, it's a long time ago, I haven't dive into this issue. Will try later.

linrongbin16 avatar Feb 17 '23 06:02 linrongbin16

Superseded by #130

j-hui avatar Jun 10 '23 07:06 j-hui