[PATCH] D84012: [clangd] Exclude preprocessed-to-nothing tokens from selection

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!



================
Comment at: clang-tools-extra/clangd/Selection.cpp:228
+         Buf.expansionsAffecting(Sel)) {
+      if (X.Expanded.empty()) {
+        for (const syntax::Token &Tok : X.Spelled) {
----------------
nit: reduce nesting via

```
if(!X.Expanded.empty())
  continue
```


================
Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:554
+TEST(SelectionTest, DisabledPreprocessor) {
+  const char *Case = R"cpp(
+    namespace ns {
----------------
this case isn't disabled PP, moreover seems to be already tested in  CommonAncestor, see:

```
      {
          R"cpp(
            void foo();
            #define CALL_FUNCTION(X) X^()^
            void bar() { CALL_FUNCTION(foo); }
          )cpp",
          nullptr,
      },
```

maybe also put a couple of tests for the macroname and the directive itself? (or just extend the test above to whole directive line?)


================
Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:568
+    #if 0
+    void fu^nc();
+    #endif
----------------
nit: i am not sure if this is worth it's own test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84012