[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:275
+bool operator<(const HighlightingToken &L, const HighlightingToken &R) {
+  return std::tie(L.R, L.Kind) < std::tie(R.R, R.Kind);;
+}
----------------
nit: remove the redundant `;`.


================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:35
+           std::vector<int>>
+getHighlightingsAnnotated(llvm::StringRef Code) {
   Annotations Test(Code);
----------------
could we split it into three functions?

- getExpectedHighlightings
- getActualHighlightings
- getExpectedEmptyLines




================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:70
 
+void checkDiffedHighlights(const char *OrgCode, const char *NewCode) {
+  std::vector<HighlightingToken> CompleteTokens1;
----------------
nit: OrgCode => OldCode, use llvm::StringRef


================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:98
+  for (unsigned I = 0, End = ActualDiffed.size(); I < End; ++I)
+    llvm::sort(ActualDiffed[I].Tokens);
+
----------------
I think the tokens are also sorted, as we pass two sorted lists of highlightings to `diffHighlightings`.


================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:282
+  std::vector<std::pair<const char *, const char *>> TestCases{{
+                                                                   R"cpp(
+      int $Variable[[A]]
----------------
nit: the code indentation is strange


================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:283
+                                                                   R"cpp(
+      int $Variable[[A]]
+      double $Variable[[B]];
----------------
The annotations in the OldCode are not used -- you only use the actual highlightings in the `checkDiffedHighlights`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475