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

Improve inspect() output for complex units #2138

Merged
merged 4 commits into from
Dec 8, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## 1.69.6

* Produce better output for numbers with complex units in `meta.inspect()` and
debugging messages.

### JS API

* Fix a bug where certain exceptions could produce `SourceSpan`s that didn't
Expand Down
62 changes: 40 additions & 22 deletions lib/src/visitor/serialize.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import 'dart:math' as math;
import 'dart:typed_data';

import 'package:charcode/charcode.dart';
import 'package:collection/collection.dart';
import 'package:source_maps/source_maps.dart';
import 'package:string_scanner/string_scanner.dart';

Expand Down Expand Up @@ -489,13 +488,8 @@ final class _SerializeVisitor

void _writeCalculationValue(Object value) {
switch (value) {
case SassNumber(value: double(isFinite: false), hasComplexUnits: true):
if (!_inspect) {
throw SassScriptException("$value isn't a valid CSS value.");
}

_writeNumber(value.value);
_buffer.write(value.unitString);
case SassNumber(hasComplexUnits: true) when !_inspect:
throw SassScriptException("$value isn't a valid CSS value.");

case SassNumber(value: double(isFinite: false)):
switch (value.value) {
Expand All @@ -507,12 +501,15 @@ final class _SerializeVisitor
_buffer.write('NaN');
}

if (value.numeratorUnits.firstOrNull case var unit?) {
_writeOptionalSpace();
_buffer.writeCharCode($asterisk);
_writeOptionalSpace();
_buffer.writeCharCode($1);
_buffer.write(unit);
_writeCalculationUnits(value.numeratorUnits, value.denominatorUnits);

case SassNumber(hasComplexUnits: true):
_writeNumber(value.value);
if (value.numeratorUnits case [var first, ...var rest]) {
_buffer.write(first);
_writeCalculationUnits(rest, value.denominatorUnits);
} else {
_writeCalculationUnits([], value.denominatorUnits);
}

case Value():
Expand All @@ -534,14 +531,36 @@ final class _SerializeVisitor
_parenthesizeCalculationRhs(operator, right.operator)) ||
(operator == CalculationOperator.dividedBy &&
right is SassNumber &&
!right.value.isFinite &&
right.hasUnits);
(right.value.isFinite
? right.hasComplexUnits
: right.hasUnits));
if (parenthesizeRight) _buffer.writeCharCode($lparen);
_writeCalculationValue(right);
if (parenthesizeRight) _buffer.writeCharCode($rparen);
}
}

/// Writes the complex numerator and denominator units beyond the first
/// numerator unit for a number as they appear in a calculation.
void _writeCalculationUnits(
List<String> numeratorUnits, List<String> denominatorUnits) {
for (var unit in numeratorUnits) {
_writeOptionalSpace();
_buffer.writeCharCode($asterisk);
_writeOptionalSpace();
_buffer.writeCharCode($1);
_buffer.write(unit);
}

for (var unit in denominatorUnits) {
_writeOptionalSpace();
_buffer.writeCharCode($slash);
_writeOptionalSpace();
_buffer.writeCharCode($1);
_buffer.write(unit);
}
}

/// Returns whether the right-hand operation of a calculation should be
/// parenthesized.
///
Expand Down Expand Up @@ -787,16 +806,15 @@ final class _SerializeVisitor
return;
}

_writeNumber(value.value);

if (!_inspect) {
if (value.hasComplexUnits) {
if (value.hasComplexUnits) {
if (!_inspect) {
throw SassScriptException("$value isn't a valid CSS value.");
}

if (value.numeratorUnits case [var first]) _buffer.write(first);
visitCalculation(SassCalculation.unsimplified('calc', [value]));
} else {
_buffer.write(value.unitString);
_writeNumber(value.value);
if (value.numeratorUnits case [var first]) _buffer.write(first);
}
}

Expand Down
8 changes: 4 additions & 4 deletions test/embedded/function_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ void main() {
..value = 1
..numerators.addAll(["em", "px", "foo"])),
inspect: true),
"1em*px*foo");
"calc(1em * 1px * 1foo)");
});

test("with one denominator", () async {
Expand All @@ -923,7 +923,7 @@ void main() {
..value = 1
..denominators.add("em")),
inspect: true),
"1em^-1");
"calc(1 / 1em)");
});

test("with multiple denominators", () async {
Expand All @@ -934,7 +934,7 @@ void main() {
..value = 1
..denominators.addAll(["em", "px", "foo"])),
inspect: true),
"1(em*px*foo)^-1");
"calc(1 / 1em / 1px / 1foo)");
});

test("with numerators and denominators", () async {
Expand All @@ -946,7 +946,7 @@ void main() {
..numerators.addAll(["em", "px"])
..denominators.addAll(["s", "foo"])),
inspect: true),
"1em*px/s*foo");
"calc(1em * 1px / 1s / 1foo)");
});
});

Expand Down
51 changes: 48 additions & 3 deletions test/output_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,55 @@ void main() {
});
});

// Tests for sass/dart-sass#417.
// Tests for sass/dart-sass#2070.
//
// Note there's no need for "in Sass" cases as it's not possible to have
// trailing loud comments in the Sass syntax.
// These aren't covered by sass-spec because the inspect format for
// non-literal values isn't covered by the spec.
group("uses a nice format to inspect numbers with complex units", () {
group("finite", () {
test("top-level", () {
expect(compileString("""
@use 'sass:meta';
a {b: meta.inspect(1px * 1em)};
"""), equalsIgnoringWhitespace('a { b: calc(1px * 1em); }'));
});

test("in calc", () {
expect(compileString("""
@use 'sass:meta';
a {b: meta.inspect(calc(1px * 1em))};
"""), equalsIgnoringWhitespace('a { b: calc(1px * 1em); }'));
});

test("nested in calc", () {
expect(compileString("""
@use 'sass:meta';
a {b: meta.inspect(calc(c / (1px * 1em)))};
"""), equalsIgnoringWhitespace('a { b: calc(c / (1px * 1em)); }'));
});

test("numerator and denominator", () {
expect(compileString("""
@use 'sass:math';
@use 'sass:meta';
a {b: meta.inspect(1px * math.div(math.div(1em, 1s), 1x))};
"""), equalsIgnoringWhitespace('a { b: calc(1px * 1em / 1s / 1x); }'));
});

test("denominator only", () {
expect(compileString("""
@use 'sass:math';
@use 'sass:meta';
a {b: meta.inspect(math.div(math.div(1, 1s), 1x))};
"""), equalsIgnoringWhitespace('a { b: calc(1 / 1s / 1x); }'));
});
});
});

// Tests for sass/dart-sass#417.
//
// Note there's no need for "in Sass" cases as it's not possible to have
// trailing loud comments in the Sass syntax.
group("preserve trailing loud comments in SCSS", () {
test("after open block", () {
expect(compileString("""
Expand Down