hokein added inline comments.
================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:333
+ ClangdServerOpts.SemanticHighlighting =
+ Params.capabilities.SemanticHighlighting;
if (Params.rootUri && *Params.rootUri)
----------------
I'd put the logic here rather than in `ClangdServer.cpp`, just
`ClangdServerOpts.SemanticHighlighting = ClangdServerOpts.HiddenFeatures &&
Params.capabilities.SemanticHighlighting;`.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:131
+ if (TokenLines.find(LineIdx) == TokenLines.end()) {
+ TokenLines[LineIdx] = {Token};
+ } else {
----------------
here we lookup the map twice, we could save one cost of lookup.
`TokenLines[LindIdx].push_back(Token)` should work.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:143
+ for (const auto &Token : Line.second) {
+ write32be(Token.R.start.character, OS);
+ write16be(Token.R.end.character - Token.R.start.character, OS);
----------------
could we have some comments explaining the details about encoding Token here?
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:144
+ write32be(Token.R.start.character, OS);
+ write16be(Token.R.end.character - Token.R.start.character, OS);
+ write16be(static_cast<int>(Token.Kind), OS);
----------------
if the token is across multiple line, we will emit an ill-formed results.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:44
+
+// Converts a vector of HighlightingTokens to a vector of
+// SemanticHighlightingInformation. The token string in
----------------
just: convert to LSP's semantic highlighting information.
================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:103
+ std::vector<HighlightingToken> Tokens{
+ HighlightingToken{HighlightingKind::Variable,
+ Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
----------------
we could drop the explicit `HighlightingToken`, just
```
... Tokens = {
{HighlightingKind::Variable, Range {...}},
{....}
}
```
================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:104
+ HighlightingToken{HighlightingKind::Variable,
+ Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
+ HighlightingToken{HighlightingKind::Function,
----------------
`Range{ /*start*/{3, 8}, /*end*/{3, 12} }` should be compilable.
================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:112
+ std::vector<SemanticHighlightingInformation> Correct = {
+ SemanticHighlightingInformation{1, "AAAAAQAEAAA="},
+ SemanticHighlightingInformation{3, "AAAACAAEAAAAAAAEAAMAAQ=="}};
----------------
here as well, just `{ {1, "...." } }`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63919/new/
https://reviews.llvm.org/D63919
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
