suggestion for buildSchemas method in SchemaValidator

[dwilbourne]

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

[eclipxe13]

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",
   ]

[eclipxe13]

about fixing the constructor... please don't -I don't like it neither- but It would introduce a compatibility break issue.

[dwilbourne]

sure I understand about the constructor. I will submit the PR and update code and create the test case.

kind regards

Doug Wilbourne

On Wed, Apr 1, 2020 at 2:08 PM Carlos C Soto notifications@github.com wrote:

about fixing the constructor... please don't -I don't like it neither- but It would introduce a compatibility break issue.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/eclipxe13/XmlSchemaValidator/issues/5#issuecomment-607407608, or unsubscribe https://github.com/notifications/unsubscribe-auth/AI6RP7UJWHX7LMFVXJVYQ5LRKN7LRANCNFSM4LZI3K6A .

[dwilbourne]

Hi Carlos -

I was getting around to updating the code snippet we discussed and found in your xml file 'books-valid.xml' a reference to a schema location as follows: 'http://localhost:8999/xsd/books.xsd'.

If you could please send me a copy of books.xsd I can finish my testing. Perhaps you will want me to update the schema location to simply be 'books.xsd' as well so that others are not subject to the configuration of localhost?

thanks and kind regards -

Doug

Doug Wilbourne Trustmark Properties LLC 757-617-3238

On Wed, Apr 1, 2020 at 2:14 PM Doug Wilbourne dougwilbourne@gmail.com wrote:

sure I understand about the constructor. I will submit the PR and update code and create the test case.

kind regards

Doug Wilbourne

On Wed, Apr 1, 2020 at 2:08 PM Carlos C Soto notifications@github.com wrote:

about fixing the constructor... please don't -I don't like it neither- but It would introduce a compatibility break issue.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/eclipxe13/XmlSchemaValidator/issues/5#issuecomment-607407608, or unsubscribe https://github.com/notifications/unsubscribe-auth/AI6RP7UJWHX7LMFVXJVYQ5LRKN7LRANCNFSM4LZI3K6A .

[eclipxe13]

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.

[dwilbourne]

Carlos - I do my development in Windows in PHPStorm, but ran the tests on a Homestead box and they all passed.

Interestingly, your original code works on the test that I added as well. Apparently $xpath-query() removes vertical whitespace and appears to convert tabs to spaces, so the explode verb worked in any case.

I left the preg_split verb in place since it is a bit more elegant and all tests passed. After I submitted the PR I noticed that the Travis CI checks all failed afterwards (?) and scrutinizer found a minor potential bug because the value of preg_split can be false instead of an array..... I feel like I have caused more harm than good! Sorry for the trouble.

kind regards -

Doug

Doug Wilbourne 757-617-3238

On Fri, Apr 3, 2020 at 1:41 PM Carlos C Soto notifications@github.com wrote:

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.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/eclipxe13/XmlSchemaValidator/issues/5#issuecomment-608571936, or unsubscribe https://github.com/notifications/unsubscribe-auth/AI6RP7WP7NUWZCK75CXW2B3RKYNURANCNFSM4LZI3K6A .

[eclipxe13]

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!

[eclipxe13]

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.