feat(lsp): inlay hints

[p00f]

Implements inlay hints by adding automatic refresh and a public interface on top of #23736

Closes #18086

[glepnir]

not sure is the good way create timer for each buffer , and in on_lines callback has changedtick param you can use pass to request like make_request(cb_bufnr, changedtick), and when lsp request callback running check nvim_get_buf_chagnetick make a compare. if before_changetick < nvim_get_buf_changetick do defer reqeust again. some undo/redo also increase changetick and also some change without text chang also increase changtick . this is my thought and I do it on documentsymbol request . it combine 1000ms change to one request .
https://github.com/nvimdev/lspsaga.nvim/blob/3546dca5560b6868612acc567f6f3c07cf61c076/lua/lspsaga/symbol/init.lua#L25

[p00f]

I don't like this myself, is there a better way of remembering which buffers have inlay hints enabled (to handle workspace/inlayHint/refresh)?

[jdrouhard]

changedtick isn't exactly the way to do version comparisons in LSP. You'd useutil.buf_versions[bufnr] since there are some situations where the changedtick increases but the LSP-aware document version does not, and we care only about which document version the LSP server knows about for a particular request. Using changedtick could actually cause the local cache of inlay hints to be out of sync with the LSP server's understanding of where they should be.

Anyway, this looks like a debounce timer? Seems ok to me. You want to avoid making new inlay hint requests until there's been a bit of time with no edit activity (after someone stops typing for a bit). Making a request on every on_lines event would be way too many requests. The alternative is to do it on a few autocmd events which might trigger less often than a dedicated timer would.

As for your other question, I think you just use your __buffers table in the refresh handler. If someone disables inlay hints for a buffer, just clear the cache for that buffer (and cancel any pending requests). I don't think it needs to be more complicated than that?

[p00f]

That is what I was doing, except how do you cancel pending requests?

[glepnir]

i had point it out changetick increase without text changed and this change will not trigger on_lines callback . and there has on_changedtick in nvim_buf_attach . these problem i had comment in prev :-) each buffer has a timer I am very confused this way . worker thread of luv will grow with the buffers count .. maybe you can ref vscode implement get some idea.
https://github.com/microsoft/vscode/tree/main/src/vs/editor/contrib/inlayHints

[jdrouhard]

changedtick isn't exactly the way to do version comparisons in LSP. You'd useutil.buf_versions[bufnr] since there are some situations where the changedtick increases but the LSP-aware document version does not, and we care only about which document version the LSP server knows about for a particular request. Using changedtick could actually cause the local cache of inlay hints to be out of sync with the LSP server's understanding of where they should be.

Anyway, this looks like a debounce timer? Seems ok to me. You want to avoid making new inlay hint requests until there's been a bit of time with no edit activity (after someone stops typing for a bit). Making a request on every on_lines event would be way too many requests. The alternative is to do it on a few autocmd events which might trigger less often than a dedicated timer would.

As for your other question, I think you just use your __buffers table in the refresh handler. If someone disables inlay hints for a buffer, just clear the cache for that buffer (and cancel any pending requests). I don't think it needs to be more complicated than that?

[clason]

When I suggested skipping the public API in #23736 , my thinking was: just enable it by default. It doesn't need to be configurable.

Does LSP actually require the enable/disable stuff to be configured client-side? Seems like this could/should be a setting that could be sent to the server, using a standard "configure" LSP request. Does LSP have something like that? Can the client-side "capabilities" map be used to enable/disable inlay hints?

Inlay hints can be quite obtrusive (even moreso than diagnostics); they should definitely be under the user's control. (I refer to the semantic tokens decision, which triggered a deluge of "my highlight is suddenly broken!!eleven!" issues for nvim-treesitter.)

[justinmk]

I don't see any other enable() methods in :help lsp. Why is this the only one?

Because we (1) started with the simple, non-controversial features ... (2) made mistakes for some of the other ones.

To avoid more mistakes can we look at vim.diagnostic.config? Instead of adding lots of foo_enable() functions, it sounds like we need vim.lsp.config that follows the model of vim.diagnostic.config.

[clason]

I believe the possibility of (easily) toggling diagnostics is something that's been missing from that API as well. I do agree (as I've written before) that it makes sense to look at this (toggling "code overlays") as a general issue.

[mfussenegger]

Does LSP actually require the enable/disable stuff to be configured client-side? Seems like this could/should be a setting that could be sent to the server, using a standard "configure" LSP request. Does LSP have something like that? Can the client-side "capabilities" map be used to enable/disable inlay hints?

Given that inlay hints are requested by the client it's also up to the client to control it. The server has no business controlling client behaviour. The capabilities indicate resolve support and dynamic registration support, not general on/off

