sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
(My first reaction was that this belongs in semanticHighlights() rather than
toSemanticTokens, but I think you got it right - the single-line restriction is
pretty unnatural from clang's point of view, and cuts across tokens generated
in different places).
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:948
+ } else {
+ // If a tokens length is past the end of the line, it should be treated
as
+ // if the token ends at the end of the line and will not wrap onto the
----------------
This wording is hard for me to follow. I think it's saying:
"If the token spans a line break, truncate it to avoid this"
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:948
+ } else {
+ // If a tokens length is past the end of the line, it should be treated
as
+ // if the token ends at the end of the line and will not wrap onto the
----------------
sammccall wrote:
> This wording is hard for me to follow. I think it's saying:
>
> "If the token spans a line break, truncate it to avoid this"
It seems it would be better to split into one token per line, rather than
simply truncating.
Is truncation for simplicity or is there a reason to prefer it?
FWIW I think this wouldn't be too hard to implement if you reordered the
tokenType/tokenModifiers above so this part is the last step, and just copied
the whole SemanticToken object from the back of the Result. But on the other
hand it's not terribly important, either.
At least I think we should have a comment for the truncate/split tradeoff.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:90
uint32_t Modifiers = 0;
Range R;
----------------
I think this deserves a comment like:
```
// This is clang's token bounds, which may span multiple lines.
// This may be split/truncated to a single line when required by LSP.
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127856/new/
https://reviews.llvm.org/D127856
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
