Repository navigation
Handle \n in quotes strings - #363
danmichaelo wants to merge 2 commits into
Conversation
merge into test
Fix parser escaping tests
|
Thanks for the PR. Had this been in 2.x, it would have been a breaking change, but we are now strict about slashes in 3.x, so we can introduce new escape sequences without being breaking. 👍 I think we'd need to support either none, or all of the common sequences. We'll definely need to add |
|
It looks like I think we should add |
@danmichaelo Does docker only expand on double quoted strings? |
|
I guess it doesn't hurt to expand for both cases, since our parser will just fail otherwise. |
|
Thanks, @danmichaelo. I've merged this, and it'll be included in the v3.5.0 release. 🚀 |
2 participants
Background: I have a
.envfile that I also share with Docker. Docker doesn't handle newlines directly, so I escape them like so:I expected this to also work with phpdotenv, so I was caught by surprise when it didn't. Is there a compelling reason not to support it, or can support be added?
I'm attaching a failing test and also a patch that makes the test go green. In my implementation, I created an array of allowed characters (
['n', 'r']), so that it can be easily expanded if more characters should also be allowed (like\t). If you don't like the implementation, please let me know how it could be improved, or just change it.Note: I guess it could be argued that
\nshould not be expanded when using single quotes (theTEST_NSvalue in themultiline.envtest). But as long as the parser currently uses the same state (QUOTED_STATE) for both single and double quotes, it seems hard to distinguish without a larger rewrite)