[BUGFIX] Allow at-rules to be parsed in strict mode #456
Conversation
Or at least it should. I found it impossible to copy and paste the previous version of the code from the GitHub diff, so went hunting for a specific version that should have contained the code to restore. Only it didn't. So it was a bit painstaking to actually revert that change. Any tips to speed up the process would be greatly appreciated. |
|
Can't fix this without knowing what the problems are. And, of course, the PHIVE set-up is a total PITA locally on Windows. |
2637ac2 to
97a4e27
Compare
OK, fixed, using info from logs (was an |
The reverts the change to `CSSList` in 134f4e6 and adds a comment that `null` is an expected return value when the end of the list (or block) is reached. It also adds a convention, for now, that a test filename beginning with `=` should mean that it is initially tested with strict parsing enabled. Fixes #352
97a4e27 to
bbc2d73
Compare
tests/ParserTest.php
Outdated
| $oParser = new Parser(file_get_contents($sDirectory . '/' . $sFileName)); | ||
| $oParser = new Parser( | ||
| \file_get_contents($sDirectory . '/' . $sFileName), | ||
| Settings::create()->withLenientParsing($sFileName[0] !== '=') |
There was a problem hiding this comment.
Instead of having separate sets of fixtures with "magic" prefixes, I'd prefer to communicate this more clearly to the reader, e.g., with a separate testcase that uses the CSS in the test code instead of reading from files. (After all, one of the purposes of tests is to communicate the intended behavior to the reader).
There was a problem hiding this comment.
I've moved the new tests to the specific test class responsible. When working within an IDE, having the test content as separate files is no probem, and helps avoid test class files with literally millions of lines to scroll through, which, even if it doesn't cause the computer to break down, is a bit of a PITA.
|
Can't figure out how to remove the files I added that are no longer wanted. |
5581cb7 to
be48242
Compare
Non-alphanumeric characters in filenames cause problems in OSs developed in the 1970s, that are still prevelant now. So must be avoided. |
|
I want to convert this from draft to live. But can't find whatever I need to do to do so. Nothing hapens when I click the 'Draft' button. |
Nope. Don't see it. |
Finally found it. What an awful UI. Thank you Microsoft. |
Co-authored-by: Oliver Klee <github@oliverklee.de>
Co-authored-by: Oliver Klee <github@oliverklee.de>
Co-authored-by: Oliver Klee <github@oliverklee.de>
Co-authored-by: Oliver Klee <github@oliverklee.de>
Co-authored-by: Oliver Klee <github@oliverklee.de>
Co-authored-by: Oliver Klee <github@oliverklee.de>
|
Wondering it we should release an 8.5.1 with this fix, since it was a regression in 8.4. |
|
Yeah, I think backporting it to 8.5.x makes sense. We probably need to change the native type declarations to PHPDoc annotations. Would you be willing to create the backport PR? And should we release 8.5.1 directly after merging it, or should we first collect some more fixes? |
Ah, yes, the 8.x branch supports older PHP versions which don't support native type declerations.
I should hopefully be able to merge the commit to main to a PR branch off the 8.x. Then revert the type hint changes.
The other outstanding PRs awaiting review mostly don't look straightforward, and may require post-review changes, so may take some time to complete. I was keen to resolve this issue as a priority since it was a regression. The others aren't, AFAIAA, so it would be on a case-by-case basis whether they can be easily backported once completed on |
The reverts the change to
CSSListin134f4e6 and adds a comment that
nullis an expected return value when the end of the list (or block) is reached.Fixes #352