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

Parse silent comments in _interpolatedDeclarationValue() #2266

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

nex3
Copy link
Contributor

@nex3 nex3 commented Jun 17, 2024

@nex3 nex3 requested a review from Goodwine June 17, 2024 23:24
@nex3 nex3 force-pushed the silent-comment-everywhere branch from 280de8c to 18cd524 Compare June 17, 2024 23:28
@@ -133,7 +133,8 @@ class Parser {
whitespace();
}

/// Consumes and ignores a silent (Sass-style) comment.
/// Consumes and ignores a single silent (Sass-style) comment, not including
/// the trailing newline.
Copy link
Member

Choose a reason for hiding this comment

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

"not including the trailing newline" makes me wonder about what happens when there are consecutive lines with silent comments, but otherwise LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function will only consume one of them, but whitespace() will consume them all and the whitespace.

@nex3 nex3 merged commit 04b6251 into main Jun 20, 2024
34 checks passed
@nex3 nex3 deleted the silent-comment-everywhere branch June 20, 2024 20:17
wroteNewline = false;
}

case $slash when silentComments && scanner.peekChar(1) == $slash:
buffer.write(rawText(loudComment));
Copy link
Contributor

Choose a reason for hiding this comment

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

should this really parse a loud comment when finding // ? Or is this dead code because of going through the previous case all the time ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right that it's incorrect and also that it's dead code. I'll send out a PR to remove it.

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.

Silent comments are incorrectly preserved in at-rule values
3 participants