Breaking change un 1.3 on selector re-parsing (again :/)

[Cerdic]

Related to #245 and compiled selector re-parsing to check they are valid

/// Utility mixin for grid.
/// @param {list} $gutters Column and row gutters (default is 40px).
/// @param {string} $breakpointName Optional breakpoint name.
@mixin grid($gutters: 40px, $breakpointName: null) {

	// Cells.
		$x: '';
		@if $breakpointName {
			$x: '\\28' + $breakpointName + '\\29';
		}
		.\31 2u#{$x}, .\31 2u\24#{$x} { width: 100%; clear: none; margin-left: 0; }
		.\31 2u\24#{$x} + *,
		.\31 u\24#{$x} + * {
			clear: left;
		}
		.\-11u#{$x} { margin-left: 91.6666666667% }
}
	
.test {
    @include grid(40px, 'xlarge');
}

produce

`$\28xlarge\29 + *` is not a valid Selector in `.\312u\24\28xlarge\29 + *, .\31u$\28xlarge\29 + *`: failed at `$\28xlarge\29 + *` ScssPhp\ScssPhp\Compiler::evalSelectors on line 1, at column 32

Whereas Sassmeister produce

.test .\31 2u\(xlarge\), .test .\31 2u\$\(xlarge\) {
  width: 100%;
  clear: none;
  margin-left: 0;
}
.test .\31 2u\$\(xlarge\) + *,
.test .\31 u\$\(xlarge\) + * {
  clear: left;
}
.test .\-11u\(xlarge\) {
  margin-left: 91.6666666667%;
}

The real bug is probably in the selector Parsing here, due to escaped chars in the selectors

[Cerdic]

(Additionaly, the call of $parser->parseSelector should be catched to throw a CompilerException with a call-stack trace to be able to know where the error is coming from in the code)

[stof]

maybe we should revert the selector re-parsing until we have a spec compliant selector handling

[Cerdic]

Maybe, that's something to consider.
The thing is it's not a full fix as there was already a re-parsing in case of the compiled selector was including a & or a , char.
8a30458#diff-bdd0d64c8e43471e6d175998fe2ab7d5ebe6487844bbc6315348250361b71a4bL1732

So you could experience the bug or not depending what kind of selector you are producing.
But at least it would fix the BC break issue, and the cases that were luckily working would kept working

[stof]

@Cerdic but your PR added also a new exception which was not there before when re-parsing: 8a30458#diff-47bc8653b21716576958e25b6d7356ecb0f0070f17554d12d2bee985ac211b26R285

[Cerdic]

You are totaly right! I forgot that point.

Looking a bit in detail, it seems here the bug is not really the re-parsing but the escaping of $ char in selectors:

The compilation of

.\31 u\24 + * {
  clear: both;
}

Produce with scssphp

.\31u$ + * {
  clear: both;
}

which is not css valid, and if you are re-parsing this you are getting a parser error.

We should have an escaped char in the result

.\31 u\$ + * {
  clear: both;
}

So the fact is with previous version of scssphp these erroneous selectors produces by scssphp were probably ignored by browsers.

That said, and unlinked to fixing this $-escaping-issue in selectors, I can totally live with reverting 8a30458 if it feels more comfortable.

[stof]

That said, and unlinked to fixing this $-escaping-issue in selectors, I can totally live with reverting 8a30458 if it feels more comfortable.

Well, if you manage to fix the escaping issue in a reasonable time, that would still be a better solution, as we would be fixing a bug. But I fear that it may take time.

[Cerdic]

I think I have an acceptable fix with #274