dotenv-rs / dotenv

Library to help supply environment variables in testing and development
MIT License
557 stars 88 forks source link

Replace line-parsing RegEx by manual code #26

Closed Swatinem closed 5 years ago

Swatinem commented 5 years ago

Replace line-parsing RegEx by manual code

This avoids pulling in regex and lazy_static and makes this a true zero-dependency crate.

codecov[bot] commented 5 years ago

Codecov Report

Merging #26 into master will increase coverage by 0.71%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #26      +/-   ##
==========================================
+ Coverage   91.09%   91.81%   +0.71%     
==========================================
  Files          18       18              
  Lines         584      623      +39     
==========================================
+ Hits          532      572      +40     
+ Misses         52       51       -1
Impacted Files Coverage Δ
dotenv/src/parse.rs 97.25% <100%> (+0.03%) :arrow_up:
dotenv_codegen_implementation/src/lib.rs 0% <0%> (ø) :arrow_up:
dotenv/src/iter.rs 96.15% <0%> (+3.84%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c41d4c9...2c25991. Read the comment docs.

Swatinem commented 5 years ago

Thanks for the quick review :-) BTW, is there some kind of official spec for all of this? Allowing whitespace around the = would be a change to the regex that was there before. Also I think I made a mistake around export. I’m not sure if it is a reserved keyword or it can just as well be a var itself, like in export="some value"?

It will take some time for me to revise, this was just a quick experiment to see how far I come :-)

ZoeyR commented 5 years ago

There is no official spec for this. It's based off of https://github.com/bkeepers/dotenv but there is no official spec. We currently implement only a subset of the parsing capabilities of the original ruby gem.

Swatinem commented 5 years ago

I completely rewrote this with something resembling a "real" parser… And updated the tests to verify the new behavior (whitespace around =, treating export as key itself).

Swatinem commented 5 years ago

some flakiness with cargo-junit

No, it was a real failure… I added a comment why those two ifs you suggested to merge need to be separate…

ZoeyR commented 5 years ago

Oh right, I see the logic issue now. I do need to fix the pipeline though to error out better though it seems.