[PATCH] D59300: [clangd] Tune the fuzzy-matching algorithm

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: clang-tools-extra/clangd/FuzzyMatch.cpp:74
 static bool isAwful(int S) { return S < AwfulScore / 2; }
-static constexpr int PerfectBonus = 3; // Perfect per-pattern-char score.
+static constexpr int PerfectBonus = 4; // Perfect per-pattern-char score.
 
----------------
could you explain this change?


================
Comment at: clang-tools-extra/clangd/FuzzyMatch.cpp:270
 int FuzzyMatcher::skipPenalty(int W, Action Last) const {
-  int S = 0;
+  if (W == 0) // Skipping the first character.
+    return 3;
----------------
If the word is "_something", do we still want to penalize skipping first char?


================
Comment at: clang-tools-extra/clangd/FuzzyMatch.cpp:275
+              // match.
+  return 0;
 }
----------------
iiuc, there is no penalty by default because consecutive matches will gain bonus. This is not trivial here. Maybe add a comment? Or consider apply penalty for non-consecutive matches and no bonus for consecutive ones.


================
Comment at: clang-tools-extra/clangd/FuzzyMatch.cpp:285
+  if (Pat[P] == Word[W] ||
+      (WordRole[W] == Head && (IsPatSingleCase || PatRole[P] == Head)))
     ++S;
----------------
could you explain the intention of this change? Is it relevant to other changes in the patch? The original looks reasonable to me. 


================
Comment at: clang-tools-extra/unittests/clangd/FuzzyMatchTests.cpp:248
   EXPECT_THAT("foo", ranks("[foo]", "[Foo]"));
   EXPECT_THAT("onMes",
-              ranks("[onMes]sage", "[onmes]sage", "[on]This[M]ega[Es]capes"));
----------------
could you add a case for "onmes" pattern?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59300