helix
helix copied to clipboard
Implement window/showMessageRequest
Picker does already a good job I think
Todo:
- [x] Capture
window/showMessageRequest
and show the choices to the user - [x] Render the message
- [x] Render the MessageType
- [x] Handle when popup is dismissed
Any pointers regarding how to render the message on top of the input correctly prompt would be really helpful
Fix https://github.com/helix-editor/helix/issues/4699
A menu would probably be more appropriate for this than a picker: https://github.com/helix-editor/helix/blob/927fa112ec049e5f40309ffdd57c314897e18bbc/helix-term/src/commands/lsp.rs#L627-L654
I think it is too small as you can see:
And it might look worse once I add the message coming with the request
Here is how it looks like when the message is added in the picker
I still need to fix the prompt in this example
I agree with @ayoub-benali here. The menu seems better suited for "inline" actions and less well suited for a large popup like this.
In fact I originally suggested the picker on matrix.
Nvim also handles this as a larger popup:
I think reusing the picker here is nice so there are fewer specical cased UI elements (and the PR is easier to review). Altough it's true that the filtering capabilities of the picker don't really fit here. Perhaps it might make more sense to create a new view that is a copy of the picker, rip out any filtering related logic and functionality and reuse the top line to display the message?
I just noticed that lsp-workspace-command
uses the same picker and as you can see the list can be quite long. The filtering capabilities do help because the user can just start typing rather than having to scroll a long list.

window/showMessageRequest
could contain similar long action list
After discussion with @pascalkuthe I moved the LSP message to the right. This way we can better handle messages that could span over one line.
Usually LSP messages are short, but I will enable softwrap once https://github.com/helix-editor/helix/pull/5420 is merged.
For metals this how the build message is displayed:
And this is for the file creation:
Is there anything still missing? Otherwise you can mark this PR as ready for review.
almost, I am just working on sending the response to the server when the file picker is dismissed. I had to add a function to the constructor and update all the call sites ^^
If your favorite language server doesn't implement this request, you can still test this feature by:
- Installing Metals https://github.com/helix-editor/helix/wiki/How-to-install-the-default-language-servers#metals
- Cloning this example project and opening via Helix in the root
- Opening any
.sbt
or.scala
file, the build message should appear.
Once the build is done or you click Don't show again
you will have to delete the .metal
directory and open Helix again to force the message again.
Hi @the-mikedavis, sorry to ping you but Is it possible to give this PR a review ?
window/showMessageRequest
is required in Metals to import the build otherwise nothing works in scala projects.
I made sure to often keep this PR conflict free with master but it is getting harder as more core changes have been merged
Hi @archseer, any chance to give this a review ? :)
Any chance we could get this merged? I would personally find this very useful! The PR seems to be ready for merge after one more review and a rebase on master. We're almost there :)
Hi @NomisIV, I rebased this PR multiple time but it doesn't help as long as @archseer doesn't have time to review. This same applies to https://github.com/helix-editor/helix/pull/5535
I would really love to see this merged. Thank you very much for your contribution @ayoub-benali!
I am also using Metals for working with Scala.
For now I am building and patching Helix locally like I did with https://github.com/helix-editor/helix/pull/5535, but I am struggling with rebasing and resolving conflicts in this PR with master
branch or latest 23.05
tag to do the same.
@ayoub-benali Could you or someone else, please, resolve the conflicts, at least with the latest 23.05
branch?
Can we include this into the next release?
Hello @the-mikedavis, @pascalkuthe and @archseer! :slightly_smiling_face: Sorry to ping you like this, but this PR has been sitting reviewed and approved by 2 maintainers for months now. It would be really great to get this merged, since it's part of the LSP specification and some servers (like for example Scala's Metals mentioned in the original issue) use it to provide core features without which usability and user experience are impacted quite heavily. Any chance to see this progress? Is there anything still needed to be done here? :slightly_smiling_face: Thank you for your work on this amazing editor (and big thanks to the author of this PR for the contribution)! :slightly_smiling_face:
I have tried to rebase this to current master and miserably failed. Currently metals (a scala LSP) is unusable without this PR. I am using neovim as a substitute which is pita.