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

Highlight definition and control block structures #2779

Merged

Conversation

rogancodes
Copy link
Contributor

Motivation

Closes #2494

Implementation

If the cursor is on a definition or control node, both the start and end of the node will be highlighted.

Automated Tests

Amended existing test case.

Manual Tests

Place the cursor on one of the definition or control nodes.

Screencast.from.25-10-24.08.44.06.AM.IST.webm

@rogancodes rogancodes requested a review from a team as a code owner October 25, 2024 03:26
@vinistock vinistock added enhancement New feature or request server This pull request should be included in the server gem's release notes labels Oct 28, 2024
@rogancodes rogancodes requested a review from vinistock October 31, 2024 16:48
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.

This is looking good!

lib/ruby_lsp/listeners/document_highlight.rb Outdated Show resolved Hide resolved
lib/ruby_lsp/listeners/document_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.

Thank you for the contribution!

@vinistock vinistock merged commit d4064db into Shopify:main Nov 4, 2024
21 checks passed
@waynehoover
Copy link

Are there any editor.semanticTokenColorCustomizations rules we need to add to make this visible? I'm not seeing it using the github light color theme in vscode.

@jbigler
Copy link

jbigler commented Nov 26, 2024

This seems to be working great for me in Neovim.
I do see an issue when I am in an ERB file. While it is able to ignore the class= attribute of normal HTML tags, it gets confused when there is a class: argument in a ruby method such as a tag builder.

@vinistock
Copy link
Member

AFAIK, all you need is for editor.occurrencesHighlight to not be turned off, which is already the default. The editor.semanticTokenColorCustomizations setting is for customizing your theme colours for semantic tokens, which is related to semantic highlighting, not document highlight.

@andyw8
Copy link
Contributor

andyw8 commented Nov 26, 2024

You can also verify that "Inspect Editor Tokens and Scopes" (via Command-P) matches this:

Screenshot 2024-11-26 at 11 48 03 AM

@waynehoover
Copy link

waynehoover commented Nov 26, 2024

Thanks, my editor.occurrencesHighlight was off, which is why it wasn't showing.

Is there anyway to make the def/end highlighting continue to highlight when I'm inside the method? Like what Zed does, here is an example:

https://share.zight.com/yAugGqED

@vinistock
Copy link
Member

That's possible, although not super conventional for the implementation of document highlight. The intent of the feature is to highlight occurrences of the thing under the cursor.

This feels more like highlighting the surrounding scope.

@jbigler
Copy link

jbigler commented Nov 27, 2024

Here's a visual example of my comment above:
image
My cursor is on the end statement, but it is confused by the class: argument and so isn't highlighting the corresponding do statement.

@vinistock
Copy link
Member

@jbigler oh, that looks like a bug in our implementation. Could you please open an issue so that we can prioritize and look into it?

@rogancodes rogancodes deleted the enhancement/higlight-block-structures branch November 29, 2024 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 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.

Add block highlights to the usual highlights
5 participants