-
-
Notifications
You must be signed in to change notification settings - Fork 442
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: noCommentText does not work when any other text is a child(#3298) #3446
Conversation
CodSpeed Performance ReportMerging #3446 will not alter performanceComparing Summary
|
let node_text = node.text().to_string(); | ||
|
||
// Replace the comments with JSX comments | ||
let new_jsx_value = COMMENT_REGEX.replace_all(&node_text, |caps: ®ex::Captures| { |
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.
This is just my preference (not a suggestion) for readability. What do you think?
let new_jsx_value = COMMENT_REGEX.replace_all(&node_text, |caps: ®ex::Captures| {
let comment = caps[0]
.trim_start_matches("//")
.trim_start_matches("/*")
.trim_end_matches("*/")
.trim();
format!("{{/*{}*/}}", comment)
});
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.
@chansuke
To accommodate Doc comments(like /** example */
), the code has been changed as follows
let new_jsx_value = COMMENT_REGEX.replace_all(&node_text, |caps: ®ex::Captures| {
let comment = caps[0].trim();
match comment {
c if c.starts_with("//") => format!("{{/* {} */}}", c[2..].trim()),
c if c.starts_with("/**") => format!("{{/** {} */}}", &c[3..c.len() - 2].trim()),
c => format!("{{/* {} */}}", &c[2..c.len() - 2].trim()),
}
});
If you have any suggestions for a better code, I welcome them!
5 │ - ····<div>text·/*·comment·*/</div> | ||
5 │ + ····<div>text·{/*comment*/}</div> |
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.
The previous suggestion was able to keep the spaces, but now we remove leading and trailing spaces between the word "comment".
Before: const·a4·=·<div>{/*·comment·*/}</div>;
Now: <div>text·{/*comment*/}</div>
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.
@ematipico
thanks your check, I had missed it...
I fixed in this commit!
9c6f601
Summary
Changed logic to judge by regex, considering the case where a comment is inserted in the middle of a paragraph.
Expanded test cases and added explanation of rule.
The quick fix feature has also been developed to fix only the commented part.
Closes #3298
Test Plan
Invalid Case
Add comments, line breaks, etc. in sentences.
Valid Case