fix: Correctly parse newline and multiline strings in dotenv files #192
Conversation
| where | ||
| newlineChar = string "\\n" >> return '\n' |
There was a problem hiding this comment.
@FranzGB I see normalChar parses any char that doesn't need escape. Do you know why that doesn't include the \n?
There was a problem hiding this comment.
Hi Cris! Yes, basically this line escapedChar = (char '\\' *> anySingle) <?> "escaped character" is eating all the \ not treating \n as a single character and leaving just the n as that is a candiate for normalChar.
There was a problem hiding this comment.
I do have another idea on how could we achieve this without adding a new function here (newlineChar <|> escapedChar <|> normalChar) but it will have to require modifying escapedChar function. Maybe we can catch up on this WDYT?
There was a problem hiding this comment.
@FranzGB LGTM! Although, I would like to understand a little bit more why this wasn't parsing the \n before.
FranzGB
force-pushed
the
bug-escape-new-line-characters
branch
from
0f52234
to
5dff2e9
Compare
Proposed Changes
\nand multiline strings in dotenv files.This pull request addresses issue #186. I wasn't sure if we should parse it as a newline or as a literal string. I decided to go with the latter. But happy to change it if needed. Let me know if you have any feedback.
Additionally, I noticed that our current tests depend heavily on the index of parsed (key, value) pairs, which could make them fragile and prone to breaking with minor changes in the fixture. I propose that we refactor these tests to make them more robust and resilient to changes. I look forward to hearing your thoughts on this.