tonerdo / dotnet-env

A .NET library to load environment variables from .env files
MIT License
427 stars 50 forks source link

Quoted comments and more #44

Closed rogusdev closed 3 years ago

rogusdev commented 3 years ago

This changes a lot. Version 2.0 for sure. Not backwards compatible with some old patterns that no one should have been using, but might have. In short: if you were consistent with how other dotenv libraries format .env files, you should be fine. (And if you are only using .env files in localdev as the README recommends, then you can't hurt production with this change! :) )

Notably, this completely replaces the parsing logic that used to exist, now using a Monadic Parser Combinator approach via Sprache. The main goal of these changes is to support multi line quoted strings for https://github.com/tonerdo/dotnet-env/pull/37#pullrequestreview-378813355 but along the way it incorporated a number of additional cleanups and requests.

This removes all the old options for flexibility in how to parse the individual lines: whitespace handling, comments vs embedded hash chars, and parsing escaped chars in quoted string. Now it is in line with how bash env vars actually work, which is how other libraries actually do .env files as well. So if you were using these options by positional bools, instead of setting them by name, you need to update your code to not accidentally change different args now!

If you want whitespace in a value, it must be quoted. There is no whitespace in env var names (identifiers, on the left hand side, LHS). There are a few new options for how to quote, and a new option to include an export (or a few other variations) prefix/precursor so that the .env file can be directly sourced to load the env vars in a terminal session.

There are no spaces between env var names/identifier LHS, the equals sign, and the value RHS. This is how bash works. It would be trivial to allow for the flexibility to have whitespace here if people wanted to break consistency with bash, but for now I chose what I consider the safer approach.

The old tests have changed in a few places -- some of the old examples are no longer valid! Many, many, many new tests have been added. And a massive shoutout to tests because a lot of edge case bugs got caught this way! Which is also to say: there are probably still a few edge cases bugs in this new approach, so please please please file issues if anything shows up!

jamesmanning commented 3 years ago

As a user I'd prefer Dictionary<string, string> instead (Postel's Law kinda), but that's easy enough as a consumer to add a ToDictionary call after - the Dictionary return would ensure keys only show up once and the enumrable doesn't, but otherwise lgtm 👍

rogusdev commented 3 years ago

Well, @jamesmanning the interesting thing about your point is that's pretty exactly why I don't want to use a dictionary -- because someone may have declared the same env var more than once and I want to report all of that, with the option for you to de-dupe into a dictionary if you want. But there certainly is an argument to be made that users will only ever want the final values. For me I think the decisive point is that the ienumerable is more expressive than the dictionary (which might have thrown away irrecoverable facts about the parsing), and calling ToDictionary is an easily added helper method here. Yw ;)

var kvps = DotNetEnv.Env.Load();
var dict = DotNetEnv.Env.ToDictionary(kvps);
rogusdev commented 3 years ago

Replaces #42 and #37 as well as resolving #38 and #40 and #43

rogusdev commented 3 years ago

Also resolves #39 now

rogusdev commented 3 years ago

Version 2.0.0 is out with this included! https://www.nuget.org/packages/DotNetEnv/2.0.0/