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

Fix #417 preserve location of trailing loud comments #849

Merged
merged 6 commits into from Jun 3, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
122 changes: 88 additions & 34 deletions lib/src/visitor/serialize.dart
Expand Up @@ -153,14 +153,16 @@ class _SerializeVisitor

void visitCssStylesheet(CssStylesheet node) {
CssNode? previous;
for (var i = 0; i < node.children.length; i++) {
var child = node.children[i];
for (var child in node.children) {
if (_isInvisible(child)) continue;

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

Expand Down Expand Up @@ -208,7 +210,7 @@ class _SerializeVisitor

if (!node.isChildless) {
_writeOptionalSpace();
_visitChildren(node.children);
_visitChildren(node.children, node);
}
}

Expand All @@ -226,7 +228,7 @@ class _SerializeVisitor
});

_writeOptionalSpace();
_visitChildren(node.children);
_visitChildren(node.children, node);
}

void visitCssImport(CssImport node) {
Expand Down Expand Up @@ -273,7 +275,7 @@ class _SerializeVisitor
() =>
_writeBetween(node.selector.value, _commaSeparator, _buffer.write));
_writeOptionalSpace();
_visitChildren(node.children);
_visitChildren(node.children, node);
}

void _visitMediaQuery(CssMediaQuery query) {
Expand All @@ -298,7 +300,7 @@ class _SerializeVisitor

_for(node.selector, () => node.selector.value.accept(this));
_writeOptionalSpace();
_visitChildren(node.children);
_visitChildren(node.children, node);
}

void visitCssSupportsRule(CssSupportsRule node) {
Expand All @@ -315,7 +317,7 @@ class _SerializeVisitor
});

_writeOptionalSpace();
_visitChildren(node.children);
_visitChildren(node.children, node);
}

void visitCssDeclaration(CssDeclaration node) {
Expand Down Expand Up @@ -1287,44 +1289,88 @@ class _SerializeVisitor
_for(value, () => _buffer.write(value.value));

/// Emits [children] in a block.
void _visitChildren(List<CssNode> children) {
void _visitChildren(List<CssNode> children, CssParentNode parent) {
Goodwine marked this conversation as resolved.
Show resolved Hide resolved
_buffer.writeCharCode($lbrace);
if (children.every(_isInvisible)) {
_buffer.writeCharCode($rbrace);
return;
}

_writeLineFeed();
CssNode? previous_;
_indent(() {
for (var i = 0; i < children.length; i++) {
var child = children[i];
if (_isInvisible(child)) continue;
CssNode? prePrevious_;
Goodwine marked this conversation as resolved.
Show resolved Hide resolved
CssNode previous = parent;
Goodwine marked this conversation as resolved.
Show resolved Hide resolved
for (var child in children) {
if (_isInvisible(child)) continue;
if (previous != parent && _requiresSemicolon(previous)) {
_buffer.writeCharCode($semicolon);
}

var previous = previous_; // dart-lang/sdk#45348
if (previous != null) {
if (_requiresSemicolon(previous)) _buffer.writeCharCode($semicolon);
_writeLineFeed();
if (previous.isGroupEnd) _writeLineFeed();
}
previous_ = child;
if (_isTrailingComment(child, previous)) {
_writeOptionalSpace();
_withoutIndendation(() => child.accept(this));
} else {
_writeLineFeed();
_indent(() {
child.accept(this);
});
}

child.accept(this);
prePrevious_ = previous;
previous = child;
}

if (previous != parent) {
if (_requiresSemicolon(previous) && !_isCompressed) {
_buffer.writeCharCode($semicolon);
}
});

if (_requiresSemicolon(previous_!) && !_isCompressed) {
_buffer.writeCharCode($semicolon);
if (prePrevious_ == parent &&
_isTrailingComment(previous, prePrevious_!)) {
_writeOptionalSpace();
} else {
_writeLineFeed();
_writeIndentation();
}
}
_writeLineFeed();
_writeIndentation();

_buffer.writeCharCode($rbrace);
}

/// Whether [node] requires a semicolon to be written after it.
bool _requiresSemicolon(CssNode? node) =>
node is CssParentNode ? node.isChildless : node is! CssComment;

/// Whether [node] represents a trailing comment when it appears after
/// [previous] in a sequence of nodes being serialized.
///
/// Note [previous] could be either a sibling of [node] or the parent of
/// [node], with [node] being the first visible child.
bool _isTrailingComment(CssNode node, CssNode previous) {
// Short-circuit in compressed mode to avoid expensive span shenanigans
// (shespanigans?), since we're compressing all whitespace anyway.
if (_isCompressed) return false;
if (node is! CssComment) return false;

var previousSpan = previous.span;
if (previous is CssParentNode && previous.children.contains(node)) {
// Walk back from just before the current node starts looking for the
// parent's left brace (to open the child block). This is safer than
Goodwine marked this conversation as resolved.
Show resolved Hide resolved
// a simple forward search of the previousSpan.text as that might contain
// other left braces.
var searchFrom = node.span.start.offset - previousSpan.start.offset - 1;
if (searchFrom < 0) {
Goodwine marked this conversation as resolved.
Show resolved Hide resolved
// This can happen when the loud comment is the first statement in a
// mixin that's later included. In that case, node.span.start.offset
// (representing the loud comment in the mixin) will be less than
// previousSpan.start.offset (representing the statement where the
// mixin is later included) ==> the loud comment cannot possibly be
// trailing.
return false;
}

var endOffset = previousSpan.text.lastIndexOf("{", searchFrom);
endOffset = math.max(0, endOffset);
previousSpan = previousSpan.file.span(
previousSpan.start.offset, previousSpan.start.offset + endOffset);
}
return node.span.start.line == previousSpan.end.line;
}

/// Writes a line feed, unless this emitting compressed CSS.
void _writeLineFeed() {
if (!_isCompressed) _buffer.write(_lineFeed.text);
Expand Down Expand Up @@ -1373,6 +1419,14 @@ class _SerializeVisitor
_indentation--;
}

/// Runs [callback] without any indentation.
void _withoutIndendation(void callback()) {
var savedIndentation = _indentation;
_indentation = 0;
callback();
_indentation = savedIndentation;
}

/// Returns whether [node] is considered invisible.
bool _isInvisible(CssNode node) {
if (_inspect) return false;
Expand Down
2 changes: 1 addition & 1 deletion test/dart_api_test.dart
Expand Up @@ -304,7 +304,7 @@ a {
expect(compileStringAsync("""
@use 'sass:meta';
@include meta.load-css("other.scss");
""", loadPaths: [d.sandbox]), completion(equals("/**/\n/**/")));
""", loadPaths: [d.sandbox]), completion(equals("/**/ /**/")));

// Give the race condition time to appear.
await pumpEventQueue();
Expand Down