sammccall 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;
----------------
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.
================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:646
+ llvm::StringRef Contents = *Draft;
+ if (llvm::SHA1::hash(llvm::makeArrayRef(Contents.bytes_begin(),
+ Contents.size())) !=
----------------
We're comparing here content in the source manager (which was read as bytes
from disk, and converted to utf8) to that from LSP (which was sent as unicode
text by the editor, and converted to utf8).
Are editors going to preserve the actual line endings from disk when sending
LSP, or are they going to use the "array-of-lines" from their editor's native
model? (e.g. join(lines, '\n'))
If the latter, we'd need to normalize line endings, possibly strip trailing
line endings, etc.
I ask because clangd-vim definitely has an "array-of-lines" model :-)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66637/new/
https://reviews.llvm.org/D66637
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
