Collector variable structure if only one or zero arguments are collected

[SomMeri]

By collector variables I mean either @arguments variable or any mixin parameter declared with ....

If collector collected multiple values, then it contains a list. However, if it collected only on variable, then it does not contains a list. It contains that one variable. That makes it impossible to differentiate between these two things:

• .mixin(1, 2, 3) // three parameters
• .mixin(1, 2, 3;) // one parameter

Full test case:

.mixin(@first, @rest...) {
  rest-length: length(@rest);
  rest: ~`"@{rest}"`;
}

div-1 {
  .mixin(box-shadow 0.2s linear);
}
div-3 {
  .mixin(box-shadow 0.2s linear, color .4s .2s ease);
}
div-2 {
  .mixin(box-shadow 0.2s linear, color .4s .2s ease, jabba dabba);
}

current output:

div-1 {
  rest-length: 0;
  rest: ;
}
div-2 {
  rest-length: 4;
  rest: [color, 0.4s, 0.2s, ease];
}
div-2 {
  rest-length: 2;
  rest: [color 0.4s 0.2s ease, jabba dabba];
}

Expected output:

div-1 {
  rest-length: 0;
  rest: []; // maybe? 
}
div-2 {
 //here is difference. It is still impossible to tell comma/space difference, but 
 //it is at least possible to know the right number of parameters
  rest-length: 1;
  rest: [[color, 0.4s, 0.2s, ease]]; 
}
div-2 {
  rest-length: 2;
  rest: [[color, 0.4s, 0.2s, ease], [jabba dabba]];
}

Background: I was trying to find out how LessHat could work if we would fix #1939 (pull request #1941). Spaces/commas/argument separator difference is lost in less->js conversion. Which might be ok if would could partially deduce them from structure which is impossible.

LessHat current prints @arguments into string and then uses javascript to parse that string.

[lukeapage]

It definitely looks wrong now. Expected output looks ok to me - more workable and would only effect js eval right? Do it in 2.0?

[SomMeri]

I looked at this. The issue is that both expression and value make no difference between list with one member and a single non-list value. No matter what context, expression treats one member list as a single value.

So, if we would fix this, it would influence more then just javascript evaluation.

For example this:

.mixin(@first, @rest...) {
  rest-length: length(@rest);
  rest: ~`"@{rest}"`;

  rest-length-1: length(first-level);
  rest-1: ~`"@{first-level}"`;
  @first-level: extract(@rest, 1);

  rest-length-2: length(second-level);
  rest-2: ~`"@{second-level}"`;
  @second-level: extract(extract(@rest, 1), 1);
}

div-3 {
  .mixin(box-shadow 0.2s linear, color .4s .2s ease);
}

now evaluates into:

// now
div-3 {
  rest-length: 4;
  rest: [color, 0.4s, 0.2s, ease];
  rest-length-1: 1;
  rest-1: color;
  rest-length-2: 1;
  rest-2: color;
}

but would evaluate into:

// after
div-3 {
  rest-length: 1; //non js change
  rest: [[color, 0.4s, 0.2s, ease]];
  rest-length-1: 4; //non js change
  rest-1: [color, 0.4s, 0.2s, ease];
  rest-length-2: 1;
  rest-2: color;
}

I would fix this by making difference between one member list and single value in expression and value (if no one objects).

[seven-phases-max]

The only problem I see is that how this will work then with a code like this:

.mixin(@args...) {
    .-(length(@args));
    .-(@i) when (@i > 0) {
        .-((@i - 1));
        padding+_: (extract(@args, @i) * 20);
    }
}

a {
    .mixin(1);
}

b {
    .mixin(1 2 3 4);
}

If I understand it right, with these changes the above example will require an is-array/is-list function and two mixins to handle list and non-list inputs separately. In other words, by making it more js-friendly we'll have to bring more js-like stuff into Less itself, specifically my main concern is countenancing of more and more conditional Less code (to pay for better support of inline-js stuff which countrary is being discouraged recently).

---

Related to #1576.

[SomMeri]

I do not think about this as being js friendly/unfriendly, that is just the context where I found that.

To your example, length of a single value can be 1 and extract of single value can behave the same way as extract of one value list.

Imagine that you would want modify your mixin to work with lists instead of numbers and generate box-shadow:

.mixin(@args...) {
    .-(length(@args));
    .-(@i) when (@i > 0) {
        .-((@i - 1));
        box-shadow+: extract(@args, @i);
        box-shadow+_: black;
    }
}

a {
    .mixin(10px 10px;);
}

b {
    .mixin(10px 10px, -20px -20px;);
}

This compiles into:

a {
  box-shadow: 10px black, 10px black;
}
b {
  box-shadow: 10px 10px black, -20px -20px black;
}

After would compile into:

a {
  box-shadow: 10px 10px black;
}
b {
  box-shadow: 10px 10px black, -20px -20px black;
}

If you have a mixin that takes list of lists [[a b][c d]], the mixin works only if outer list has at least two members. If the outer list contains only one list inside [[a b]], then it becomes indistinguishable from list of values [a b].

[SomMeri]

Btw, the above is made up example, not something I would need. My only goal is to close this issue one way (wont fix) or the other (fix it).

I have time for bug fixing now, so I want to fix as many bugs as possible (while I remember how less.js works).

[seven-phases-max]

To your example, length of a single value can be 1 and extract of single value

Yes, but the length of @args... = (1 2 3 4) (as well as of (1, 2, 3, 4;)) will also be 1 hence we'll need an is-array conditional there (sure my example is also completely artificial, as in fact there ... is only necessary to handle (1, 2, 3, 4) input etc.).

---

If the outer list contains only one list inside [[a b]], then it becomes indistinguishable from list of values [a b].

Yes, this is an issue too. Curiously I face it quite often myself recently. The trick I use for such cases:

.mixin(@args) {
    box-shadow+: @args black;
}

.mixin(@args...) when (default()) {
    .-(length(@args));
    .-(@i) when (@i > 0) {
        .-((@i - 1));
        .mixin(extract(@args, @i));
    }
}

a {
    .mixin(10px 10px);
}

b {
    .mixin(10px 10px, -20px -20px);
}

which is not so quite evident on its own of course (So, yep, there's complication price for not distinguishing [[a b]] and [a b] by extract/length).

---

Thinking of it more, I guess I'll support the proposed changes (I'm afraid they seem to be unavoidable anyway as we'll have all this "working with arrays/lists code" growing). So the only thing to keep in mind that these are breaking changes for certain "pure Less code" snippets too (I'm afraid I've already propagated a couple of such snippets at the SO already :(

[lukeapage]

I think it makes sense to make the change.. but I'm scared it requires a major version update since it is a breaking change..

[matthew-dean]

Stuff like this worries me:

.mixin(@args...) when (default()) {
    .-(length(@args));
    .-(@i) when (@i > 0) {
        .-((@i - 1));
        .mixin(extract(@args, @i));
    }
}

... because such complication always seems to me to be an indication of a syntax hole. Just like mixin libraries using JavaScript.

It seems like what's missing in this example (but correct me if I'm wrong) is not necessarily a way to distinguish list types and iterate through like a function, but a way to bind a list to a mixin, declaratively.

Something like:

.mixin(@args) {
    box-shadow+: @args black;
}
a {
    each(10px 10px, .mixin); // or, obviously, just .mixin(10px 10px);
}

b {
    each(10px 10px, -20px -20px, .mixin);
}

And, for @seven-phases-max:

.mixin(@args) {
  padding+_: (@args*20);
}
a {
    .mixin(1);
}

b {
    each(1, 2, 3, 4, .mixin);  // or each(1 2 3 4, .mixin)
}

That requires no complex .-() loops, nor counting of elements or extracting by number of lists, nor JavaScript. You could still apply guards to differentiate behavior, since each value would be passed to the mixin as a distinct, single value. Seems a hell of a lot cleaner.

I'm not sure if the mixin name seems more intuitive as the first value or the last value or which order would be more CSS-like. But something like this should work. Does it omit any common use case?

[matthew-dean]

Oh, and obviously, that means you could pass a variable into a mixin without knowing if the variable is a list or not.

.mixin(@args) {
    box-shadow+: @args black;
}
@values: 10px 10px, -20px -20px;
a {
    each(@values, .mixin); 
}

[seven-phases-max]

@matthew-dean

Stuff like this worries me:

