refactor: incremental update of inlay hints and more options

jose-elias-alvarez / nvim-lsp-ts-utils

Utilities to improve the TypeScript development experience for Neovim's built-in LSP client.

The Unlicense

437 stars 18 forks source link

refactor: incremental update of inlay hints and more options #101

Closed yioneko closed 2 years ago

yioneko commented 2 years ago

Previously, the inlay hints are dropped and requested as a whole on every update of text. This will cause apparent lag in large file, and tsserver is even less responsive as it's busy dealing with the large amounts of inlay hints. By requesting and updating the hints incrementally, the performance will be much better.

Three extra options are exposed to help customization.

inlay_hints_priority is used to resolve the conflict with other extmarks set on the same line.
inlay_hints_throttle configures the throttle time of requests as the text may change much frequently.
inlay_hints_format allow user to customize the highlight and text of different kind of hints, because the original text format of hint is confusing when messed up on the single line (param: and : return).

jose-elias-alvarez commented 2 years ago

I haven't done a ton of JavaScript work lately but this seems to be working well. Let me know what you think about the config issue, but other than that I think this is okay to merge.

yioneko commented 2 years ago

Sorry for late response, previously I'm busy with personal stuff but these days I might have enough time to pick it up.

I think we should not rely on updatetime and bring the throttle time option back since there is an issue for CursorHold: https://neovim.discourse.group/t/psa-dont-rely-on-cursorhold-yet/1813. Also I think setting default to 150ms is more reasonable since nvim now use it as default debounce time for lsp servers: https://github.com/neovim/neovim/pull/16908

And the another thing worth notice is that the changes introduce in https://github.com/jose-elias-alvarez/nvim-lsp-ts-utils/commit/4655fbcb85b55bc34c80aa3e9400ca069b3f1363 makes the throttle behavior actually becomes debounced since the on_lines events are accumulated and only the last response will be accepted and handled. I'm not sure whether it will be alright or better be reverted.

jose-elias-alvarez commented 2 years ago

Sorry for late response, previously I'm busy with personal stuff but these days I might have enough time to pick it up.

I think we should not rely on updatetime and bring the throttle time option back since there is an issue for CursorHold: https://neovim.discourse.group/t/psa-dont-rely-on-cursorhold-yet/1813. Also I think setting default to 150ms is more reasonable since nvim now use it as default debounce time for lsp servers: neovim/neovim#16908

And the another thing worth notice is that the changes introduce in 4655fbc makes the throttle behavior actually becomes debounced since the on_lines events are accumulated and only the last response will be accepted and handled. I'm not sure whether it will be alright or better be reverted.

Yep, definitely agreed about updatetime and the default setting. I don't think debouncing changes is a big deal (it might reduce flickering, actually) so I'm happy with the way things are working now.

jose-elias-alvarez commented 2 years ago

Everything looks good to me, and I've been using this without issues. Let me know when you think it's ready and I'll merge it.

yioneko commented 2 years ago

Sure, I think there is no other issue.

jose-elias-alvarez commented 2 years ago

Great – thank you for your work on this!