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 all 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
9 changes: 9 additions & 0 deletions lib/src/util/span.dart
Expand Up @@ -77,6 +77,15 @@ extension SpanExtensions on FileSpan {
_scanIdentifier(scanner);
return subspan(scanner.position).trimLeft();
}

/// Whether [this] FileSpan contains the [target] FileSpan.
///
/// Validates the FileSpans to be in the same file and for the [target] to be
/// within [this] FileSpan inclusive range [start,end].
bool contains(FileSpan target) =>
file.url == target.file.url &&
start.offset <= target.start.offset &&
end.offset >= target.end.offset;
}

/// Consumes an identifier from [scanner].
Expand Down
118 changes: 82 additions & 36 deletions lib/src/visitor/serialize.dart
Expand Up @@ -21,6 +21,7 @@ import '../util/character.dart';
import '../util/no_source_map_buffer.dart';
import '../util/number.dart';
import '../util/source_map_buffer.dart';
import '../util/span.dart';
import '../value.dart';
import 'interface/css.dart';
import 'interface/selector.dart';
Expand Down Expand Up @@ -153,14 +154,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 +211,7 @@ class _SerializeVisitor

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

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

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

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

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

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

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

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

void visitCssDeclaration(CssDeclaration node) {
Expand Down Expand Up @@ -1286,45 +1289,80 @@ class _SerializeVisitor
void _write(CssValue<String> value) =>
_for(value, () => _buffer.write(value.value));

/// Emits [children] in a block.
void _visitChildren(List<CssNode> children) {
/// Emits [parent.children] in a block.
void _visitChildren(CssParentNode parent) {
_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;
CssNode? previous;
for (var child in parent.children) {
if (_isInvisible(child)) continue;

var previous = previous_; // dart-lang/sdk#45348
if (previous != null) {
if (_requiresSemicolon(previous)) _buffer.writeCharCode($semicolon);
_writeLineFeed();
if (previous.isGroupEnd) _writeLineFeed();
}
previous_ = child;
if (previous != null && _requiresSemicolon(previous)) {
_buffer.writeCharCode($semicolon);
}

child.accept(this);
if (_isTrailingComment(child, previous ?? parent)) {
_writeOptionalSpace();
_withoutIndendation(() => child.accept(this));
} else {
_writeLineFeed();
_indent(() {
child.accept(this);
});
}
});

if (_requiresSemicolon(previous_!) && !_isCompressed) {
_buffer.writeCharCode($semicolon);
prePrevious = previous;
previous = child;
}
_writeLineFeed();
_writeIndentation();

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

if (prePrevious == null && _isTrailingComment(previous, parent)) {
_writeOptionalSpace();
} else {
_writeLineFeed();
_writeIndentation();
}
}

_buffer.writeCharCode($rbrace);
}

/// Whether [node] requires a semicolon to be written after it.
bool _requiresSemicolon(CssNode? node) =>
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;

if (!previous.span.contains(node.span)) {
return node.span.start.line == previous.span.end.line;
}

// 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 a
// simple forward search of the previous.span.text as that might contain
// other left braces.
var searchFrom = node.span.start.offset - previous.span.start.offset - 1;
var endOffset = previous.span.text.lastIndexOf("{", searchFrom);
endOffset = math.max(0, endOffset);
var span = previous.span.file.span(
previous.span.start.offset, previous.span.start.offset + endOffset);
return node.span.start.line == span.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 +1411,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