[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks, I'm fine with the current approach, feel free to add unittests.



================
Comment at: clang-tools-extra/clangd/Protocol.h:1212
   std::string Tokens;
+  /// Is the line in an inactive preprocessor branch?
+  bool IsInactive = false;
----------------
could you add some documentation describing this is a clangd extension?


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:469
+      auto &AddedLine = DiffedLines.back();
+      for (auto Iter = AddedLine.Tokens.begin();
+           Iter != AddedLine.Tokens.end();) {
----------------
it took me a while to understand this code,

If the NewLine is `IsInactive`, it just contains a single token whose range is [0, 0), can't we just?

```

if (NewLine.back().Tokens.empty()) continue;

bool InactiveLine = NewLine.back().Tokens.front().Kind == InactiveCode;
assert(InactiveLine && NewLine.back().Tokens.size() == 1 && "IncativeCode must have a single token");
DiffedLines.back().IsInactive = true;
```


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:46
   Macro,
+  InactiveCode,
 
----------------
we should document this Kind as it is different than other kinds, it is for line-style I believe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536