vscode-languageserver-node (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation20 Commits2 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

@sam-mccall yes, this all has to go under proposed. And it is easier to keep the markdown spec part in sync if it is part of this repository as well. Adding it to the spec can be done when we think that the proposed API has reach some stability.

rwols mentioned this pull request

Jun 12, 2021

@dbaeumer Gentle ping to get this back on your radar :-)

We've got a server implementation of this in clangd which is coming up to a release cycle, and are looking to add matching support in a client soon. Having the protocol landed in some form (proposed, or even just rough agreement on the protocol structure) would let us align on that. IME it's hard to rip out nonstandard extensions later especially if they're "richer" than the standard replacement.

I was out on vacation. I will try to find out where VS Code is with that. What we can do is to add something that mirrors VS Code's proposed API. But then the proposed API might change if the VS Code API changes.

I followed up with the VS Code team and they expect changes in the realm of inlay Hints.

Given that and the fact that the PR doesn't contain any server implementation I will currently not merge it.

@sam-mccall are you willing to add the server code as well? Otherwise it is hard to write tests for this.

@dbaeumer thanks for following up.

I'm willing to add node server code & tests if you think this it's likely it can be merged in that case.
(If it's going to be blocked waiting for VSCode changes anyway, then I'd probably wait)

@sam-mccall I would propose to wait for now and see what happens in VS Code.

I'd like to second this issue.
VSCode now has a toggle like this:

Can you give us a status update? It seems many language-extensions are blocked on this PR, although admittedly I didn't take the time to fully understand the chain of things that need to happen.

Related issue: microsoft/language-server-protocol#956
I'm coming from this Dart extension issue: Dart-Code/Dart-Code#3609

VS Code will finalize the API this month. After that I think it is a good time to look into the PR again.

Anyone interested in preparing a new PR or updating this one? Based on reviews we did we changed the VS Code API to allow more features (like navigating on inlay hints) which resulted in changes in the API.

I can take another pass at it but probably not for a few weeks.

If someone else wants to jump on it before then, please do!

Another slight inconsistency is textDocument/inlayHint (singular) in the current LSP proposal vs. textDocument/semanticTokens (plural). A lot of other textDocument requests are singular too though, so perhaps naming the request in singular is more idiomatic?

fwcd mentioned this pull request

Mar 8, 2022

9 tasks

I called it viewPort since this is how we called it in the inline values. I chatted with @jrieken that we should also make it consistent in the VS Code API.

In general all request are singular. The semantic token one slipped in and it was too late to correct it since it already moved out of proposed and I didn't want to break it.

If we choose range we could make it consistent since inline values is still proposed.

OK. We will use range since a client can ask for a range that is not the viewPort.

I will close the PR. The feature has shipped in 3.17