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

Fri Mar 6 07:41:51 PST 2020

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

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.

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

================
Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:117
         // fuzzymatch score for every item.
         // The sortText (which reflects clangd ranking) breaks the tie.
         //
----------------
"This also prevents VSCode from filtering out any results due to differences in how fuzzy filtering is applied."

================
Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:125
             let items = (Array.isArray(list) ? list : list.items).map(item => {
+              // ^ is the position.
+              //  Example 1: std::u_p^
----------------
While the detailed examples/debugging instructions are nice for me, I don't think keeping them in the code here is worthwhile - I'd suggest just adding the one line to the comment above and removing this block.

For the dot-to-arrow example specifically it might be worth filing a clangd bug and linking to it in the comment above.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75739