Skip to content

Add automatic ".env" compatibility#438

Closed
aude wants to merge 2 commits intodirenv:masterfrom
aude:env-file
Closed

Add automatic ".env" compatibility#438
aude wants to merge 2 commits intodirenv:masterfrom
aude:env-file

Conversation

@aude
Copy link
Copy Markdown

@aude aude commented Jan 3, 2019

This is work towards using .env if there is no .envrc (#284).

I love what direnv does, and would really like to use it more 😄 Having automatic "fallback" to .env would remove a lot of friction for me. I figured the best way to achieve that would be to help implement it.

The code is WIP now. It works, but might be a bit wrong/incoherent. I would appreciate any feedback, I'm quite new to Go.

  1. Naming and coding style could be off with regard to the rest of the project, just change it however you'd like.
  2. .env files are not loaded "layered" now, same as .envrc. I would personally like .env loading to work by default like source_up. Thoughts?
  3. I changed DIRENV_DIR to DIRENV_FILE, to keep track of the RC file used. Is that breaking anything?
  4. RC file "detection" is done by simply checking the file name. I think this is a bit cheap, maybe there should be a type attribute on the RC object instead. Thoughts?
  5. I haven't added any tests for .env loading. I'd like to add some, but could take some advice what should be tested.

@zimbatm
Copy link
Copy Markdown
Member

zimbatm commented Jan 9, 2019

This introduces a few issues with backward-compatibility:

  • people rely on the existence of $DIRENV_DIR to check if an environment has been loaded for example
  • the .env might get loaded even if this is not the user's intention
    I don't have a great solution to fix that unfortunately. Ideally the behaviour would be configurable in some way.

@aude
Copy link
Copy Markdown
Author

aude commented Jan 10, 2019

Thanks for checking!

  • I can change the code to have both DIRENV_FILE and DIRENV_DIR, that sounds better now knowing that DIRENV_DIR is used.
  • The .env will only be loaded if the user has accepted it via direnv allow, just like for .envrc. If you haven't allowed the .env file, you'll get this error message:
    direnv: error .env is blocked. Run `direnv allow` to approve its content.

Does this answer your questions?

@aude
Copy link
Copy Markdown
Author

aude commented Jan 10, 2019

I have been using this fork for the last week now without issues, for what it's worth.

@aude
Copy link
Copy Markdown
Author

aude commented Jan 15, 2019

Added back DIRENV_DIR, so that's not an issue anymore.

@alvis
Copy link
Copy Markdown

alvis commented Feb 28, 2019

Thank you @aude. This PR is highly anticipated!
@zimbatm: shell we merge this?

@aude aude closed this Apr 4, 2019
@aude
Copy link
Copy Markdown
Author

aude commented Apr 4, 2019

I did not mean to close this PR. I just rebased it on master, to keep the PR up to date.

I think one approach here could be to merge it and get it out to people. Then we could add improvements as we go, if needed.

I've been using this code successfully since January 3, 2019, so that could vouch for it working.

@aude aude reopened this Apr 4, 2019
@aude aude changed the title WIP: add automatic ".env" compatibility Add automatic ".env" compatibility Apr 4, 2019
@aude aude mentioned this pull request Apr 4, 2019
Closed
@schickling
Copy link
Copy Markdown

schickling commented Apr 4, 2019

:shipit:

@zimbatm
Copy link
Copy Markdown
Member

zimbatm commented Apr 6, 2019

@aude I added some minimal testing, which probably needs to be fixed. Having it work on your machine is not nearly sufficient to make sure it doesn't break other people's workflows as direnv is used in many different contexts. Please also amend the documentation to advertise the fact that direnv is now also looking for .env files.

@aude
Copy link
Copy Markdown
Author

aude commented Apr 9, 2019

Thanks for taking a look. I'll take a look at the tests and add documentation.

@zimbatm zimbatm force-pushed the master branch 8 times, most recently from a4e430c to 6d567f3 Compare May 21, 2019 10:22
@aude
Copy link
Copy Markdown
Author

aude commented Jul 4, 2019

Sorry for letting you guys down here. I lost motivation some months ago, I'm currently not motivated to finish the PR.

Hoping that changes, can't promise anything though.

@joaomcarlos
Copy link
Copy Markdown

joaomcarlos commented Nov 14, 2019

:shipit: !

@varac
Copy link
Copy Markdown

varac commented May 5, 2020

Ping, any progress here ?

@zimbatm
Copy link
Copy Markdown
Member

zimbatm commented May 5, 2020

@varac do you want to pick this up?

@JakeElder
Copy link
Copy Markdown

JakeElder commented Jun 23, 2020

Is this likely to be merged? I'd like to see this functionality included. Great work with direnv, thank you.

@zimbatm
Copy link
Copy Markdown
Member

zimbatm commented Jun 23, 2020

It depends on somebody stepping up and fixing the PR.

@JakeElder
Copy link
Copy Markdown

JakeElder commented Jun 23, 2020

It depends on somebody stepping up and fixing the PR.

Understood. I'd like to have a go but have very little time, knowledge of bash and experience with go, so I'm not best equipped 🤓. Thanks again for the package.

@robert-r-meyer
Copy link
Copy Markdown

robert-r-meyer commented Feb 5, 2021

@aude @zimbatm I'd be happy to finish this off given commit permissions to the fork.

@felipecrs felipecrs mentioned this pull request Nov 26, 2021
Merged
@felipecrs
Copy link
Copy Markdown
Contributor

felipecrs commented Nov 26, 2021

I created a PR as continuation of this: #845

It should be ready to review, any feedback is welcome.

@zimbatm
Copy link
Copy Markdown
Member

zimbatm commented Dec 24, 2021

Released in v2.30.1!

@zimbatm zimbatm closed this Dec 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@aude @zimbatm @alvis @schickling @joaomcarlos @varac @JakeElder @robert-r-meyer @felipecrs