[PATCH] D157080: [clangd] Symbol search includes parameter types for (member) functions

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 14 06:22:49 PDT 2023


sammccall added a comment.

LSP is fairly clear here and our behavior here seems to be pretty correct.

- `name` is "the name of this symbol",
- detail is "More detail for this symbol, e.g the signature of a function.".

We're providing the type as detail: `void()` (for functions just `()` might be better, or better still: `() -> void`).

Anyway we're providing the data, VSCode is choosing not to display it. It does show it in the hierarchical symbol view (if you are inside the symbol and click on its name in the breadcrumbs along the top), and also in the outline view.
It seems that the VSCode folks have decided that the since the ctrl-shift-O view shows results as a flat list, the "grey text" slot is used to show hierarchy rather than detail.
I'm afraid that's up to them, you can file feature requests to have them change it.

Indeed it looks like the MS cpptools extension works around this by putting the detail into the name, but I don't think we should be emulating this: it violates the spec intent, is likely to break things in other editors. (cpptools don't care about this, they only support vscode).



================
Comment at: clang-tools-extra/clangd/Protocol.cpp:899
+  if (!S.detail.empty()) {
+    if (S.kind == SymbolKind::Method || S.kind == SymbolKind::Function) {
+      llvm::StringRef Detail{S.detail};
----------------
DocumentSymbol is already a structure modelling LSP, this function's job is to serialize it, not rearrange the fields.

Moreover, we shouldn't render the detail into a string and then go back later and attempt to parse it, but just produce the data we want in the first place (in FindSymbols.cpp, where it can be unittested).


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

https://reviews.llvm.org/D157080



More information about the cfe-commits mailing list