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

Added tramp support #68

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

siddharthverma314
Copy link

Closes #47.

I just followed the suggestions from the emacs manual. I have tested this locally. Let me know if you want me to change anything!

@sellout
Copy link

sellout commented Feb 24, 2021

This isgreat. I'd love to see it merged. For the time being, I'll just patch it locally.

@sellout
Copy link

sellout commented Jun 11, 2022

Wondering if there could be some feedback on this? I've been patching locally, but @siddharthverma314 has had to keep this patch in sync, and it's just broken again.

@mogorman
Copy link

mogorman commented Aug 3, 2022

any chance this could be updated this feature would be really useful

@siddharthverma314
Copy link
Author

Hey all, sorry for the late reply -- I don't use this package anymore. However, I've performed a rebase in hopes that people still find this useful! I haven't tested this properly, please let me know if it works and if not feel free to send a patch!

@mjrusso
Copy link

mjrusso commented Feb 23, 2023

@siddharthverma314 thanks for putting this PR together (and the rebase)!

I recently switched to using your fork and I've noticed that direnv doesn't unload properly; the PATH updates (for example when changing directories), but existing entries are not removed. I'm not sure if this is specific to my config (I use direnv with asdf), however I can reproduce on multiple machines, and I don't have this issue with the latest release of emacs-direnv. Quickly skimming the PR, I don't see anything obvious that would cause this behaviour. Just raising this is in case anyone else has seen this.

(Sidenote: this happens when running locally; I haven't tested over TRAMP yet.)

@sellout
Copy link

sellout commented Aug 25, 2023

I started having an issue with this, and I think it’s because I’ve set up connection-local variables.

I get a stack overflow (or whatever the max-depth error is) where, basically, any TRAMP call occurs, TRAMP tries to apply my connection-local vars, which then calls direnv--maybe-update-environment, which calls file-directory-p, which triggers another TRAMP call, which tries to apply my connection-local variables, which calls direnv--maybe-update-environment, etc.

I’ve worked around it for now by having direnv give up if we’re already in a direnv call:

(defvar direnv--updating-environment nil)   ; added

(defun direnv--maybe-update-environment ()
  "Maybe update the environment."
  (unless direnv--updating-environment      ; added
    (let ((direnv--updating-environment t)) ; added
      (with-current-buffer (window-buffer)
        (let ((directory-name (direnv--directory)))
          (when (and directory-name
                     (not (string-equal direnv--active-directory directory-name))
                     (file-directory-p directory-name))
            (direnv-update-directory-environment directory-name)))))))

@sellout
Copy link

sellout commented Aug 29, 2023

Oh, another thing this PR could use is a defcustom to toggle whether direnv should be loaded over TRAMP.

Or maybe rather than a toggle, a list of external methods that are either supported or unsupported. E.g., even if I don’t want to use direnv over remote connections, using it with sudoedit might still be good.

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.

add support for TRAMP
4 participants