sammccall added a comment.
I'd suggest splitting out the DocumentSymbol changes into a separate patch from
the FoldingRanges patches.
================
Comment at: clang-tools-extra/clangd/FindSymbols.cpp:140
SourceLocation NameLoc = nameLocation(ND, SM);
// getFileLoc is a good choice for us, but we also need to make sure
// sourceLocToPosition won't switch files, so we call getSpellingLoc on top
of
----------------
this whole block seems likely to be obsolete if you switch to
toHalfOpenFileRange
================
Comment at: clang-tools-extra/clangd/FindSymbols.cpp:167
SI.range =
Range{sourceLocToPosition(SM, BeginLoc), sourceLocToPosition(SM,
EndLoc)};
+ // Include trailing '}' token for most symbol kinds.
----------------
As far as I can tell, this line is just always wrong: EndLoc points at the
*start* of the last token in the range, so the last token will be omitted from
the range in all cases.
================
Comment at: clang-tools-extra/clangd/FindSymbols.cpp:168
Range{sourceLocToPosition(SM, BeginLoc), sourceLocToPosition(SM,
EndLoc)};
+ // Include trailing '}' token for most symbol kinds.
+ if (SK == SymbolKind::Class || SK == SymbolKind::Struct ||
----------------
I can't see any conceptual reason for this to be conditional.
================
Comment at: clang-tools-extra/clangd/SemanticSelection.h:28
+llvm::Expected<std::vector<FoldingRange>> getFoldingRanges(ParsedAST &AST);
+
----------------
Can we still have a high-level description of what this function does?
```
Returns a list of ranges whose contents might be collapsible in an editor.
This should include large scopes, preprocessor blocks etc.
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82436/new/
https://reviews.llvm.org/D82436
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
