This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Added option to enable semantic highlighting via an experimental capability
AbandonedPublic

Authored by jvikstrom on Jul 18 2019, 7:10 AM.

Details

Summary

Adds option to enable semantic highlighting as an experimental capability. The reason for this is that it is not possible to add textDocument capabilities in the vscode extension as it is a strongly typed argument. The experimental capability is a normal JS like object and we can therefore send the capability there. However we can't remove the textDocument capability as theia relies on the capability to be in textDocument. (The same reasoning is why the server sends the TM scopes in the experimental capability if it got the capability as experimental).

Event Timeline

jvikstrom created this revision.Jul 18 2019, 7:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2019, 7:10 AM
jvikstrom updated this revision to Diff 210559.Jul 18 2019, 7:14 AM

Made Protocol comment be doxygen.

sammccall added inline comments.Jul 18 2019, 7:50 AM
clang-tools-extra/clangd/ClangdLSPServer.cpp
428

these comments (and also the one in Protocol.h) lack context: it's easy to understand what they're saying when looking at this patch, but hard when just looking at the code.

Maybe something like "There are two slightly different versions of the semantic highlighting capabilities: one in the 'main' namespace (supported by Theia), and one in the 'experimental' namespace (supported by VSCode)"

nridge added a subscriber: nridge.EditedJul 27 2019, 5:25 PM

I believe this is not necessary. You can add text document capabilities in the vscode client extension like this:

class SemanticHighlightingFeature implements vscodelc.StaticFeature {
  fillClientCapabilities(capabilities: vscodelc.ClientCapabilities): void {
    const textDocumentCapabilities:
        vscodelc.TextDocumentClientCapabilities & { semanticHighlightingCapabilities?: { semanticHighlighting: boolean } }
        = capabilities.textDocument;
    textDocumentCapabilities.semanticHighlightingCapabilities = { semanticHighlighting: true };
  }
}

...

clangdClient.registerFeature(new SemanticHighlightingFeature());

What this is doing is creating a new type on the fly which extends vscodelc.TextDocumentCapabilities with a new optional field semanticHighlightingCapabilities, and then casts capabilities.textDocument to that type.

jvikstrom abandoned this revision.Jul 30 2019, 2:33 AM

I believe this is not necessary. You can add text document capabilities in the vscode client extension like this:

class SemanticHighlightingFeature implements vscodelc.StaticFeature {
  fillClientCapabilities(capabilities: vscodelc.ClientCapabilities): void {
    const textDocumentCapabilities:
        vscodelc.TextDocumentClientCapabilities & { semanticHighlightingCapabilities?: { semanticHighlighting: boolean } }
        = capabilities.textDocument;
    textDocumentCapabilities.semanticHighlightingCapabilities = { semanticHighlighting: true };
  }
}

...

clangdClient.registerFeature(new SemanticHighlightingFeature());

What this is doing is creating a new type on the fly which extends vscodelc.TextDocumentCapabilities with a new optional field semanticHighlightingCapabilities, and then casts capabilities.textDocument to that type.

That definitely works, I had no idea that was a feature you could use in typescript. Thanks

Abandoning in favor of using type intersection in typescript instead.