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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: unittests/clangd/DraftStoreTests.cpp:27
+
+static int rangeLength(StringRef Code, const Range &Rng) {
+  size_t Start = positionToOffset(Code, Rng.start);
----------------
ilya-biryukov wrote:
> no need for `static`, since function is already inside an anonymous namespace.
> remove it?
This comment is marked as done, but not addressed.


================
Comment at: unittests/clangd/DraftStoreTests.cpp:38
+// Send the changes one by one to updateDraft, verify the intermediate results.
+
+static void stepByStep(llvm::ArrayRef<IncrementalTestStep> Steps) {
----------------
ilya-biryukov wrote:
> NIT: remove empty line after comment?
> NIT: make the comment doxygen-like (i.e. start with `///`)?
This comment is marked as done, but not addressed.


================
Comment at: unittests/clangd/DraftStoreTests.cpp:42
+  Annotations InitialSrc(Steps.front().Src);
+  const char Path[] = "/hello.cpp";
+
----------------
ilya-biryukov wrote:
> NIT: use `constexpr StringLiteral Path("/hello.cpp")` instead?
> It's a `StringRef` with size known at compile-time.
This comment is marked as done, but not addressed.


================
Comment at: unittests/clangd/DraftStoreTests.cpp:58
+    llvm::Expected<std::string> Result = DS.updateDraft(Path, {Event});
+    EXPECT_TRUE(!!Result);
+    EXPECT_EQ(*Result, SrcAfter.code());
----------------
ilya-biryukov wrote:
> Use `ASSERT_TRUE(!!Result)`, the code below won't work for failure results anyway.
This comment is marked as done, but not addressed.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44272