[PATCH] D83914: [clangd] Plan features for FoldingRanges

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:246
+
+          if ([[B && I > 42 || ~([[++I]])]]) {[[
+            ++I;
----------------
sammccall wrote:
> confused about [[++I]] - why is this foldable but not I > 42 etc - is this because of the containing parens?
> 
> FWIW, Personally, I'd want the whole if condition to be  foldable (maybe only if it spans lines), and only paren subexpressions, arg lists etc that span lines to be foldable. It's hard to imagine a UX that could expose folding small parens in a way that's not annoying. (e.g. shows lots of extra UI elements, or prevents folding the outer expression)
Sounds good! Added a separate expressions test to Misc, too.


================
Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:329
+      )cpp",
+      // Argument lists.
+      R"cpp(
----------------
sammccall wrote:
> missing lambdas, blocks, objc methods.
> cover bodies in the same tests?
> capture lists of lambdas?
What kind of blocks? Do you mean statement groups? What do you mean by "cover bodies in the same tests"? What kind of bodies?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83914