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

skip comments for preprocessors #3894

Conversation

tanhauhau
Copy link
Member

Fix #3047 (comment)

need to skip html comments when regexping <script> and <style> tag.

}) as Replacement
)
);
str.replace(re, (match, attributes, content, offset) => {
Copy link
Member

@Conduitry Conduitry Nov 11, 2019

Choose a reason for hiding this comment

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

Were you just making this change for code style reasons? Part of the reason it was written like it was was so that replace_async would work with arbitrarily many capture groups. With these changes, it looks like this will only work with exactly two capture groups. Any more or fewer won't be passed on to the callback correctly, and offset will also receive the wrong argument.

@tanhauhau tanhauhau force-pushed the tanhauhau/skip-comment-for-preprocessors branch from 08acda4 to 5c53cf2 Compare November 11, 2019 16:23
}) as Replacement
)
);
if (args.slice(1, args.length - 2).some(Boolean)) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm still not sure what the changes to replace_async are for. The intention was that it be a low-level utility function, behaving a lot like String.prototype.replace, except able to work with callbacks that return promises. If you need different behavior than that, it should be handled in the callback you pass to replace_async.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh yea you are right. i've updated my changes.

@tanhauhau tanhauhau force-pushed the tanhauhau/skip-comment-for-preprocessors branch from 5c53cf2 to 3fb9626 Compare November 12, 2019 01:01
@Rich-Harris Rich-Harris merged commit d67b448 into sveltejs:master Nov 14, 2019
@tanhauhau tanhauhau deleted the tanhauhau/skip-comment-for-preprocessors branch November 14, 2019 23:53
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.

Bug: "<!-- -->"does not work inside script tag
3 participants