[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

This revision was automatically updated to reflect the committed changes. Closed by commit rL327711: Move DraftMgr from ClangdServer to ClangdLSPServer (authored by simark, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44408 Files: cl

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

simark updated this revision to Diff 138581. simark marked 2 inline comments as done. simark added a comment. Address latest review comments. Also, update the comments in DraftStore.{h,cpp} that were outdated. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44408 Files: clang

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.h:78 + /// Calls forceReparse() on all currently opened files. + /// As a result, this method may be very expensive. NIT: there is no `forceReparse()` anymore, maybe remove its mention fr

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

simark updated this revision to Diff 138569. simark marked 6 inline comments as done. simark added a comment. Changes based on review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44408 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServ

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:495 +llvm::Optional ClangdLSPServer::getDocument(PathRef File) { + llvm::Optional Contents = DraftMgr.getDraft(File); + if (!Contents) This function is equivalent to `return DraftMgr.ge

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

simark updated this revision to Diff 138490. simark added a comment. Rebase on current master, addressing previous comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44408 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/Cla

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

ilya-biryukov added a comment. In https://reviews.llvm.org/D44408#1037520, @simark wrote: > I see, I made this as a separate patch: > > https://reviews.llvm.org/D44484 I LGTMed it, so feel free to submit it. However, if you do it in this patch it's also fine. Repository: rCTE Clang Tools Ex

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

simark added a comment. In https://reviews.llvm.org/D44408#1037327, @ilya-biryukov wrote: > In https://reviews.llvm.org/D44408#1036941, @simark wrote: > > > I don't see how to avoid adding the Contents parameter to the codeComplete > > and signatureHelp methods, since they needed to fetch the do

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

ilya-biryukov added a comment. In https://reviews.llvm.org/D44408#1036941, @simark wrote: > I don't see how to avoid adding the Contents parameter to the codeComplete > and signatureHelp methods, since they needed to fetch the document from the > draft manager and pass it to the backend. You

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

simark added a comment. I don't see how to avoid adding the Contents parameter to the codeComplete and signatureHelp methods, since they needed to fetch the document from the draft manager and pass it to the backend. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44408

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

simark updated this revision to Diff 138344. simark added a comment. Rebase on https://reviews.llvm.org/D44462 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44408 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

simark added a comment. In https://reviews.llvm.org/D44408#1036846, @ilya-biryukov wrote: > We shouldn't add `Contents` parameter to every method, it would defeat the > purpose of caching ASTs and won't allow to properly manage lifetimes of the > indexes. Makes sense. > The most tricky part

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

ilya-biryukov added a reviewer: ilya-biryukov. ilya-biryukov requested changes to this revision. ilya-biryukov added a comment. This revision now requires changes to proceed. We shouldn't add `Contents` parameter to every method, it would defeat the purpose of caching ASTs and won't allow to prop

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

simark updated this revision to Diff 138253. simark added a comment. Rebase Non-trivial rebase on today's master. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44408 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

simark created this revision. Herald added subscribers: cfe-commits, ioeric, jkorous-apple, ilya-biryukov, mgorny, klimek. This patch moves the draft manager closer to the edge of Clangd, from ClangdServer to ClangdLSPServer. This will make it easier to implement incremental document sync, by ma