[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

ilya-biryukov created this revision. ilya-biryukov added a reviewer: hokein. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric. Only run completion when we were trigerred on '->' and '::', otherwise send an error code in return. To avoid automatically invoking completions in c

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

ilya-biryukov added a comment. This still needs tests, will add them before committing the change. However, the implementation should be good for review. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55994/new/ https://reviews.llvm.org/D55994 __

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

hokein added a comment. The code looks mostly good to me. Comment at: clangd/ClangdLSPServer.cpp:789 +bool ClangdLSPServer::isSporadicCompletion( +const CompletionParams &Params) const { The method name is a bit confusing to me, I didn't get its meaning a

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

ilya-biryukov updated this revision to Diff 179844. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Address comments - Added a test Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55994/new/ https://reviews.llvm.org/D5

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:811 + if (Trigger == ">") +return (*Code)[*Offset - 2] != '-'; // trigger only on '->'. + if (Trigger == ":") hokein wrote: > Checking `Offset` is not always right for rare cases li

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clangd/ClangdLSPServer.cpp:811 + if (Trigger == ">") +return (*Code)[*Offset - 2] != '-'; // trigger only on '->'. + if (Trigger == ":") ---

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

jkorous added a comment. This looks like a work around LSP imperfection indeed. Are you going to push for change in LSP? Something like `CompletionOptions/triggerCharacters` -> `CompletionOptions/triggerStrings`? export interface CompletionOptions { /** * The server provides

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

hokein added a comment. In D55994#1343669 , @jkorous wrote: > This looks like a work around LSP imperfection indeed. > > Are you going to push for change in LSP? Something like > `CompletionOptions/triggerCharacters` -> `CompletionOptions/triggerStrings`?

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

ilya-biryukov added a comment. In D55994#1343669 , @jkorous wrote: > This looks like a work around LSP imperfection indeed. Totally. > Are you going to push for change in LSP? Something like > `CompletionOptions/triggerCharacters` -> `CompletionOptions

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

jkorous added a comment. @hokein I think you are correct. I meant that it would be possible to make LSP more generic by using trigger strings instead of trigger characters.. @ilya-biryukov Yes, that's true. I would still expect some performance gain in more restrictive filtering on client's side

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

ilya-biryukov added a comment. In D55994#1344717 , @jkorous wrote: > @ilya-biryukov Yes, that's true. I would still expect some performance gain > in more restrictive filtering on client's side - not sure how big though. > Anyway, seems like LSP folks th

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

This revision was automatically updated to reflect the committed changes. Closed by commit rL350304: [clangd] Check preceding char when completion triggers on ':' or '>' (authored by ibiryukov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.ll