Add ellipsis support when calling a mixin

[philschatz]

This adds support for expanding arguments that were collapsed using .mixin(@foo; @rest...) { } by being able to write .mixin2(@rest...);

An example file that uses this feature extensively can be found at https://gist.github.com/philschatz/839d32ef87bc22ba5231

Also, thanks for adding plugin support! If you're interested, I've used it for:

1. dead-simple CSS Code Coverage for Mocha/Jasmine+BlanketJS and http://coveralls.io (and Demo)
2. CSS Polyfills library that implements several CSS3 Module Drafts from W3C (and GitHub)

[seven-phases-max]

I don't have any particular opinion so far, just doing some tests. My first observation: the way it is implemented it's not just "expand argument that was collapsed using ..." but actually a more generic "expand any value list as mixin argument list"... E.g.:

@list: 1 2 3;

a {.mixin(@list)}    // 1 argument
b {.mixin(@list...)} // 3 arguments
c {.wrapper(@list)}  // 1 argument
                     // etc.

.mixin(@a1) {
    a1: @a1;
}

.mixin(@a1, @a2, @a3) {
    a1: @a1;
    a2: @a2;
    a3: @a3;
}

.wrapper(@args...) {
    .mixin(@args...); // 1 argument of `c` expands as 3 arguments
}

output:

a {
  a1: 1 2 3;
}
b {
  a1: 1;
  a2: 2;
  a3: 3;
}
c {
  a1: 1;
  a2: 2;
  a3: 3;
}

(Note that I'm not saying it's bad :)

[lukeapage]

I like the syntax.. I've been thinking about the benefits and use-cases. I see you use it alot to extend mixins and just pass the rest of the argument list. Its kind of a work-around for the consumer of the mixin passing it as one argument to start with e.g.

.mixin-call('a'; 1, 2, 3;);

And the code addition isn't that much more complicated, so I'm leaning towards taking it. Can you offer any more use-cases and reasons for this to be useful?

[philschatz]

Thanks!

It might be useful for extending a 3rd-party lib that uses variable-argument mixins; something like https://github.com/visionmedia/nib but for LESS.

Also, even with the workaround, I believe it is still not possible to partially match a list (.mixin('a'; 1, 2, @anything...)) but I don't have any specific examples offhand.

[lukeapage]

@jonschlinkert @matthew-dean @SomMeri would be good to get your opinions since it sounds like myself and @seven-phases-max are more on the pro side.

I'm only hesitant because we want to make sure every feature is needed and is worth the maintenance effort and language complexity add

[SomMeri]

@lukeapage I do not see any harm that could be caused by this and if it is useful, then why not.

The ... working for any list seems more like an advantage and I can imagine using ... syntax for list unfolding anywhere we decide to unfold whatever.

Looping like this is even more readable then looping with index variable and what not:

.mixin(@a, @b...) {
  do-something: @a;
  .mixin(@b...); //looping
}

Disclaimer: I did not tested it, only read comments here so far.

[seven-phases-max]

I just would like to put a few notes. As I already mentioned this feature is not limited only to mixins arguments but applies to any value lists (though I guess it was unintentional). In other words, it also can be used to do the work that some hypothetical expand(@list) and concat(@list, ...) built-in functions would do (except the fact that build-in function won't be able to return argument list).
So it's quite powerful feature in general with surprisingly lightweight implementation.
(And though technically this does not completely cancel some need for expand(@list) and concat(@list, ...) it's still quite handy bonus to eliminating/simplifying some really ugly loops).

[matthew-dean]

I haven't personally ever written a (...) mixin, as I've always been somewhat confused why they exist. I guess it's for when you'd want a mixin to "match" and add some additional properties / values, without consuming the arguments? The Less documentation has a lot of examples of different argument configurations you can make for matching mixins, but no examples why you'd want to use specific ones.

So, I feel unqualified to give my opinion on this feature, as I don't see many use cases to use ... to begin with. Is it really used that much in the wild? If so, what are the best cases?

[rjgotten]

The proposed syntax offers a nice and tidy equivalent of how one would iterate over a list using list comprehension in pure functional languages.

I reaaaaaaa---lly like it. 👍
Hope it gets added.

[seven-phases-max]

Yep, I also always keep this feature in mind (I even have a special Less setup with this feature enabled for my own use :) Hoping someday it gets enough votes to get in.

[matthew-dean]

@seven-phases-max I'll reiterate that I don't have enough experience with these to put in any strong opinion. If you, @lukeapage, and @rjgotten are on board, shove that puppy in.

[rjgotten]

I think the only thing that irks me a bit is that this probably wouldn't handle the case of list-of-list correctly and would produce a wrong result when it arrives on the last member of an outer list.

What does the following produce?

.mixin(@head, @tail...) {
  @first: extract(@head, 1);
  @second: extract(@head, 2);

  @{first}: @second;
  .mixin(@tail...);
}

.class {
  @values : a A, b B, c C;
  .mixin(@values...);
}

Does this produce what I would expect? I.e.

.class {
  a : A;
  b : B;
  c : C;
}

[seven-phases-max]

For your example it's:

SyntaxError: No matching definition was found for .mixin()

I guess this problem is related to #1943.

[rjgotten]

Maybe my example is just flawed. What happens if we explicitly add the base case of the tail recursion?

That is; what if we do:

.mixin() {}
.mixin(@head, @tail...) {
  @first: extract(@head, 1);
  @second: extract(@head, 2);

  @{first}: @second;
  .mixin(@tail...);
}

.class {
  @values : a A, b B, c C;
  .mixin(@values...);
}

[seven-phases-max]

Hmm, indeed, you're right. Empty one is required in this case as a terminator for an empty tail in the end. And yes, on the final call the c C tail expands to @head: c, @tail: C.
But this is not this feature fault, it's again more like a #1943 problem.

[rjgotten]

But this is not this feature fault, it's again more like a #1943 problem.

Yes. That one is issue #1943 at play for sure. ^_^
I guess it is one more potential reason to fix it.

[StreetStrider]

👍

[lukeapage]

@seven-phases-max it seems like the general view is a +, I'm happy - do you want to merge it in?

[seven-phases-max]

@lukeapage Yes, I'll do then.

[seven-phases-max]

Done.

[philschatz]

🎉 🎈 🏆 Yay! It's happening! 🎈 🎉

[StreetStrider]

Thats great :party: