bash-it icon indicating copy to clipboard operation
bash-it copied to clipboard

new plugin: safe_autoenv

Open oz123 opened this issue 3 months ago • 11 comments

Exports variables from .env when starting a shell or cd'ing to a directory. When leaving the directory the variables from .env will be unset.

Given:

$ mkdir -pv /tmp/foo/bar/baz mkdir: created directory '/tmp/foo' mkdir: created directory '/tmp/foo/bar' mkdir: created directory '/tmp/foo/bar/baz' $ echo CLOWN=sideshowbob > /tmp/foo/bar/baz/.env $ echo MASTER_CLOWN="Krusty the Clown" > /tmp/foo/bar/.env

$ cd /tmp/foo/bar/baz/ Processing /tmp/foo/bar/.env Processing /tmp/foo/bar/baz/.env $ cd ../ Unsetting CLOWN (from /tmp/foo/bar/baz) $ cd baz/ Processing /tmp/foo/bar/baz/.env cd ../../ Unsetting MASTER_CLOWN (from /tmp/foo/bar) Unsetting CLOWN (from /tmp/foo/bar/baz)

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [ ] My code follows the code style of this project.
  • [ ] If my change requires a change to the documentation, I have updated the documentation accordingly.
  • [ ] I have read the CONTRIBUTING document.
  • [ ] If I have added a new file, I also added it to clean_files.txt and formatted it using lint_clean_files.sh.
  • [ ] I have added tests to cover my changes, and all the new and existing tests pass.

oz123 avatar Oct 08 '25 15:10 oz123

@oz123 ?

seefood avatar Oct 11 '25 16:10 seefood

Fixed shellcheck warnings (SC2207, SC2155) and artisan completion issues (SC2034, SC2016). All pre-commit hooks now pass. ✅

seefood avatar Oct 11 '25 19:10 seefood

Hi! Thanks for fixing the stuff for me. I already did some of it but have not pushed it. Does that mean you are going to merge it? anything else needed?

oz123 avatar Oct 12 '25 10:10 oz123

I guess @hyperupcall (who is a contributor of Bash-it and also the maintainer of autoenv) may give good suggestions.

By the way, the plugin name contains safe, but what kind of safety does it indicate?

akinomyoga avatar Oct 12 '25 11:10 akinomyoga

Does that mean you are going to merge it? anything else needed?

Yes, I am just about to publish 3.2.0 and if it's fixed by Friday (see all the remarks from @akinomyoga ) it will be in there. have a go...

seefood avatar Oct 12 '25 12:10 seefood

but what kind of safety does it indicate?

I think Oz meant that it unsets vars once you CD out of a directory, however I don't see how it restores a previous value. needs a stack and all.

seefood avatar Oct 12 '25 12:10 seefood

Summary of findings:

Runtime code requiring bash 3.3+:

  1. plugins/available/pack.plugin.bash - ✅ Now has version check added (PR #2358) - Uses associative arrays (declare -A) requiring bash 4.0+ - Already had inline checks on lines 162 and 230 for bash > 3
  2. plugins/available/history-eternal.plugin.bash:4 - ✅ Already has version check - Requires bash 4.3+ for unlimited history - Returns early with warning if version < 4.3
  3. plugins/available/percol.plugin.bash:18 - ✅ Already has version check - Requires bash 4.0+ - Returns early with warning if version < 4
  4. vendor/github.com/gaelicWizard/bash-progcomp/defaults.completion.bash:51 - Vendor file (excluded from linting) - Has bash 4.0 check but only for performance optimization - Falls back to bash 3 compatible code

All runtime plugins requiring bash 3.3+ now have appropriate version checks and early returns.

seefood avatar Oct 13 '25 07:10 seefood

Thank you for your investigations!

For pack.plugin.bash: OK, I haven't noticed there is an existing check for the version check in pack.plugin.bash. It seems intentional that declare -A flaghash 2> /dev/null in pack.plugin.bash (where the error message is suppressed by the redirection to /dev/null) causes an error in Bash < 4.0. If so, it probably works without checking the Bash version at the beginning of the file.

I checked how the associative arrays flaghash and aliashash are used in pack.plugin.bash. However, flaghash is set but not referenced anywhere. Conversely, aliashash is referenced but not set anywhere. If that is true, it seems to me that we can safely remove all codes related to flaghash and aliashash. But maybe I missed something. @seefood Do you have an idea?

akinomyoga avatar Oct 13 '25 10:10 akinomyoga

Anyway, we need a Bash version check also for this new plugin (as long as declare -A is used).

akinomyoga avatar Oct 13 '25 10:10 akinomyoga

seems to me that we can safely remove all codes related to flaghash and aliashash

I'll read it a bit later, I have a feeling shellcheck would have caught any problems like that.

However, right now I am glued to the TV, as a long war is coming to an end here and it's a happy news day I can't easily rip myself out of and do work :)

seefood avatar Oct 13 '25 10:10 seefood

Perhaps "autounset-autoenv" or "autocleanup-autoenv" fit better?

LIFO direnv...

seefood avatar Oct 14 '25 21:10 seefood