Skip to content

Conversation

ghostwriter
Copy link
Contributor

$ By default, the match must occur at the end of the string or before \n at the end of the string; in multiline mode, it must occur before the end of the line or before \n at the end of the line.
https://docs.microsoft.com/en-us/dotnet/standard/base-types/regular-expression-language-quick-reference

closes #138

@jasonmccreary
Copy link
Collaborator

While I understand since I followed the issue, but it seems rather cryptic.

I'd rather use \R instead of \r?$ or normalize $contents beforehand in a similar way.

We should also add a parser test where we pass in a string with Windows line endings to verify this change.

> `$` By default, the match must occur at the end of the string or before `\n` at the end of the string; in multiline mode, it must occur before the end of the line or before `\n` at the end of the line.
> https://docs.microsoft.com/en-us/dotnet/standard/base-types/regular-expression-language-quick-reference

closes #138
@ghostwriter
Copy link
Contributor Author

I'd rather use \R instead of \r?$ or normalize $contents beforehand in a similar way.

I have chosen to normalize $contents with str_replace(["\r\n", "\r"], "\n", $content);

We should also add a parser test where we pass in a string with Windows line endings to verify this change.

The test gives a false positive on newer versions of macOS. It must pass on Mac (OS 9 and below) and Windows OS for confidence. (unless I missed something)

@jasonmccreary jasonmccreary merged commit 4dd672a into laravel-shift:master Apr 27, 2020
@jasonmccreary
Copy link
Collaborator

Thanks.

@ghostwriter ghostwriter deleted the bugfix/fix-regular-expression-for-windows branch April 27, 2020 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

softDeletes not working
2 participants