Added the code for the StaticFeature that must be registered to the client. Also decoding the notification data into objects. Did not register it to the client yet.
Details
- Reviewers
hokein ilya-biryukov - Commits
- rG020eea0c16a2: [clangd] Added the vscode SemanticHighlighting feature code but did not enable…
rCTE368568: [clangd] Added the vscode SemanticHighlighting feature code but did not enable…
rL368568: [clangd] Added the vscode SemanticHighlighting feature code but did not enable…
Diff Detail
- Repository
- rL LLVM
Event Timeline
| clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts | ||
|---|---|---|
| 37 ↗ | (On Diff #214334) | nit: name it SemanticHighlightingFeature. |
| 38 ↗ | (On Diff #214334) | I believe the type of lookup table returned from clangd is a string[][]. I'd name it scopeLookupTable |
| 62 ↗ | (On Diff #214334) | I think the params here is defined as SemanticHighlightingParams from the LSP proposal, could we use the names as defined in the LSP for these structures (e.g. SemanticHighlightingParams, SemanticHighlightingInformation)? |
Address comments.
| clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts | ||
|---|---|---|
| 62 ↗ | (On Diff #214334) | Renaming HighlightingToken to SemanticHighlightingToken to be consistent with those names as well. |
| clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts | ||
|---|---|---|
| 8 ↗ | (On Diff #214364) | sorry for not being clear in the previous comment, I think we should mirror definitions from the LSP proposal, the same to the SemanticHighlightingInformation. You may need to add a vscode-languageserver-types dependency. |
| 32 ↗ | (On Diff #214364) | I think here we should use SemanticHighlightingParams in the generic type parameter list |
| 38 ↗ | (On Diff #214364) | could you add a comment on this? |
| 63 ↗ | (On Diff #214364) | this seems doesn't do anything, can we defer it to a follow-up patch? |
mostly good with a few nits.
| clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts | ||
|---|---|---|
| 9 ↗ | (On Diff #214613) | could you add a comment describing this is a mirror of LSP definition? |
| 16 ↗ | (On Diff #214613) | nit: I'd remove this blank line to make SemanticHighlightingParams and SemanticHighlightingInformation group closer as they are mirrors of LSP definitions. |
| 27 ↗ | (On Diff #214613) | the comment seems not precise to me, this is the data decoded from the base64-encoded string sent by clangd. |
| 33 ↗ | (On Diff #214613) | nit: scope lookup table. |
| 34 ↗ | (On Diff #214613) | nit: maybe name it scopeIndex. |