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

Number comparison is different to ruby sass #590

Closed
xzyfer opened this issue Oct 29, 2014 · 7 comments · Fixed by #770
Closed

Number comparison is different to ruby sass #590

xzyfer opened this issue Oct 29, 2014 · 7 comments · Fixed by #770

Comments

@xzyfer
Copy link
Contributor

xzyfer commented Oct 29, 2014

I've was looking the current failing specs and noticed a common theme of broken number comparisons. A bunch of outstanding bugs boil down to the following evaluating to true in ruby sass and false in libsass.

a { 
  a: 1/2;
  b: 0.5;
  c: (1/2);
  d: 1/2 == 0.5;
  e: (1/2) == 0.5;
}

Output in Sass 3.4

a {
  a: 1/2;
  b: 0.5;
  c: 0.5;
  d: true;
  e: true; }

Output in Libsass 3.0.0

a {
  a: 1/2;
  b: 0.5;
  c: 0.5;
  d: false;
  e: true; }

Specs added sass/sass-spec#137.

@QuLogic
Copy link
Contributor

QuLogic commented Oct 30, 2014

Integer division?

@xzyfer
Copy link
Contributor Author

xzyfer commented Oct 30, 2014

I've updated the description with more information. I appears coercions to compatible types for equality operations aren't happening.

i.e. 1/2 == 0.5 is being treated literally as 1/2 == 0.5.

The coercion is happening as expected for other arithmetic expressions .i.e. 1/2 * 2 correctly produces 1;

@am11
Copy link
Contributor

am11 commented Nov 2, 2014

@xzyfer, nice job catching this bug! 👍
I found some more interesting stuff (related).

Consider the following code:

(taken from https://raw.githubusercontent.com/less/less.js/../test/less/operations.less, replaced @ with $):

#operations {
  color: (#110000 + #000011 + #001100); // #111111
  height: (10px / 2px + 6px - 1px * 2); // 9px
  width: (2 * 4 - 5em); // 3em
  .spacing {
    height: (10px / 2px+6px-1px*2);
    width: (2  * 4-5em);
  }
  substraction: (20 - 10 - 5 - 5); // 0
  division: (20 / 5 / 4); // 1
}

$x: 4;
$y: 12em;

.with-variables {
  height: ($x + $y); // 16em
  width: (12 + $y); // 24em
  size: (5cm - $x); // 1cm
}

.with-functions {
  color: (rgb(200, 200, 200) / 2);
  color: (2 * hsl(0, 50%, 50%));
  color: (rgb(10, 10, 10) + hsl(0, 50%, 50%));
}

$z: -2;

.negative {
  height: (2px + $z); // 0px
  width: (2px - $z); // 4px
}

.shorthands {
  padding: -1px 2px 0 -4px; //
}

.rem-dimensions {
  font-size: (20rem / 5 + 1.5rem); // 5.5rem
}

.colors {
  color: #123; // #112233
  border-color: (#234 + #111111); // #334455
  background-color: (#222222 - #fff); // #000000
  .other {
    color: (2 * #111); // #222222
    border-color: (#333333 / 3 + #111); // #222222
  }
}

.negations {
    $var: 4px;
    variable: (-$var); // 4
    variable1: (-$var + $var); // 0
    variable2: ($var + -$var); // 0
    variable3: ($var - -$var); // 8
    variable4: (-$var - -$var); // 0
    paren: (-($var)); // -4px
    paren2: (-(2 + 2) * -$var); // 16
}

Less output (as is: https://raw.githubusercontent.com/less/less.js/../test/css/operations.css):

#operations {
  color: #111111;
  height: 9px;
  width: 3em;
  substraction: 0;
  division: 1;
}
#operations .spacing {
  height: 9px;
  width: 3em;
}
.with-variables {
  height: 16em;
  width: 24em;
  size: 1cm;
}
.with-functions {
  color: #646464;
  color: #ff8080;
  color: #c94a4a;
}
.negative {
  height: 0px;
  width: 4px;
}
.shorthands {
  padding: -1px 2px 0 -4px;
}
.rem-dimensions {
  font-size: 5.5rem;
}
.colors {
  color: #123;
  border-color: #334455;
  background-color: #000000;
}
.colors .other {
  color: #222222;
  border-color: #222222;
}
.negations {
  variable: -4px;
  variable1: 0px;
  variable2: 0px;
  variable3: 8px;
  variable4: 0px;
  paren: -4px;
  paren2: 16px;
}

Ruby Sass output is same as above.

Libsass output:

#operations {
  color: #111111;
  height: 9px;
  width: 3em;
  substraction: 0;
  division: 1; }
  #operations .spacing {
    height: 17px-1px;
    width: 8 -5em; }

.with-variables {
  height: 16em;
  width: 24em;
  size: 1cm; }

.with-functions {
  color: #646464;
  color: #ff8080;
  color: #c94a4a; }

.negative {
  height: 0px;
  width: 4px; }

.shorthands {
  padding: -1px 2px 0 -4px; }

.rem-dimensions {
  font-size: 5.5rem; }

.colors {
  color: #123;
  border-color: #334455;
  background-color: black; }
  .colors .other {
    color: #222222;
    border-color: #222222; }

.negations {
  variable: -4px;
  variable1: 0px;
  variable2: 0px;
  variable3: 8px;
  variable4: 0px;
  paren: -4px;
  paren2: 16px; }

Perhaps we can use this test case as is? It is covering units conversion and basic arithmetic on values.

@xzyfer
Copy link
Contributor Author

xzyfer commented Nov 2, 2014

Nice find @am11 . IMO specs should aim to be more granular. I'd build a series to specs addressing the exact use cases where numbers are and aren't being handled correctly. There are a lot of different use cases which will likely be addressed incrementally.

@xzyfer
Copy link
Contributor Author

xzyfer commented Nov 2, 2014

It's worth noting fixing this behaviour will fix ~6-10 currently failing specs.

@mirisuzanne
Copy link

@xzyfer This bug seems to be only partially fixed in 3.1 (I haven't tested against 3.2). The problem still exists when you retrieve a fraction from a map. See my example on SassMeister.

@xzyfer
Copy link
Contributor Author

xzyfer commented May 4, 2015

I can confirm this issue is fixed in 3.2 @ericam

.test {
  integer: true;
  decimal: true;
  fraction: true; }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants