simark marked 2 inline comments as done. simark added inline comments.
================ Comment at: clangd/ClangdServer.h:159 /// constructor will receive onDiagnosticsReady callback. void addDocument(PathRef File, StringRef Contents, + WantDiagnostics WantDiags = WantDiagnostics::Auto, ---------------- simark wrote: > ilya-biryukov wrote: > > I don't think we should do this on `ClangdServer` level. We will have to > > copy the whole file anyway before passing it to clang, so there are no > > performance wins here and it complicates the interface. > > I suggest we update the document text from diffs on `ClangdLSPServer` level > > and continue passing the whole document to `ClangdServer`. > > It would mean `DraftMgr` will need to move to `ClangdLSPServer`, but it's > > fine. > > > `ClangdServer` also needs `DraftMgr` for `forceReparse` and > `reparseOpenedFiles`. I guess that too would move to `ClangdLSPServer`? I'm > just not sure how to adapt the unit tests, since we don't have unittests > using `ClangdLSPServer` (yet). Actually, `forceReparse` doesn't really need `DraftMgr` (it's only used for the assert), only `reparseOpenedFiles` needs it. So in the test, we can manually call `forceReparse` on all the files by hand... this just means that `reparseOpenedFiles` won't be tested anymore. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44272 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
