godotenv
godotenv copied to clipboard
joho
Add multi-line values support
Fixes #117 issue with multi-line values parsing.
Had to refactor existing parser to implement this. Theoretically, a new parser should be faster, since less iterations over source slice are required to do parsing.
The new parser passes all tests in godotenv_test.go, so it doesn't introduce any breaking changes.
Upd: If anyone needs this fix, you can a branch with fix or just use the latest tag from my fork.
Replace approach
replace (
github.com/joho/godotenv => github.com/x1unix/godotenv v1.3.1-0.20200910042738-acd8c1e858a6
)
Different package approach
go get -u github.com/x1unix/[email protected]
I would like to support multiline. Do you know if this behaviour is consistent with the ruby & node dotenv libraries?
@joho I don't use ruby or node so I don't know, but this is a correct behavior for environment variables in shell scripts.
Sorry for the delay, I'm planning on doing a maintenance pass over all my outstanding GitHub over the Xmas break
@joho I know you're busy. Is there any sort of help that someone else could give to get this moving forward?
@coolaj86 currently I'm just using my fork until this PR is "on review":
You can temporary redirect dependency to a fork in go.mod:
replace (
github.com/joho/godotenv => github.com/x1unix/godotenv v1.3.1-0.20200910042738-acd8c1e858a6
)
@milon-bs23 you can switch to my branch until this PR is not merged:
go.mod
replace (
github.com/joho/godotenv => github.com/x1unix/godotenv v1.3.1-0.20200910042738-acd8c1e858a6
)
Also, you can try to use the latest version from by repo (v1.4.0) but it has a different package URL (x1unix/godotenv).
I'm doing some manual testing on my end to ensure this change is in line with https://github.com/bkeepers/dotenv/blob/master/spec/dotenv/parser_spec.rb#L198-L234
If so, I'll merge and cut a release.
@x1unix in testing your PR I've found a problem with windows newlines.
To give myself extra piece of mind that your multiline behavior matches the canonical ruby implementation I copied a few of their testcases over into fixtures/quoted.env. They work find on linux & mac, but fail the build on windows.
You can see what I've done on #156
I have some unrelated build cleanup work to do which I'll pull into another PR, but if you can get the windows linebreak issue resolved I can merge whenever.
@joho thank you for the reply, I'll take a look at Windows build in a few days :)
Nice patch, works nicely but I've noticed that it breaks preserving inline comments :(
This PR is now indirectly merged via #156
I'm going to leave it on main without a numbered release for a little while (or do a beta release number) to let any regressions fall out first.
So this is really on me for not reviewing close enough but there were a couple of problems in this PR.
The first was in the category of what I expected and was procrastinating about - essentially having undefined behaviour I hadn't written a test for that would be missed with the new parser. #204 was it.
In trying to fix it in #205, I noticed the bigger problem.
The new parser replaced the existing parser, but didn't clean out the old private functions, and not all of the tests were updated to point at the new parser. This was particularly problematic as TestParsing has all the odd edge cases and wasn't updated. I've updated all tests to call the newer code paths (and deleted the old stale paths) and there's a few more failing cases.
I'll try and get the tests passing again at some point this week but I've got a pretty full schedule, so if anyone involved here wanted to finish #205 off sooner I'd appreciate it.