Closed mjordan closed 7 years ago
Confirming that I got OK (52 tests, 72 assertions)
on my OS X machine. I don't have easy access to a Windows machine setup with MIK on it just yet, so we'll use the fact that this change was used in a production instance as proof that it passes a smoke test.
@mjordan Under the first section of the description of this pull-request, you wrote: "Fixes a problem with the CSV Books filegetter on Windows, and cleans up the code in the CSV Filegetter class substantially." Just to make sure that the correct number of modified files were included in this pull-request, I should take the last part of that sentence to read: "cleans up the code in the CSV [Books] Filegetter class substantially." Is that correct? Thank you in advance.
Correct, src/filegetters/CsvBooks.php was the only file that I modified.
Thanks for testing!
Github issue: (#428)
What does this Pull Request do?
Fixes a problem with the CSV Books filegetter on Windows, and cleans up the code in the CSV Filegetter class substantially.
What's new?
The regex at https://github.com/MarcusBarnes/mik/blob/master/src/filegetters/CsvBooks.php#L158 fails on Windows, where
DIRECTORY_SEPARATOR
is\
, which escapes the succeeding character. This PR wraps the regex inpreg_quote()
to prevent this from happening.Also, this PR refactors/simplifies the filegetter so we can remove the function
determineObjItems()
.How should this be tested?
This work is testable. Running
phpunit
in master and issue-428 branches should yield the same results (52 tests, 72 assertions).You will need a Windows environment to run a smoke test, but in the absence of one, I can attest that the changed code works, on Windows at any rate, because I used MIK yesterday with these changes in place to process this 1100-page book: http://digital.lib.sfu.ca/islandora/object/rarebooks:1/pages.
Interested parties
@MarcusBarnes