hokein added inline comments.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:255
+ // ArrayRefs to the current line in the highlights.
+ ArrayRef<HighlightingToken> NewLine(Highlightings.data(),
+ Highlightings.data()),
----------------
jvikstrom wrote:
> hokein wrote:
> > IIUC, we are initializing an empty ArrayRef, if so, how about using
> > `NewLine(Highlightings.data(), /*length*/0)`?
> > `NewLine(Highlightings.data(), Highlightings.data())` is a bit weird, it
> > took me a while to understand the purpose.
> I couldn't do that because the `ArrayRef(const T *data, size_t length)` and
> `ArrayRef(const T *begin, const T *end)` were ambiguous. Added a cast to cast
> 0 to a size_t which solved it.
you could also do it like `NewLine(Highlightings.data(), /*length*/0UL);`,
which saves you a `static_cast<>`.
================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:281
+ )cpp"}},
+ {{5},
+ {
----------------
so the empty lines are stored separately, which is not easy to figure out from
the testing code snippet. I think we should make the empty lines more obvious
in the code snippet (maybe use an `Empty` annotations?) , and explicitly verify
the empty lines.
What do you think refining the test like below, just annotate the diff tokens?
I find it is easy to spot the diff tokens.
```
struct Testcase {
Code Before;
Code After;
};
std::vector<TestCase> cases = {
{
R"cpp(
int a;
int b;
int c;
)cpp",
R"cpp(
int a;
$Empty[[]] // int b
int $Variable[[C]];
)cpp"
}
}
oldHighlightings = getSemanticHighlightings(OldAST);
newHighlightings = getSemanticHighlightings(NewAST);
diffHighlightings = diff...;
// verify the diffHighlightings has the expected empty lines ("Empty"
annotations).
// verify the diffHighlightings has the expected highlightings (the regular
annotations);
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64475/new/
https://reviews.llvm.org/D64475
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
