godotengine / godot-vscode-plugin

Godot development tools for VSCode
MIT License
1.55k stars 164 forks source link

Open documentation from "go to definition" on native symbols #541

Closed levidavidmurray closed 10 months ago

levidavidmurray commented 10 months ago

This has been bothering me for a while, so figured I'd do something about it.

https://github.com/godotengine/godot-vscode-plugin/assets/46084870/2f087dd0-5ef7-41d8-b6eb-525ce2f4ea69

Added the following settings to control this behaviour:

The GDScript LSP can sometimes struggle to resolve a symbol and returns multiple types

image

You can cast the value to get around this, but I opted to add a QuickPick menu to allow you to select from the given options

https://github.com/godotengine/godot-vscode-plugin/assets/46084870/dc21a637-445a-4ab8-9f94-c1d9477f3328

DaelonSuzuka commented 10 months ago

This has been bothering me for a while, so figured I'd do something about it.

First off, thank you for your interest in the extension and for putting the time into making this PR. I greatly appreciate the effort it takes to start a new contribution to a project.

That said, I have good news and I have "bad" news...

The good news is that you've done a solid job here. This is a reasonable implementation, especially considering the convoluted state of the LSP Client components. I especially like the quick pick for resolving ambiguous definitions.

The "bad" news is that this feature has already been implemented in #529:

Code_pnOxz7hvPA

levidavidmurray commented 10 months ago

The "bad" news is that this feature has already been implemented in https://github.com/godotengine/godot-vscode-plugin/pull/529:

Haha, all good! This was mostly a project taken on to procrastinate working on my game 🤡 Definitely should've taken a closer look at any existing PRs.

I use VSCodeVim and didn't realize the problems my solution has with CTRL+clicking (I just use g+d). Hooking into the textDocument/definition message isn't a viable solution because there could be plenty of cases where that message occurs without going to definition (such as holding CTRL and hovering over a symbol).

I see you did things the correct way and created an actual vscode.DefinitionProvider! I did have some problems getting some things working properly, but I'll close this out and move the discussion to your PR.