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

Variable scope in parameters #139

Open
LeighBicknell opened this issue Jan 15, 2014 · 6 comments
Open

Variable scope in parameters #139

LeighBicknell opened this issue Jan 15, 2014 · 6 comments

Comments

@LeighBicknell
Copy link

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
Copy link

kenorb commented Nov 11, 2014

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

@richthegeek
Copy link
Owner

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
Copy link

kenorb commented Nov 11, 2014

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
Copy link
Owner

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
Copy link
Owner

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
Copy link

kenorb commented Nov 11, 2014

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.

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

No branches or pull requests

3 participants