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

Make direct direnv loading default #18536

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

WeetHet
Copy link
Contributor

@WeetHet WeetHet commented Sep 30, 2024

I've been running with direct direnv loading for a while now and haven't experienced any significant issues other than #18473. Making it default would make direnv integration more reliable and consistent. I've also updated the docs a bit to ensure that they represent current status of direnv integration

Release Notes:

  • Made direnv integration use direct (direnv export json) mode by default instead of relying on a shell hook, improving consistency and reliability of direnv detection

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Sep 30, 2024
@mrnugget mrnugget self-assigned this Sep 30, 2024
@mrnugget mrnugget merged commit 215bce1 into zed-industries:main Sep 30, 2024
10 checks passed
@mrnugget
Copy link
Member

Thanks!

@mrnugget
Copy link
Member

Ah damn, maybe I was too quick to merge. What happens now if someone doesn't have direnv installed? Won't we then always try to spawn the process and fail and put noise in the logs?

@WeetHet
Copy link
Contributor Author

WeetHet commented Sep 30, 2024

Ah damn, maybe I was too quick to merge. What happens now if someone doesn't have direnv installed? Won't we then always try to spawn the process and fail and put noise in the logs?

It's fine, we check for direnv and if there's none we return Ok(None), which gets us an empty direnv environment and no spam in the logs

@WeetHet WeetHet deleted the direct-direnv-default branch September 30, 2024 13:50
@WeetHet
Copy link
Contributor Author

WeetHet commented Sep 30, 2024

let Ok(direnv_path) = which::which("direnv") else {
return Ok(None);
};

@mrnugget
Copy link
Member

Sweet, thanks!

noaccOS pushed a commit to noaccOS/zed that referenced this pull request Oct 19, 2024
I've been running with direct direnv loading for a while now and haven't
experienced any significant issues other than zed-industries#18473. Making it default
would make direnv integration more reliable and consistent. I've also
updated the docs a bit to ensure that they represent current status of
direnv integration

Release Notes:

- Made direnv integration use direct (`direnv export json`) mode by
default instead of relying on a shell hook, improving consistency and
reliability of direnv detection
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants