Use ShellKind::try_quote whenever we need to quote shell args (#41104)

Jakub Konka created

Re-reverts
https://github.com/zed-industries/zed/commit/8f4646d6c357409cfc286104088b4d21f2bea851
with fixes

Release Notes:

- N/A

Change summary

Cargo.lock                                      |   4 
crates/askpass/src/askpass.rs                   |  12 
crates/dap_adapters/Cargo.toml                  |   1 
crates/dap_adapters/src/javascript.rs           |   4 
crates/debugger_ui/Cargo.toml                   |   1 
crates/debugger_ui/src/new_process_modal.rs     |  14 +
crates/project/Cargo.toml                       |   1 
crates/project/src/environment.rs               |   4 
crates/project/src/lsp_store/lsp_ext_command.rs |   3 
crates/project/src/terminals.rs                 |  23 +-
crates/remote/Cargo.toml                        |   1 
crates/remote/src/transport/ssh.rs              |  81 ++++---
crates/remote/src/transport/wsl.rs              |  24 +
crates/remote_server/src/headless_project.rs    |   2 
crates/task/src/task.rs                         | 103 ++-------
crates/terminal/src/terminal.rs                 |  14 
crates/terminal/src/terminal_settings.rs        |   2 
crates/util/src/paths.rs                        |  34 +--
crates/util/src/shell.rs                        | 196 +++++++++++++++---
crates/util/src/shell_builder.rs                |   7 
crates/util/src/shell_env.rs                    |   2 
crates/util/src/util.rs                         |   5 
22 files changed, 310 insertions(+), 228 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -4532,7 +4532,6 @@ dependencies = [
  "paths",
  "serde",
  "serde_json",
- "shlex",
  "smol",
  "task",
  "util",
@@ -4758,7 +4757,6 @@ dependencies = [
  "serde_json",
  "serde_json_lenient",
  "settings",
- "shlex",
  "sysinfo 0.37.2",
  "task",
  "tasks_ui",
@@ -12926,7 +12924,6 @@ dependencies = [
  "settings",
  "sha2",
  "shellexpand 2.1.2",
- "shlex",
  "smallvec",
  "smol",
  "snippet",
@@ -13839,7 +13836,6 @@ dependencies = [
  "serde",
  "serde_json",
  "settings",
- "shlex",
  "smol",
  "tempfile",
  "thiserror 2.0.17",

crates/askpass/src/askpass.rs 🔗

@@ -20,7 +20,7 @@ use futures::{
 };
 use gpui::{AsyncApp, BackgroundExecutor, Task};
 use smol::fs;
-use util::{ResultExt as _, debug_panic, maybe, paths::PathExt};
+use util::{ResultExt as _, debug_panic, maybe, paths::PathExt, shell::ShellKind};
 
 /// Path to the program used for askpass
 ///
@@ -199,9 +199,15 @@ impl PasswordProxy {
         let current_exec =
             std::env::current_exe().context("Failed to determine current zed executable path.")?;
 
+        // TODO: inferred from the use of powershell.exe in askpass_helper_script
+        let shell_kind = if cfg!(windows) {
+            ShellKind::PowerShell
+        } else {
+            ShellKind::Posix
+        };
         let askpass_program = ASKPASS_PROGRAM
             .get_or_init(|| current_exec)
-            .try_shell_safe()
+            .try_shell_safe(shell_kind)
             .context("Failed to shell-escape Askpass program path.")?
             .to_string();
         // Create an askpass script that communicates back to this process.
@@ -343,7 +349,7 @@ fn generate_askpass_script(askpass_program: &str, askpass_socket: &std::path::Pa
     format!(
         r#"
         $ErrorActionPreference = 'Stop';
-        ($args -join [char]0) | & "{askpass_program}" --askpass={askpass_socket} 2> $null
+        ($args -join [char]0) | & {askpass_program} --askpass={askpass_socket} 2> $null
         "#,
         askpass_socket = askpass_socket.display(),
     )

crates/dap_adapters/Cargo.toml 🔗

@@ -35,7 +35,6 @@ log.workspace = true
 paths.workspace = true
 serde.workspace = true
 serde_json.workspace = true
-shlex.workspace = true
 smol.workspace = true
 task.workspace = true
 util.workspace = true

crates/dap_adapters/src/javascript.rs 🔗

@@ -6,7 +6,7 @@ use gpui::AsyncApp;
 use serde_json::Value;
 use std::{path::PathBuf, sync::OnceLock};
 use task::DebugRequest;
-use util::{ResultExt, maybe};
+use util::{ResultExt, maybe, shell::ShellKind};
 
 use crate::*;
 
@@ -67,7 +67,7 @@ impl JsDebugAdapter {
                     .get("type")
                     .filter(|value| value == &"node-terminal")?;
                 let command = configuration.get("command")?.as_str()?.to_owned();
-                let mut args = shlex::split(&command)?.into_iter();
+                let mut args = ShellKind::Posix.split(&command)?.into_iter();
                 let program = args.next()?;
                 configuration.insert("runtimeExecutable".to_owned(), program.into());
                 configuration.insert(

crates/debugger_ui/Cargo.toml 🔗

@@ -60,7 +60,6 @@ serde.workspace = true
 serde_json.workspace = true
 serde_json_lenient.workspace = true
 settings.workspace = true
-shlex.workspace = true
 sysinfo.workspace = true
 task.workspace = true
 tasks_ui.workspace = true

crates/debugger_ui/src/new_process_modal.rs 🔗

@@ -32,7 +32,7 @@ use ui::{
     SharedString, Styled, StyledExt, ToggleButton, ToggleState, Toggleable, Tooltip, Window, div,
     h_flex, relative, rems, v_flex,
 };
-use util::{ResultExt, rel_path::RelPath};
+use util::{ResultExt, rel_path::RelPath, shell::ShellKind};
 use workspace::{ModalView, Workspace, notifications::DetachAndPromptErr, pane};
 
 use crate::{attach_modal::AttachModal, debugger_panel::DebugPanel};
@@ -839,7 +839,11 @@ impl ConfigureMode {
             };
         }
         let command = self.program.read(cx).text(cx);
-        let mut args = shlex::split(&command).into_iter().flatten().peekable();
+        let mut args = ShellKind::Posix
+            .split(&command)
+            .into_iter()
+            .flatten()
+            .peekable();
         let mut env = FxHashMap::default();
         while args.peek().is_some_and(|arg| arg.contains('=')) {
             let arg = args.next().unwrap();
@@ -1265,7 +1269,11 @@ impl PickerDelegate for DebugDelegate {
             })
             .unwrap_or_default();
 
-        let mut args = shlex::split(&text).into_iter().flatten().peekable();
+        let mut args = ShellKind::Posix
+            .split(&text)
+            .into_iter()
+            .flatten()
+            .peekable();
         let mut env = HashMap::default();
         while args.peek().is_some_and(|arg| arg.contains('=')) {
             let arg = args.next().unwrap();

crates/project/Cargo.toml 🔗

@@ -72,7 +72,6 @@ serde_json.workspace = true
 settings.workspace = true
 sha2.workspace = true
 shellexpand.workspace = true
-shlex.workspace = true
 smallvec.workspace = true
 smol.workspace = true
 snippet.workspace = true

crates/project/src/environment.rs 🔗

@@ -4,7 +4,7 @@ use language::Buffer;
 use remote::RemoteClient;
 use rpc::proto::{self, REMOTE_SERVER_PROJECT_ID};
 use std::{collections::VecDeque, path::Path, sync::Arc};
-use task::Shell;
+use task::{Shell, shell_to_proto};
 use util::ResultExt;
 use worktree::Worktree;
 
@@ -198,7 +198,7 @@ impl ProjectEnvironment {
                         .proto_client()
                         .request(proto::GetDirectoryEnvironment {
                             project_id: REMOTE_SERVER_PROJECT_ID,
-                            shell: Some(shell.clone().to_proto()),
+                            shell: Some(shell_to_proto(shell.clone())),
                             directory: abs_path.to_string_lossy().to_string(),
                         });
                 cx.spawn(async move |_, _| {

crates/project/src/lsp_store/lsp_ext_command.rs 🔗

@@ -657,6 +657,7 @@ impl LspCommand for GetLspRunnables {
                     );
                     task_template.args.extend(cargo.cargo_args);
                     if !cargo.executable_args.is_empty() {
+                        let shell_kind = task_template.shell.shell_kind(cfg!(windows));
                         task_template.args.push("--".to_string());
                         task_template.args.extend(
                             cargo
@@ -682,7 +683,7 @@ impl LspCommand for GetLspRunnables {
                                 // That bit is not auto-expanded when using single quotes.
                                 // Escape extra cargo args unconditionally as those are unlikely to contain `~`.
                                 .flat_map(|extra_arg| {
-                                    shlex::try_quote(&extra_arg).ok().map(|s| s.to_string())
+                                    shell_kind.try_quote(&extra_arg).map(|s| s.to_string())
                                 }),
                         );
                     }

crates/project/src/terminals.rs 🔗

@@ -167,18 +167,19 @@ impl Project {
                     match remote_client {
                         Some(remote_client) => match activation_script.clone() {
                             activation_script if !activation_script.is_empty() => {
-                                let activation_script = activation_script.join("; ");
+                                let separator = shell_kind.sequential_commands_separator();
+                                let activation_script =
+                                    activation_script.join(&format!("{separator} "));
                                 let to_run = format_to_run();
-                                let args =
-                                    vec!["-c".to_owned(), format!("{activation_script}; {to_run}")];
+                                let shell = remote_client
+                                    .read(cx)
+                                    .shell()
+                                    .unwrap_or_else(get_default_system_shell);
+                                let arg = format!("{activation_script}{separator} {to_run}");
+                                let args = shell_kind.args_for_shell(false, arg);
+
                                 create_remote_shell(
-                                    Some((
-                                        &remote_client
-                                            .read(cx)
-                                            .shell()
-                                            .unwrap_or_else(get_default_system_shell),
-                                        &args,
-                                    )),
+                                    Some((&shell, &args)),
                                     env,
                                     path,
                                     remote_client,
@@ -547,7 +548,7 @@ fn create_remote_shell(
         Shell::WithArguments {
             program: command.program,
             args: command.args,
-            title_override: Some(format!("{} — Terminal", host).into()),
+            title_override: Some(format!("{} — Terminal", host)),
         },
         command.env,
     ))

crates/remote/Cargo.toml 🔗

@@ -34,7 +34,6 @@ rpc = { workspace = true, features = ["gpui"] }
 serde.workspace = true
 serde_json.workspace = true
 settings.workspace = true
-shlex.workspace = true
 smol.workspace = true
 tempfile.workspace = true
 thiserror.workspace = true

crates/remote/src/transport/ssh.rs 🔗

@@ -203,17 +203,6 @@ impl AsMut<Child> for MasterProcess {
     }
 }
 
-macro_rules! shell_script {
-    ($fmt:expr, $($name:ident = $arg:expr),+ $(,)?) => {{
-        format!(
-            $fmt,
-            $(
-                $name = shlex::try_quote($arg).unwrap()
-            ),+
-        )
-    }};
-}
-
 #[async_trait(?Send)]
 impl RemoteConnection for SshRemoteConnection {
     async fn kill(&self) -> Result<()> {
@@ -738,21 +727,23 @@ impl SshRemoteConnection {
         delegate.set_status(Some("Extracting remote development server"), cx);
         let server_mode = 0o755;
 
+        let shell_kind = ShellKind::Posix;
         let orig_tmp_path = tmp_path.display(self.path_style());
+        let server_mode = format!("{:o}", server_mode);
+        let server_mode = shell_kind
+            .try_quote(&server_mode)
+            .context("shell quoting")?;
+        let dst_path = dst_path.display(self.path_style());
+        let dst_path = shell_kind.try_quote(&dst_path).context("shell quoting")?;
         let script = if let Some(tmp_path) = orig_tmp_path.strip_suffix(".gz") {
-            shell_script!(
+            format!(
                 "gunzip -f {orig_tmp_path} && chmod {server_mode} {tmp_path} && mv {tmp_path} {dst_path}",
-                server_mode = &format!("{:o}", server_mode),
-                dst_path = &dst_path.display(self.path_style()),
             )
         } else {
-            shell_script!(
-                "chmod {server_mode} {orig_tmp_path} && mv {orig_tmp_path} {dst_path}",
-                server_mode = &format!("{:o}", server_mode),
-                dst_path = &dst_path.display(self.path_style())
-            )
+            format!("chmod {server_mode} {orig_tmp_path} && mv {orig_tmp_path} {dst_path}",)
         };
-        self.socket.run_command("sh", &["-c", &script]).await?;
+        let args = shell_kind.args_for_shell(false, script.to_string());
+        self.socket.run_command("sh", &args).await?;
         Ok(())
     }
 
@@ -886,8 +877,12 @@ impl SshSocket {
     // into a machine. You must use `cd` to get back to $HOME.
     // You need to do it like this: $ ssh host "cd; sh -c 'ls -l /tmp'"
     fn ssh_command(&self, program: &str, args: &[impl AsRef<str>]) -> process::Command {
+        let shell_kind = ShellKind::Posix;
         let mut command = util::command::new_smol_command("ssh");
-        let mut to_run = shlex::try_quote(program).unwrap().into_owned();
+        let mut to_run = shell_kind
+            .try_quote(program)
+            .expect("shell quoting")
+            .into_owned();
         for arg in args {
             // We're trying to work with: sh, bash, zsh, fish, tcsh, ...?
             debug_assert!(
@@ -895,9 +890,10 @@ impl SshSocket {
                 "multiline arguments do not work in all shells"
             );
             to_run.push(' ');
-            to_run.push_str(&shlex::try_quote(arg.as_ref()).unwrap());
+            to_run.push_str(&shell_kind.try_quote(arg.as_ref()).expect("shell quoting"));
         }
-        let to_run = format!("cd; {to_run}");
+        let separator = shell_kind.sequential_commands_separator();
+        let to_run = format!("cd{separator} {to_run}");
         self.ssh_options(&mut command, true)
             .arg(self.connection_options.ssh_url())
             .arg("-T")
@@ -906,7 +902,7 @@ impl SshSocket {
         command
     }
 
-    async fn run_command(&self, program: &str, args: &[&str]) -> Result<String> {
+    async fn run_command(&self, program: &str, args: &[impl AsRef<str>]) -> Result<String> {
         let output = self.ssh_command(program, args).output().await?;
         anyhow::ensure!(
             output.status.success(),
@@ -1080,7 +1076,10 @@ impl SshConnectionOptions {
             "-w",
         ];
 
-        let mut tokens = shlex::split(input).context("invalid input")?.into_iter();
+        let mut tokens = ShellKind::Posix
+            .split(input)
+            .context("invalid input")?
+            .into_iter();
 
         'outer: while let Some(arg) = tokens.next() {
             if ALLOWED_OPTS.contains(&(&arg as &str)) {
@@ -1243,6 +1242,7 @@ fn build_command(
 ) -> Result<CommandTemplate> {
     use std::fmt::Write as _;
 
+    let shell_kind = ShellKind::new(ssh_shell, false);
     let mut exec = String::new();
     if let Some(working_dir) = working_dir {
         let working_dir = RemotePathBuf::new(working_dir, ssh_path_style).to_string();
@@ -1252,29 +1252,38 @@ fn build_command(
         const TILDE_PREFIX: &'static str = "~/";
         if working_dir.starts_with(TILDE_PREFIX) {
             let working_dir = working_dir.trim_start_matches("~").trim_start_matches("/");
-            write!(exec, "cd \"$HOME/{working_dir}\" && ",).unwrap();
+            write!(exec, "cd \"$HOME/{working_dir}\" && ",)?;
         } else {
-            write!(exec, "cd \"{working_dir}\" && ",).unwrap();
+            write!(exec, "cd \"{working_dir}\" && ",)?;
         }
     } else {
-        write!(exec, "cd && ").unwrap();
+        write!(exec, "cd && ")?;
     };
-    write!(exec, "exec env ").unwrap();
+    write!(exec, "exec env ")?;
 
     for (k, v) in input_env.iter() {
-        if let Some((k, v)) = shlex::try_quote(k).ok().zip(shlex::try_quote(v).ok()) {
-            write!(exec, "{}={} ", k, v).unwrap();
-        }
+        write!(
+            exec,
+            "{}={} ",
+            k,
+            shell_kind.try_quote(v).context("shell quoting")?
+        )?;
     }
 
     if let Some(input_program) = input_program {
-        write!(exec, "{}", shlex::try_quote(&input_program).unwrap()).unwrap();
+        write!(
+            exec,
+            "{}",
+            shell_kind
+                .try_quote(&input_program)
+                .context("shell quoting")?
+        )?;
         for arg in input_args {
-            let arg = shlex::try_quote(&arg)?;
-            write!(exec, " {}", &arg).unwrap();
+            let arg = shell_kind.try_quote(&arg).context("shell quoting")?;
+            write!(exec, " {}", &arg)?;
         }
     } else {
-        write!(exec, "{ssh_shell} -l").unwrap();
+        write!(exec, "{ssh_shell} -l")?;
     };
 
     let mut args = Vec::new();

crates/remote/src/transport/wsl.rs 🔗

@@ -2,7 +2,7 @@ use crate::{
     RemoteClientDelegate, RemotePlatform,
     remote_client::{CommandTemplate, RemoteConnection, RemoteConnectionOptions},
 };
-use anyhow::{Result, anyhow, bail};
+use anyhow::{Context, Result, anyhow, bail};
 use async_trait::async_trait;
 use collections::HashMap;
 use futures::channel::mpsc::{Sender, UnboundedReceiver, UnboundedSender};
@@ -441,6 +441,7 @@ impl RemoteConnection for WslRemoteConnection {
             bail!("WSL shares the network interface with the host system");
         }
 
+        let shell_kind = ShellKind::new(&self.shell, false);
         let working_dir = working_dir
             .map(|working_dir| RemotePathBuf::new(working_dir, PathStyle::Posix).to_string())
             .unwrap_or("~".to_string());
@@ -448,19 +449,26 @@ impl RemoteConnection for WslRemoteConnection {
         let mut exec = String::from("exec env ");
 
         for (k, v) in env.iter() {
-            if let Some((k, v)) = shlex::try_quote(k).ok().zip(shlex::try_quote(v).ok()) {
-                write!(exec, "{}={} ", k, v).unwrap();
-            }
+            write!(
+                exec,
+                "{}={} ",
+                k,
+                shell_kind.try_quote(v).context("shell quoting")?
+            )?;
         }
 
         if let Some(program) = program {
-            write!(exec, "{}", shlex::try_quote(&program)?).unwrap();
+            write!(
+                exec,
+                "{}",
+                shell_kind.try_quote(&program).context("shell quoting")?
+            )?;
             for arg in args {
-                let arg = shlex::try_quote(&arg)?;
-                write!(exec, " {}", &arg).unwrap();
+                let arg = shell_kind.try_quote(&arg).context("shell quoting")?;
+                write!(exec, " {}", &arg)?;
             }
         } else {
-            write!(&mut exec, "{} -l", self.shell).unwrap();
+            write!(&mut exec, "{} -l", self.shell)?;
         }
 
         let wsl_args = if let Some(user) = &self.connection_options.user {

crates/remote_server/src/headless_project.rs 🔗

@@ -781,7 +781,7 @@ impl HeadlessProject {
         envelope: TypedEnvelope<proto::GetDirectoryEnvironment>,
         mut cx: AsyncApp,
     ) -> Result<proto::DirectoryEnvironment> {
-        let shell = task::Shell::from_proto(envelope.payload.shell.context("missing shell")?)?;
+        let shell = task::shell_from_proto(envelope.payload.shell.context("missing shell")?)?;
         let directory = PathBuf::from(envelope.payload.directory);
         let environment = this
             .update(&mut cx, |this, cx| {

crates/task/src/task.rs 🔗

@@ -3,7 +3,6 @@
 mod adapter_schema;
 mod debug_format;
 mod serde_helpers;
-mod shell_builder;
 pub mod static_source;
 mod task_template;
 mod vscode_debug_format;
@@ -12,23 +11,22 @@ mod vscode_format;
 use anyhow::Context as _;
 use collections::{HashMap, HashSet, hash_map};
 use gpui::SharedString;
-use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use std::borrow::Cow;
 use std::path::PathBuf;
 use std::str::FromStr;
-use util::get_system_shell;
 
 pub use adapter_schema::{AdapterSchema, AdapterSchemas};
 pub use debug_format::{
     AttachRequest, BuildTaskDefinition, DebugRequest, DebugScenario, DebugTaskFile, LaunchRequest,
     Request, TcpArgumentsTemplate, ZedDebugConfig,
 };
-pub use shell_builder::{ShellBuilder, ShellKind};
 pub use task_template::{
     DebugArgsRequest, HideStrategy, RevealStrategy, TaskTemplate, TaskTemplates,
     substitute_variables_in_map, substitute_variables_in_str,
 };
+pub use util::shell::{Shell, ShellKind};
+pub use util::shell_builder::ShellBuilder;
 pub use vscode_debug_format::VsCodeDebugTaskFile;
 pub use vscode_format::VsCodeTaskFile;
 pub use zed_actions::RevealTarget;
@@ -318,81 +316,32 @@ pub struct TaskContext {
 #[derive(Clone, Debug)]
 pub struct RunnableTag(pub SharedString);
 
-/// Shell configuration to open the terminal with.
-#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema, Hash)]
-#[serde(rename_all = "snake_case")]
-pub enum Shell {
-    /// Use the system's default terminal configuration in /etc/passwd
-    #[default]
-    System,
-    /// Use a specific program with no arguments.
-    Program(String),
-    /// Use a specific program with arguments.
-    WithArguments {
-        /// The program to run.
-        program: String,
-        /// The arguments to pass to the program.
-        args: Vec<String>,
-        /// An optional string to override the title of the terminal tab
-        title_override: Option<SharedString>,
-    },
+pub fn shell_from_proto(proto: proto::Shell) -> anyhow::Result<Shell> {
+    let shell_type = proto.shell_type.context("invalid shell type")?;
+    let shell = match shell_type {
+        proto::shell::ShellType::System(_) => Shell::System,
+        proto::shell::ShellType::Program(program) => Shell::Program(program),
+        proto::shell::ShellType::WithArguments(program) => Shell::WithArguments {
+            program: program.program,
+            args: program.args,
+            title_override: None,
+        },
+    };
+    Ok(shell)
 }
 
-impl Shell {
-    pub fn program(&self) -> String {
-        match self {
-            Shell::Program(program) => program.clone(),
-            Shell::WithArguments { program, .. } => program.clone(),
-            Shell::System => get_system_shell(),
-        }
-    }
-
-    pub fn program_and_args(&self) -> (String, &[String]) {
-        match self {
-            Shell::Program(program) => (program.clone(), &[]),
-            Shell::WithArguments { program, args, .. } => (program.clone(), args),
-            Shell::System => (get_system_shell(), &[]),
-        }
-    }
-
-    pub fn shell_kind(&self, is_windows: bool) -> ShellKind {
-        match self {
-            Shell::Program(program) => ShellKind::new(program, is_windows),
-            Shell::WithArguments { program, .. } => ShellKind::new(program, is_windows),
-            Shell::System => ShellKind::system(),
-        }
-    }
-
-    pub fn from_proto(proto: proto::Shell) -> anyhow::Result<Self> {
-        let shell_type = proto.shell_type.context("invalid shell type")?;
-        let shell = match shell_type {
-            proto::shell::ShellType::System(_) => Self::System,
-            proto::shell::ShellType::Program(program) => Self::Program(program),
-            proto::shell::ShellType::WithArguments(program) => Self::WithArguments {
-                program: program.program,
-                args: program.args,
-                title_override: None,
-            },
-        };
-        Ok(shell)
-    }
-
-    pub fn to_proto(self) -> proto::Shell {
-        let shell_type = match self {
-            Shell::System => proto::shell::ShellType::System(proto::System {}),
-            Shell::Program(program) => proto::shell::ShellType::Program(program),
-            Shell::WithArguments {
-                program,
-                args,
-                title_override: _,
-            } => proto::shell::ShellType::WithArguments(proto::shell::WithArguments {
-                program,
-                args,
-            }),
-        };
-        proto::Shell {
-            shell_type: Some(shell_type),
-        }
+pub fn shell_to_proto(shell: Shell) -> proto::Shell {
+    let shell_type = match shell {
+        Shell::System => proto::shell::ShellType::System(proto::System {}),
+        Shell::Program(program) => proto::shell::ShellType::Program(program),
+        Shell::WithArguments {
+            program,
+            args,
+            title_override: _,
+        } => proto::shell::ShellType::WithArguments(proto::shell::WithArguments { program, args }),
+    };
+    proto::Shell {
+        shell_type: Some(shell_type),
     }
 }
 

crates/terminal/src/terminal.rs 🔗

@@ -67,7 +67,7 @@ use thiserror::Error;
 use gpui::{
     App, AppContext as _, Bounds, ClipboardItem, Context, EventEmitter, Hsla, Keystroke, Modifiers,
     MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, Pixels, Point, Rgba,
-    ScrollWheelEvent, SharedString, Size, Task, TouchPhase, Window, actions, black, px,
+    ScrollWheelEvent, Size, Task, TouchPhase, Window, actions, black, px,
 };
 
 use crate::mappings::{colors::to_alac_rgb, keys::to_esc_str};
@@ -277,7 +277,7 @@ pub struct TerminalError {
     pub directory: Option<PathBuf>,
     pub program: Option<String>,
     pub args: Option<Vec<String>>,
-    pub title_override: Option<SharedString>,
+    pub title_override: Option<String>,
     pub source: std::io::Error,
 }
 
@@ -446,14 +446,14 @@ impl TerminalBuilder {
         struct ShellParams {
             program: String,
             args: Option<Vec<String>>,
-            title_override: Option<SharedString>,
+            title_override: Option<String>,
         }
 
         impl ShellParams {
             fn new(
                 program: String,
                 args: Option<Vec<String>>,
-                title_override: Option<SharedString>,
+                title_override: Option<String>,
             ) -> Self {
                 log::info!("Using {program} as shell");
                 Self {
@@ -514,10 +514,8 @@ impl TerminalBuilder {
                 working_directory: working_directory.clone(),
                 drain_on_exit: true,
                 env: env.clone().into_iter().collect(),
-                // We do not want to escape arguments if we are using CMD as our shell.
-                // If we do we end up with too many quotes/escaped quotes for CMD to handle.
                 #[cfg(windows)]
-                escape_args: shell_kind != util::shell::ShellKind::Cmd,
+                escape_args: shell_kind.tty_escape_args(),
             }
         };
 
@@ -823,7 +821,7 @@ pub struct Terminal {
     pub last_content: TerminalContent,
     pub selection_head: Option<AlacPoint>,
     pub breadcrumb_text: String,
-    title_override: Option<SharedString>,
+    title_override: Option<String>,
     scroll_px: Pixels,
     next_link_id: usize,
     selection_phase: SelectionPhase,

crates/terminal/src/terminal_settings.rs 🔗

@@ -66,7 +66,7 @@ fn settings_shell_to_task_shell(shell: settings::Shell) -> Shell {
         } => Shell::WithArguments {
             program,
             args,
-            title_override,
+            title_override: title_override.map(Into::into),
         },
     }
 }

crates/util/src/paths.rs 🔗

@@ -15,7 +15,7 @@ use std::{
     sync::LazyLock,
 };
 
-use crate::rel_path::RelPath;
+use crate::{rel_path::RelPath, shell::ShellKind};
 
 static HOME_DIR: OnceLock<PathBuf> = OnceLock::new();
 
@@ -84,9 +84,7 @@ pub trait PathExt {
     fn multiple_extensions(&self) -> Option<String>;
 
     /// Try to make a shell-safe representation of the path.
-    ///
-    /// For Unix, the path is escaped to be safe for POSIX shells
-    fn try_shell_safe(&self) -> anyhow::Result<String>;
+    fn try_shell_safe(&self, shell_kind: ShellKind) -> anyhow::Result<String>;
 }
 
 impl<T: AsRef<Path>> PathExt for T {
@@ -164,24 +162,16 @@ impl<T: AsRef<Path>> PathExt for T {
         Some(parts.into_iter().join("."))
     }
 
-    fn try_shell_safe(&self) -> anyhow::Result<String> {
-        #[cfg(target_os = "windows")]
-        {
-            Ok(self.as_ref().to_string_lossy().to_string())
-        }
-
-        #[cfg(not(target_os = "windows"))]
-        {
-            let path_str = self
-                .as_ref()
-                .to_str()
-                .with_context(|| "Path contains invalid UTF-8")?;
-
-            // As of writing, this can only be fail if the path contains a null byte, which shouldn't be possible
-            // but shlex has annotated the error as #[non_exhaustive] so we can't make it a compile error if other
-            // errors are introduced in the future :(
-            Ok(shlex::try_quote(path_str)?.into_owned())
-        }
+    fn try_shell_safe(&self, shell_kind: ShellKind) -> anyhow::Result<String> {
+        let path_str = self
+            .as_ref()
+            .to_str()
+            .with_context(|| "Path contains invalid UTF-8")?;
+        shell_kind
+            .try_quote(path_str)
+            .as_deref()
+            .map(ToOwned::to_owned)
+            .context("Failed to quote path")
     }
 }
 

crates/util/src/shell.rs 🔗

@@ -1,6 +1,53 @@
+use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use std::{borrow::Cow, fmt, path::Path, sync::LazyLock};
 
+/// Shell configuration to open the terminal with.
+#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema, Hash)]
+#[serde(rename_all = "snake_case")]
+pub enum Shell {
+    /// Use the system's default terminal configuration in /etc/passwd
+    #[default]
+    System,
+    /// Use a specific program with no arguments.
+    Program(String),
+    /// Use a specific program with arguments.
+    WithArguments {
+        /// The program to run.
+        program: String,
+        /// The arguments to pass to the program.
+        args: Vec<String>,
+        /// An optional string to override the title of the terminal tab
+        title_override: Option<String>,
+    },
+}
+
+impl Shell {
+    pub fn program(&self) -> String {
+        match self {
+            Shell::Program(program) => program.clone(),
+            Shell::WithArguments { program, .. } => program.clone(),
+            Shell::System => get_system_shell(),
+        }
+    }
+
+    pub fn program_and_args(&self) -> (String, &[String]) {
+        match self {
+            Shell::Program(program) => (program.clone(), &[]),
+            Shell::WithArguments { program, args, .. } => (program.clone(), args),
+            Shell::System => (get_system_shell(), &[]),
+        }
+    }
+
+    pub fn shell_kind(&self, is_windows: bool) -> ShellKind {
+        match self {
+            Shell::Program(program) => ShellKind::new(program, is_windows),
+            Shell::WithArguments { program, .. } => ShellKind::new(program, is_windows),
+            Shell::System => ShellKind::system(),
+        }
+    }
+}
+
 #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub enum ShellKind {
     #[default]
@@ -185,32 +232,20 @@ impl ShellKind {
             .unwrap_or_else(|| program.as_os_str())
             .to_string_lossy();
 
-        if program == "powershell" || program == "pwsh" {
-            ShellKind::PowerShell
-        } else if program == "cmd" {
-            ShellKind::Cmd
-        } else if program == "nu" {
-            ShellKind::Nushell
-        } else if program == "fish" {
-            ShellKind::Fish
-        } else if program == "csh" {
-            ShellKind::Csh
-        } else if program == "tcsh" {
-            ShellKind::Tcsh
-        } else if program == "rc" {
-            ShellKind::Rc
-        } else if program == "xonsh" {
-            ShellKind::Xonsh
-        } else if program == "sh" || program == "bash" {
-            ShellKind::Posix
-        } else {
-            if is_windows {
-                ShellKind::PowerShell
-            } else {
-                // Some other shell detected, the user might install and use a
-                // unix-like shell.
-                ShellKind::Posix
-            }
+        match &*program {
+            "powershell" | "pwsh" => ShellKind::PowerShell,
+            "cmd" => ShellKind::Cmd,
+            "nu" => ShellKind::Nushell,
+            "fish" => ShellKind::Fish,
+            "csh" => ShellKind::Csh,
+            "tcsh" => ShellKind::Tcsh,
+            "rc" => ShellKind::Rc,
+            "xonsh" => ShellKind::Xonsh,
+            "sh" | "bash" | "zsh" => ShellKind::Posix,
+            _ if is_windows => ShellKind::PowerShell,
+            // Some other shell detected, the user might install and use a
+            // unix-like shell.
+            _ => ShellKind::Posix,
         }
     }
 
@@ -363,14 +398,27 @@ impl ShellKind {
         match self {
             ShellKind::PowerShell => Some('&'),
             ShellKind::Nushell => Some('^'),
-            _ => None,
+            ShellKind::Posix
+            | ShellKind::Csh
+            | ShellKind::Tcsh
+            | ShellKind::Rc
+            | ShellKind::Fish
+            | ShellKind::Cmd
+            | ShellKind::Xonsh => None,
         }
     }
 
     pub const fn sequential_commands_separator(&self) -> char {
         match self {
             ShellKind::Cmd => '&',
-            _ => ';',
+            ShellKind::Posix
+            | ShellKind::Csh
+            | ShellKind::Tcsh
+            | ShellKind::Rc
+            | ShellKind::Fish
+            | ShellKind::PowerShell
+            | ShellKind::Nushell
+            | ShellKind::Xonsh => ';',
         }
     }
 
@@ -378,29 +426,103 @@ impl ShellKind {
         shlex::try_quote(arg).ok().map(|arg| match self {
             // If we are running in PowerShell, we want to take extra care when escaping strings.
             // In particular, we want to escape strings with a backtick (`) rather than a backslash (\).
-            // TODO double escaping backslashes is not necessary in PowerShell and probably CMD
-            ShellKind::PowerShell => Cow::Owned(arg.replace("\\\"", "`\"")),
-            _ => arg,
+            ShellKind::PowerShell => Cow::Owned(arg.replace("\\\"", "`\"").replace("\\\\", "\\")),
+            ShellKind::Cmd => Cow::Owned(arg.replace("\\\\", "\\")),
+            ShellKind::Posix
+            | ShellKind::Csh
+            | ShellKind::Tcsh
+            | ShellKind::Rc
+            | ShellKind::Fish
+            | ShellKind::Nushell
+            | ShellKind::Xonsh => arg,
         })
     }
 
+    pub fn split(&self, input: &str) -> Option<Vec<String>> {
+        shlex::split(input)
+    }
+
     pub const fn activate_keyword(&self) -> &'static str {
         match self {
             ShellKind::Cmd => "",
             ShellKind::Nushell => "overlay use",
             ShellKind::PowerShell => ".",
-            ShellKind::Fish => "source",
-            ShellKind::Csh => "source",
-            ShellKind::Tcsh => "source",
-            ShellKind::Posix | ShellKind::Rc => "source",
-            ShellKind::Xonsh => "source",
+            ShellKind::Fish
+            | ShellKind::Csh
+            | ShellKind::Tcsh
+            | ShellKind::Posix
+            | ShellKind::Rc
+            | ShellKind::Xonsh => "source",
         }
     }
 
     pub const fn clear_screen_command(&self) -> &'static str {
         match self {
             ShellKind::Cmd => "cls",
-            _ => "clear",
+            ShellKind::Posix
+            | ShellKind::Csh
+            | ShellKind::Tcsh
+            | ShellKind::Rc
+            | ShellKind::Fish
+            | ShellKind::PowerShell
+            | ShellKind::Nushell
+            | ShellKind::Xonsh => "clear",
+        }
+    }
+
+    #[cfg(windows)]
+    /// We do not want to escape arguments if we are using CMD as our shell.
+    /// If we do we end up with too many quotes/escaped quotes for CMD to handle.
+    pub const fn tty_escape_args(&self) -> bool {
+        match self {
+            ShellKind::Cmd => false,
+            ShellKind::Posix
+            | ShellKind::Csh
+            | ShellKind::Tcsh
+            | ShellKind::Rc
+            | ShellKind::Fish
+            | ShellKind::PowerShell
+            | ShellKind::Nushell
+            | ShellKind::Xonsh => true,
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    // Examples
+    // WSL
+    // wsl.exe --distribution NixOS --cd /home/user -- /usr/bin/zsh -c "echo hello"
+    // wsl.exe --distribution NixOS --cd /home/user -- /usr/bin/zsh -c "\"echo hello\"" | grep hello"
+    // wsl.exe --distribution NixOS --cd ~ env RUST_LOG=info,remote=debug .zed_wsl_server/zed-remote-server-dev-build proxy --identifier dev-workspace-53
+    // PowerShell from Nushell
+    // nu -c overlay use "C:\Users\kubko\dev\python\39007\tests\.venv\Scripts\activate.nu"; ^"C:\Program Files\PowerShell\7\pwsh.exe" -C "C:\Users\kubko\dev\python\39007\tests\.venv\Scripts\python.exe -m pytest \"test_foo.py::test_foo\""
+    // PowerShell from CMD
+    // cmd /C \" \"C:\\\\Users\\\\kubko\\\\dev\\\\python\\\\39007\\\\tests\\\\.venv\\\\Scripts\\\\activate.bat\"& \"C:\\\\Program Files\\\\PowerShell\\\\7\\\\pwsh.exe\" -C \"C:\\\\Users\\\\kubko\\\\dev\\\\python\\\\39007\\\\tests\\\\.venv\\\\Scripts\\\\python.exe -m pytest \\\"test_foo.py::test_foo\\\"\"\"
+
+    #[test]
+    fn test_try_quote_powershell() {
+        let shell_kind = ShellKind::PowerShell;
+        assert_eq!(
+            shell_kind
+                .try_quote("C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest \"test_foo.py::test_foo\"")
+                .unwrap()
+                .into_owned(),
+            "\"C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest `\"test_foo.py::test_foo`\"\"".to_string()
+        );
+    }
+
+    #[test]
+    fn test_try_quote_cmd() {
+        let shell_kind = ShellKind::Cmd;
+        assert_eq!(
+            shell_kind
+                .try_quote("C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest \"test_foo.py::test_foo\"")
+                .unwrap()
+                .into_owned(),
+            "\"C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest \\\"test_foo.py::test_foo\\\"\"".to_string()
+        );
+    }
+}

crates/task/src/shell_builder.rs → crates/util/src/shell_builder.rs 🔗

@@ -1,8 +1,5 @@
-use util::shell::get_system_shell;
-
-use crate::Shell;
-
-pub use util::shell::ShellKind;
+use crate::shell::get_system_shell;
+use crate::shell::{Shell, ShellKind};
 
 /// ShellBuilder is used to turn a user-requested task into a
 /// program that can be executed by the shell.

crates/util/src/shell_env.rs 🔗

@@ -35,8 +35,8 @@ async fn capture_unix(
     use std::os::unix::process::CommandExt;
     use std::process::Stdio;
 
-    let zed_path = super::get_shell_safe_zed_path()?;
     let shell_kind = ShellKind::new(shell_path, false);
+    let zed_path = super::get_shell_safe_zed_path(shell_kind)?;
 
     let mut command_string = String::new();
     let mut command = std::process::Command::new(shell_path);

crates/util/src/util.rs 🔗

@@ -9,6 +9,7 @@ pub mod rel_path;
 pub mod schemars;
 pub mod serde;
 pub mod shell;
+pub mod shell_builder;
 pub mod shell_env;
 pub mod size;
 #[cfg(any(test, feature = "test-support"))]
@@ -295,12 +296,12 @@ fn load_shell_from_passwd() -> Result<()> {
 }
 
 /// Returns a shell escaped path for the current zed executable
-pub fn get_shell_safe_zed_path() -> anyhow::Result<String> {
+pub fn get_shell_safe_zed_path(shell_kind: shell::ShellKind) -> anyhow::Result<String> {
     let zed_path =
         std::env::current_exe().context("Failed to determine current zed executable path.")?;
 
     zed_path
-        .try_shell_safe()
+        .try_shell_safe(shell_kind)
         .context("Failed to shell-escape Zed executable path.")
 }