[PATCH] D42484: [clangd] Limit completion results.

[Sam McCall via Phabricator via cfe-commits]

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

Thank you!



================
Comment at: clangd/tool/ClangdMain.cpp:128
+static llvm::cl::opt<int> LimitCompletionResult(
+    "limit-completion-results",
+    llvm::cl::desc("Limit the number of completion results returned by clangd. "
----------------
nit: our flag names are generally too long, I think. is completion-limit enough?


================
Comment at: clangd/tool/ClangdMain.cpp:131
+                   "0 means no limit."),
+    llvm::cl::init(100), llvm::cl::Hidden);
+
----------------
I think we probably want to keep the default at 0 for now. I'm fine with 100 personally, but talk to Ilya if you want to change the default :-)

This shouldn't be hidden.


================
Comment at: clangd/tool/ClangdMain.cpp:224
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
+  CCOpts.Limit =  LimitCompletionResult;
   // Initialize and run ClangdLSPServer.
----------------
nit: whitespace


================
Comment at: unittests/clangd/CodeCompleteTests.cpp:541
+      {func("ns::both"), cls("ns::Index")}, Opts);
+  EXPECT_EQ(Results.items.size(), Opts.Limit);
+}
----------------
also assert Results.isIncomplete


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42484