MyIntervals
`Rule::listDelimiterForRule` needs to return different order for many properties
Continuing from #789.
The above-mentioned method is only currently special-casing font properties and the fix for the above.
Many properties' values are comma-separated before being space-separated. It might be better to make that the default, then special-case the others. But doing so risks a breaking change. I don't know which properties use / as a separator. I can't think of any, but there must be some, otherwise it wouldn't be there.
Things like box-shadow will not currently be parsed correctly into the expected object structure, though will still render back out OK.
The method was introduced in https://github.com/MyIntervals/PHP-CSS-Parser/commit/9ed24ad97b6586029fa23af7daaebbbe3eaf6251.
From the references, / as delimiter might only be needed for border-image and not any other property.
From the references,
/as delimiter might only be needed forborder-imageand not any other property.
I think we introduced it because we wanted to parse font: Helvetica 12px/2 (which is interpreted as font-family: Helvetica; font-size: 12px; line-height: 2).
From the references,
/as delimiter might only be needed forborder-imageand not any other property.I think we introduced it because we wanted to parse
font: Helvetica 12px/2(which is interpreted asfont-family: Helvetica; font-size: 12px; line-height: 2).
Yes, that occurred to me after I wrote the above.
Thinking about various properties, and reviewing the docs on MDN, it seems to me that the best general case would be
['/', ' ', ',']
I.e. most values are first separated by commas, then by spaces, then somtimes by slashes.
I can't think of any exceptions to this, though that doesn't mean there aren't any.
In the case of font-*, I can't think of a value component that might contain a comma apart from a typeface name, in which case it would surely need to be in quotes.
For the next major release, I would suggest simply using the above separator precedence for all cases. We are allowed to make potentially breaking changes in a major release, but I think doing so would fix more issues than it creates. We can then special-case certain properties that may need a differerent ordering. There are many that need separation by commas first.
@sabberworm, @oliverklee WDYT?
In the case of
font-*, I can't think of a value component that might contain a comma apart from a typeface name, in which case it would surely need to be in quotes.
Actually, I can: a list of typefaces can be provided, and to complicate matters futher, the names may contain spaces and don't need to be enclosed in quotes even if they do. E.g. I think font: Times, Times New Roman, serif 16px/1.5; is valid. However, judging by the DocBlock for the RuleValueList class which gives a similar example, I think this is parsed correctly, and is why a different separator ordering for font* is needed, with space as the 'outermost' delimeter.