Skip to content

Commit

Permalink
Environment loading fixes (zed-industries#19144)
Browse files Browse the repository at this point in the history
Closes zed-industries#19040
Addresses the problem with annoying error messages on windows (see
comment from SomeoneToIgnore on zed-industries#18567)

Release Notes:

- Fixed the bug where language servers from PATH would sometimes be
prioritised over the ones from `direnv`
- Stopped running environment loading on windows as it didn't work
anyways due to `SHELL` not being set
  • Loading branch information
WeetHet authored Oct 16, 2024
1 parent a77ec94 commit 1286198
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 17 deletions.
10 changes: 7 additions & 3 deletions crates/project/src/direnv.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::environment::EnvironmentErrorMessage;
use std::process::ExitStatus;

#[cfg(not(any(test, feature = "test-support")))]
#[cfg(not(any(target_os = "windows", test, feature = "test-support")))]
use {collections::HashMap, std::path::Path, util::ResultExt};

#[derive(Clone)]
Expand Down Expand Up @@ -30,14 +30,18 @@ impl From<DirenvError> for Option<EnvironmentErrorMessage> {
}
}

#[cfg(not(any(test, feature = "test-support")))]
pub async fn load_direnv_environment(dir: &Path) -> Result<HashMap<String, String>, DirenvError> {
#[cfg(not(any(target_os = "windows", test, feature = "test-support")))]
pub async fn load_direnv_environment(
env: &HashMap<String, String>,
dir: &Path,
) -> Result<HashMap<String, String>, DirenvError> {
let Ok(direnv_path) = which::which("direnv") else {
return Err(DirenvError::NotFound);
};

let Some(direnv_output) = smol::process::Command::new(direnv_path)
.args(["export", "json"])
.envs(env)
.env("TERM", "dumb")
.current_dir(dir)
.output()
Expand Down
42 changes: 28 additions & 14 deletions crates/project/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,19 @@ async fn load_shell_environment(
(Some(fake_env), None)
}

#[cfg(not(any(test, feature = "test-support")))]
#[cfg(all(target_os = "windows", not(any(test, feature = "test-support"))))]
async fn load_shell_environment(
_dir: &Path,
_load_direnv: &DirenvSettings,
) -> (
Option<HashMap<String, String>>,
Option<EnvironmentErrorMessage>,
) {
// TODO the current code works with Unix $SHELL only, implement environment loading on windows
(None, None)
}

#[cfg(not(any(target_os = "windows", test, feature = "test-support")))]
async fn load_shell_environment(
dir: &Path,
load_direnv: &DirenvSettings,
Expand All @@ -225,18 +237,6 @@ async fn load_shell_environment(
(None, Some(message))
}

let (direnv_environment, direnv_error) = match load_direnv {
DirenvSettings::ShellHook => (None, None),
DirenvSettings::Direct => match load_direnv_environment(dir).await {
Ok(env) => (Some(env), None),
Err(err) => (
None,
<Option<EnvironmentErrorMessage> as From<DirenvError>>::from(err),
),
},
};
let direnv_environment = direnv_environment.unwrap_or(HashMap::default());

let marker = "ZED_SHELL_START";
let Some(shell) = std::env::var("SHELL").log_err() else {
return message("Failed to get login environment. SHELL environment variable is not set");
Expand Down Expand Up @@ -279,7 +279,6 @@ async fn load_shell_environment(

let Some(output) = smol::process::Command::new(&shell)
.args(["-l", "-i", "-c", &command])
.envs(direnv_environment)
.output()
.await
.log_err()
Expand All @@ -305,5 +304,20 @@ async fn load_shell_environment(
parsed_env.insert(key, value);
});

let (direnv_environment, direnv_error) = match load_direnv {
DirenvSettings::ShellHook => (None, None),
DirenvSettings::Direct => match load_direnv_environment(&parsed_env, dir).await {
Ok(env) => (Some(env), None),
Err(err) => (
None,
<Option<EnvironmentErrorMessage> as From<DirenvError>>::from(err),
),
},
};

for (key, value) in direnv_environment.unwrap_or(HashMap::default()) {
parsed_env.insert(key, value);
}

(Some(parsed_env), direnv_error)
}

0 comments on commit 1286198

Please sign in to comment.