[PATCH] D63989: [Clangd] Added hidden command line option -tweaks to specify which tweaks to enable

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/ClangdServer.h:145
+    /// Returns true if the StringRef is a tweak that should be enabled
+    std::function<bool(llvm::StringRef)> TweakFilter;
+    
----------------
ClangdServer::Options needs to be default-constructible with sensible default options.

Can you either inline-initialize this `= [](llvm::StringRef) { return true; }` (if possible), or have the code handle TweakFilter==nullptr as accepting anything?


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:542
   Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
+  Opts.TweakFilter = [&](llvm::StringRef TweakToSearch) {
+    // return true if any tweak matches the TweakToSearch
----------------
it would be clearer to set this only `if TweakList.getNumOccurrences()`, rather than using it inside the lambda, I think


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:544
+    // return true if any tweak matches the TweakToSearch
+    for (auto Tweak : TweakList) {
+      if (TweakToSearch == Tweak)
----------------
return llvm::find(TweakList, TweakToSearch) != TweakList.end() ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63989