go-langserver icon indicating copy to clipboard operation
go-langserver copied to clipboard

Workspace Symbol Provider lists all struct method definitions instead of just the struct definition

Open ramya-rao-a opened this issue 8 years ago • 9 comments

I did a Workspace Symbols Search for "Win" in with https://github.com/rogpeppe/godef as a workspace in Visual Studio Code.

Actual: In the background you can see that Addr is a method on Win. Same goes for everything appearing after Addr in the list.

screen shot 2017-02-13 at 7 24 47 pm

Expected: screen shot 2017-02-13 at 7 29 15 pm

ramya-rao-a avatar Feb 14 '17 03:02 ramya-rao-a

If you do a workspace/symbols request, would you expect those results to show?

keegancsmith avatar Feb 14 '17 08:02 keegancsmith

I don't understand the issue. The actual screenshot looks 100% correct to me

felixfbecker avatar Feb 14 '17 08:02 felixfbecker

@felixfbecker yes, I would expect a search query to match all possible fields we return in symbol information. The ordering that vscode does is also useful in this case. But maybe matching these values is actually not that useful for end users. Keen to hear the rational.

keegancsmith avatar Feb 14 '17 08:02 keegancsmith

If you do a workspace/symbols request, would you expect those results to show?

Nope

I don't understand the issue. The actual screenshot looks 100% correct to me

Workspace Symbol Search is more like Go To Definition and not Find All References. Therefore, one would want to see the results matching just the definition and not all references. https://code.visualstudio.com/docs/extensions/language-support#_show-all-all-symbol-definitions-in-folder

ramya-rao-a avatar Feb 14 '17 16:02 ramya-rao-a

@ramya-rao-a I understand, but as a user, I see it as a feature if a workspace/symbol query for a class also returns methods of that class, meaning my query is applied not only to name, but also to containerName.

felixfbecker avatar Feb 14 '17 16:02 felixfbecker

@felixfbecker Interesting.. I wonder if we can have a setting in VS Code to control that. This could be a whole new feature. Created a feature request in the language server repo for this. https://github.com/Microsoft/language-server-protocol/issues/183

Coming back to how the protocol is defined at the moment, @keegancsmith can we only match the query with name and not the containerName so that we can have feature parity?

ramya-rao-a avatar Feb 14 '17 18:02 ramya-rao-a

Coming back to how the protocol is defined at the moment

It isn't defined anywhere afaik: https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#workspace_symbol

Would like to know this because the PHP LS also matches more than just name

felixfbecker avatar Feb 14 '17 19:02 felixfbecker

Oh the woes of documentation...

You are right, the protocol doesnt explicitly call out that the query should match the name only.

I was making my statement based on the below from https://code.visualstudio.com/docs/extensions/language-support#_show-all-all-symbol-definitions-in-folder

image

Updated https://github.com/Microsoft/language-server-protocol/issues/183 description asking for clarification

ramya-rao-a avatar Feb 14 '17 19:02 ramya-rao-a

@ramya-rao-a I understand and agree it should only return symbol definitions, but the question is whether it should for example also return method definitions if you queried for a class name (because the containerName matches the query). I find it useful as a user if it does

felixfbecker avatar Feb 14 '17 20:02 felixfbecker