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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
