Fixes #1421 - re-parses variable-interpolated elements to selectors

[matthew-dean]

• If an element is a variable, this re-parses once evaluated to see if it contains any selectors - Fixes Variable selector "string" improperly parses comma #1421 Output of interpolated selector with nested rules is different than written out #3098 Using variables as selector, issue in inheritance #1817 Idea: Support declarative list expansion into selectors (syntax not decided yet) #1694 Feature Request: Support lists for selector interpolation #2544 Parent selector with mutiple selector in var. #1546 & cannot be used in variable selectors #2080 Selector interpolation: Shouldn't variables be evaluated before the code parse? #2909

[seven-phases-max]

What about more complex tests like:

.bar {
    foo-@{classes}&:hover, baz {...}
}

etc?

[matthew-dean]

Shit, that borks it entirely. I reverted this PR. Any other test cases you can think of? Basically, I put in the test cases that people had filed as their use cases, but yeah it should be more robust.

[matthew-dean]

Technically, the result of that should be an error, but it doesn't even get close. I THINK that's because I had it jump out if it's an error.

[matthew-dean]

I had this working perfectly with:

.bar {
    foo-@{classes}&:hover, baz {...}
}

and more complex cases, and then I tried....

@c: ~'.a, .b';
@d: ~' > .c';
@e: ~' + .d';

@{c}@{d}@{e} {
  foo: bar;
}

...aaaaaand wasted the rest of my weekend lol.

The problem was I had a kind of "selector merging" behavior with something like:

@textClasses: ~'[class="text"], .text';

input@{textClasses} {
  background: red;
}

//outputting
input[class="text"],
input.text {
  background: red;
}

That is, if we assume that interpolated text should be merged into the selector list, then it stands to reason that it should merged with any adjacent elements and combinators.

All of that I got working, but naïvely. As soon as there were multiple vars containing multiple selectors, elements, and combinators, it borked. Not only would it be inconsistent behavior, it's not clear what the expected behavior SHOULD be.

Maybe a more rational approach is to try to stringify everything in the selector, including the eval'd variable, THEN re-parse the entire selector list, rather than try to parse the variable and merge. That would not allow that input[class="text"], input.text output, but that seems to have add-on misery with multiple vars and combinators.

I dunno. My brain has died now.

[seven-phases-max]

I was thinking of how we actually would expect my (and your latest) example to compile - and now I think that it's quite questionable if must not compile to the "naive" thing (if I correctly understand what you mean):
e.g.:

@classes: ~".a, .b, .c";
foo-@{classes}&:hover, baz {...}

is probably expected to compile to just foo-.a, .b, .cbar:hover, baz {...} (quite weird, but valid).
I.e. no "combinatorial explosion" goes here.

Same way:

@textClasses: ~'[class="text"], .text';

input@{textClasses} {
  background: red;
}

Is (probably!) fine to compile to input[class="text"], .text
Actually the more I think of it the more I get to expect it should not explode.

---

What I'd expect to explode would look something like this probably:

@textClasses: ~'[class="text"], .text';

input {
&@{textClasses} {
  background: red;
}

now that example (same as #1421 I guess?) is most likely expected (I think) to result in:

input[class="text"],
input.text {
  background: red;
}

---

On the other hand I can see how someone else would expect all of above examples to explode. Hmm... I.e. either variant may be correct depending on how exactly we think of the "concatenation within same selector" (in contrast to nesting selectors with or without space). So I am for the naive implementation probably 70% vs. 30% :) (even if does not fully cover all of issues/requests referenced above).

Weird. Complicated. More insights needed.

[matthew-dean]

@seven-phases-max

is probably expected to compile to just foo-.a, .b, .cbar:hover, baz {...} (quite weird, but valid).

🤔

Hmm... so... if I understand correctly, you think it would make sense to parse as a selector list unless....there are adjacent elements, in which case, render as-is? (foo-@{classes}&:hover is stored as 4 elements, I believe: ['foo-', @{classes}, &, :hover`]

What I mean is.....

Currently, if you wrote this:

@classes: ~".a, .b, .c";
bar {
  foo-@{classes}&:hover, baz {
    .d, .e {
    	foo: bar;
    }
  }
}

You get this:

foo-.a, .b, .cbar:hover .d,
bar baz .d,
foo-.a, .b, .cbar:hover .e,
bar baz .e {
  foo: bar;
}

So if you defaulted to that behavior, you'd have strings sometimes becoming selectors (which propagate down to child selectors) and sometimes they wouldn't; they would behave as strings.

Which I guess is okay and certainly would be easier to code.

Maybe this is the rule:

1. If a variable is the only element, it can be re-parsed as a selector list and add to the selector list. Ex:

@var: ~".a, .b";
@{var}, .c {
  &:hover {
    foo: bar;
  }
}
// output
.a:hover,
.b:hover,
.c:hover {
  foo: bar;
}

2. If a variable is interpolated as part of an element, it can't expand to a selector list. Less doesn't know what you want, and coding it is too damn hard.

@var: ~".a, .b";
@{var}-box {
  &:hover {
    foo: bar;
  }
}
//output
.a, .b-box:hover {
  foo: bar;
}

3. The workaround for #2 would be simple:

@var: ~".a, .b";
@{var} {
  &-box {
    &:hover {
      foo: bar;
    }
  }
}

How 'bout dat?

[seven-phases-max]

How 'bout dat?

+1.

if I understand correctly, you think it would make sense to parse as a selector list unless....there are adjacent elements,

In a perfect world it would go upside-down. First we expand all the variables and only then we reparse the commas of the whole resulting selector "string" of the current level. I understand though that this is impossible counting how selectors are stored in the tree.

_{Counting other issues and related language problems (like for example :extend concatenated/interpolated selectors and similar things) it looks like switching the internal representation from "arrays of arrays of supposedly single elements" to "a normalized selector string to be reparsed at the last moment" (or something like that) is the only way to go eventually. But that would be a huge refactong :(}

So taking all this into account I guess what you suggest is fine. I.e. if the compiler can reparse then it does, if not then not (and "result of anything beyond that single @{var} thing is explicitly undefined/unspecified and a subject to change").

[matthew-dean]

@seven-phases-max

In a perfect world it would go upside-down. First we expand all the variables and only then we reparse the commas of the whole resulting selector "string" of the current level. I understand though that this is impossible counting how selectors are stored in the tree.

Yeah, it's not impossible, but the current Less lib doesn't easily allow serializing selectors (or partial CSS of any kind) without probably some hacktastic magic. So yeah, if you're on board with the simpler method in the meantime, I'll go that route, unless I think of an easy way to serialize / re-parse. Re-parsing is much easier in 3.0, but it's still non-trivial.