Convert Sass to Dart 3 style #2038
Conversation
nex3
force-pushed
the
dart3
branch
2 times, most recently
from
3a7c041
to
2c34d3d
Compare
nex3
force-pushed
the
main
branch
2 times, most recently
from
9cff4ca
to
6c59213
Compare
| @@ -13,7 +13,8 @@ import 'comment.dart'; | |||
| import 'style_rule.dart'; | |||
|
|
|||
| /// A statement in a plain CSS syntax tree. | |||
| abstract class CssNode extends AstNode { | |||
| @sealed | |||
There was a problem hiding this comment.
Just to confirm my own understanding: this uses the @sealed annotation rather than the sealed keyword because the former allows extensions within the same package while the latter only allows extensions within the same library, right?
There was a problem hiding this comment.
Yep. See also dart-lang/language#3113
| if (rest case var rest?) "${_parenthesizeArgument(rest)}...", | ||
| if (keywordRest case var keywordRest?) | ||
| "${_parenthesizeArgument(keywordRest)}..." |
There was a problem hiding this comment.
Is there something extra that using a case expression gets us here? If not, if (rest != null) reads cleaner to me.
There was a problem hiding this comment.
Yes: Dart doesn't do type inference on fields, so if (rest != null) would give an error when using rest in a non-nullable context. This way, we create a new non-nullable variable so inference isn't necessary. (In other words, it allows us to delete var rest = this.rest and var keywordRest = this.keywordRest above.)
| right.contents.length > 1); | ||
| var rightNeedsParens = switch (right) { | ||
| BinaryOperationExpression(:var operator) => | ||
| operator.precedence <= operator.precedence && |
There was a problem hiding this comment.
I'm a little confused by what's going on here. It seems like we're just comparing operator.precedence (and on the line below, operator) to itself.
There was a problem hiding this comment.
Oops, yeah, this needs some this.s
| codeUnit == $hash && | ||
|
|
||
| case $backslash && var codeUnit: | ||
| case var codeUnit when codeUnit == quote: |
There was a problem hiding this comment.
Is there a reason why case quote && var codeUnit doesn't work here?
There was a problem hiding this comment.
You can't reference variables in patterns, only constants. See dart-lang/language#3064
| var next = text.codeUnitAt(i + 1); | ||
| if (isWhitespace(next) || isHex(next)) { | ||
| buffer.writeCharCode($space); | ||
| switch (text.codeUnitAt(i)) { |
There was a problem hiding this comment.
Given how many var codeUnits we need below, would it make more sense to just assign it outside the switch?
|
|
||
| case Value_Value.map: | ||
| return value.map.entries.isEmpty | ||
| ? const SassMap.empty() | ||
| : SassMap({ | ||
| for (var entry in value.map.entries) | ||
| _deprotofy(entry.key): _deprotofy(entry.value) | ||
| for (var Value_Map_Entry(:key, :value) in value.map.entries) |
There was a problem hiding this comment.
Does the pairs getter not work here?
There was a problem hiding this comment.
No, this isn't a Dart map, it's a custom protocol buffer type. (It's not even a protocol buffer map because it needs to guarantee entry order is preserved.)
See sass/sass-spec#1925