Skip to content

Add definitions for Rails callbacks #297

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

Merged
merged 4 commits into from
Mar 22, 2024

Conversation

aryan-soni
Copy link
Contributor

Implements definitions for callbacks in Rails, building on the approach leveraged for definitions inside ruby-lsp.

This PR also abstracts away the collection of Rails callbacks that we keep, since both the document symbol and definition features now rely on it. Moreover, generally, this PR introduces the definitions feature to the add-on, as it was not present previously.

@aryan-soni aryan-soni force-pushed the as/definition-dsl-argument-methods branch from 24cd909 to fa58345 Compare March 20, 2024 17:45
@aryan-soni aryan-soni added the enhancement New feature or request label Mar 20, 2024
@aryan-soni aryan-soni marked this pull request as ready for review March 20, 2024 17:47
@aryan-soni aryan-soni requested a review from a team as a code owner March 20, 2024 17:47
@aryan-soni aryan-soni requested review from andyw8 and vinistock March 20, 2024 17:47
@andyw8
Copy link
Contributor

andyw8 commented Mar 20, 2024

misc/definition.gif is just copied from ruby-lsp, right? We should probably have one that's more focused on Rails features.

@andyw8
Copy link
Contributor

andyw8 commented Mar 20, 2024

I guess this is limited currently since it can't jump to definitions in another class (e.g. a common authenticate method defined in ApplicationController), but once we have full method indexing it should be possible to make it more capable.

module RubyLsp
module Rails
module Support
module Callbacks
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe CallbacksList to emphasize there's no behaviour in here?

Copy link
Contributor

@andyw8 andyw8 left a comment

Choose a reason for hiding this comment

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

Looking good, just a few tweaks needed.

@aryan-soni aryan-soni force-pushed the as/definition-dsl-argument-methods branch 2 times, most recently from f672239 to 1a37bfe Compare March 21, 2024 18:01
@aryan-soni aryan-soni requested review from vinistock and andyw8 March 21, 2024 18:03
@aryan-soni aryan-soni force-pushed the as/definition-dsl-argument-methods branch from 1a37bfe to ec42db4 Compare March 21, 2024 18:27
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

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

Nice 🚀

@vinistock
Copy link
Member

Let's add definition to this list.

@aryan-soni aryan-soni force-pushed the as/definition-dsl-argument-methods branch from ec42db4 to 41114a9 Compare March 21, 2024 21:44
@aryan-soni aryan-soni merged commit 621b2e6 into main Mar 22, 2024
@aryan-soni aryan-soni deleted the as/definition-dsl-argument-methods branch March 22, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants