[PATCH] D97983: [clangd] strictly respect formatting range

[Quentin Chateau via Phabricator via cfe-commits]

qchateau created this revision.
qchateau added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
qchateau requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Clang format will often format a range of code wider then the requested format range. Returning a replacement outside of the requested range can mess with clients. Keeping the formatting in range of what the client asks gives more expected results and will limit the impact of partial formatting in diffs.

A specific example: a problem often occurs when using VSCode in "format modified lines" mode. The client will ask clangd to format two non-continugous lines, separately. Clangd replies to both requests with replacements for both lines, effectively providing four replacements in total. VSCode will either apply replacements twice (messing up the results) or simply error out and skip the formatting.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97983

Files:
  clang-tools-extra/clangd/ClangdServer.cpp


Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -423,10 +423,24 @@
     if (!Changed)
       return CB(Changed.takeError());
 
-    CB(IncludeReplaces.merge(format::reformat(
+    auto Replacements = IncludeReplaces.merge(format::reformat(
         Style, *Changed,
         tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
-        File)));
+        File));
+    clang::tooling::Replacements ReplacementsInRange;
+    for (const auto &Repl : Replacements) {
+      tooling::Range ReplRange{
+          Repl.getOffset(),
+          std::max<unsigned int>(Repl.getLength(),
+                                 Repl.getReplacementText().size()),
+      };
+      if (ReplRange.overlapsWith(Ranges.front())) {
+        if (auto Err = ReplacementsInRange.add(Repl)) {
+          CB(std::move(Err));
+        }
+      }
+    }
+    CB(ReplacementsInRange);
   };
   WorkScheduler->runQuick("Format", File, std::move(Action));
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97983.328312.patch
Type: text/x-patch
Size: 1105 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210304/ffc2ec77/attachment.bin>