[PATCH] D44272: [clangd] Support incremental document syncing

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 21 09:50:56 PDT 2018


ilya-biryukov added inline comments.


================
Comment at: clangd/DraftStore.h:36
   /// Replace contents of the draft for \p File with \p Contents.
-  void updateDraft(PathRef File, StringRef Contents);
+  void addDraft(PathRef File, StringRef Contents);
+
----------------
simark wrote:
> ilya-biryukov wrote:
> > Could we add versions from LSP's `VersionedTextDocumentIdentifier` here and make the relevant sanity checks?
> > Applying diffs to the wrong version will cause everything to fall apart, so we should detect this error and signal it to the client as soon as possible.
> I agree that applying diffs to the wrong version will break basically everything, but even if we detect a version mismatch, I don't see what we could do, since we don't have a mean to report the error to the client.  The only thing we could do is log it (which we already do.
If we couldn't apply a diff, we should return errors from all future operations on this file until it gets closed and reopened. Otherwise clangd and the editor would see inconsistent contents for the file and results provided by clangd would be unreliable.
The errors from any actions on the invalid file would actually be visible to the users.

The simplest way to achieve that is to remove the file from `DraftStore` and `ClangdServer` on errors from `updateDraft`.
This will give "calling action on non-tracked file" errors for future operations and the actual root cause can be figured out from the logs.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44272





More information about the cfe-commits mailing list