[PATCH] D130081: Add foldings for multi-line comment.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:176
 
 // FIXME(kirillbobyrev): Collect comments, PP conditional regions, includes and
 // other code regions (e.g. public/private/protected sections of classes,
----------------
nit: update the fixme, comment is supported now.


================
Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:231
+    while (T->Kind == tok::comment && T != Tokens.end()) {
+      End = offsetToPosition(Code, StartOffset(*T) + OriginalToken(*T).Length);
+      T++;
----------------
we can save some cost if we first get the final End comment, and calculate the offset from it.


================
Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:235
+    if (Start.line < End.line)
+      Result.push_back(ToFoldingRange(Start, End, "comment"));
+  }
----------------
since LSP defines 3 kinds (`region`, `comment`, `import`), I'd suggest defining these string literals in the `Protocol.h`, rather than using the magic string here. 


================
Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:351
+        int c;
+        [[// A comment 
+        // expanding more than
----------------
For this case, my personal preference would be to have 3 different foldings, rather than a union one.

If you think about cases like below, it makes more sense to have individual folding per comment.

```
/* comment 1
*/

/* comment 2
*/
```

```
/* comment 1
*/

// comment 2
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130081