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

fix: remove try block #949

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: remove try block #949

wants to merge 1 commit into from

Conversation

avioli
Copy link

@avioli avioli commented May 25, 2022

try {} except e {} is now deprecated (elvish v0.18.0) and will be removed in the next version - 0.19.0.

Since direnv will most probably run in pre-0.18.0 elvish environments it is maybe for the best to remove the try block.

As far as I can see - its use is to catch exceptions from running direnv export elvish, which should only happen if direnv vanishes from the user's path. Any other error should most probably fail anyway, rather than echo the exception but proceed.

`try {} except e {}` is now deprecated (elvish v0.18.0) and will be
removed in the next version - 0.19.0.

Since direnv will most probably run in pre-0.18.0 elvish
environments it is maybe for the best to remove the try block.

As far as I can see - its use is to catch exceptions from running
`direnv export elvish`, which should only happen if direnv vanishes
from the user's path. Any other error should most probably fail
anyway, rather than echo the exception but proceed.
@avioli
Copy link
Author

avioli commented May 25, 2022

If I cd into a directory with .envrc I used to get:

direnv: error /tmp/.envrc is blocked. Run `direnv allow` to approve its content
[&reason=[&cmd-name=/opt/homebrew/bin/direnv &exit-status=1 &pid=96438 &type=external-cmd/exited]]

now I get

direnv: error /tmp/.envrc is blocked. Run `direnv allow` to approve its content
Exception: /opt/homebrew/bin/direnv exited with 1
/Users/avioli/.config/elvish/lib/direnv.elv, line 3: 	var m = [("/opt/homebrew/bin/direnv" export elvish | from-json)]

but at the end - the change of the directory is not prevented.

@zimbatm
Copy link
Member

zimbatm commented Jun 21, 2022

@icidasset @mmlb do you have any opinion on this PR?

@mmlb
Copy link
Contributor

mmlb commented Jun 21, 2022

Hmm I get that it doesn't really matter that the exception is not caught but it's not something I'd like to do if this were my project. Why not just s/except/catch/? That preserves the current behavior, works in 0.18 and is the correct spelling going forward.

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

Successfully merging this pull request may close these issues.

None yet

3 participants