Skip to content

Commit

Permalink
Fix sass#417 preserve location of trailing loud comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
nickbehrens committed Oct 16, 2019
1 parent 6b8c168 commit 32e18b5
Show file tree
Hide file tree
Showing 11 changed files with 138 additions and 32 deletions.
4 changes: 4 additions & 0 deletions lib/src/ast/css/comment.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,9 @@ abstract class CssComment extends CssNode {
/// compressed mode.
bool get isPreserved;

/// Whether this comment follows non-comment text on a line and should remain
/// attached to that non-comment text when being serialized.
bool get isTrailing;

T accept<T>(CssVisitor<T> visitor) => visitor.visitCssComment(this);
}
4 changes: 3 additions & 1 deletion lib/src/ast/css/modifiable/comment.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ import 'node.dart';
class ModifiableCssComment extends ModifiableCssNode implements CssComment {
final String text;
final FileSpan span;
final bool _isTrailing;

bool get isPreserved => text.codeUnitAt(2) == $exclamation;
bool get isTrailing => _isTrailing;

ModifiableCssComment(this.text, this.span);
ModifiableCssComment(this.text, this.span, this._isTrailing);

T accept<T>(ModifiableCssVisitor<T> visitor) => visitor.visitCssComment(this);
}
6 changes: 5 additions & 1 deletion lib/src/ast/sass/statement/loud_comment.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ class LoudComment implements Statement {

FileSpan get span => text.span;

LoudComment(this.text);
/// Whether this comment follows non-comment text on a line and should remain
/// attached to that non-comment text when being serialized.
final bool isTrailing;

LoudComment(this.text, this.isTrailing);

T accept<T>(StatementVisitor<T> visitor) => visitor.visitLoudComment(this);

Expand Down
10 changes: 8 additions & 2 deletions lib/src/parse/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,17 @@ class Parser {
}

/// Consumes whitespace, but not comments.
///
/// Returns whether any newlines were seen in the consumed whitespace.
@protected
void whitespaceWithoutComments() {
bool whitespaceWithoutComments() {
bool sawAnyNewlines = false;
while (!scanner.isDone && isWhitespace(scanner.peekChar())) {
scanner.readChar();
if (isNewline(scanner.readChar())) {
sawAnyNewlines = true;
}
}
return sawAnyNewlines;
}

/// Consumes spaces and tabs.
Expand Down
3 changes: 2 additions & 1 deletion lib/src/parse/sass.dart
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,8 @@ class SassParser extends StylesheetParser {
}
if (!buffer.trailingString.trimRight().endsWith("*/")) buffer.write(" */");

return LoudComment(buffer.interpolation(scanner.spanFrom(start)));
// The sass syntax does not support trailing loud comments ==> isTrailing = false.
return LoudComment(buffer.interpolation(scanner.spanFrom(start)), false);
}

void whitespace() {
Expand Down
39 changes: 31 additions & 8 deletions lib/src/parse/scss.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ class ScssParser extends StylesheetParser {

List<Statement> children(Statement child()) {
scanner.expectChar($lbrace);
whitespaceWithoutComments();
var children = <Statement>[];
_whitespaceAndOptionalTrailingLoudComment(children);
while (true) {
switch (scanner.peekChar()) {
case $dollar:
Expand All @@ -76,7 +76,7 @@ class ScssParser extends StylesheetParser {
whitespaceWithoutComments();
break;
case $asterisk:
children.add(_loudComment());
children.add(_loudComment(/* isTrailing= */ false));
whitespaceWithoutComments();
break;
default:
Expand All @@ -87,7 +87,7 @@ class ScssParser extends StylesheetParser {

case $semicolon:
scanner.readChar();
whitespaceWithoutComments();
_whitespaceAndOptionalTrailingLoudComment(children);
break;

case $rbrace:
Expand All @@ -103,7 +103,7 @@ class ScssParser extends StylesheetParser {

List<Statement> statements(Statement statement()) {
var statements = <Statement>[];
whitespaceWithoutComments();
_whitespaceAndOptionalTrailingLoudComment(statements);
while (!scanner.isDone) {
switch (scanner.peekChar()) {
case $dollar:
Expand All @@ -117,7 +117,7 @@ class ScssParser extends StylesheetParser {
whitespaceWithoutComments();
break;
case $asterisk:
statements.add(_loudComment());
statements.add(_loudComment(/* isTrailing= */ false));
whitespaceWithoutComments();
break;
default:
Expand All @@ -129,7 +129,7 @@ class ScssParser extends StylesheetParser {

case $semicolon:
scanner.readChar();
whitespaceWithoutComments();
_whitespaceAndOptionalTrailingLoudComment(statements);
break;

default:
Expand Down Expand Up @@ -163,7 +163,7 @@ class ScssParser extends StylesheetParser {
}

/// Consumes a statement-level loud comment block.
LoudComment _loudComment() {
LoudComment _loudComment(bool isTrailing) {
var start = scanner.state;
scanner.expect("/*");
var buffer = InterpolationBuffer()..write("/*");
Expand All @@ -182,7 +182,8 @@ class ScssParser extends StylesheetParser {
if (scanner.peekChar() != $slash) break;

buffer.writeCharCode(scanner.readChar());
return LoudComment(buffer.interpolation(scanner.spanFrom(start)));
return LoudComment(
buffer.interpolation(scanner.spanFrom(start)), isTrailing);

case $cr:
scanner.readChar();
Expand All @@ -200,4 +201,26 @@ class ScssParser extends StylesheetParser {
}
}
}

/// Consumes whitespace and optionally a trailing loud comment, adding the
/// trailing loud comment to [statements] if consumed.
///
/// A trailing loud comment is only possible if the initially consumed
/// whitespace lacks any newlines. If that's the case and a loud comment
/// immediately follows, the loud comment will also be consumed.
void _whitespaceAndOptionalTrailingLoudComment(List<Statement> statements) {
bool sawAnyNewlines = whitespaceWithoutComments();
if (_peekForStartOfLoudComment()) {
bool isTrailing = !sawAnyNewlines;
statements.add(_loudComment(isTrailing));
whitespaceWithoutComments();
}
}

/// Returns whether the scanner is pointing at the start of a loud comment.
bool _peekForStartOfLoudComment() {
return !scanner.isDone &&
scanner.peekChar() == $slash &&
scanner.peekChar(1) == $asterisk;
}
}
5 changes: 3 additions & 2 deletions lib/src/visitor/async_evaluate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1500,7 +1500,7 @@ class _EvaluateVisitor
}

_parent.addChild(ModifiableCssComment(
await _performInterpolation(node.text), node.span));
await _performInterpolation(node.text), node.span, node.isTrailing));
return null;
}

Expand Down Expand Up @@ -2424,7 +2424,8 @@ class _EvaluateVisitor
_endOfImports++;
}

_parent.addChild(ModifiableCssComment(node.text, node.span));
_parent
.addChild(ModifiableCssComment(node.text, node.span, node.isTrailing));
}

Future<void> visitCssDeclaration(CssDeclaration node) async {
Expand Down
2 changes: 1 addition & 1 deletion lib/src/visitor/clone_css.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class _CloneCssVisitor implements CssVisitor<ModifiableCssNode> {
}

ModifiableCssComment visitCssComment(CssComment node) =>
ModifiableCssComment(node.text, node.span);
ModifiableCssComment(node.text, node.span, node.isTrailing);

ModifiableCssDeclaration visitCssDeclaration(CssDeclaration node) =>
ModifiableCssDeclaration(node.name, node.value, node.span,
Expand Down
9 changes: 5 additions & 4 deletions lib/src/visitor/evaluate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// DO NOT EDIT. This file was generated from async_evaluate.dart.
// See tool/grind/synchronize.dart for details.
//
// Checksum: f4f4c5d1cbc9894d14b6d8ce7c1a3c09146db9ba
// Checksum: 56067ad7a31ee7adf5bb4e9d484fe95f38d3762c
//
// ignore_for_file: unused_import

Expand Down Expand Up @@ -1498,8 +1498,8 @@ class _EvaluateVisitor
_endOfImports++;
}

_parent.addChild(
ModifiableCssComment(_performInterpolation(node.text), node.span));
_parent.addChild(ModifiableCssComment(
_performInterpolation(node.text), node.span, node.isTrailing));
return null;
}

Expand Down Expand Up @@ -2409,7 +2409,8 @@ class _EvaluateVisitor
_endOfImports++;
}

_parent.addChild(ModifiableCssComment(node.text, node.span));
_parent
.addChild(ModifiableCssComment(node.text, node.span, node.isTrailing));
}

void visitCssDeclaration(CssDeclaration node) {
Expand Down
58 changes: 46 additions & 12 deletions lib/src/visitor/serialize.dart
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,12 @@ class _SerializeVisitor

if (previous != null) {
if (_requiresSemicolon(previous)) _buffer.writeCharCode($semicolon);
_writeLineFeed();
if (_shouldWriteLineFeedBeforeChild(child)) {
_writeLineFeed();
} else {
_writeOptionalSpace();
}

if (previous.isGroupEnd) _writeLineFeed();
}
previous = child;
Expand Down Expand Up @@ -1085,23 +1090,41 @@ class _SerializeVisitor
return;
}

_writeLineFeed();
bool indentNextChild;
if (_shouldWriteLineFeedBeforeChildren(children)) {
_writeLineFeed();
indentNextChild = true;
} else {
_writeOptionalSpace();
indentNextChild = false;
}

CssNode previous;
_indent(() {
for (var i = 0; i < children.length; i++) {
var child = children[i];
if (_isInvisible(child)) continue;
for (var i = 0; i < children.length; i++) {
var child = children[i];
if (_isInvisible(child)) continue;

if (previous != null) {
if (_requiresSemicolon(previous)) _buffer.writeCharCode($semicolon);
if (previous != null) {
if (_requiresSemicolon(previous)) _buffer.writeCharCode($semicolon);
if (_shouldWriteLineFeedBeforeChild(child)) {
_writeLineFeed();
if (previous.isGroupEnd) _writeLineFeed();
indentNextChild = true;
} else {
_writeOptionalSpace();
indentNextChild = false;
}
previous = child;
if (previous.isGroupEnd) _writeLineFeed();
}
previous = child;

child.accept(this);
if (indentNextChild) {
_indentation++;
}
});
child.accept(this);
if (indentNextChild) {
_indentation--;
}
}

if (_requiresSemicolon(previous) && !_isCompressed) {
_buffer.writeCharCode($semicolon);
Expand All @@ -1115,6 +1138,17 @@ class _SerializeVisitor
bool _requiresSemicolon(CssNode node) =>
node is CssParentNode ? node.isChildless : node is! CssComment;

/// Whether [child] should have a line feed written before the child is serialized.
bool _shouldWriteLineFeedBeforeChild(CssNode child) =>
child is! CssComment || !(child as CssComment).isTrailing;

/// Whether [children] should have a line feed written before the children are serialized.
bool _shouldWriteLineFeedBeforeChildren(List<CssNode> children) {
return children.isEmpty ||
!(children.first is CssComment) ||
!((children.first as CssComment).isTrailing);
}

/// Writes a line feed, unless this emitting compressed CSS.
void _writeLineFeed() {
if (!_isCompressed) _buffer.write(_lineFeed.text);
Expand Down
30 changes: 30 additions & 0 deletions test/output_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,34 @@ void main() {
});
});
});

// Regression test for sass/dart-sass#417.
group("preserve trailing loud comments", () {
// No need for "in Sass" cases as it's not possible to have
// trailing loud comments in the Sass syntax.
group("in SCSS", () {
test("after open block", () {
expect(compileString("""
selector { /* please don't move me */
name: value;
}"""), equals("""
selector { /* please don't move me */
name: value;
}"""));
});
test("after declaration", () {
expect(compileString("""
selector {
name: value; /* please don't move me */
}"""), equals("""
selector {
name: value; /* please don't move me */
}"""));
});
test("after top-level statement", () {
expect(compileString("@rule; /* please don't move me */"),
equals("@rule; /* please don't move me */"));
});
});
});
}

0 comments on commit 32e18b5

Please sign in to comment.