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