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

Go to declaration service #734

Merged
merged 2 commits into from
Nov 9, 2022
Merged

Go to declaration service #734

merged 2 commits into from
Nov 9, 2022

Conversation

gfontorbe
Copy link
Contributor

Solves #596
Added a default implementation of the go to declaration service.

For languages like Langium, for what I understand, go to declaration and go to definition achieve the same thing.

Should we instead provide an abstract implementation?

@gfontorbe gfontorbe requested a review from msujew October 25, 2022 14:09
@msujew msujew added the LSP Language Server Protocol integration label Oct 25, 2022
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

I don't believe we need a default service for the declaration provider. An interface declaration for the service is already enough. As the service is highly semantic specific, we can't really do anything here. So it's probably best to just remove the implementation and tests.

packages/langium/src/services.ts Outdated Show resolved Hide resolved
@gfontorbe gfontorbe requested a review from msujew November 9, 2022 08:37
@gfontorbe
Copy link
Contributor Author

I removed the implementation and the tests

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks 👍

@gfontorbe gfontorbe merged commit 824f098 into main Nov 9, 2022
@gfontorbe gfontorbe deleted the gfontorbe/goto-declaration branch November 9, 2022 14:50
@spoenemann spoenemann added this to the v1.0.0 milestone Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LSP Language Server Protocol integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants