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

C-like: Made all comments greedy #2680



Copy link

@RunDevelopment RunDevelopment commented Dec 30, 2020

This fixes #2679.

The problem was that multiline comments in C-like weren't greedy. This usually isn't a problem because a non-greedy first grammar rule behaves amlost as if it was greedy with one exception: greedy rematching.

Basically, previously matched tokens have to be overwritten by greedy tokens but non-greedy tokens can't do that. This then results in the token stream seen in #2679:



I also had to make a minor adjustment to Coffeescript because the tests failed. The problem there was that the delimiter token was matched before comment which meant that the lookbehind group of the comment pattern didn't accept. This is a general problem with greedy rules and lookbehinds that cannot be fixed without ES2018 lookbehinds (#1708).

Copy link

@github-actions github-actions bot commented Dec 30, 2020

JS File Size Changes (gzipped)

A total of 2 files have changed, with a combined diff of +11 B (+1.0%).

file master pull size diff % diff
components/prism-clike.min.js 446 B 446 B 0 Bytes 0%
components/prism-coffeescript.min.js 638 B 649 B +11 B +1.7%

Generated by 🚫 dangerJS against 1877c3d

@RunDevelopment RunDevelopment merged commit 0a3932f into PrismJS:master Dec 30, 2020
8 checks passed
@RunDevelopment RunDevelopment deleted the c-like-non-greedy-comment branch Dec 30, 2020
Copy link

@mAAdhaTTah mAAdhaTTah commented Dec 30, 2020

@RunDevelopment I'm good to release a new version later this evening. Is there anything else you want to get merged for it?

Copy link
Member Author

@RunDevelopment RunDevelopment commented Dec 30, 2020

Nope. All other PRs need further work and/or offer little immediate benefit to our users.

I'll make you the changelog.

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

Successfully merging this pull request may close these issues.

2 participants