-
Notifications
You must be signed in to change notification settings - Fork 445
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
Conversation
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. |
200de47
to
778dc3d
Compare
778dc3d
to
089a90b
Compare
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.
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. |
just a reminder :) |
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. |
Top work on this PR and I'll have to merge this up. Thanks a lot for contribution. 👍 🎉 |
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? |
ping @theskumar |
For future readers, per comment above
POSIX-compliant expansion of default values inside the
There doesn't appear to be a mechanism for defining defaults with |
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.