phpsass icon indicating copy to clipboard operation
phpsass copied to clipboard

Variable scope in parameters

Open LeighBicknell opened this issue 11 years ago • 6 comments

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.

LeighBicknell avatar Jan 15 '14 16:01 LeighBicknell

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

kenorb avatar Nov 11 '14 12:11 kenorb

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.

richthegeek avatar Nov 11 '14 12:11 richthegeek

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

kenorb avatar Nov 11 '14 14:11 kenorb

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 avatar Nov 11 '14 15:11 richthegeek

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

richthegeek avatar Nov 11 '14 15:11 richthegeek

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.

kenorb avatar Nov 11 '14 15:11 kenorb