[PATCH] D44882: [clangd] Implementation of workspace/symbol request

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle marked an inline comment as done.
malaperle added inline comments.


================
Comment at: clangd/FindSymbols.cpp:31
+
+  if (supportedSymbolKinds &&
+      std::find(supportedSymbolKinds->begin(), supportedSymbolKinds->end(),
----------------
MaskRay wrote:
> MaskRay wrote:
> > malaperle wrote:
> > > MaskRay wrote:
> > > > 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
> > > I have a client that throws an exception when the symbolkind is not known and the whole request fails, so I think it's worth checking. But if it's too slow I can look at making it faster. Unfortunately, I cannot patch any upstream project :)
> > https://github.com/gluon-lang/languageserver-types/blob/master/src/lib.rs#L2016
> > 
> > LanguageClient-neovim returns empty candidate list if one of the candidates has unknown SymbolKind. Apparently they should be more tolerant and there is an issue tracking it.
> If it was not an internal confidential client, I would like to know its name, unless the confidentiality includes the existence of the client.
Sorry, I should have explained a bit more my thought. I see that an older-ish version of Monaco is not handling unknown symbols kinds well. I don't really know if it's our integration with Monaco or Monaco itself (the IDE is Theia). But my thought is that there are clients out there that follow the protocol "correctly" by choosing not to handle those unknown symbol kinds gracefully. I am not that concerned about a single client in particular, just general support of clients out there. As a server, I think it makes sense to support clients that follow the protocol, especially since 1.x and 2.x are not that old. I don't think it's the crux of the complexity of this patch.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882