MaskRay added inline comments.

================
Comment at: clangd/FindSymbols.cpp:31
+
+  if (supportedSymbolKinds &&
+      std::find(supportedSymbolKinds->begin(), supportedSymbolKinds->end(),
----------------
This std::find loop adds some overhead to each candidate... In my experience 
the client usually doesn't care about the returned symbol kinds, they are used 
to give a category name. You can always patch the upstream to add missing 
categories.

This is one instance where LSP is over specified. nvm I don't have strong 
opinion here


================
Comment at: clangd/Protocol.cpp:206
+
+bool fromJSON(const json::Expr &Params, WorkspaceClientCapabilities &R) {
+  json::ObjectMapper O(Params);
----------------
This copy-pasting exposes another problem that the current  `fromJSON` `toJSON` 
approach is too verbose and you may find other space-efficient serialization 
format convenient .... Anyway, they can always be improved in the future.


================
Comment at: clangd/tool/ClangdMain.cpp:235
 
+  clangd::WorkspaceSymbolOptions WorkspaceSymOpts;
+  WorkspaceSymOpts.Limit = LimitResults;
----------------
I know command line options are easy for testing purpose but they are clumsy 
for users. You need a "initialization option" <-> "command line option" bridge.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to