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

Document highlight shouldn't include the block for a method invocation #2357

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

andyw8
Copy link
Contributor

@andyw8 andyw8 commented Jul 24, 2024

Motivation

Closes #2332

Implementation

Highlight only the invocation's name, not its block.

Automated Tests

Added a new fixture and expectation.

Manual Tests

Open the fixture, move the cursor over foo and ensure only the foo part is highlighted in all invocations.

@andyw8 andyw8 added the bugfix This PR will fix an existing bug label Jul 24, 2024
@andyw8 andyw8 force-pushed the andyw8/fix-block-highlighting branch from 9252aa4 to c581617 Compare July 24, 2024 18:25
@andyw8 andyw8 added the server This pull request should be included in the server gem's release notes label Jul 24, 2024
test/fixtures/highlight.rb Outdated Show resolved Hide resolved
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.

If I understand the issue correctly, the problem was more related to the DefNode than the CallNode. I think we might be highlighting the entire method declaration instead of just its name.

The call node changes are good too, but we need the def node handling to be improved to fix the reported issue.

lib/ruby_lsp/listeners/document_highlight.rb Outdated Show resolved Hide resolved
test/fixtures/highlight.rb Outdated Show resolved Hide resolved
@andyw8 andyw8 force-pushed the andyw8/fix-block-highlighting branch 2 times, most recently from 18b9a1c to 571b7db Compare July 25, 2024 14:53
@andyw8 andyw8 marked this pull request as ready for review July 25, 2024 14:53
@andyw8 andyw8 requested a review from a team as a code owner July 25, 2024 14:53
@andyw8 andyw8 requested review from st0012 and vinistock July 25, 2024 14:53
@andyw8 andyw8 force-pushed the andyw8/fix-block-highlighting branch 2 times, most recently from 1cf9387 to 6bc9b93 Compare July 25, 2024 14:55
@andyw8 andyw8 force-pushed the andyw8/fix-block-highlighting branch from 6bc9b93 to 67494f4 Compare July 25, 2024 15:10
@andyw8 andyw8 enabled auto-merge (squash) July 25, 2024 15:10
@andyw8 andyw8 mentioned this pull request Jul 25, 2024
1 task
@andyw8 andyw8 changed the title Document highlight shouldn't include the block for a method Document highlight shouldn't include the block for a method invocation Jul 25, 2024
@andyw8 andyw8 merged commit edfe065 into main Jul 25, 2024
35 checks passed
@andyw8 andyw8 deleted the andyw8/fix-block-highlighting branch July 25, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable method highlighting
3 participants