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

fix(#652): remove usage of lookbehind regex with wildcard lengths #47

Merged
merged 5 commits into from
Apr 15, 2024

Conversation

alehechka
Copy link
Contributor

@alehechka alehechka commented Mar 30, 2024

Overview

Resolves: a-h/templ#652.

This PR removes the usage of wildcards in all of the lookbehind expressions mentioned in a-h/templ#652.

For (?<=\s*}) lookbehind expressions, most were able to just remove the wildcard \s and work the same. The else expression did need some adjustments however to properly address the syntax highlighting of the match it's covering.

For the script/style elements, these were previously added and later updated to fix inline tags. The way the lookbehind expression was previously used was allowing the #element match to find it first and lookbehind for script/style to start the new match. Instead, I moved #script-element and #style-element above #element in the top-level patterns so they will be matched first and made them more exact matching with proper closing brackets (>). I also copied down the HTML tokens into these captures.

Through testing a few different scenarios of each area I saw no regression, but would be worth a double check from different eyes.

@joerdav
Copy link
Contributor

joerdav commented Apr 12, 2024

I think I'll implement some tests using https://github.com/PanAeon/vscode-tmgrammar-test then merge this :)

@joerdav
Copy link
Contributor

joerdav commented Apr 12, 2024

Added this here: #50

The diffs are a bit hard to read but I think as long as they don't change thats a good sign I think.

I've only added snapshots, but this package also supports unit style tests.

@joerdav
Copy link
Contributor

joerdav commented Apr 12, 2024

@alehechka Can you update your branch with main please?

@alehechka
Copy link
Contributor Author

@joerdav updated!

@joerdav
Copy link
Contributor

joerdav commented Apr 12, 2024

Thanks, @alehechka your changes seem to break the snapshot tests, though they seem to be expected.

You can update the snapshots with xc test-update if you use xc or if not npm run test:update

@alehechka
Copy link
Contributor Author

@joerdav, sweet, got that fixed! Cool to see that the grammar tests have been added!

@joerdav
Copy link
Contributor

joerdav commented Apr 12, 2024

Great thanks! On Monday I'll test this branch out then merge :)

@joerdav joerdav merged commit 25eeddc into templ-go:main Apr 15, 2024
1 check passed
@alehechka alehechka deleted the fix_652 branch April 15, 2024 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature: add syntax highlighting support to Github - needs Grammar Regex Adjustments
2 participants