suggestion for buildSchemas method in SchemaValidator
Closed this issue · 8 comments
currently at line 120: $parts = array_values(array_filter(explode(' ', $content)));
the W3 spec indicates whitespace, not just ' ', so in order not to messed up by carriage return or some other whitespace, consider preg_split. Also, not sure that array_filter with no callback is necessary since explode only returns false in the case where the delimiter is an empty string.
$parts = preg_split('/[\s]+/', $content);
If you would like, I can submit the pull request and change this as well as fix the constructor (DOMDocument only) and implement the createFromString method. Let me know and thanks for this work.
regards -
Doug Wilbourne
dougwilbourne@gmail.com
Good catch! It would be great to have a PR about this. Thanks in advance.
Could you please patch and also create a test case on SchemaValidatorTest to demonstrate it is working fine?
As you already notice, using the regex /\s+/
removes the need for array_filter
.
preg_split('/\s+/', "foo bar \n baz \t\r\n\n\r zee");
[
"foo",
"bar",
"baz",
"zee",
]
about fixing the constructor... please don't -I don't like it neither- but It would introduce a compatibility break issue.
The files you are asking for are in tests/public/xsd/
When the phpunit start (see tests/bootstrap.php
), it creates a php standalone web server using tests/public/
as webroot
... I think that it would make the current test suite incompatible with Windows
You can start the required endpoint http://localhost:8999/
by running php -S 127.0.0.1:8999 -t tests/public/
I would need to review why is important to use a web server and not use just local file system. Maybe separate those tests to a different folder.
I agree that preg_split
is more elegant, and I think it should be more expensive than the original one, but not as expensive to not use it.
The warning about returning false
is real, but it can be solved using $foo = preg_split('/\s+/', $bar) ?: []
.
I will commit your PR and prepare some other minor updates (like license year) to make it pass on travis and scrutinizer.
Thanks for your contribution!
Thanks for your contribution! It was merged and release version 2.1.2.
It should be the last version of 2.x series, I will release version 3 on the next days that introduce many breaking compatibility changes, including minimal php version of 7.3.