Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support POSIX parameter expansion #30

Merged
merged 1 commit into from
Sep 8, 2016
Merged

Conversation

hugochinchilla
Copy link
Contributor

I needed this so much that made a PR without even asking if you would be intereseted on the feature, hope you like it.

Includes proper testing.

@theskumar
Copy link
Owner

This is something that I had in my already. Thanks for sending the PR. I'll have a look at the implementation in detail and let u know any issues. Expect 1-2 days of time, as I am bit preoccupied with work.

@hugochinchilla
Copy link
Contributor Author

I have reworked the PR, I forgot to test some cases and it wasn't working as expected.

POSIX expansion is working as in most dotenv libraries but it is not truly POSIX compilant.

${FOO} and FOO_${BAR} will be replaced without problems, but supporting default values in the form of ${FOO:-default_value} will not.

Truly posix expansion can be achieved using https://github.com/kojiromike/parameter_expansion but I didn't want to include external libraries without asking you first. For me the current behavior is good enough.

@hugochinchilla
Copy link
Contributor Author

just a reminder :)

@theskumar
Copy link
Owner

I am thinking of adding a note about not supporting the default values in POSIX expansion for now. This support can be always be added in new release if we getting any requests for it.

I'm just going over the PR and will let you if I find anything.

@theskumar
Copy link
Owner

Top work on this PR and I'll have to merge this up. Thanks a lot for contribution. 👍 🎉

@twosigmajab
Copy link

Haven't used this project before so was just checking out the README, and noticed it says "Note: Default Value Expansion is not supported as of yet, see #30.)" But it looks like this issue was resolved over two years ago, is that right? If so, I'm surprised no one asked about this sooner -- it looks like this project is still maintained and plenty of people are still using it, right?

@twosigmajab
Copy link

ping @theskumar

@al-the-x
Copy link

al-the-x commented Jan 7, 2019

For future readers, per comment above

POSIX expansion is working as in most dotenv libraries but it is not truly POSIX compilant.

${FOO} and FOO_${BAR} will be replaced without problems, but supporting default values in the form of ${FOO:-default_value} will not.

POSIX-compliant expansion of default values inside the .env file is not yet implemented, hence @theskumar's comment

I am thinking of adding a note about not supporting the default values in POSIX expansion for now. This support can be always be added in new release if we getting any requests for it.

There doesn't appear to be a mechanism for defining defaults with python-dotenv aside from using the idiomatic methods of dict like setdefault or get on the os.environ instance or with os.getenv.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants