kadircet added inline comments.
================ Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1088 + + if (Trigger == "/") { // trigger only on '#include.../' + auto StartOfLine = Code->rfind('\n', *Offset); ---------------- i think instead of regex, it would be better to just perform a linear search to make sure line looks something like this(up to the Offset): ```{whitespaces}#{whitespaces}include{whitespaces}{",<}{not {",>}}``` IIRC, regex had a bad performance and we would like to trigger as quickly as possible. @sammccall might've more context about this one. ================ Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1090 + auto StartOfLine = Code->rfind('\n', *Offset); + if (StartOfLine == llvm::StringRef::npos) { + StartOfLine = 0; ---------------- i suppose this if should only enclose the `StartOfLine = 0;` statement, not the rest? ================ Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1092 + StartOfLine = 0; + llvm::Regex IncPattern("^#[[:space:]]*include.*"); + return IncPattern.match( ---------------- as i mentioned above, the line doesn't have to start with `#`, and we cannot expect arbitrary stuff after include, we must make sure current point is inside quotes(") or brackets(<) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64391/new/ https://reviews.llvm.org/D64391 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits