[PATCH] D64624: [clangd] Added highlighting to enum constants.

[Johan Vikström via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366045: [clangd] Added highlighting to enum constants. 
(authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64624?vs=209514=209769#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64624/new/

https://reviews.llvm.org/D64624

Files:
  clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
  clang-tools-extra/trunk/clangd/SemanticHighlighting.h
  clang-tools-extra/trunk/clangd/test/semantic-highlighting.test
  clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -119,6 +119,10 @@
   addToken(Loc, HighlightingKind::Enum);
   return;
 }
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::EnumConstant);
+  return;
+}
 if (isa(D)) {
   addToken(Loc, HighlightingKind::Variable);
   return;
@@ -249,6 +253,8 @@
 return "entity.name.type.class.cpp";
   case HighlightingKind::Enum:
 return "entity.name.type.enum.cpp";
+  case HighlightingKind::EnumConstant:
+return "variable.other.enummember.cpp";
   case HighlightingKind::Namespace:
 return "entity.name.namespace.cpp";
   case HighlightingKind::NumKinds:
Index: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
@@ -37,7 +37,8 @@
   {HighlightingKind::Function, "Function"},
   {HighlightingKind::Class, "Class"},
   {HighlightingKind::Enum, "Enum"},
-  {HighlightingKind::Namespace, "Namespace"}};
+  {HighlightingKind::Namespace, "Namespace"},
+  {HighlightingKind::EnumConstant, "EnumConstant"}};
   std::vector ExpectedTokens;
   for (const auto  : KindToString) {
 std::vector Toks = makeHighlightingTokens(
@@ -103,12 +104,19 @@
   }
 )cpp",
 R"cpp(
-  enum class $Enum[[E]] {};
-  enum $Enum[[EE]] {};
+  enum class $Enum[[E]] {
+$EnumConstant[[A]],
+$EnumConstant[[B]],
+  };
+  enum $Enum[[EE]] {
+$EnumConstant[[Hi]],
+  };
   struct $Class[[A]] {
 $Enum[[E]] EEE;
 $Enum[[EE]] ;
   };
+  int $Variable[[I]] = $EnumConstant[[Hi]];
+  $Enum[[E]] $Variable[[L]] = $Enum[[E]]::$EnumConstant[[B]];
 )cpp",
 R"cpp(
   namespace $Namespace[[abc]] {
@@ -118,7 +126,7 @@
   namespace $Namespace[[cde]] {
 struct $Class[[A]] {
   enum class $Enum[[B]] {
-Hi,
+$EnumConstant[[Hi]],
   };
 };
   }
@@ -129,7 +137,7 @@
 $Namespace[[abc]]::$Namespace[[bcd]]::$Namespace[[cde]];
   $Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[AA]];
   $Namespace[[vwz]]::$Class[[A]]::$Enum[[B]] $Variable[[AAA]] =
-$Namespace[[vwz]]::$Class[[A]]::$Enum[[B]]::Hi;
+$Namespace[[vwz]]::$Class[[A]]::$Enum[[B]]::$EnumConstant[[Hi]];
   ::$Namespace[[vwz]]::$Class[[A]] $Variable[[B]];
   ::$Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[BB]];
 )cpp"};
Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.h
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.h
@@ -28,6 +28,7 @@
   Function,
   Class,
   Enum,
+  EnumConstant,
   Namespace,
 
   NumKinds,
Index: clang-tools-extra/trunk/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/trunk/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/trunk/clangd/test/semantic-highlighting.test
@@ -17,6 +17,9 @@
 # CHECK-NEXT:"entity.name.type.enum.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
+# CHECK-NEXT:"variable.other.enummember.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.namespace.cpp"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64624: [clangd] Added highlighting to enum constants.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom marked an inline comment as done.
jvikstrom added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:257
+  case HighlightingKind::EnumConstant:
+return "constant.other.enum.cpp";
   case HighlightingKind::Namespace:

