[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D131154#3706062 <https://reviews.llvm.org/D131154#3706062>, @kadircet wrote:

> thanks, i think we should change the behaviour to not fold the last line in any case.

+1, I think we should always do it for all bracket cases, but we probably don't want it for other cases (comment), right?



================
Comment at: clang-tools-extra/clangd/Protocol.cpp:395
+    if (auto *Folding = TextDocument->getObject("foldingRange")) {
+      if (auto LineFolding = Folding->getBoolean("lineFoldingOnly")) {
+        R.LineFoldingOnly = *LineFolding;
----------------
nit: remove the `{}` for the single-statement body.


================
Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:225
+                                       Position End) {
+    // For LineFoldingsOnly clients, do not fold the last line if it
+    // contains tokens after `End`.
----------------
kadircet wrote:
> i don't think the extra check for "if there are more tokens" is really useful here, in a scenario like:
> ```
> if (foo) {
>   ...
> }
> ```
> do we actually want to fold away the closing `}` as well ? (i don't think we do)
Yeah, it also introduces some inconsistencies (depending on whether there are trailing comments after the closed brackets).

We probably want the folding to work the same way for the following cases.

```
namespace foo {
 ...
} // namespace foo
```

```
namespace foo {
  ...
}
```






Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131154