Read() default behaviour works more like Overload than Load()

[HartS]

When reading a series of files to a map, successive .env files which define a variable already set by files earlier in the list will overwrite the value of that variable. Since this is different from the behaviour of Load() (which is consistent with Load() from the reference dotenv ruby implementation), this difference should be documented.

Here's a gist for demonstration purposes

[joho]

I'm thinking that it's nearly time for a v2 - I've got a couple of issues representing drift from the reference implementation that I'm not sure I can address with the current API.

Do you think it would be more worthwhile to adapt the behaviour of Read or document the difference?

[HartS]

Documenting the difference is certainly preferable to leaving it open to misinterpretation as an immediate measure :) As for what to do in v2, I'm not sure. Read() isn't in the reference API, so while it may violate expectations undocumented, I wouldn't consider it a bug if behaviour was clearly documented. In the event that software is already relying on the current Read() behaviour (intentionally or not), changing it may break programs which upgrade in subtle ways also.

I would also suspect that functions named LoadMap() and OverloadMap() would be more clear in intention (especially for users familiar with ruby dotenv), and if you decide to go that route, Read() could still delegate to OverloadMap() for legacy support.

[geozelot]

I just implemented godotenv and went mental about my services doing exactly the opposite of what they were supposed to...I seriously think you should add some big bold notes about this somewhere; there's so much explanation as to why godotenv does not override keys, but then there's this mixup still. Or am I missing sth.? I could open a PR, but it seems you have some larger overhaul in mind?

[tonglil]

Yup, just stumbled upon this issue as well. In my opinion this should be considered a bug and fixed when it was first found out.

The function is documented as Read all env (with same file loading semantics as Load)... but it truly does not.

The correct implementation should look like:

		for key, value := range individualEnvMap {
-			envMap[key] = value
+			if _, ok := envMap[key]; !ok {
+				envMap[key] = value
+			}
		}