Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support qualified names in workspace/symbol #31

Closed
HighCommander4 opened this issue May 15, 2020 · 5 comments · Fixed by #32
Closed

Support qualified names in workspace/symbol #31

HighCommander4 opened this issue May 15, 2020 · 5 comments · Fixed by #32

Comments

@HighCommander4
Copy link
Contributor

Currently, a query like Class::Method does not return any results, but it would be useful if it did. The alternatives of searching for Class and then manually navigating to Method within the class, or searching for Method and choosing among potentially many classes that declare a method with that name, are more labour-intensive.

@kadircet
Copy link
Member

normally this works, just not in a user friendly way ...

First of all, the match needs to be exact, so you should start from most outer namespace if there are any. In addition to that matching (for the scope) is case sensitive :/

For example for a symbol like ::clang::clangd::ClangdServer::rename:

  • rename is the method name
  • clang::clangd is the namespace
  • ClangdServer is the class name.

one has to exactly type clang::clangd::ClangdServer::r :/

So in your case, searching for Class::m should actually work, if Class is in global namespace.

I suppose making scope matching use suffix match rather than an exact match should be relatively easy.
It is unclear whether this would have any "bad" effects on symbol ranking though, but I wouldn't expect that to be the case.
I am also not familiar with dex, but a naive implementation [1] of suffix matching could end up doubling/tripling memory usage, not sure how much memory this would correspond to in absolute terms.

1: storing same symbol in multiple places, e.g. rename is currently only stored in clang::clangd::ClangdServer. For a suffix match strategy we could store it in ClangdServer, clangd::ClangdServer and clang::clangd::ClangdServer

@HighCommander4
Copy link
Contributor Author

one has to exactly type clang::clangd::ClangdServer::r :/

I did try this, and it does not work for me. The query clang finds the namespace declaration, but any query beginning with clang:: fails to find any symbols. It's as if the moment colons appear in the query, it stops matching anything.

@kadircet
Copy link
Member

Hmm, that's interesting. Could you provide the full logs? Normally I get:

image

That's what the query looks like:

V[21:54:01.481] <<< {
   "id": 308,
   "jsonrpc": "2.0",
   "method": "workspace/symbol",
   "params": {
     "query": "clang::"
   }
}

Maybe vscode (or whatever editor/plugin you are using) is sending a different query somehow?

@HighCommander4
Copy link
Contributor Author

I'm using vscode. The logs show that clangd is sending back results, but vscode is... ignoring them?

For example, for this test file:

namespace Foo {
  class C {};
}

If I invoke "Go to Symbol in Workspace" and enter the query Foo::, I see the following traffic:

V[16:24:44.528] <<< {"id":49,"jsonrpc":"2.0","method":"workspace/symbol","params":{"query":"Foo::"}}

I[16:24:44.528] <-- workspace/symbol(49)
V[16:24:44.529] Dex query tree: false
V[16:24:44.529] Dex query tree: false
I[16:24:44.529] --> reply:workspace/symbol(49) 0 ms
V[16:24:44.529] >>> {"id":49,"jsonrpc":"2.0","result":[{"containerName":"Foo","kind":5,"location":{"range":{"end":{"character":9,"line":1},"start":{"character":8,"line":1}},"uri":"file:///home/nr/dev/projects/testing/vscode-test/foo.cpp"},"name":"C"}]}

which looks like the expected result, but vscode just shows "No matching workspace symbols".

@hokein
Copy link
Collaborator

hokein commented May 18, 2020

I suspect this is a similar issue that we encountered in the code completion -- looks like VSCode applies the fuzzy match only on symbol.name, so in your case, the query token is Foo::, and the name of returned result is C, so this is not a match, and VSCode throws it away :(

not sure this is by design in VSCode, I think we could do the same hack in our vscode extension to make it support qualified name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants