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

This revision was automatically updated to reflect the committed changes. Closed by commit rG7a514c9bf8f2: [clangd] Implement textDocument/foldingRange (authored by kbobyrev). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82436/new/

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

kbobyrev updated this revision to Diff 277683. kbobyrev marked an inline comment as done. kbobyrev added a comment. Remove llvm::Optional from character fields. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82436/new/

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

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Nice, thanks! Comment at: clang-tools-extra/clangd/Protocol.h:1523 + unsigned startLine = 0; + llvm::Optional startCharacter; + unsigned endLine = 0;

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

kbobyrev updated this revision to Diff 277382. kbobyrev added a comment. Rebase on top of master. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82436/new/ https://reviews.llvm.org/D82436 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

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

kbobyrev updated this revision to Diff 277381. kbobyrev marked 5 inline comments as done. kbobyrev added a comment. Isolate DocumentSymbol range changes into D83668 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

sammccall added inline comments. Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:205 + +TEST(SemanticSelection, FoldingRanges) { + const char *Tests[] = { nit, the test is usually named after the API/feature, so this should be

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

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

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

kbobyrev updated this revision to Diff 277359. kbobyrev added a comment. Updated a couple of comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82436/new/ https://reviews.llvm.org/D82436 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

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

kbobyrev updated this revision to Diff 277357. kbobyrev marked 4 inline comments as done. kbobyrev added a comment. Add tests and fix DocumentSymbol ranges. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82436/new/ https://reviews.llvm.org/D82436

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

sammccall added a comment. In D82436#2142631 , @kbobyrev wrote: > In D82436#2141953 , @sammccall wrote: > > > Tests :-) > > > I was hoping glorious DocumentSymbols super tested API would shield me from > that :P

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

kbobyrev marked an inline comment as done. kbobyrev added a comment. In D82436#2141953 , @sammccall wrote: > Tests :-) I was hoping glorious DocumentSymbols super tested API would shield me from that :P Didn't think of reasonable testing strategy but

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

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

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

kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/FindSymbols.cpp:281 + Range.startCharacter = Symbol.range.start.character; + Range.endLine = Symbol.range.end.line; + Range.endCharacter = Symbol.range.end.character; sammccall wrote: >

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

kbobyrev updated this revision to Diff 275929. kbobyrev marked 7 inline comments as done. kbobyrev added a comment. Hide FoldingRanges feature behind the flag. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82436/new/

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

sammccall added a comment. I think this looks reasonable, but I'd like to make sure we have a plan going forward otherwise the behavior/assumptions tend to calcify. I think RAV and adding other region types are clear, but maybe we should discuss lines/filtering behavior offline a bit.

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

kbobyrev updated this revision to Diff 274741. kbobyrev marked 7 inline comments as done. kbobyrev added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82436/new/ https://reviews.llvm.org/D82436 Files:

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

kbobyrev updated this revision to Diff 274743. kbobyrev added a comment. Remove unused variable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82436/new/ https://reviews.llvm.org/D82436 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

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

kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/FindSymbols.cpp:281 + Range.startCharacter = Symbol.range.start.character; + Range.endLine = Symbol.range.end.line; + Range.endCharacter = Symbol.range.end.character; sammccall wrote: > How

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

sammccall added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:967 + URIForFile FileURI = Params.textDocument.uri; + Server->foldingRanges( + Params.textDocument.uri.file(), one we fix the signature, you should just be able to

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

kbobyrev added a comment. @sammccall ping 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

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

kbobyrev created this revision. kbobyrev added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. This patch introduces basic textDocument/foldingRange support. It relies on