-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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: nested comments and strings, new regexp utils #7650
Conversation
If ok I'll apply util to the modules mentioned by #7549 😊 |
I think it is ok to have this refactoring in a single PR, yes. I like that the regexes were combined in a single one. About the utils, maybe CleanString shouldn't extend String? Now you have the string itself, |
@patak-dev how about this. I think if(s[0] === '`' || s[0] === '"' || s[0] === "'") {
const [start, end] = findEmptyStringRawIndex(cleanString, s, index)
match[xxx] = cleanString.raw.slice(start, end)
} |
If need a new PR, I think should merge this PR first. 👀 #7549 dependence on But there is a known bug that has not been resolved that the nested string template syntax does not match. Since the previous regexp match should just ignore the comment, adding a string template might have a destructive error, since most of the time all the code is processed. |
maybe string template don't need to replace all. #7647 const str = `${a${`c${`d`}`}b}`
const clean = `${\0${`\0${`\0`}`}\0}` |
Because regular expressions can't be written, we can only use lex methods such as |
@poyoho interesting. Using a lexer only for template strings may be fast enough 🤔 Note: if you think it is hard to divide it in two, then let's keep it together and ask for others to check it out. |
It's okay, it's easy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for handling these bugs :)
Description
fix: #7632
fix: #7647
mentioned #7549
Additional context
fix comment nested string, string nested comment
and try to create regexes general utilities #7549
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).