Well, just FYI in my real code this stuff looks like this:

@import "for";

.mixin(@args...) when (default()) {
    .for(@args); .-each(@v) {
        .mixin(@v);
    }
}

so you can have syntactic sugar if you want.

[matthew-dean]

It makes sense to me to have a native each() function for lists. I've seen you do some awesome tricks with mixins, but a native feature would surface that to more users, and it's still a bit less verbose than your example (and requires no imports).

[seven-phases-max]

Moved my comment about loops to the gist to not clutter this issue with unrelated stuff.

[matthew-dean]

Well, as far as method of implementation, that doesn't seem so important to me, so much as getting rid of the more complex Less code or JavaScript shims required for some functionality. Do you think each() or the examples you've listed above would be limited in usage? To me, it seems more useful than some of the functions Less currently has. I'd be more likely to use this than extract() for example.

[seven-phases-max]

Do you think each() or the examples you've listed above would be limited in usage?

Yep. For instance it does not help you to write the loop body directly w/o a dedicated mixin which makes it even more verbose for basic loops (example in my gist above). Besides it really just encodes only one pattern (even if it's the most used one) - so if I will need to loop through an array in reverse will I have to propose reverse-each() and so on and so on? :) Btw., I also forgot to mention related #2270.

[matthew-dean]

Why would you need to loop through an array in reverse? I mean, in terms of CSS properties? I can see modification of items on the list for output, but I can't think of a scenario where someone would be inputting a list in the reverse from what the CSS value required.

And in general, I feel the same about any looping using Less: if it is the only way to solve a common use case, to me it indicates a syntax hole. The fact that you can use a loop in Less to solve a problem doesn't indicate to me that a problem has been (elegantly) solved, just that there's a possible workaround. A loop is trivial for programmers, but it's a way more complex concept for a designer who knows CSS but has never coded, especially the way they're typically implemented in Less. A Less loop is probably the most complex Less code that someone new to Less would encounter. So, I've long been on the lookout for something simpler in concept. If there are multiple simpler somethings to replace a loop structure, that seems fine, if it makes the language and concepts more accessible. The avoidance of complex programming structures is one of the advantages Less has over Sass. It's a lot more accessible for people of different backgrounds.

[seven-phases-max]

So, I've long been on the lookout for something simpler in concept.

And that's why we have

.mixin(@args...) {
    property: @args; // no loop is necessary
}

But as soon as you have to introduce some each you're automatically targeting a more developer-like audience since things like each imply familiarity with more complex stuff like callbacks/lambdas etc. Well, let's try to imagine how a basic loop can look like if meant for a designer-guy... Can we really have something more simple than just:

.for(@array, {
    box-shadow+: @item black;
});

? (and this is already currently available syntax, so this looks more like a matter of "standard library"/documentation stuff).

[seven-phases-max]

@matthew-dean Don't you mind if I'll move our whole discussion to #2270? It would be really unfortunate to link to #1943 when the changes proposed here are implemented.

[matthew-dean]

That's fine.

[seven-phases-max]

If I understand it right, with these changes the above example will require an is-array/is-list function and two mixins to handle list and non-list inputs separately.

Correcting myself, actually it's not that dramatic as I thought above...
For that my loop snippet above the fix will be as simple as:

.mixin(@args_...) {
    @args: extract(args_, 1); // <- here's the trick
    .-(length(@args));
    .-(@i) when (@i > 0) {
        .-((@i - 1));
        padding+_: (extract(@args, @i) * 20);
    }
}

a {
    .mixin(1);
}

b {
    .mixin(1 2 3 4);
}

So no is-array/is-list stuff, extra conditions and other scary things will be nessecary to distinguish between single and list values (simply because extract(alone, 1) returns alone :).

[matthew-dean]

@seven-phases-max Does your last comment mean this can be closed?

[seven-phases-max]

@matthew-dean

Does your last comment mean this can be closed?

No, it's just I revoked my initial objections to the fix proposed by @SomMeri (toning it down from "very breaking" to just "subtly breaking").
I.e. the issue and the feature-request to fix it (the change in @arguments/@rest format) is still valid.

But since the problem is pretty subtle itself (3 years of inactivity here) I guess it can be safely closed untill someone rises something like this again.