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: change lineNumbers regex to respect positioning/existence of showLineNumbers{x} #75

Merged
merged 2 commits into from
Apr 19, 2023
Merged

fix: change lineNumbers regex to respect positioning/existence of showLineNumbers{x} #75

merged 2 commits into from
Apr 19, 2023

Conversation

gendelbendel
Copy link
Contributor

When showLineNumbers exists with a value to start at, like showLineNumbers{3}, the line highlighter potentially grabs that number instead of the appropriate line highlight group, causing line highlights to not function correctly.

The new regex essentially checks to see if there are any characters before the { other than white space and filters them out. If there is a white space before the {, it's the correct group for line highlights.

If you run the additional test, but without the changes in src/index.js, the test fails. It then passes with the changes.

@netlify
Copy link

netlify bot commented Apr 18, 2023

Deploy Preview for rehype-pretty-code canceled.

Name Link
🔨 Latest commit a5a49c3
🔍 Latest deploy log https://app.netlify.com/sites/rehype-pretty-code/deploys/643f06b3e3a0550008fe53a7

src/index.js Outdated
@@ -198,7 +198,7 @@ export default function rehypePrettyCode(options = {}) {
meta = meta.replace(tiltleMatch?.[0] ?? '', '');

const lineNumbers = meta
? rangeParser(meta.match(/{(.*)}/)?.[1] ?? '')
? rangeParser(meta.match(/(?<!\S){(.*?)}/)?.[1] ?? '')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this breaks in older Safaris

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh shoot, because of the negative lookbehind huh.

What about (?:^|\s){(.*?)}

@atomiks atomiks merged commit 270c29d into rehype-pretty:master Apr 19, 2023
5 checks passed
@atomiks
Copy link
Collaborator

atomiks commented Apr 19, 2023

tests did seem to pass!

@gendelbendel gendelbendel deleted the highlight-lines-and-show-line-numbers branch April 21, 2023 03:51
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.

None yet

2 participants