[PATCH] D82436: [clangd] Implement textDocument/foldingRange

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Tests :-)



================
Comment at: clang-tools-extra/clangd/Protocol.h:1522
+struct FoldingRange {
+  unsigned startLine;
+  llvm::Optional<unsigned> startCharacter;
----------------
nit: =0 (and on endLine)


================
Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:107
+  // FIXME(kirillbobyrev): getDocumentSymbols() is conveniently available but
+  // limited (e.g. doesn't yield blocks inside functions). Replace this with a
+  // more general RecursiveASTVisitor implementation instead.
----------------
I think this should mention the contents-of-blocks vs whole-nodes issue too.


================
Comment at: clang-tools-extra/clangd/SemanticSelection.h:28
 
+/// Retrieves folding ranges in the "main file" section of given AST.
+llvm::Expected<std::vector<FoldingRange>> getFoldingRanges(ParsedAST &AST);
----------------
I think "main file" section is just a for-now implementation detail, I'd leave this out.


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:300
+opt<bool> FoldingRanges{
+    "folding-rangees",
+    cat(Features),
----------------
rangees -> ranges


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82436