Consistently use floating-point numbers everywhere #1802
Conversation
lib/src/util/number.dart
Outdated
| var result = fuzzyCheckRange(number, min, max); | ||
| if (result != null) return result; | ||
| throw RangeError.range( | ||
| number, min, max, name, "must be between $min and $max"); | ||
| } | ||
|
|
||
| /// Return [num1] modulo [num2], using Sass's modulo semantics, which it | ||
| /// inherited from Ruby and which differ from Dart's. |
There was a problem hiding this comment.
Should we mention the more general names for each language's semantics here? Wikipedia says that Ruby/Sass uses floored division, while Dart uses Euclidean division.
Now that I think of it, it doesn't look like we define Sass's modulo semantics in the spec (either the existing ones or the floating point proposal). Should we do that?
There was a problem hiding this comment.
Good catch! sass/sass#3393
lib/src/util/number.dart
Outdated
| /// digit. | ||
| bool fuzzyEquals(num number1, num number2) { | ||
| if (number1 == number2) return true; | ||
| // Using `!<=` here rather than `>` means we filter out `Infinity |
There was a problem hiding this comment.
should this use <= and not !<= in the comment ?
There was a problem hiding this comment.
and isn't this missing a backtick at the end of the line ?
There was a problem hiding this comment.
I think this comment is just no longer necessary.
CHANGELOG.md
Outdated
| @@ -10,6 +31,10 @@ | |||
| * Add an optional `argumentName` parameter to `SassScriptException()` to make it | |||
| easier to throw exceptions associated with particular argument names. | |||
|
|
|||
| * Most APIs that previously returned `num` now return `double`. All APIs | |||
| continue to _accept_ `num`, although in Dart 2.0.0 most of these APIs will be | |||
There was a problem hiding this comment.
Do you have any insight about which APIs may be excluded here (the reason for saying most of in this sentence) ?
As I'm working on version 2 of scssphp (to make it fully spec compliant and easier to maintain), I think it would make sense that scssphp would use float (PHP's name for the double type) for arguments instead of float|int (which is the equivalent of Dart's num type), as we don't have existing consumers of those APIs yet for which we need to provide BC. So I'd like to know if there are some places for which I should not do the migration.
There was a problem hiding this comment.
You should be safe to just use float everywhere. I'll update this to remove "most of".
Closes #1794
See sass/sass-spec#1826