Fixes #3147 #2715

[matthew-dean]

• Adds permissive parsing for at-rules and custom properties
• Fixes Parser ignores strings at an unknown at-rule prelude (thus failing at { and ; there) #3147 Less fails to parse valid Custom Property values (ex: @apply polyfill) #2715

Custom property values and unknown at-rule entities are essentially treated as literals that replace variable references, meaning you can write:

@function-name: regexp;
@d-value: 15;
@-moz-document @function-name("(\d{0,@{d-value}})") {
	a {
		color: red;
	}
}
@iostat: 1;
.var {
  --fortran: read (*, *, iostat=@iostat) radius, height;
}

The only rule with permissive parsing is that you have matching {}()[]"', and I've added error tests for those. Each block ignores anything except its closing block, and only the top-most block is looking for a final parsing token(s).

[matthew-dean]

I was thinking about this a bit more, and realized that before this is merged, it would need to address this:

1. If we assume that variables can be used for custom property values or custom at-rules, then that means that variable values need permissive parsing as well, and should be able to contain any value. Parsing of the content of vars should be done upon usage. There should be enough "late-parsing" architecture in place in 3.x for that to work without a lot of changes.
2. Technically this would apply to properties as well, but Less kind of already does this. That is, if there is nothing to "resolve", it parses/saves property values as anonymous values. So at eval time, that's when a (non-custom) property would need to follow a particular structure to not throw an error.

It would mean that errors would be thrown at eval time and not parsing time, so the nature of errors would change, but as both are done as a single "pass" from parse to render, it shouldn't matter to the user. (It might mean that, depending on evaluation order and how it might differ from parse order, that 2 errors in a file might switch which fires first, but again, that shouldn't necessarily matter.)

Thoughts?

[matthew-dean]

Accidentally pressed the "close" button.

[seven-phases-max]

Thoughts?

Btw., indeed, the example at #3147 is questionable for this part also:

@function-name: regexp;
@-moz-document @{function-name} ...

Nothing in documentation suggests @{function-name} should be expanded to regexp (as the op probably expects). It was a separate issue/request that was never agreed upon.
Technically the valid variable usage there would be @-moz-document @function-name ...
(Basically we don't have any color: @{rgba-string}(...); or @keyframes @{bla-bla} ... like things).

In general, this:

If we assume that variables can be used for custom property values or custom at-rules, then that means that variable values need permissive parsing as well, and should be able to contain any value.

brings us to "Less variables are not text-replacement-macros" discussion at #3059.
(Which is in summary: yes, the fundamental difference between "variables" and "macros" is that a value of the latter simply can't be and shouldn't be validated).

---

Aside of above I guess my only concern about the PR would be the following kind of code:

@-moz-whatever (foo: "(" @boom-boom ")") {
  // ...
}

Will it work?

[matthew-dean]

@seven-phases-max

Just tested. The output of :

@-moz-whatever (foo: "(" @boom-boom ")") {
  bar: foo;
}

is the same:

@-moz-whatever (foo: "(" @boom-boom ")") {
  bar: foo;
}

That's the expected output, yes? Or do you mean it should be a regular var substitution?

[matthew-dean]

Re: this

Nothing in documentation suggests @{function-name} should be expanded to regexp (as the op probably expects). It was a separate issue/request that was never agreed upon.

I think I see what you mean. That would be a breaking change, correct? Let me think on this.

[matthew-dean]

TL;DR - I allowed variables to "fallback" to permissive parsing, to allow them to be inserted wherever. But it also allows a few other neat possibilities and would close other bugs, but it would also probably need a major Less version bump.

I also fixed it so it wasn't forcing / breaking at-rules and custom properties to use @{} for variables, which, you were right, didn't make sense outside of quotes.

@seven-phases-max With the latest commit, this input:

@function-name: regexp;
@d-value: 15;
@-moz-document @function-name("(\d{0,@{d-value}})") {
	a {
		color: red;
	}
}

.custom-property {
  --this: () => {
    basically anything until final semi-colon;
    even other stuff; // i\'m serious;
  };
  @this: () => {
    basically anything until final semi-colon;
    even other stuff; // i\'m serious;
  };
  --that: @this;
  @red: lighten(red, 10%);
  --custom-color: @red;
  custom-color: $--custom-color;
}

@iostat: 1;
.var {
  --fortran: read (*, *, iostat=@iostat) radius, height;
}

@boom-boom: bam;
@-moz-whatever (foo: "(" @boom-boom ")") {
  bar: foo;
}

@selectorList: #selector, .bar, foo[attr="blah"];
@{selectorList} {
  bar: value;
}

@size: 640px;
@tablet: (min-width: @size);
@media @tablet {
  .holy-crap {
    this: works;
  }
}

Produces this output:

@-moz-document regexp("(\d{0,15})") {
  a {
    color: red;
  }
}
.custom-property {
  --this: () => {
    basically anything until final semi-colon;
    even other stuff; // i'm serious;
  };
  --that: () => {
    basically anything until final semi-colon;
    even other stuff; // i'm serious;
  };
  --custom-color: #ff3333;
  custom-color: #ff3333;
}
.var {
  --fortran: read (*, *, iostat=1) radius, height;
}
@-moz-whatever (foo: "(" bam ")") {
  bar: foo;
}
#selector, .bar, foo[attr="blah"] {
  bar: value;
}
@media (min-width: 640px) {
  .holy-crap {
    this: works;
  }
}

[matthew-dean]

See my comment here. It looks like what I'm treating as valid in this PR pretty closely matches what Chrome considers valid, with the exception that I'm removing escapes, which I probably shouldn't, according to Chrome behavior.

[seven-phases-max]

So I can't see anything wrong with it now. It should do the trick I guess.

_{Well, I suspect one still can break it with some /* { ; ) */ in between (can it?) but that's not very significant edge case.}

[matthew-dean]

@seven-phases-max

Well, I suspect one still can break it with some /* { ; ) */ in between (can it?) but that's not very significant edge case.

I may try to make the permissiveValue() function in the parser actually part of a sub-expression, so that the parser can try to catch/evaluate outer Less functions and comments. But I don't want to over-engineer it. IMO this is an improvement and allows a lot of possibilities.

Speaking of, when I'm ready to merge, and later release, what are your thoughts on version #? As in, since this will allow @tablet: (min-width: 640px); @media @tablet { ... }, and naked selectors, and well assigning almost anything to a variable, and we weren't before, is this a 4.0? It's technically not a breaking change, but it is a potentially big feature. So is it still only a minor version bump?

[seven-phases-max]

Bumping it to 4.x will somewhat postpone its spread I guess (since in many cases it will need explicit upgrade from 3.x). But I'm fine with any variant. 3.5 maybe?

[matthew-dean]

@seven-phases-max Yeah, I was thinking a jump to 3.5 would be good. K. I'll look at the selectors branch as well. That might fit well together with this once merged.

Can you look at #3223? Thanks for your help reviewing lately!

[matthew-dean]

@seven-phases-max Do you understand the parser well enough to have insight into how I could change this to "try" to parse a custom property as a value(), but then back out and parse as a permissiveValue() if it fails? Everything I've tried just throws errors after it starts parsing as a value. But I don't get how other types of entities / nodes can get "tried" without throwing errors. Some seem to back out gracefully; others don't.

[matthew-dean]

@seven-phases-max Never mind, figured it out. See: #3228