Skip to content

add highlight for do...end blocks#3061

Closed
vicocamacho wants to merge 2 commits intoShopify:mainfrom
vicocamacho:feat/add-block-highlight
Closed

add highlight for do...end blocks#3061
vicocamacho wants to merge 2 commits intoShopify:mainfrom
vicocamacho:feat/add-block-highlight

Conversation

@vicocamacho
Copy link
Contributor

@vicocamacho vicocamacho commented Jan 13, 2025

Motivation

Closes #2929

As I was investigating the problem with the issue above I discovered that the highlight functionality for do blocks was not implemented.

Implementation

This PR enhances the existing Highlight feature to add support for highlighting blocks.

Automated Tests

Added new expectations using the existing "do block" fixture files.

Manual Tests

Go to the start of a do block, it should highlight its matching end

@vicocamacho vicocamacho requested a review from a team as a code owner January 13, 2025 20:13
@vicocamacho
Copy link
Contributor Author

I have signed the CLA!

@jbigler
Copy link

jbigler commented Feb 5, 2025

Great!

Wow, this reveals a different problem I was having then. When I tested your code the correct matching term was highlighted along with the wrong one. So I was actually using a legacy regex based highlighter without realizing it. 🙄

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2025

This pull request is being marked as stale because there was no activity in the last 2 months

@github-actions github-actions bot added Stale and removed Stale labels Apr 7, 2025
@github-actions
Copy link
Contributor

This pull request is being marked as stale because there was no activity in the last 2 months

@github-actions github-actions bot added the Stale label Jun 25, 2025
@github-actions github-actions bot closed this Jul 10, 2025
@vinistock
Copy link
Member

@vicocamacho I'm confused about this one. Is this PR related to #2929? Or does it just implement highlighting the matching do and end tokens for blocks?

The description has the closes keyword, but my understanding is that the issue was related to tree sitter.

Let me know if you want to bring this PR back.

@vicocamacho
Copy link
Contributor Author

hey @vinistock,

While I was investigating the error reported in #2929, I discovered that do ... end were not being highlighted. I implemented support for them with the intention of fixing the reported issue.

It turns out the issue was actually caused by Regex-based highlighting in Neovim.

So while technically this PR does not solve that bug report, it does add a "missing" feature.

I'm happy to bring back the PR if you think it is worth having.

@vinistock
Copy link
Member

Yes! Let's do it. We should highlight both matching do..end, but also matching if|unless|while|until|for|def|class|module and their respective end tokens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Highlight block correctly in ERB file

3 participants