phpsass icon indicating copy to clipboard operation
phpsass copied to clipboard

Variables cannot be changed from within a for loop

Open chrisinajar opened this issue 12 years ago • 4 comments

If you define a variable in a function, you cannot change it from within a for loop. Example code is below.

@mixin test-mixin(val) {
  left: test-function(val);
}

@function test-function($var) {
  $inner-var: "Wrong!";

  @for $i from 1 through 1 {
    $inner-var: "It worked!";
  }

  @return $inner-var;
}


body {
  @include test-mixin('woot');
}

If i replace the line inside the for loop with a return statement then the correct value appears, so I know the loop is running.

chrisinajar avatar Dec 13 '12 15:12 chrisinajar

If I comment out line 92 of SassForNode.php it appears to work

$context = new SassContext($context);

I'm not sure if there are adverse affects of simply passing the same context in, but it works...

chrisinajar avatar Dec 13 '12 18:12 chrisinajar

I was just thinking it's probably a context issue! I think it's actually an issue with how variables are handled generally.

The way variables work now (similar to how PHP handles it):

  1. a variable assign occurs ($x: 42)
  2. the variable x gets set to 42 in the current SassContext

The way it should actually work (similar to how JS handles it):

  1. a variable assign occurs ($x: 42)
  2. the parents of the current SassContext are searched for existance of $x
  3. if it exists, change the value of the variable within that context
  4. if it does not exist, set it within the local context.

Of course, it could simply be a case of loops not requiring their own contexts! The only issue I can think of (which is not really an issue) would be the lop variables potentially "bleeding" out of their intended context.

richthegeek avatar Dec 14 '12 15:12 richthegeek

That makes sense. I looked at the context parenting code a little bit, I think your description of how it should work is exactly on.

If I find the time I'll take a stab at fixing this issue, otherwise I'll be watching closely for a fix from you whenever you similarly find the time (I know how that can be)

I think that for loops should have their own contexts, however removing that line is a quick fix for now to keep things going. I'm trying to get Bourbon to work but it's proving much more difficult that I had expected. Hopefully I can make a few more pull requests / bug reports as I continue to work through the Bourbon issues.

Thanks!

chrisinajar avatar Dec 14 '12 18:12 chrisinajar

@chrisinajar did you ever get bourbon to work?

drsii avatar Mar 20 '15 13:03 drsii