Looping stopped to be possible

[SomMeri]

I was abusing less.js little bit today and I think that I found a regression. It used to be possible to loop in less.js-1.3.3 (see #869 (comment) ):

.spanX (@index) when (@index > 0) {
  loop: member @index;
  .spanX(@index - 1);
}
.spanX (@index) when (@index =< 0) { 
  loop: end;
}

Less-1.3.3.js output:

#usePlace {
  loop: member 5;
  loop: member 4;
  loop: member 3;
  loop: member 2;
  loop: member 1;
  loop: end;
}

Less-1.4.0-beta.js downloaded 11.3.2013 does not support it anymore. Less-1.4.0-beta.js output:

#usePlace {
  loop: member 5;
}

Motivation: I wanted to combine the extract function with vendor prefix workaround #1199 (comment) . Improved mixin would create vendor prefixed declarations out of list of vendor prefixes and declaration .vendorPrefixes(@declaration, @prefixes).

[jonschlinkert]

I won't be able to test until this evening, but if there are indeed issues with looping, Bootstrap's (2.3.1 and 3.0.0) grid might have problems:

.spanX (@index) when (@index > 0) {
      .span@{index} { .span(@index); }
      .spanX(@index - 1);
    }
...

[matthew-dean]

Looping, AFAIK, was not an official feature. It would be good to support something like this, but probably in a different way.

[jonschlinkert]

The looping just a result of the when keyword guarding against the recursive mixin. So this was technically a supported feature. And I just did some tests and verified that this is in fact a bug, and it does break Bootstrap's grid in wip-3.0.0 (and presumably 2.3.1. if memory serves me it uses the same mixins/guards).

I labeled this a bug and high priority.

cc @lukeapage

[matthew-dean]

I'm just saying if looping was available, it probably was unintentionally supported (using it would be technically a "hack"), so it is subject to breakage. But fair enough that people have found it useful.

[jonschlinkert]

No here is a quote from the issue where fat decided to use guards:

it's fairly complicated. I'm going to work with @cloudhead to make this cleaner - but this seems to be the right path...

Found here: twbs/bootstrap#1952 (comment)

The only reason I knew that, is that I was in that conversation.

[jonschlinkert]

crap I didn't meant to message cloudhead, I thought that a quote would filter that out...

[SomMeri]

I do not see what part of that solution is not official - that solution uses only mixins, guards and expressions. What I mean is that it does not abuse some syntax gimmick or a bug (such as space handling not being strict or something similar). Combinations of official features should not amount to unofficial feature.

I think that looping is just an easy to find manifestation of a problem that may show up also in other unexpected contexts.

[matthew-dean]

Well, what I meant is that I don't know that there is any guarantee as to the order of evaluation of mixins. A looping syntax presumes a sequential evaluation of each loop.

However, that's more an academic explanation than anything, and there's probably no specific reason to break this behavior, but if it's officially supported, we should note it and make test cases. No test case == not guaranteed functionality.

[lukeapage]

I tried 1.4.0 beta on bootstrap and it works with the recursion there.

I cannot reproduce. The code above goes wrong because there is no parenthesis around @index - 1.

.spanX (@index) when (@index > 0) {
  loop: member @index;
  .spanX((@index - 1));
}
.spanX (@index) when (@index = 0) { 
  loop: end;
}
.a {
 .spanX(5);
}

[lukeapage]

btw when I said I tried bootstrap, I then compared the output to see if it was identical.

[SomMeri]

Closing because .spanX((@index - 1)); works. Require brackets to do maths #1151 will require some time to use to.

[jonschlinkert]

The code above goes wrong because there is no parenthesis around @Index - 1.

Good catch @lukeapage. I looked at that code and didn't notice that it was missing a set of parens. As @SomMeri points out, it's just going to take time to get used to. This is exactly the kind of thing I was hoping to catch with less-tests.

[jonschlinkert]

btw when I said I tried bootstrap, I then compared the output to see if it was identical.

@lukeapage I just noticed that you said that, did you use less-tests or just your normal setup? I actually had bootstrap in there and removed it, but I could add it back in if it's useful.

[lukeapage]

It would be useful to integrate into our tests that it gets bootstrap, compiles and compares..

I generally get a version of bootstrap, compile with node and my local less, compile with node global lessc from last release, then compare in windiff..

[jonschlinkert]

that scenario you just described is exactly the kind of thing I created less-tests for.

I'm going to try to do a few things that make it easier, so that when you run less-tests it will (optionally):

• automatically pull down Bootstrap, or a specific version of Bootstrap, or some other arbitrary lib
• run some tests against those files
• output some kind of useful information about the results

ideally this will remove some of the manual effort

[matthew-dean]

Aha! The math requirement. It's too bad we don't have LESS "warnings" yet (compile but log things like "Looks like you may have forgotten parentheses for math?".