-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cycle through hover results from multiple LSPs #10122
base: master
Are you sure you want to change the base?
Conversation
Also, seems there are some concurrency issues in the integration test. And i tried to run Any idea? |
c61cef2
to
8310f64
Compare
I have very similar concerns/review as in #9974:
|
65d7105
to
9f9016b
Compare
Thank for the review and comments. The new push removed the unnecessary handler codes, and updated the keymap as suggested. |
9f9016b
to
a5f9ca8
Compare
This seems to be working great! Merge with
|
42555c3
to
f6aa0e4
Compare
Great work! I've been exploring the use of a language server to show git blame information on hover, which was pretty much useless without this. Only issue I noticed during development of the language server is that when the language server is slow or times out it will block messages from other language servers. |
1510f49
to
3313c60
Compare
Thanks for your comment! The Git LSP sounds like interesting idea. Regarding the blocking behaviour, it might be a bit complicated to manage UI interaction with the async responses from each LSPs. I am thinking about if it should display hovers preserving the order of LSPs in config, or by the response sequence from LSPs. It might better to be a separate PR as improvement. I have rebased onto master with some conflict resolved. |
replaces #9744 with improved UX.
inspired by #9974
This PR refactor LSP hover feature with the event system, with support of cycling through hover results from multiple LSPs.
keybinding:
Shift+Left
for previous hoverShift+Right
for next hover