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 Regexp used for CSS comment detection #32

Merged
merged 6 commits into from Nov 24, 2021

Conversation

rmacklin
Copy link
Contributor

@rmacklin rmacklin commented Nov 24, 2021

Addresses #30 (comment)

Previously the * was not escaped in the SOURCE_MAPPING_PATTERN Regexp, so it was being interpreted as a "zero or more" quantifier on the previous character (/), rather than as a literal * as intended. This meant that the Regexp matched the prefix # sourceMappingURL= even when it did not appear inside a CSS comment (since zero leading / characters is a valid match). This PR fixes the Regexp to properly escape the * character, so that it'll only match /*# sourceMappingURL=.

The test added in 9dafe50 demonstrates an example where the Regexp was previously working incorrectly (it fails when the * is not escaped). Additionally, I changed the Regexp literals to use %r syntax so that we don't have to escape the / characters to make them easier to follow (1b030f6), and I made the pattern a bit stricter so that we can ignore comments that don't appear at the beginning of the line when searching for comments to replace (1dd7799).

Previously the `*` was not escaped in this Regexp, so it was being
interpreted as a "zero or more" quantifier on the previous character,
rather than as a literal `*` as intended. This meant that the Regexp
matched the prefix `# sourceMappingURL=` even when it did not appear
inside a CSS comment. Now it'll only match `/*# sourceMappingURL=`.
Again, the intent here was to match the beginning of a CSS comment
string (i.e. to match the literal prefix `/*`), but this was actually
matching "zero or more" `/` characters because the `*` was not escaped
in the Regexp.
This way we don't have to escape literal `/` characters, which makes the
Regexp easier to follow.
The `sourceMappingURL` comments always appear at the beginning of the
line, so we can ignore any comments that appear later in a line when
searching for comments to replace.
before the `.map` extension (i.e. the Regexp doesn't need to match
`/*# sourceMappingURL=.map`)
@rmacklin rmacklin mentioned this pull request Nov 24, 2021
@dhh dhh merged commit 920730e into rails:main Nov 24, 2021
@rmacklin rmacklin deleted the fix-css-comment-matching-pattern branch November 24, 2021 21:56
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