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

Mon Mar 9 02:39:34 PDT 2020

hokein marked 2 inline comments as done.
hokein added a comment.

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.

> 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...

> How does this handle cases where an object overloads the -> operator or worse still overloads the operator and has conflicting names when accessed with .  or ->.

This patch shouldn't affect the existing behavior for the mentioned case,  it is handled by Sema. VSCode will show both candidates (one of them will correct the `.` while the other will not).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75739