Proposed extensions: textDocument/selectionRange#441
Proposed extensions: textDocument/selectionRange#441dbaeumer merged 1 commit intomicrosoft:masterfrom
Conversation
|
Status: I've got the code to compile, but I haven't actually tested it with my server. Not sure how all proposed bits, in the VS Code and in the lsp client library, should fit together. |
c40a2a4 to
222a999
Compare
|
Status update: I've successfully (after fiddling with npm quite a bit) built my extension with |
|
The server impl is read, and server and client successfully work together: rust-lang/rust-analyzer@fffa069! The only quirk is that current insider's does not have the latest version of proposed API (it uses Range instead of SelectionRange), so I had to tweak client library a bit to work with old API. |
|
Great to see that you got it working. Please ping when the API and implementation is finalized and then we can merge it in. |
|
Sure! I believe this is currently blocked on VS Code side (some questions there). @jrieken could you ping me once VS Code side is "finalized" (I understand that this is provisional API, hence quotes). |
|
@matklad So far we are pretty happy with our proposal. Tho we haven't implemented any useful provider yet ;-) The idea is to that in the next weeks and to validate our design with that. Having said that, we will likely keep the API in proposed for another month. @dbaeumer Isn't an option for LSP proposal? Have this as proposed protocol first and validate it with some of our own language brains, like JSON and CSS? |
222a999 to
165c80c
Compare
165c80c to
54480ec
Compare
|
I've rebased and switched to the new API, but I am having hard-time actually testing my client with this code (I had success the time before this, but I can't reproduce it). Is there anything more to using my fork of npm gives me some weird errors about extraneous packages :( whyyyyy |
|
@matklad I usually use links. (e.g. I go into the node_modules folder delete |
|
@dbaeumer that did the trick, thanks! I've verified that all the pieces work together, including my server. |
|
I will merge this in and ship a next version as soon as we released the next VS Code stable version which contains the necessary proposed API. |
|
fyi - after implementing some provider and actually using them we got the feeling of going 180 of the single position vs multi position. The VS Code API will very likely be changed to accept multiple positions and return an array of ranges |
|
I merged the PR in and made new next version: |
|
@jrieken I am a little confused about VS Code state here. Are you switching to multi-position in the end? The current proposed API is still single-position. |
|
Yeah, switching to multiple positions is the current plan. Not sure why @dbaeumer already merged this... |
|
@matklad @jrieken It's probably @dbaeumer feels that the LSP API itself will not change so he would like adopters to start trying out and using the LSP API. Or do we now anticipate the LSP API changing also? |
|
This got released as a next version (*-next.1) to get feedback on the LSP level as well. Normal adopters will not see that version and we are free to change the LSP API to multiple positions which I suggest to do. |
|
In addition the feature is in proposed state and users need to opt in using it. |
|
@dbaeumer FYI, the vscode API changed again:
|
|
@matklad I noticed that as well. Are you willing to work on a adoption PR. We can merge it in as soon as we have another stabled release with the new API. |
|
@dbaeumer yeah, I'll most likely handle this withing a week |
|
done: #474 (review) |
Implementation for microsoft/language-server-protocol#613