-
Notifications
You must be signed in to change notification settings - Fork 464
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
Regression: Call function does not destructure variables #2034
Comments
Issue [#2034](sass/libsass#2034)
Issue [#2034](sass/libsass#2034)
This is because an early check was added in function call evaluation, since it was needed to avoid the evaluation of the arguments for |
I refactored the argument logic to match Ruby Sass in 3.3.3. It should be correct. The issue is more likely which how we're evaling |
Bug happens here: https://github.com/sass/libsass/blob/master/src/eval.cpp#L842 |
This is a more complete spec test: $args: mix, #fff, #000, 20%;
$params: #fff, #000, 20%;
.foo {
test: call($args...);
test: mix(#fff, #000, 20%);
test: call(mix, #fff, #000, 20%);
test: call(mix, $params...);
} Currently yields:
Expected is: .foo {
test: #333333;
test: #333333;
test: #333333;
test: #333333; } So we can probably get away here with one additional check. @xzyfer I also do not like to add such patches, but I see it as a legit way to add the needed behavior until someone (mostly me) gets around to do the actual refactoring. And it has proven to help to have these patches in place, specially if they are documented accordingly. The only small thing that worries me a little is that this pattern may break lazy evaluation as fixed in #1986. To put it short we tend to get away with those patches quite far (and I do keep cleaning up after them). |
When doing:
Ruby Sass will output:
But Libsass will output:
This is happening with:
node-sass 3.5.3
wrappinglibsass 3.3.5
.The text was updated successfully, but these errors were encountered: