richthegeek / phpsass

A compiler for SASS/SCSS written in PHP, brought up to date (approx 3.2) from a fork of PHamlP: http://code.google.com/p/phamlp/
http://phpsass.com/
382 stars 83 forks source link

Variable scope in parameters #139

Open LeighBicknell opened 10 years ago

LeighBicknell commented 10 years ago

This one's easier to explain in code:

@function foo($param)
{
    @return $param;
}
@mixin bar($paramOne, $paramTwo: foo($paramOne))
{
    test: $paramTwo;
}
#test { @include bar(foobar); }

You'd expect this to return

#test { test: foobar; }

However when passing $paramOne to function foo $paramOne looses it's value.

kenorb commented 9 years ago

The same here. Probably related: https://www.drupal.org/node/2204793

richthegeek commented 9 years ago

Isn't this expected behavior?

From a programming perspective, many languages don't allow use of the arguments within the function definition.

I assume (hope) that defining $paramTwo within the mixin body would work as expected.

kenorb commented 9 years ago

It seems Ruby version works with that syntax. http://sassmeister.com/gist/78080cdec3a519c93f2a Source: Can I use arguments within the function definition? at SO

richthegeek commented 9 years ago

I see.

I think the fix to this is changing extractArgs [1] to add the arguments to the scope as they are processed, rather than afterwards. Unfortunately, they are processed in process_arguments [2] before this, so it needs a comprehensive refactoring.

If you think you can manage it, a PR will be happily accepted.

[1] https://github.com/richthegeek/phpsass/blob/master/script/SassScriptFunction.php#L201 [2] https://github.com/richthegeek/phpsass/blob/master/script/SassScriptFunction.php#L47

richthegeek commented 9 years ago

It might be possible to fix it within SassMixinNode [1] but then the behavior should also change in any other parametered directives, so if it can be fixed within SassScriptFunction then that'd be better.

[1] https://github.com/richthegeek/phpsass/blob/master/tree/SassMixinNode.php#L71

kenorb commented 9 years ago

Thanks, I'll see what I can do. Currently that kind of syntax is used by _vertical_rhythm.scss file which basically breaks some external CSSes (because of this bug) when using it with Drupal sassy module which using this parser. But it seems the recent version of _vertical_rhythm.scss in Compass doesn't use the same syntax.