Skip to content

Commit

Permalink
Address PR feedback.
Browse files Browse the repository at this point in the history
Revert parser changes.  Instead look at node span line numbers to determine whether a loud comment is trailing.

Add some more test cases.
  • Loading branch information
nickbehrens committed Oct 18, 2019
1 parent 6272552 commit d937f87
Show file tree
Hide file tree
Showing 10 changed files with 122 additions and 123 deletions.
4 changes: 0 additions & 4 deletions lib/src/ast/css/comment.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,5 @@ 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: 1 addition & 3 deletions lib/src/ast/css/modifiable/comment.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@ 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, this._isTrailing);
ModifiableCssComment(this.text, this.span);

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

FileSpan get span => text.span;

/// 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);
LoudComment(this.text);

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

Expand Down
22 changes: 4 additions & 18 deletions lib/src/parse/sass.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import '../ast/sass.dart';
import '../interpolation_buffer.dart';
import '../logger.dart';
import '../util/character.dart';
import '../value.dart';
import 'stylesheet.dart';

/// A parser for the indented syntax.
Expand Down Expand Up @@ -99,21 +98,9 @@ class SassParser extends StylesheetParser {
scanner.readChar();
next = scanner.peekChar();
}
var url = scanner.substring(start.position);
var span = scanner.spanFrom(start);

if (isPlainImportUrl(url)) {
// Serialize [url] as a Sass string because [StaticImport] expects it to
// include quotes.
return StaticImport(
Interpolation([SassString(url).toString()], span), span);
} else {
try {
return DynamicImport(parseImportUrl(url), span);
} on FormatException catch (innerError) {
error("Invalid URL: ${innerError.message}", span);
}
}

return DynamicImport(parseImportUrl(scanner.substring(start.position)),
scanner.spanFrom(start));
}

bool scanElse(int ifIndentation) {
Expand Down Expand Up @@ -306,8 +293,7 @@ class SassParser extends StylesheetParser {
}
if (!buffer.trailingString.trimRight().endsWith("*/")) buffer.write(" */");

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

void whitespace() {
Expand Down
39 changes: 8 additions & 31 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(/* isTrailing= */ false));
children.add(_loudComment());
whitespaceWithoutComments();
break;
default:
Expand All @@ -87,7 +87,7 @@ class ScssParser extends StylesheetParser {

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

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

List<Statement> statements(Statement statement()) {
var statements = <Statement>[];
_whitespaceAndOptionalTrailingLoudComment(statements);
whitespaceWithoutComments();
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(/* isTrailing= */ false));
statements.add(_loudComment());
whitespaceWithoutComments();
break;
default:
Expand All @@ -129,7 +129,7 @@ class ScssParser extends StylesheetParser {

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

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

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

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

case $cr:
scanner.readChar();
Expand All @@ -201,26 +200,4 @@ 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: 2 additions & 3 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, node.isTrailing));
await _performInterpolation(node.text), node.span));
return null;
}

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

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

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, node.isTrailing);
ModifiableCssComment(node.text, node.span);

ModifiableCssDeclaration visitCssDeclaration(CssDeclaration node) =>
ModifiableCssDeclaration(node.name, node.value, node.span,
Expand Down
9 changes: 4 additions & 5 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: 56067ad7a31ee7adf5bb4e9d484fe95f38d3762c
// Checksum: f4f4c5d1cbc9894d14b6d8ce7c1a3c09146db9ba
//
// ignore_for_file: unused_import

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

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

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

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

void visitCssDeclaration(CssDeclaration node) {
Expand Down
Loading

0 comments on commit d937f87

Please sign in to comment.