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

Add GoToTypeDefinition provider #5094

Merged
merged 9 commits into from
Mar 24, 2022
Merged

Conversation

jtschuster
Copy link
Member

@jtschuster jtschuster commented Mar 6, 2022

PR OmniSharp/omnisharp-roslyn#2315 add a /gototypedefinition endpoint. This PR adds a VS Code provider that utilizes the endpoint.

Fixes #4251

PR OmniSharp/omnisharp-roslyn#2315 add a gototypedefinition endpoint. This PR adds a provider that utilizes the endpoint.
@333fred 333fred self-requested a review March 9, 2022 21:25
@333fred
Copy link
Member

333fred commented Mar 9, 2022

I'll try to take a look at this over the next couple of nights.

// the defintion is in source
if (goToTypeDefinitionResponse && goToTypeDefinitionResponse.Definitions) {

for (const definition of goToTypeDefinitionResponse.Definitions) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic should be sharable with https://github.com/OmniSharp/omnisharp-vscode/blob/090acf09c8/src/features/definitionProvider.ts#L36, pretty much in its entirety. Let's refactor these so they don't have to be maintained separately.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated to use the same CSharpDefinitionProvider class to implement both DefinitionProvider and TypeDefinitionProvider, and put as much as I could into a shared function.

@jtschuster jtschuster requested a review from 333fred March 11, 2022 04:30
Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (commit 6). @JoeRobich for another review.

@333fred 333fred requested a review from JoeRobich March 11, 2022 04:40
@jtschuster
Copy link
Member Author

Forgot I was using a local build of Omnisharp with the new endpoint for local testing. The "/gototypedefinition" endpoint was just added recently, so this will fail in CI until it uses the next Omnisharp release that includes it.

@JoeRobich
Copy link
Member

Thanks @jtschuster!

@JoeRobich JoeRobich merged commit e316b14 into dotnet:master Mar 24, 2022
@crawlchange
Copy link

F12 is still not working. "Go to Type Definition" or "Go to Definition" are still not working. Does OmniSharp do ANYTHING at all?

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 this pull request may close these issues.

"Go to type definition" and other navigations not working
4 participants