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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to