dotenv-hs icon indicating copy to clipboard operation
dotenv-hs copied to clipboard

pick last instead of first value

Open rudymatela opened this issue 5 years ago • 4 comments

This makes it so that we prefer latest than earlier occurrences of variable settings.

Before the change

$ cat > my_script.hs
#!/bin/bash
echo $FOO
echo $GOO
^D

$ cat > .env
#!/bin/sh
FOO=first
GOO=second
FOO=third
^D

$ dotenv -f.env ./my_script.hs
first
second

After the change

$ cat > my_script.hs
#!/bin/bash
echo $FOO
echo $GOO
^D

$ cat > .env
#!/bin/sh
FOO=first
GOO=second
FOO=third
^D

$ dotenv -f.env ./my_script.hs
third
second

This makes dotenv-hs consistent with both Python's dotenv and Bash/Sh's source.

The implementation may look innefficient with

reverse . nubBy (...) . reverse

however it is as efficient as the surrounding code which is O(n^2) overall as Data.List.union is O(n^2) already.

The ultimate reverse may not be needed, but I have kept it so the order in which variables are set is unchanged.

This fixes issue #121.

rudymatela avatar Apr 29 '20 16:04 rudymatela

Needless to say: this may or may not be desirable. One can argue that preferring the last occurence instead of the first is a personal preference. I do prefer pciking the last one for consistency with other tools.

So feel free to reject this PR if you disagree. :-)

rudymatela avatar Apr 29 '20 16:04 rudymatela

Thanks for the contribution @rudymatela :smile:

If other dotenv tools work this way, I suppose that's the de facto standard, we should follow it as well. Although, I think this could be a breaking change in case someone is already expecting dotenv to set the first ocurrence rather than the last one.

I'd like to hear your thoughts @jsl @sestrella @juanpaucar

CristhianMotoche avatar Apr 30 '20 13:04 CristhianMotoche

In personal preference with .bashrc or .zshrc configurations I have always come to expect that the latest value overrides previous values. If the python equivalent is using that approach, then, I find it even more desirable to go on that way

juanpaucar avatar Apr 30 '20 14:04 juanpaucar

@rudymatela Thank you for submitting this patch, I wonder if duplicate keys in a .env value is a use case or a user input error? If it is an error, maybe we should report this back to the user?

sestrella avatar May 04 '20 12:05 sestrella

Hello @rudymatela I really appreciate your contribution to the dotenv-hs library. Your solution is simple and straightforward. However, I prefer to go with the change done in #161 since it follows an approach similar to the one from other libraries from other languages.

I'm going to prepare a release with this change so that I can close #121 and this PR, which have been open for a very long time now.

Once again, thanks a lot for using and contributing to the library. I really appreciate your contributions and any future PR is very welcome.

CristhianMotoche avatar Apr 18 '23 16:04 CristhianMotoche

One more time, thanks for the contribution @rudymatela The last dotenv release (0.11.0.0) contains the fix for this. So, I'll close this PR.

CristhianMotoche avatar Apr 19 '23 18:04 CristhianMotoche