[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
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`.
----------------
hokein wrote:
> 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 {
>   ...
> }
> ```
> 
> 
> 
> 
right. i think for comments we've got a bunch of different cases. for the block comments, i think we want to exclude the last line all the time again, e.g:
```
/*[[ foo
     bar
]]*/
```
i got 2 reasons:
1- this is somewhat the same as being able to see opening and closing brace.
2- when there's any token following the comment-block terminator, we actually mustn't hide them.

for back-to-back single-line comments, we should always fold the last line as well, eg:
```
//[[ foo
// bar]]
```

so i think the model here is always include the first line and don't include the last line if there are tokens after the folding range on the same line, as you've already proposed here, but we need to make sure that folding range doesn't include the terminators (like `}` and `*/`). That way we'll ensure a line with a terminator is always visible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131154