Skip to content

Commit

Permalink
Improve inspect() output for complex units (sass#2138)
Browse files Browse the repository at this point in the history
Closes sass#2070
  • Loading branch information
nex3 authored and Ludwig Meysel committed Feb 20, 2024
1 parent bdc95ec commit be20a48
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 29 deletions.
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

0 comments on commit be20a48

Please sign in to comment.