[PATCH] D50449: [clangd] Support textEdit in addition to insertText.

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 10 05:38:02 PDT 2018


ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks for the change!
Could we add an option to clangd to switch it on? (VSCode does not work, but our hacked-up ycm integration seems to work, right?)



================
Comment at: clangd/CodeComplete.cpp:1310
+  // other.
+  for (const auto &FixIt : FixIts) {
+    if (IsRangeConsecutive(FixIt.range, LSP.textEdit->range)) {
----------------
kadircet wrote:
> ilya-biryukov wrote:
> > Maybe keep the `reserve` call? (we could reserve one extra element, but that's fine)
> Actually we could have much more than one extra element, not for the current situation but may be in the future when we have more fixit completions.
We can't have more than one edit adjacent to the completion identifier, right? Otherwise they'll conflict.
It is possible to have multiple edits which are consectutive and the last of them is adjacent to the completion identifier, though. But I don't think we handle those cases here anyway.

But I'm not too worried about leaving out the reserve call either. At the very worst we could waste some memory on a single completion item, but we shouldn't keep too many around at the same time anyway, so feel free to ignore this one.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50449





More information about the cfe-commits mailing list