[PATCH] D64391: [CodeComplete] an option to suppress endings for header completion

[Kadir Cetinkaya via Phabricator via cfe-commits]

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