[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked an inline comment as done.
kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:644
+          assert(It.second.Edits && "TextEdits hasn't been generated?");
+          if (auto Draft = DraftMgr.getDraft(It.first())) {
+            llvm::StringRef Contents = *Draft;
----------------
sammccall wrote:
> ilya-biryukov wrote:
> > This callback is called asynchronously and the version inside `DraftMgr` may be newer than the one we used to calculate the original offsets inside replacements at this point.
> > 
> > We should not rely on `DraftMgr`, doing conversions on the source buffers inside `SourceManager` is probably the most reliable option we have.
> I think you've misunderstood what this code is doing (it needs comments!)
> 
> The SourceManager contains the code that we've calculated the edits against, by definition. So checking against it does nothing.
> 
> When we send the edits to the client, it's going to apply them to the file if it's not open, but to the dirty buffer if it is open.
> If the dirty buffer has different contents than what we saw (and calculated edits against) then the edits will be wrong - that's what this code guards against.
> 
> So looking at the latest available content in the draftmgr (at the end of the request, not the start!) is the right thing here, I think.
> This callback is called asynchronously and the version inside DraftMgr may be newer than the one we used to calculate the original offsets inside replacements at this point.

That's exactly what we are checking though?

We want to bail out if user has a "different" version of the source code in their editor by the time we generated edits. Since editors will most likely try to apply the edits onto the version in editor.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66637/new/

https://reviews.llvm.org/D66637