hokein added a comment.
thanks mostly good. Thinking more about the name, we should align with the LSP
proposal, see my comments inline.
================
Comment at: clang-tools-extra/clangd/SemanticHighlight.cpp:20
+// Collects all semantic tokens in an ASTContext.
+class SemanticTokenCollector
+ : public RecursiveASTVisitor<SemanticTokenCollector> {
----------------
HighlightingTokenCollector.
================
Comment at: clang-tools-extra/clangd/SemanticHighlight.h:17
+
+enum class SemanticHighlightKind {
+ Variable,
----------------
LSP proposal is using `Highlighting` rather than `Highlight`, let's align with
the LSP proposal, using `Highlighting` in our names (comments, function,
class, and files).
The name seems too verbose, let's drop the `Semantic`, just use
`HighlightingKind`.
================
Comment at: clang-tools-extra/clangd/SemanticHighlight.h:23
+// Contains all information needed for the highlighting a token.
+struct SemanticToken {
+ SemanticHighlightKind Kind;
----------------
here use `HighlightingToken`.
================
Comment at: clang-tools-extra/clangd/SemanticHighlight.h:32
+// main AST.
+std::vector<SemanticToken> getSemanticHighlights(ParsedAST &AST);
+
----------------
getSemanticHighlightings.
================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightTests.cpp:46
+
+TEST(SemanticTokenCollector, GetsCorrectTokens) {
+ const char *TestCases[] = {
----------------
nit: SemanticTokenCollector => SemanticHighlighting
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63559/new/
https://reviews.llvm.org/D63559
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