hokein wrote:
> hokein wrote:
> > could you check the tm scope on vscode? They seem to use 
> > `variable.other.enummember`.
> this comment is undone, vscode is using 
> https://github.com/microsoft/vscode/blob/master/extensions/cpp/syntaxes/cpp.tmLanguage.json#L10024
Oh yeah, I was going to write a comment about keeping it as 
`constant.other.enum` or something similar to it. Because it's not really a 
variable. But as we've stuck to the same TM scopes as vscode we should probably 
keep doing that so I'll change to `variable.other.enummember` and land.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64624/new/

https://reviews.llvm.org/D64624



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64624: [clangd] Added highlighting to enum constants.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:257
+  case HighlightingKind::EnumConstant:
+return "constant.other.enum.cpp";
   case HighlightingKind::Namespace:

hokein wrote:
> could you check the tm scope on vscode? They seem to use 
> `variable.other.enummember`.
this comment is undone, vscode is using 
https://github.com/microsoft/vscode/blob/master/extensions/cpp/syntaxes/cpp.tmLanguage.json#L10024


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64624/new/

https://reviews.llvm.org/D64624



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64624: [clangd] Added highlighting to enum constants.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 209514.
jvikstrom marked 2 inline comments as done.
jvikstrom added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64624/new/

https://reviews.llvm.org/D64624

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -37,7 +37,8 @@
   {HighlightingKind::Function, "Function"},
   {HighlightingKind::Class, "Class"},
   {HighlightingKind::Enum, "Enum"},
-  {HighlightingKind::Namespace, "Namespace"}};
+  {HighlightingKind::Namespace, "Namespace"},
+  {HighlightingKind::EnumConstant, "EnumConstant"}};
   std::vector ExpectedTokens;
   for (const auto  : KindToString) {
 std::vector Toks = makeHighlightingTokens(
@@ -103,12 +104,19 @@
   }
 )cpp",
 R"cpp(
-  enum class $Enum[[E]] {};
-  enum $Enum[[EE]] {};
+  enum class $Enum[[E]] {
+$EnumConstant[[A]],
+$EnumConstant[[B]],
+  };
+  enum $Enum[[EE]] {
+$EnumConstant[[Hi]],
+  };
   struct $Class[[A]] {
 $Enum[[E]] EEE;
 $Enum[[EE]] ;
   };
+  int $Variable[[I]] = $EnumConstant[[Hi]];
+  $Enum[[E]] $Variable[[L]] = $Enum[[E]]::$EnumConstant[[B]];
 )cpp",
 R"cpp(
   namespace $Namespace[[abc]] {
@@ -118,7 +126,7 @@
   namespace $Namespace[[cde]] {
 struct $Class[[A]] {
   enum class $Enum[[B]] {
-Hi,
+$EnumConstant[[Hi]],
   };
 };
   }
@@ -129,7 +137,7 @@
 $Namespace[[abc]]::$Namespace[[bcd]]::$Namespace[[cde]];
   $Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[AA]];
   $Namespace[[vwz]]::$Class[[A]]::$Enum[[B]] $Variable[[AAA]] =
-$Namespace[[vwz]]::$Class[[A]]::$Enum[[B]]::Hi;
+$Namespace[[vwz]]::$Class[[A]]::$Enum[[B]]::$EnumConstant[[Hi]];
   ::$Namespace[[vwz]]::$Class[[A]] $Variable[[B]];
   ::$Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[BB]];
 )cpp"};
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -17,6 +17,9 @@
 # CHECK-NEXT:"entity.name.type.enum.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
+# CHECK-NEXT:"constant.other.enum.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.namespace.cpp"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -28,6 +28,7 @@
   Function,
   Class,
   Enum,
+  EnumConstant,
   Namespace,
 
   NumKinds,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -119,6 +119,10 @@
   addToken(Loc, HighlightingKind::Enum);
   return;
 }
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::EnumConstant);
+  return;
+}
 if (isa(D)) {
   addToken(Loc, HighlightingKind::Variable);
   return;
@@ -249,6 +253,8 @@
 return "entity.name.type.class.cpp";
   case HighlightingKind::Enum:
 return "entity.name.type.enum.cpp";
+  case HighlightingKind::EnumConstant:
+return "constant.other.enum.cpp";
   case HighlightingKind::Namespace:
 return "entity.name.namespace.cpp";
   case HighlightingKind::NumKinds:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64624: [clangd] Added highlighting to enum constants.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

mostly good.




Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:122
 }
