[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks for formatting test cases, I had it in mind but forgot to mention in last round.

LGTM



================
Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:113
+  auto Best = Candidates.begin();
+  for (auto It = Candidates.begin(); It != Candidates.end(); ++It) {
+    if (It->second > Best->second)
----------------
nit: `for(auto &It : Candidates)`


================
Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:117
+    // Select the first one in the lexical order if we have multiple candidates.
+    if (It->second == Best->second && It->first() < Best->first())
+      Best = It;
----------------
nit: `else if`


================
Comment at: clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp:196
+  } TestCases[] = {
+      {R"cpp(// empty, no header found)cpp", llvm::None},
+      {R"cpp(
----------------
kadircet wrote:
> again no need for raw literals
not done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67907