A user could change the server capability, to trick the client into thinking it's not supported, but dynamic capabilities make that problematic too. (We currently use this approach to turn semantic tokens off, but we may have to move away from that).
An option could be to extend vim.lsp.start/start_client with flags for the different features. E.g. start({ ..., formatting = false, semantic_tokens = false, inlay_hints = true, ... })
I think there's a bit a difference to vim.diagnostic.config in that users first have to start a client with lsp.start or lsp.start_client, so the namespace model is different too.

A vim.lsp.config could act as a skeleton, but it would need a way to distinguish between settings for all clients or specific clients.

When I suggested skipping the public API in #23736 , my thinking was: just enable it by default. It doesn't need to be configurable.

Inlay hints was always the first thing I turned off in intellij as it's very intrusive. We can discuss the default but it definitely needs an option to turn it off. And I'd say not just the display but the requests as well. Users who don't use it shouldn't need to spend processing power on it.

[p00f]

@justinmk I think vscode has a thing where it shows inlay hints as long as you hold certain keys

Not saying this should be the reference, just addressing

didn't think that toggling inlay hints on/off at will was a common thing

[gpanders]

I sincerely believe that "runtime choice" is the primary use case for inlay hints,

I could be out of touch, but I didn't think that toggling inlay hints on/off at will was a common thing. But again (1) the door is open for that after we decide a long-term interface, (2) doesn't need to block the minimal approach.

FWIW I didn’t think that either until I tried it. And my almost immediate thought was that I wanted to be able to flip them on when needed, but disable them otherwise. Personally, I find them far too noisy to leave enabled all the time.

[justinmk]

@mfussenegger

A user could change the server capability, to trick the client into thinking it's not supported

But the client also declares its capabilities (vim.lsp.start({capabilities={...}})), don't see why a "trick" is needed.

An option could be to extend vim.lsp.start/start_client with flags for the different features. E.g. start({ ..., formatting = false, semantic_tokens = false, inlay_hints = true, ... })

Let's forget that. coc.nvim for example, suggests there is a ridiculous amount of configuration people want to fiddle with. Can't jam that all into vim.lsp.start(), and see below...

A vim.lsp.config could act as a skeleton, but it would need a way to distinguish between settings for all clients or specific clients.

And also "specific buffers" (sigh)--but I think that is not an important use-case.

vim.lsp.get_active_clients()[1] shows me:

{
  config = {
    _on_attach = <function 3>,
    autostart = true,
    capabilities = { … },
    settings = {
      Lua = {
        diagnostics = {
          globals = { "vim" }
        },
        runtime = { … },
      }
    },
  },
  server_capabilities = { … },
  ...
}

I could imagine a vim.lsp.buf.config({inlay_hints=...}) interface that updates the current-buffer LSP clients. But that will require some thought, so to unblock this PR...

Proposal

To unblock this PR, all that's needed is:

1. rename the function(s) to vim.lsp.buf.* instead of vim.lsp.buf_* (I'm not aware of any new convention in favor of buf_ over .buf.*, and we should stay consistent)
2. reduce the number of functions from 3 to 1. The main use-case is dynamic enable/disable, anyone who wants to fiddle more can read the docs and figure it out, they don't need super convenient functions. https://github.com/neovim/neovim/pull/23984/files#r1225924616

[justinmk]

Convention is:

• 0 = current buffer
• nil = all buffers (don't necessarily need to implement this here)

[p00f]

done

[clason]

Would be nice to do this in a follow-up, though, as now you can't just map to vim.lsp.buf.inlay_hints() (like you can with most other vim.lsp.buf functions).

[justinmk]

LGTM (after CI is passing), will wait for other approvals, ping after a few days if not merged already.

Thanks for your persistence and patience!

[mfussenegger]

Approved this, but I think we should look into doing some API adjustments before this gets released.

Inlay hints have optional textEdits, and we could (or should?) allow users to apply them. That will likely need another function. With the current approach this would mean something like vim.lsp.buf.inlay_hints_run() (or apply or apply_edits or whatever). Which would just forward to _inlay_hints.xy().

I'm thinking maybe we could go with a callable module approach similar to how vim.iter works. We'd have vim.lsp.inlay_hints() to toggle, enable or disable and could add additional functionality as needed in vim.lsp.inlay_hints.xy().
This would remove the indirection/forwarding to a private module, and we could keep stuff properly encapsulated via lua locals and it's module system.

The same approach would also work for codelens. We could toggle/enable/disable via vim.lsp.codelens(), but we still have vim.lsp.codelens.on_codelens for the handler and vim.lsp.codelens.run() to execute a lens.