purcell / envrc

Emacs support for direnv which operates buffer-locally
379 stars 35 forks source link

Run from before-hack-local-variables-hook #16

Closed astoff closed 1 year ago

astoff commented 3 years ago

Adding envrc--update to before-hack-local-variables-hook instead of enabling envrc-mode seems to work fine, when combined with inheritenv, and would eliminate the overhead associated to every buffer creation, if I understand things correctly. It also seems more consistent to add envrc variables only to buffers that receive dir-local variables.

Should this mode of operation be officially supported?

Then only downside I see in this approach is that it's now unavoidable that envrc--update runs after the major mode hooks. A way around this issue would be to provide a macro on the lines of

(defmacro after-hack-local-variables (&rest body)
  "Arrange for BODY to run after buffer-local variables are set."
  `(add-hook 'hack-local-variables-hook (lambda () ,@body) nil 'local))
astoff commented 3 years ago

PS: I'm unable to find a practical case where the above macro is necessary — or, for that matter, where the readme warning about the order of activation of envrc-global-mode applies. Flycheck and Eglot, for instance, already have some logic to sufficiently delay the starting of external processes.

PPS: I realize now that hack-local-variables dutifully checks if there is at least one buffer-local variable before running ~its hooks, so an advice on that function would actually be needed. But this doesn't change the general idea.~ before-hack-local-variables-hook. So actually hack-local-variables-hook would be the right hook to use in this case.

purcell commented 3 years ago

This is an interesting option, thanks, I will take a look in due course.

downside I see in this approach is that it's now unavoidable that envrc--update runs after the major mode hooks

Hmm. I'm guessing that a minor mode's hooks would always run after the major mode's hooks anyway: you can usually see the major mode while a minor mode hook is executing.

Flycheck and Eglot, for instance, already have some logic to sufficiently delay the starting of external processes.

Interesting, for some reason in early local testing I concluded that the minor mode hook ordering was important. When packages don't handle this situation like Flycheck does, it's somewhat easier for users to reason about mode hook order in isolation than it is to understand the relative order or hack-local-variables-hook and minor mode hooks.

astoff commented 3 years ago

Hmm. I'm guessing that a minor mode's hooks would always run after the major mode's hooks anyway: you can usually see the major mode while a minor mode hook is executing.

Yes, but I believe that running envrc--update from hack-local-variables-hook would cause it to run after the activation of minor modes (at least those which are activated via the major mode hook; I don't know exactly when globalized minor modes kick in).

Interesting, for some reason in early local testing I concluded that the minor mode hook ordering was important.

I'm sure it is. I was just commenting that Flycheck is not among the problematic examples.

zackw commented 1 year ago

Abstractly it seems like the right place to do this is hack-dir-local--get-variables, but there's no official hook for that function, nor does it look straightforward to advise it.

astoff commented 1 year ago

Since I've implemented this idea in another package (buffer-env), I'll close the ticket.