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

Update environment earlier #80

Closed
wants to merge 1 commit into from
Closed

Update environment earlier #80

wants to merge 1 commit into from

Conversation

wyuenho
Copy link
Contributor

@wyuenho wyuenho commented Aug 12, 2022

This PR should fix #17 and #28.

The problems are firstly, before-hack-local-variables-hook doesn't get run if the file has not file local variables, and secondly post-command-hook is run after all the major mode hooks. If any major mode hooks expects exec-path to be the most updated value, the current implementation does not work for this expectation. It also runs too frequently unnecessarily.

To update the environment as early as possible, this PR moves the decision to just before the major mode hooks are run but after the major mode has been set up, after a window buffer change and after a window is selected.

In addition, this PR resets the environment to the HOME dir's whenever the user selects a non-file and non-whitelisted window buffer so as to mitigate the perception problem that justified the entire existence of this package.

@wyuenho wyuenho changed the title Update environment earlier and more reliably Update environment earlier Aug 12, 2022
(direnv--maybe-update-environment))
(setq direnv--active-directory default-directory)
(add-hook 'change-major-mode-after-body-hook #'direnv--maybe-update-environment)
(add-to-list 'window-buffer-change-functions #'direnv--maybe-update-environment-wsc)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative implementation could be hooking into buffer-list-update-hook, but this works equally well. @wbolster what do you think?

@wyuenho
Copy link
Contributor Author

wyuenho commented Jul 21, 2023

@wbolster WDYT?

@wbolster
Copy link
Owner

wbolster commented Jul 21, 2023

ah yes, i gave this a try earlier.

some of the behavioural changes actually annoyed me: i lost my direnv environment even though i didn't want to. examples are opening a new scratch buffer to jot down some notes or copy-paste stuff from elsewhere, switching to a flycheck error window, opening an emacs debugger (e.g. on error). sure, expanding direnv-non-file-modes with a ton of other things could prevent that, but it feels like a negated way of expressing the behaviour i want. i understand this is a preference, e.g. envrc.el uses the opposite approach, but perhaps there is a better way? a direnv-linger setting or something like that? 🤔

i don't remember any issue with the change to the hooks/events that trigger (potential) direnv updated.

@wyuenho
Copy link
Contributor Author

wyuenho commented Jul 23, 2023

examples are opening a new scratch buffer to jot down some notes or copy-paste stuff from elsewhere, switching to a flycheck error window

Why would you want a per directory environment when you are doing these things?

a direnv-linger setting

What does this do? Is this a boolean flag to keep the current behavior? If nil, the behavior of this PR kicks in?

i don't remember any issue with the change to the hooks/events that trigger (potential) direnv updated.

I'm not sure what you mean, does it mean this PR doesn't interfere with normal emacs ops?

@wyuenho
Copy link
Contributor Author

wyuenho commented Aug 28, 2023

I'm going to close this as I've switched to envrc as it makes no sense to change the entire Emacs process's environment late.

@wyuenho wyuenho closed this Aug 28, 2023
@wyuenho wyuenho deleted the early-reliable-update-env branch August 28, 2023 17:28
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.

direnv loads too late sometimes
2 participants