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.

Sign up for GitHub

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

Jump to bottom

Add method support to References request #2650

Merged
merged 10 commits into from
Oct 4, 2024
Merged

Add method support to References request #2650

merged 10 commits into from
Oct 4, 2024

Conversation

andyw8
Copy link
Contributor

@andyw8 andyw8 commented Oct 2, 2024 •
edited
Loading

This allows the use of Find All References for methods.

As discussed in #2640, we cannot have full confidence in the matches due the lack of a type system.

Note: In VS Code, Rename Symbol will show as available for methods, but isn't yet implemented.

Co-authored with @vinistock

Closes #2640

@andyw8 andyw8 changed the base branch from vs-add-references-support to vs-add-rename-support October 2, 2024 16:14
@vinistock vinistock force-pushed the vs-add-rename-support branch 4 times, most recently from 21f56af to 377e988 Compare October 2, 2024 20:18
Base automatically changed from vs-add-rename-support to main October 2, 2024 20:39
@andyw8 andyw8 force-pushed the andyw8/add-method-references branch from 97055d7 to 26fe65e Compare October 3, 2024 19:04
@andyw8 andyw8 added the enhancement New feature or request label Oct 3, 2024
@andyw8 andyw8 added the server This pull request should be included in the server gem's release notes label Oct 3, 2024
@andyw8 andyw8 changed the title Add method references Add method support to References request Oct 3, 2024
@andyw8 andyw8 marked this pull request as ready for review October 3, 2024 19:56
@andyw8 andyw8 requested a review from a team as a code owner October 3, 2024 19:56
@andyw8
Copy link
Contributor Author

andyw8 commented Oct 3, 2024

@vinistock should we also try cover attr_* references, or does that add too much complexity?

st0012
st0012 approved these changes Oct 3, 2024
View reviewed changes
Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

Test this with irb and it works well 👍 Just left some small suggestions.

lib/ruby_lsp/requests/references.rb Outdated Show resolved Hide resolved
lib/ruby_indexer/test/reference_finder_test.rb Show resolved Hide resolved
vinistock
vinistock reviewed Oct 4, 2024
View reviewed changes
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. We can get rename working after this

lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb Show resolved Hide resolved
lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb Outdated Show resolved Hide resolved
lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb Outdated Show resolved Hide resolved
@vinistock
Copy link
Member

@vinistock should we also try cover attr_* references, or does that add too much complexity?

We should definitely do it, but you can do that in a subsequent PR.

@andyw8 andyw8 mentioned this pull request Oct 4, 2024
Open
@andyw8 andyw8 enabled auto-merge (squash) October 4, 2024 14:59
@andyw8 andyw8 merged commit 46b0411 into main Oct 4, 2024
38 checks passed
@andyw8 andyw8 deleted the andyw8/add-method-references branch October 4, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@st0012 st0012 st0012 approved these changes

@vinistock vinistock vinistock approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request server This pull request should be included in the server gem's release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add references support for methods
3 participants
@andyw8 @vinistock @st0012