[PATCH] D75739: [clangd][vscode] Enable dot-to-arrow fixes in clangd completion.

Mon Mar 9 07:30:44 PDT 2020

sammccall added a comment.

In D75739#1912051 <https://reviews.llvm.org/D75739#1912051>, @hokein wrote:

> In D75739#1909637 <https://reviews.llvm.org/D75739#1909637>, @sammccall wrote:
>
> > So the core of this is a better version of my patch, thanks!
> >  It does work around the problem with dot-to-arrow, which is useful and we should definitely enable that.
> >
> > We get into this mess because we merge two edits together, I think to avoid LSP's vague prohibition about edits related to the cursor. My vote would be:
> >
> > 1. check this patch in, but without claiming it as the correct and carefully architected fix for dot-to-arrow
> > 2. try turning off range-merging, and test it in a couple of clients (including vscode *without* this patch)
> > 3. if it works, make that change in clangd
>
>
> I think if we do that, we are likely to go back to the issue <https://github.com/microsoft/language-server-protocol/issues/543> that sending additionalEdit related to the cursor, and LSP clearly states it is not supported -- `Additional text edits should be used to change text unrelated to the current cursor position`, and it would not work in VSCode.

Sure, well if it doesn't work, then we should keep this hack instead.

>> However I actually think the dot-to-arrow problem is our bug at this point. LSP is vague but the discussion on language-server-protocol#898 very sensibly links filterText to textEdit.range. If we're sending filterText = "foo", textEdit.range = ".f" then I don't think that *is* a match.
> 
> Regarding the mismatch between filterText and textEdit.range, there is another way to make them matched -- sending `filterText=".foo"` (rather than `foo`) from clangd,  vscode should work without this patch, but it looks like a hack in clangd to me...

Yes. And worse, it's a hack that's pretty hard to implement :-)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75739