+if(isa(D)) {
+  addToken(Loc, HighlightingKind::EnumConstant);

nit: clang-format.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:257
+  case HighlightingKind::EnumConstant:
+return "constant.other.enum.cpp";
   case HighlightingKind::Namespace:

could you check the tm scope on vscode? They seem to use 
`variable.other.enummember`.



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:107
 R"cpp(
   enum class $Enum[[E]] {};
   enum $Enum[[EE]] {};

since we have a dedicated testcase for `enum`, could you extend the testcase 
here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64624/new/

https://reviews.llvm.org/D64624



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64624: [clangd] Added highlighting to enum constants.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom created this revision.
jvikstrom added reviewers: hokein, sammccall, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

VSCode does not have a scope for enum constants. So they were placed under 
"constant.other.enum" as that seems to be the most correct scope for enum 
constants. However, this makes theia color them blue (the same color it uses 
for keywords).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64624

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -37,7 +37,8 @@
   {HighlightingKind::Function, "Function"},
   {HighlightingKind::Class, "Class"},
   {HighlightingKind::Enum, "Enum"},
-  {HighlightingKind::Namespace, "Namespace"}};
+  {HighlightingKind::Namespace, "Namespace"},
+  {HighlightingKind::EnumConstant, "EnumConstant"}};
   std::vector ExpectedTokens;
   for (const auto  : KindToString) {
 std::vector Toks = makeHighlightingTokens(
@@ -118,7 +119,7 @@
   namespace $Namespace[[cde]] {
 struct $Class[[A]] {
   enum class $Enum[[B]] {
-Hi,
+$EnumConstant[[Hi]],
   };
 };
   }
@@ -129,9 +130,20 @@
 $Namespace[[abc]]::$Namespace[[bcd]]::$Namespace[[cde]];
   $Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[AA]];
   $Namespace[[vwz]]::$Class[[A]]::$Enum[[B]] $Variable[[AAA]] =
-$Namespace[[vwz]]::$Class[[A]]::$Enum[[B]]::Hi;
+$Namespace[[vwz]]::$Class[[A]]::$Enum[[B]]::$EnumConstant[[Hi]];
   ::$Namespace[[vwz]]::$Class[[A]] $Variable[[B]];
   ::$Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[BB]];
+)cpp",
+R"cpp(
+  enum $Enum[[ABC]] {
+$EnumConstant[[Hi]],
+  };
+  int $Variable[[I]] = $EnumConstant[[Hi]];
+  enum class $Enum[[BC]] {
+$EnumConstant[[A]],
+$EnumConstant[[B]],
+  };
+  $Enum[[BC]] $Variable[[L]] = $Enum[[BC]]::$EnumConstant[[B]];
 )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -17,6 +17,9 @@
 # CHECK-NEXT:"entity.name.type.enum.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
+# CHECK-NEXT:"constant.other.enum.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.namespace.cpp"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -28,6 +28,7 @@
   Function,
   Class,
   Enum,
+  EnumConstant,
   Namespace,
 
   NumKinds,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -119,6 +119,10 @@
   addToken(Loc, HighlightingKind::Enum);
   return;
 }
+if(isa(D)) {
+  addToken(Loc, HighlightingKind::EnumConstant);
+  return;
+}
 if (isa(D)) {
   addToken(Loc, HighlightingKind::Variable);
   return;
@@ -249,6 +253,8 @@
 return "entity.name.type.class.cpp";
   case HighlightingKind::Enum:
 return "entity.name.type.enum.cpp";
+  case HighlightingKind::EnumConstant:
+return "constant.other.enum.cpp";
   case HighlightingKind::Namespace:
 return "entity.name.namespace.cpp";
   case HighlightingKind::NumKinds:


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -37,7 +37,8 @@
   {HighlightingKind::Function, "Function"},
   {HighlightingKind::Class, "Class"},
   {HighlightingKind::Enum, "Enum"},
-  {HighlightingKind::Namespace, "Namespace"}};
+  {HighlightingKind::Namespace, "Namespace"},
+  {HighlightingKind::EnumConstant, "EnumConstant"}};
   std::vector ExpectedTokens;
   for (const auto  :