Skip to content

update the direnv hook#628

Merged
amtoine merged 3 commits intonushell:mainfrom
amtoine:update-direnv-hook
Oct 4, 2023
Merged

update the direnv hook#628
amtoine merged 3 commits intonushell:mainfrom
amtoine:update-direnv-hook

Conversation

@amtoine
Copy link
Copy Markdown
Member

@amtoine amtoine commented Oct 2, 2023

i wanted to install and run direnv with Nushell, came accross this hook and thought i could update it a bit 😋

changelog

  • make config.nu possible to source => it will add the hook to the list of hooks, without overwriting
  • will run the hook on environment change thank to $env.config.hooks.env_change.PWD instead of $env.config.hooks.pre_prompt => i think this is the most controversial change, we can discuss that of course
  • checks if direnv is in the PATH before running the rest of the hook
  • uses a oneliner with default to load the environment
  • uses directly a closure instead of string
  • the comments were out of date, so i removed them

i tested this before and after and i think this works just as before 😌

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Oct 2, 2023

Just guessing here but it seems that one reason to run a hook in pre-prompt is to allow something from the direnv to update the prompt. Maybe you can do that too in the env_change hook. Not sure.

@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented Oct 2, 2023

Just guessing here but it seems that one reason to run a hook in pre-prompt is to allow something from the direnv to update the prompt. Maybe you can do that too in the env_change hook. Not sure.

not sure 🤔

one thing we could do is define a constant in that script that one could source and add to either the env_change or the pre_prompt?

@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented Oct 4, 2023

@fdncred
let me know if 6c01cf9 makes this any better 😌

Copy link
Copy Markdown
Contributor

@fdncred fdncred left a comment

Choose a reason for hiding this comment

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

Seems like it explain two ways to do it, so it looks better to me. I don't use direnv so I can't test it.

@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented Oct 4, 2023

@fdncred
i just ran, from nu -n,

$env.config.hooks.env_change.PWD = []
$env.config.hooks.env_change.PWD = (
    $env.config.hooks.env_change.PWD | append (source ~/documents/repos/github.com/amtoine/nu_scripts/hooks/direnv/config.nu)
)

went into a directory with a .envrc and direnv kicked in 😉

@amtoine amtoine merged commit 1019cca into nushell:main Oct 4, 2023
@amtoine amtoine deleted the update-direnv-hook branch October 4, 2023 17:33
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.

2 participants