hokein added a comment.
just some random comments from my side.
> At least for the AST-based folds, I'd like some formalism like:
+1. I think we need some general rules (mental model) to guide us how the
folding works.
Some editors (e.g. VSCode) have their builtin folding-range, it is worth to
investigate how the built folding-range interacts with one provided by the
language server.
From my experience, VSCode's built folding-range kind of works though this is
not perfect, would be nice to have some concrete examples see the improvements,
we may want to prioritize those during the implementation.
================
Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:261
+
+ if (B && ([["subexpression" != "multi-line" &&
+ 1 > 0]]))
----------------
this example seems vague, it is folded because
1. it is cross-line
2. it is wrapped by `()`
or both?
================
Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:284
+ )cpp",
+ // For.
+ R"cpp(
----------------
nit: for completeness, add a for-range case.
================
Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:326
+ int Y = 42;
+ bool B = 15;]]
+ if ([[B]]) {[[ ++X; ]]}
----------------
not sure about this case.
if we decide to support this (btw, the test on L242 should be adjusted too),
would be nice to support the following usages.
```
// forward decls
[[class A;
class B;
class C;]]
// using decls;
[[using ns::A;
using ns::B;
using ns::C;]]
```
================
Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:556
+ R"cpp(
+ //[[ Some documentation.
+ // @param
----------------
this `[[` seems mismatched, if I read the code correctly.
btw, this example contains too many brackets, considering split into simpler
ones.
================
Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:598
+
+ #include [["math.h"]]
+ )cpp",
----------------
another option: we could just fold the whole preamble region, it seems easier,
and provides a visible to see preamble range :), the downside is that any
comments/macros will be folded together.
================
Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:601
+ R"cpp(
+ #define true [[false]]
+
----------------
These macros are quite simple, I'm not sure this is useful to fold them. I'd
just not fold it, but if the macro body is complicated (multi-lines), it is
worth folding it.
================
Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:679
+ struct BarStruct {[[]]};
+ struct FwdDeclaration;
+ )cpp",
----------------
looks like we're missing case of inheritance, e.g. a class inherits multiple
classes
```
class B : public A,
private C {};
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83914/new/
https://reviews.llvm.org/D83914
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
