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

[Haojian Wu via Phabricator via cfe-commits]

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 == ":")
----------------
ilya-biryukov wrote:
> hokein wrote:
> > Checking `Offset` is not always right for rare cases like (`bar:/*commment*/:`), a robust way is to use lexer and get the token at the current position, but I don't think it is worth (these rare cases should not happen when people write code). Maybe add a comment documenting the limitation?
> Done. Note that `bar:/*comment*/:` is not actually a qualifier, i.e. a single token cannot be split in half by the comment.
> But the comment is totally valid: use the lexer would allow to filter out more things, e.g. we don't want to auto-trigger inside a comment, etc.
Yeah, you are right `a single token cannot be split in half by the comment`. Thanks for the clarification.


================
Comment at: test/clangd/completion-auto-trigger.test:13
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":3,"character":5},"context":{"triggerKind":2,"triggerCharacter":">"}}}
+#      CHECK:  "id": 2,
----------------
nit: would be nicer if we add a comment illustrating what this request tests for. For this case, it is `a->^`, IIUC.


================
Comment at: test/clangd/completion-auto-trigger.test:26
+# CHECK-NEXT:        "label": " size",
+# CHECK-NEXT:        "sortText": "3eacccccsize",
+# CHECK-NEXT:        "textEdit": {
----------------
`sortText` is implementation details, maybe use `{{.*}}size`, the same below.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55994