-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: Implement goto_declaration support #9380
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
Conversation
I wonder if goto_definition fallback logically belongs to the client? |
Another natural candidate here are trait methods. |
Good point, it feels like it could belong to the client.
Oh right, I was wondering if there was something else that has the notion of declaration vs definition in rust 👍 |
See #2541 for thoughts on this.
…On Tue, Jun 22, 2021, 8:04 PM Lukas Wirth ***@***.***> wrote:
I wonder if goto_definition fallback logically belongs to the client?
Good point, it feels like it could belong to the client.
Another natural candidate here are trait methods.
Oh right, I was wondering if there was something else that has the notion
of declaration vs definition in rust 👍
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#9380 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABBACRBZY6ROHBDM65ERGBTTUEQHRANCNFSM47EIM6XQ>
.
|
crates/ide/src/goto_declaration.rs
Outdated
_ => return None, | ||
} | ||
})(); | ||
res.or_else(|| goto_definition::goto_definition(db, position)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let’s put this or_else in the handlers for now, otherwise, LGTM!
bors r=matklad |
This is just a simple implementation that falls back to
goto_definition
for everything but modules where it goes to the actual module declaration if possible.