kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:202
FR.endLine = End.line;
+ FR.startCharacter = Start.character;
FR.endCharacter = End.character;
----------------
can you put it back to its previous location (i.e. right after startline)
================
Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:251
+ // Always show last line of a block comment.
+ if (StartPosition(*LastComment).line != EndPosition(*LastComment).line) {
+ End.line--;
----------------
i think this will end up trimming the foldings for cases like:
```
// foo\
bar\
baz
```
can we check the comment introducer instead? i.e. `//` vs `/*`. also let's
amend the startposition to be `Startoffset +2` similar to what we do with
braces, to make sure starting sentinel is not included in the folding range.
================
Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:391
+ /* No folding for this comment.
+ */ int b_token=0;
+ )cpp",
----------------
usaxena95 wrote:
> kadircet wrote:
> > it'd be nice to have a test case like:
> > ```
> > template <[[typename foo, class bar]]> struct baz {[[]]};
> > ```
> >
> > which isn't useful for line-only folding clients, but something we should
> > be able to support going forward.
> I don't understand. We do not return any single-line ranges.
> I think lines are never long enough to have helpful foldings (atleast in
> formatted code). It would only clutter IMO.
> I don't understand. We do not return any single-line ranges.
I think that's just a convenient solution we went with today because it's
convenient and easier. i don't think that's the final UX we want to have.
> I think lines are never long enough to have helpful foldings (atleast in
> formatted code). It would only clutter IMO.
Even if that was the case template arguments can get really long, especially in
the presence of SFINAE, so there's definitely value in folding them.
Also just to be clear, i wasn't suggesting to have the implementation for them.
I was just saying let's leave the testcases here, in disabled state, to remind
us to have support for them in the future (and properly handle even for single
line case).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131154/new/
https://reviews.llvm.org/D131154
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
