[PATCH] D97123: [clangd] Support FixIts that use InsertFromRange instead of inserting raw text

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.


================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:695
+      if (Insert.empty() && FixIt.InsertFromRange.isValid()) {
+        bool InvalidInsert = false;
+        Insert = Lexer::getSourceText(FixIt.InsertFromRange, SM, *LangOpts,
----------------
kadircet wrote:
> nit: I would rather make `!Invalid` a condition and just make use of it in here as well.
Good point, that's much nicer.


================
Comment at: clang-tools-extra/clangd/SourceCode.cpp:555
+    auto Insert = Lexer::getSourceText(FixIt.InsertFromRange, M, L, &Invalid);
+    if (!Invalid)
+      Result.newText = Insert.str();
----------------
kadircet wrote:
> it feels like correct semantics would be to make this function fail now. as the resulting value will likely be used for editing the source code and we don't want to propagate some garbage.
While I agree with this in principal. We currently don't handle the case where RemoveRange doesn't correspond to a FileCharRange which would likely need propagating. Though likely in a separate patch. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97123