editor: Make hover popover delay strictly respect hover_popover_delay setting #41149
Conversation
cla-bot
bot
added
the
cla-signed
| if let Some(delay) = delay { | ||
| delay.await; | ||
| } |
There was a problem hiding this comment.
The intent here is so to start the lsp request before the actual delay, such that when the request takes longer its not as noticeable (as we won't have to wait for the request as the delay expired).
I think we want to retain the previous system, but try to split the user configured delay into two parts. That is we could try removing HOVER_REQUEST_DELAY_MILLIS from the user delay. That way we also do not spam the server with requests.
There was a problem hiding this comment.
Thank you, I think I roughly understand how this works now.
But if we just subtract HOVER_REQUEST_DELAY_MILLIS, then if the user sets a delay smaller than that value, wouldn’t it cause every hover action to trigger an LSP request?
Or maybe we could make HOVER_REQUEST_DELAY_MILLIS smaller, so that the extra waiting time isn’t as noticeable — for example, set it to around 50 ms?
There was a problem hiding this comment.
Yea I think we do want a minimum here either way, reducing this to 50 is probably fine.
There was a problem hiding this comment.
Thank you for your time! In the latest commit, I kept the early LSP request strategy while dynamically adjusting the early request timing to maintain consistency with the configured delay. I believe this change helps keep the application responsive without putting too much load on the LSP server.
Veykril
changed the title
3 participants
Previously, the hover popover delay was implemented using two overlapping timers, which caused the minimum delay to always be at least HOVER_REQUEST_DELAY_MILLIS, regardless of the hover_popover_delay setting.
This change updates the logic to wait for hover_popover_delay only, ensuring the total delay is always equals to hover_popover_delay . As a result, the hover popover now appears after the intended delay, matching the user's configuration more accurately.
Release Notes: