util: Fix shell environment fetching with `cmd` (#42093)

Lukas Wirth created

Release Notes:

- Fixed shell environment fetching failing when having `cmd` configured
as terminal shell

Change summary

crates/project/src/environment.rs |  83 ++++++--------
crates/util/src/shell_env.rs      | 178 ++++++++++++--------------------
2 files changed, 104 insertions(+), 157 deletions(-)

Detailed changes

crates/project/src/environment.rs 🔗

@@ -333,60 +333,51 @@ async fn load_directory_shell_environment(
             .into()
     };
 
-    if cfg!(target_os = "windows") {
+    let (shell, args) = shell.program_and_args();
+    let mut envs = util::shell_env::capture(shell.clone(), args, abs_path)
+        .await
+        .with_context(|| {
+            tx.unbounded_send("Failed to load environment variables".into())
+                .ok();
+            format!("capturing shell environment with {shell:?}")
+        })?;
+
+    if cfg!(target_os = "windows")
+        && let Some(path) = envs.remove("Path")
+    {
+        // windows env vars are case-insensitive, so normalize the path var
+        // so we can just assume `PATH` in other places
+        envs.insert("PATH".into(), path);
+    }
+    // If the user selects `Direct` for direnv, it would set an environment
+    // variable that later uses to know that it should not run the hook.
+    // We would include in `.envs` call so it is okay to run the hook
+    // even if direnv direct mode is enabled.
+    let direnv_environment = match load_direnv {
+        DirenvSettings::ShellHook => None,
         // Note: direnv is not available on Windows, so we skip direnv processing
         // and just return the shell environment
-        let (shell, args) = shell.program_and_args();
-        let mut envs = util::shell_env::capture(shell.clone(), args, abs_path)
-            .await
-            .with_context(|| {
-                tx.unbounded_send("Failed to load environment variables".into())
-                    .ok();
-                format!("capturing shell environment with {shell:?}")
-            })?;
-        if let Some(path) = envs.remove("Path") {
-            // windows env vars are case-insensitive, so normalize the path var
-            // so we can just assume `PATH` in other places
-            envs.insert("PATH".into(), path);
-        }
-        Ok(envs)
-    } else {
-        let (shell, args) = shell.program_and_args();
-        let mut envs = util::shell_env::capture(shell.clone(), args, abs_path)
+        DirenvSettings::Direct if cfg!(target_os = "windows") => None,
+        DirenvSettings::Direct => load_direnv_environment(&envs, &dir)
             .await
             .with_context(|| {
-                tx.unbounded_send("Failed to load environment variables".into())
+                tx.unbounded_send("Failed to load direnv environment".into())
                     .ok();
-                format!("capturing shell environment with {shell:?}")
-            })?;
-
-        // If the user selects `Direct` for direnv, it would set an environment
-        // variable that later uses to know that it should not run the hook.
-        // We would include in `.envs` call so it is okay to run the hook
-        // even if direnv direct mode is enabled.
-        let direnv_environment = match load_direnv {
-            DirenvSettings::ShellHook => None,
-            DirenvSettings::Direct => load_direnv_environment(&envs, &dir)
-                .await
-                .with_context(|| {
-                    tx.unbounded_send("Failed to load direnv environment".into())
-                        .ok();
-                    "load direnv environment"
-                })
-                .log_err(),
-        };
-        if let Some(direnv_environment) = direnv_environment {
-            for (key, value) in direnv_environment {
-                if let Some(value) = value {
-                    envs.insert(key, value);
-                } else {
-                    envs.remove(&key);
-                }
+                "load direnv environment"
+            })
+            .log_err(),
+    };
+    if let Some(direnv_environment) = direnv_environment {
+        for (key, value) in direnv_environment {
+            if let Some(value) = value {
+                envs.insert(key, value);
+            } else {
+                envs.remove(&key);
             }
         }
-
-        Ok(envs)
     }
+
+    Ok(envs)
 }
 
 async fn load_direnv_environment(

crates/util/src/shell_env.rs 🔗

@@ -136,124 +136,80 @@ async fn capture_windows(
         std::env::current_exe().context("Failed to determine current zed executable path.")?;
 
     let shell_kind = ShellKind::new(shell_path, true);
-    let env_output = match shell_kind {
+    if let ShellKind::Posix
+    | ShellKind::Csh
+    | ShellKind::Tcsh
+    | ShellKind::Rc
+    | ShellKind::Fish
+    | ShellKind::Xonsh = shell_kind
+    {
+        return Err(anyhow::anyhow!("unsupported shell kind"));
+    }
+    let mut cmd = crate::command::new_smol_command(shell_path);
+    let cmd = match shell_kind {
         ShellKind::Posix
         | ShellKind::Csh
         | ShellKind::Tcsh
         | ShellKind::Rc
         | ShellKind::Fish
         | ShellKind::Xonsh => {
-            return Err(anyhow::anyhow!("unsupported shell kind"));
-        }
-        ShellKind::PowerShell => {
-            let output = crate::command::new_smol_command(shell_path)
-                .args([
-                    "-NonInteractive",
-                    "-NoProfile",
-                    "-Command",
-                    &format!(
-                        "Set-Location '{}'; & '{}' --printenv",
-                        directory.display(),
-                        zed_path.display()
-                    ),
-                ])
-                .stdin(Stdio::null())
-                .stdout(Stdio::piped())
-                .stderr(Stdio::piped())
-                .output()
-                .await?;
-
-            anyhow::ensure!(
-                output.status.success(),
-                "PowerShell command failed with {}. stdout: {:?}, stderr: {:?}",
-                output.status,
-                String::from_utf8_lossy(&output.stdout),
-                String::from_utf8_lossy(&output.stderr),
-            );
-            output
-        }
-        ShellKind::Elvish => {
-            let output = crate::command::new_smol_command(shell_path)
-                .args([
-                    "-c",
-                    &format!(
-                        "cd '{}'; {} --printenv",
-                        directory.display(),
-                        zed_path.display()
-                    ),
-                ])
-                .stdin(Stdio::null())
-                .stdout(Stdio::piped())
-                .stderr(Stdio::piped())
-                .output()
-                .await?;
-
-            anyhow::ensure!(
-                output.status.success(),
-                "Elvish command failed with {}. stdout: {:?}, stderr: {:?}",
-                output.status,
-                String::from_utf8_lossy(&output.stdout),
-                String::from_utf8_lossy(&output.stderr),
-            );
-            output
-        }
-        ShellKind::Nushell => {
-            let output = crate::command::new_smol_command(shell_path)
-                .args([
-                    "-c",
-                    &format!(
-                        "cd '{}'; {}'{}' --printenv",
-                        directory.display(),
-                        shell_kind
-                            .command_prefix()
-                            .map(|prefix| prefix.to_string())
-                            .unwrap_or_default(),
-                        zed_path.display()
-                    ),
-                ])
-                .stdin(Stdio::null())
-                .stdout(Stdio::piped())
-                .stderr(Stdio::piped())
-                .output()
-                .await?;
-
-            anyhow::ensure!(
-                output.status.success(),
-                "Nushell command failed with {}. stdout: {:?}, stderr: {:?}",
-                output.status,
-                String::from_utf8_lossy(&output.stdout),
-                String::from_utf8_lossy(&output.stderr),
-            );
-            output
+            unreachable!()
         }
-        ShellKind::Cmd => {
-            let output = crate::command::new_smol_command(shell_path)
-                .args([
-                    "/c",
-                    &format!(
-                        "cd '{}'; '{}' --printenv",
-                        directory.display(),
-                        zed_path.display()
-                    ),
-                ])
-                .stdin(Stdio::null())
-                .stdout(Stdio::piped())
-                .stderr(Stdio::piped())
-                .output()
-                .await?;
-
-            anyhow::ensure!(
-                output.status.success(),
-                "Cmd command failed with {}. stdout: {:?}, stderr: {:?}",
-                output.status,
-                String::from_utf8_lossy(&output.stdout),
-                String::from_utf8_lossy(&output.stderr),
-            );
-            output
-        }
-    };
-
-    let env_output = String::from_utf8_lossy(&env_output.stdout);
+        ShellKind::PowerShell => cmd.args([
+            "-NonInteractive",
+            "-NoProfile",
+            "-Command",
+            &format!(
+                "Set-Location '{}'; & '{}' --printenv",
+                directory.display(),
+                zed_path.display()
+            ),
+        ]),
+        ShellKind::Elvish => cmd.args([
+            "-c",
+            &format!(
+                "cd '{}'; {} --printenv",
+                directory.display(),
+                zed_path.display()
+            ),
+        ]),
+        ShellKind::Nushell => cmd.args([
+            "-c",
+            &format!(
+                "cd '{}'; {}'{}' --printenv",
+                directory.display(),
+                shell_kind
+                    .command_prefix()
+                    .map(|prefix| prefix.to_string())
+                    .unwrap_or_default(),
+                zed_path.display()
+            ),
+        ]),
+        ShellKind::Cmd => cmd.args([
+            "/c",
+            "cd",
+            &directory.display().to_string(),
+            "&&",
+            &zed_path.display().to_string(),
+            "--printenv",
+        ]),
+    }
+    .stdin(Stdio::null())
+    .stdout(Stdio::piped())
+    .stderr(Stdio::piped());
+    let output = cmd
+        .output()
+        .await
+        .with_context(|| format!("command {cmd:?}"))?;
+    anyhow::ensure!(
+        output.status.success(),
+        "Command {cmd:?} failed with {}. stdout: {:?}, stderr: {:?}",
+        output.status,
+        String::from_utf8_lossy(&output.stdout),
+        String::from_utf8_lossy(&output.stderr),
+    );
+    // "cmd" "/c" "cd \'C:\\Workspace\\salsa\\\'; \'C:\\Workspace\\zed\\zed\\target\\debug\\zed.exe\' --printenv"
+    let env_output = String::from_utf8_lossy(&output.stdout);
 
     // Parse the JSON output from zed --printenv
     serde_json::from_str(&env_output)