Respecting line endings in AnnotationScanner #6676
Conversation
…eature/newlines-code-scanner
| @@ -160,7 +160,7 @@ protected function tokenize() | |||
| } | |||
| $currentChar = $stream[$streamIndex]; | |||
| $matches = array(); | |||
| $currentLine = (preg_match('#(.*)\n#', $stream, $matches, null, $streamIndex) === 1) ? $matches[1] : substr($stream, $streamIndex); | |||
| $currentLine = (preg_match('#(.*?)(?:\n|\r|\r\n)#', $stream, $matches, null, $streamIndex) === 1) ? $matches[1] : substr($stream, $streamIndex); | |||
There was a problem hiding this comment.
Could be simplified: (?:\n|\r\n?)
|
@waltertamboer needs a test case with a test asset with non-unix newlines, I suppose. |
|
Can actually be crafted in the test case by explicitly using escape sequences like "\r\n" or "\n\r" depending on tested system |
|
@Ocramius I'm more than happy to update this PR but it's a bit unclear what you want me to do. I probably can't create a test file that has other line endings because when people check out the code and git changes line endings, the test will break. |
|
@waltertamboer what is needed is a test asset with DOS/Windows line endings, and a respective test :-) |
…ewlines-code-scanner
…ows, Linux and Mac line endings.
|
@Ocramius Done. Sorry for the delay. I updated the existing unit test so that it is tested with all type of line endings. |
|
@waltertamboer thanks! |
|
@waltertamboer merged, thanks. |
|
@waltertamboer one remark, please never ever EVER merge Instead, use I simply cherry-picked the commits out of this PR, so it doesn't result as merged, but just as closed. Thanks for sorting this out, very simple and clean fix :-) |
|
Lesson learned. Thanks |
The annotation scanner only checked for line feeds ("\n"), carriage returns were not respected causing problems on files that were contained Mac or Windows line endings ("\r" or "\r\n").