Add proposed textDocument/inlayHints to protocol & client.

[sam-mccall]

This addresses the FR microsoft/language-server-protocol#956
And corresponds to the spec in microsoft/language-server-protocol#1249

[sam-mccall]

@dbaeumer I'm guessing that as a new feature, this should all go under "proposed"?
Does this mean I should fold the spec text (i.e. microsoft/language-server-protocol#1249) into this PR as a protocol.inlayHints.proposed.md file?

[dbaeumer]

@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.

[sam-mccall]

@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.

[dbaeumer]

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.

[dbaeumer]

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.

[sam-mccall]

@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)

[dbaeumer]

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

[RedyAu]

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

[dbaeumer]

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

[rami3l]

@dbaeumer Gentle reminder: microsoft/vscode#16221 (comment) is now closed.

[dbaeumer]

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.

[sam-mccall]

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!

[dbaeumer]

A first draft version is available here:

vscode-languageserver-node/protocol/src/common/proposed.InlayHints.ts

Line 6 in 582ea52

import { RequestHandler, RequestHandler0 } from 'vscode-jsonrpc';

[fwcd]

@dbaeumer Thanks for the nice work, just a question:

vscode-languageserver-node/protocol/src/common/proposed.inlayHint.ts

Line 254 in 37cf23b

viewPort: Range;

In the proposed LSP API, the range parameter of InlayHintParams is called viewPort whereas both the new VSCode API and rust-analyzer call it range. Any reason for not picking range for LSP too?

[matklad]

Yeah, viewPort would also be inconsistent with LSP's semantic highlighhting:

vscode-languageserver-node/protocol/src/common/protocol.semanticTokens.ts

Line 230 in 37cf23b

range: Range;

[fwcd]

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?

[dbaeumer]

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.

[dbaeumer]

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

[dbaeumer]

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

[dbaeumer]

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