diff --git a/Cargo.lock b/Cargo.lock index eb5df527243bf130f7bd11735f767ed51807bc6b..0b24221bb6594478b70e50be0c03e2456b97e402 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4531,6 +4531,7 @@ dependencies = [ "paths", "serde", "serde_json", + "shlex", "smol", "task", "util", @@ -4756,6 +4757,7 @@ dependencies = [ "serde_json", "serde_json_lenient", "settings", + "shlex", "sysinfo 0.37.2", "task", "tasks_ui", @@ -12924,6 +12926,7 @@ dependencies = [ "settings", "sha2", "shellexpand 2.1.2", + "shlex", "smallvec", "smol", "snippet", @@ -13836,6 +13839,7 @@ dependencies = [ "serde", "serde_json", "settings", + "shlex", "smol", "tempfile", "thiserror 2.0.17", diff --git a/crates/askpass/src/askpass.rs b/crates/askpass/src/askpass.rs index 9b5d5848270b13c29e84f5601a60511f8956988c..dfe8a96ee6f19510df06948f94af48d621515747 100644 --- a/crates/askpass/src/askpass.rs +++ b/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, shell::ShellKind}; +use util::{ResultExt as _, debug_panic, maybe, paths::PathExt}; /// Path to the program used for askpass /// @@ -199,15 +199,9 @@ 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(shell_kind) + .try_shell_safe() .context("Failed to shell-escape Askpass program path.")? .to_string(); // Create an askpass script that communicates back to this process. diff --git a/crates/dap_adapters/Cargo.toml b/crates/dap_adapters/Cargo.toml index 253674c0f3da16574b4303faf679abeb310756d8..1593f51cf326b06f6c865d8bca8a8b4712511ff1 100644 --- a/crates/dap_adapters/Cargo.toml +++ b/crates/dap_adapters/Cargo.toml @@ -35,6 +35,7 @@ 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 diff --git a/crates/dap_adapters/src/javascript.rs b/crates/dap_adapters/src/javascript.rs index 68f5ca7e7976640c5b3e44ec5e2e2b880a6c2407..8c90bfc7c054f147336f9c6330d5f1d4a847d588 100644 --- a/crates/dap_adapters/src/javascript.rs +++ b/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, shell::ShellKind}; +use util::{ResultExt, maybe}; 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 = ShellKind::Posix.split(&command)?.into_iter(); + let mut args = shlex::split(&command)?.into_iter(); let program = args.next()?; configuration.insert("runtimeExecutable".to_owned(), program.into()); configuration.insert( diff --git a/crates/debugger_ui/Cargo.toml b/crates/debugger_ui/Cargo.toml index c1a0657c0ed93508acb330a98dc6d1c1ee91c570..28866f0d273ce990b51615157412c9b120220d7b 100644 --- a/crates/debugger_ui/Cargo.toml +++ b/crates/debugger_ui/Cargo.toml @@ -60,6 +60,7 @@ 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 diff --git a/crates/debugger_ui/src/new_process_modal.rs b/crates/debugger_ui/src/new_process_modal.rs index e12c768e12b1e098e150027c89d05695c59c51f6..c7cfedf5a2d03fa6d89d28a412eefba750a2e5be 100644 --- a/crates/debugger_ui/src/new_process_modal.rs +++ b/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, shell::ShellKind}; +use util::{ResultExt, rel_path::RelPath}; use workspace::{ModalView, Workspace, notifications::DetachAndPromptErr, pane}; use crate::{attach_modal::AttachModal, debugger_panel::DebugPanel}; @@ -839,11 +839,7 @@ impl ConfigureMode { }; } let command = self.program.read(cx).text(cx); - let mut args = ShellKind::Posix - .split(&command) - .into_iter() - .flatten() - .peekable(); + let mut args = shlex::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(); @@ -1269,11 +1265,7 @@ impl PickerDelegate for DebugDelegate { }) .unwrap_or_default(); - let mut args = ShellKind::Posix - .split(&text) - .into_iter() - .flatten() - .peekable(); + let mut args = shlex::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(); diff --git a/crates/project/Cargo.toml b/crates/project/Cargo.toml index d9285a8c24ec5130dd8ce8abf5bbd77c830e0f3f..0297611d101ad883c3d68d7dff9e0a92f00c5b71 100644 --- a/crates/project/Cargo.toml +++ b/crates/project/Cargo.toml @@ -72,6 +72,7 @@ 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 diff --git a/crates/project/src/environment.rs b/crates/project/src/environment.rs index 4f669545668834a6d93e62a18e5bb4944e01e2d9..4dd1e94b9fce6bcf99e49402b8e5d05ece041f40 100644 --- a/crates/project/src/environment.rs +++ b/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, shell_to_proto}; +use task::Shell; 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_to_proto(shell.clone())), + shell: Some(shell.clone().to_proto()), directory: abs_path.to_string_lossy().to_string(), }); cx.spawn(async move |_, _| { diff --git a/crates/project/src/lsp_store/lsp_ext_command.rs b/crates/project/src/lsp_store/lsp_ext_command.rs index 5066143244da890a63ead6650cb61fdb71d3964a..c79e3df178290fa614e08a8abd85a527a696b003 100644 --- a/crates/project/src/lsp_store/lsp_ext_command.rs +++ b/crates/project/src/lsp_store/lsp_ext_command.rs @@ -657,7 +657,6 @@ 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 @@ -683,7 +682,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| { - shell_kind.try_quote(&extra_arg).map(|s| s.to_string()) + shlex::try_quote(&extra_arg).ok().map(|s| s.to_string()) }), ); } diff --git a/crates/project/src/terminals.rs b/crates/project/src/terminals.rs index cf25735e4bf490847e20c92b19e791f8bef56b9b..4a0a1790b49449fd82b8aeff58f6c11c8e63261b 100644 --- a/crates/project/src/terminals.rs +++ b/crates/project/src/terminals.rs @@ -168,19 +168,20 @@ impl Project { match remote_client { Some(remote_client) => match activation_script.clone() { activation_script if !activation_script.is_empty() => { - let separator = shell_kind.sequential_commands_separator(); - let activation_script = - activation_script.join(&format!("{separator} ")); + let activation_script = activation_script.join("; "); let to_run = format_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); - + let args = vec![ + "-c".to_owned(), + format!("{activation_script}; {to_run}"), + ]; create_remote_shell( - Some((&shell, &args)), + Some(( + &remote_client + .read(cx) + .shell() + .unwrap_or_else(get_default_system_shell), + &args, + )), env, path, remote_client, @@ -561,7 +562,7 @@ fn create_remote_shell( Shell::WithArguments { program: command.program, args: command.args, - title_override: Some(format!("{} — Terminal", host)), + title_override: Some(format!("{} — Terminal", host).into()), }, command.env, )) diff --git a/crates/remote/Cargo.toml b/crates/remote/Cargo.toml index d1a91af9a5decc88b4c70c69001ba6dad18e4b8b..02560484922fd5b02b348a74493c0af5ca4f78d1 100644 --- a/crates/remote/Cargo.toml +++ b/crates/remote/Cargo.toml @@ -34,6 +34,7 @@ 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 diff --git a/crates/remote/src/transport/ssh.rs b/crates/remote/src/transport/ssh.rs index 745391e17ee90b183e517a8ce4c4ab7006493758..a1337c2d65c74b882e19dd832359e297a13b9236 100644 --- a/crates/remote/src/transport/ssh.rs +++ b/crates/remote/src/transport/ssh.rs @@ -203,6 +203,17 @@ impl AsMut 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<()> { @@ -727,24 +738,21 @@ 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") { - format!( - "gunzip -f {orig_tmp_path} && chmod {server_mode} {tmp_path} && mv {tmp_path} {dst_path}" + shell_script!( + "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 { - format!("chmod {server_mode} {orig_tmp_path} && mv {orig_tmp_path} {dst_path}") + 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()) + ) }; - let script = shell_kind.try_quote(&script).context("shell quoting")?; - let args = shell_kind.args_for_shell(false, script.to_string()); - self.socket.run_command("sh", &args).await?; + self.socket.run_command("sh", &["-c", &script]).await?; Ok(()) } @@ -878,12 +886,8 @@ 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]) -> process::Command { - let shell_kind = ShellKind::Posix; let mut command = util::command::new_smol_command("ssh"); - let mut to_run = shell_kind - .try_quote(program) - .expect("shell quoting") - .into_owned(); + let mut to_run = shlex::try_quote(program).unwrap().into_owned(); for arg in args { // We're trying to work with: sh, bash, zsh, fish, tcsh, ...? debug_assert!( @@ -891,10 +895,9 @@ impl SshSocket { "multiline arguments do not work in all shells" ); to_run.push(' '); - to_run.push_str(&shell_kind.try_quote(arg.as_ref()).expect("shell quoting")); + to_run.push_str(&shlex::try_quote(arg.as_ref()).unwrap()); } - let separator = shell_kind.sequential_commands_separator(); - let to_run = format!("cd{separator} {to_run}"); + let to_run = format!("cd; {to_run}"); self.ssh_options(&mut command, true) .arg(self.connection_options.ssh_url()) .arg("-T") @@ -903,7 +906,7 @@ impl SshSocket { command } - async fn run_command(&self, program: &str, args: &[impl AsRef]) -> Result { + async fn run_command(&self, program: &str, args: &[&str]) -> Result { let output = self.ssh_command(program, args).output().await?; anyhow::ensure!( output.status.success(), @@ -1077,10 +1080,7 @@ impl SshConnectionOptions { "-w", ]; - let mut tokens = ShellKind::Posix - .split(input) - .context("invalid input")? - .into_iter(); + let mut tokens = shlex::split(input).context("invalid input")?.into_iter(); 'outer: while let Some(arg) = tokens.next() { if ALLOWED_OPTS.contains(&(&arg as &str)) { @@ -1243,7 +1243,6 @@ fn build_command( ) -> Result { 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(); @@ -1253,41 +1252,29 @@ 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}\" && ",)?; + write!(exec, "cd \"$HOME/{working_dir}\" && ",).unwrap(); } else { - write!(exec, "cd \"{working_dir}\" && ",)?; + write!(exec, "cd \"{working_dir}\" && ",).unwrap(); } } else { - write!(exec, "cd && ")?; + write!(exec, "cd && ").unwrap(); }; - write!(exec, "exec env ")?; + write!(exec, "exec env ").unwrap(); for (k, v) in input_env.iter() { - write!( - exec, - "{}={} ", - k, - shell_kind.try_quote(v).context("shell quoting")? - )?; + if let Some((k, v)) = shlex::try_quote(k).ok().zip(shlex::try_quote(v).ok()) { + write!(exec, "{}={} ", k, v).unwrap(); + } } if let Some(input_program) = input_program { - write!( - exec, - "{}", - shell_kind - .try_quote(&input_program) - .context("shell quoting")? - )?; + write!(exec, "{}", shlex::try_quote(&input_program).unwrap()).unwrap(); for arg in input_args { - write!( - exec, - " {}", - shell_kind.try_quote(&arg).context("shell quoting")? - )?; + let arg = shlex::try_quote(&arg)?; + write!(exec, " {}", &arg).unwrap(); } } else { - write!(exec, "{ssh_shell} -l")?; + write!(exec, "{ssh_shell} -l").unwrap(); }; let mut args = Vec::new(); diff --git a/crates/remote/src/transport/wsl.rs b/crates/remote/src/transport/wsl.rs index 4ccadee73f99804675c51c9eb6007419b2cacfa7..4eca7c4d5295e4baf8b2812763a02c32701959f7 100644 --- a/crates/remote/src/transport/wsl.rs +++ b/crates/remote/src/transport/wsl.rs @@ -2,7 +2,7 @@ use crate::{ RemoteClientDelegate, RemotePlatform, remote_client::{CommandTemplate, RemoteConnection, RemoteConnectionOptions}, }; -use anyhow::{Context, Result, anyhow, bail}; +use anyhow::{Result, anyhow, bail}; use async_trait::async_trait; use collections::HashMap; use futures::channel::mpsc::{Sender, UnboundedReceiver, UnboundedSender}; @@ -441,7 +441,6 @@ impl RemoteConnection for WslRemoteConnection { bail!("WSL shares the network interface with the host system"); } - let shell = 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()); @@ -449,26 +448,19 @@ impl RemoteConnection for WslRemoteConnection { let mut exec = String::from("exec env "); for (k, v) in env.iter() { - write!( - exec, - "{}={} ", - k, - shell.try_quote(&v).context("shell quoting")? - )?; + if let Some((k, v)) = shlex::try_quote(k).ok().zip(shlex::try_quote(v).ok()) { + write!(exec, "{}={} ", k, v).unwrap(); + } } if let Some(program) = program { - write!( - exec, - "{}", - shell.try_quote(&program).context("shell quoting")? - )?; + write!(exec, "{}", shlex::try_quote(&program)?).unwrap(); for arg in args { - let arg = shell.try_quote(&arg).context("shell quoting")?; - write!(exec, " {}", &arg)?; + let arg = shlex::try_quote(&arg)?; + write!(exec, " {}", &arg).unwrap(); } } else { - write!(&mut exec, "{} -l", self.shell)?; + write!(&mut exec, "{} -l", self.shell).unwrap(); } let wsl_args = if let Some(user) = &self.connection_options.user { diff --git a/crates/remote_server/src/headless_project.rs b/crates/remote_server/src/headless_project.rs index 588ee836084e350010cfe552d3f294d5f3ae0bcf..83000c8bac3b409a0dad07490a5f028e482f0662 100644 --- a/crates/remote_server/src/headless_project.rs +++ b/crates/remote_server/src/headless_project.rs @@ -774,7 +774,7 @@ impl HeadlessProject { envelope: TypedEnvelope, mut cx: AsyncApp, ) -> Result { - 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| { diff --git a/crates/util/src/shell_builder.rs b/crates/task/src/shell_builder.rs similarity index 98% rename from crates/util/src/shell_builder.rs rename to crates/task/src/shell_builder.rs index 7e52b67b35f6f3d21ea5e3ad5a0632cd46344125..a6504f4eb765a8a144343691b82cdca9a6802cbd 100644 --- a/crates/util/src/shell_builder.rs +++ b/crates/task/src/shell_builder.rs @@ -1,5 +1,8 @@ -use crate::shell::get_system_shell; -use crate::shell::{Shell, ShellKind}; +use util::shell::get_system_shell; + +use crate::Shell; + +pub use util::shell::ShellKind; /// ShellBuilder is used to turn a user-requested task into a /// program that can be executed by the shell. diff --git a/crates/task/src/task.rs b/crates/task/src/task.rs index 280bf5ccdb91271d7ff738654d507573c9d667d4..bfb84ced944cda758c7c453f561ca4ec13220c07 100644 --- a/crates/task/src/task.rs +++ b/crates/task/src/task.rs @@ -3,6 +3,7 @@ mod adapter_schema; mod debug_format; mod serde_helpers; +mod shell_builder; pub mod static_source; mod task_template; mod vscode_debug_format; @@ -11,22 +12,23 @@ 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; @@ -316,32 +318,81 @@ pub struct TaskContext { #[derive(Clone, Debug)] pub struct RunnableTag(pub SharedString); -pub fn shell_from_proto(proto: proto::Shell) -> anyhow::Result { - 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) +/// 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, + /// An optional string to override the title of the terminal tab + title_override: Option, + }, } -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), +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 { + 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), + } } } diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index 68ac3d3e290953363a3246c41652149e1ed5da1f..fa42a94e932a81d171ffc871393a30abf965678f 100644 --- a/crates/terminal/src/terminal.rs +++ b/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, Size, Task, TouchPhase, Window, actions, black, px, + ScrollWheelEvent, SharedString, 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, pub program: Option, pub args: Option>, - pub title_override: Option, + pub title_override: Option, pub source: std::io::Error, } @@ -445,14 +445,14 @@ impl TerminalBuilder { struct ShellParams { program: String, args: Option>, - title_override: Option, + title_override: Option, } impl ShellParams { fn new( program: String, args: Option>, - title_override: Option, + title_override: Option, ) -> Self { log::debug!("Using {program} as shell"); Self { @@ -514,8 +514,10 @@ 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.tty_escape_args(), + escape_args: shell_kind != util::shell::ShellKind::Cmd, } }; @@ -822,7 +824,7 @@ pub struct Terminal { pub last_content: TerminalContent, pub selection_head: Option, pub breadcrumb_text: String, - title_override: Option, + title_override: Option, scroll_px: Pixels, next_link_id: usize, selection_phase: SelectionPhase, diff --git a/crates/terminal/src/terminal_settings.rs b/crates/terminal/src/terminal_settings.rs index b8576a1de308d8bf3bd098907018b94cb73eefa0..9bb5ffb517b15225eed711a6d4e31e2977626d0a 100644 --- a/crates/terminal/src/terminal_settings.rs +++ b/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.map(Into::into), + title_override, }, } } diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index 20187bf7376861ebd03e02f7fb006428c1c51ec4..0743601839cc31e0e3a4c9d6c936aab7edce5837 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -15,7 +15,7 @@ use std::{ sync::LazyLock, }; -use crate::{rel_path::RelPath, shell::ShellKind}; +use crate::rel_path::RelPath; static HOME_DIR: OnceLock = OnceLock::new(); @@ -84,7 +84,9 @@ pub trait PathExt { fn multiple_extensions(&self) -> Option; /// Try to make a shell-safe representation of the path. - fn try_shell_safe(&self, shell_kind: ShellKind) -> anyhow::Result; + /// + /// For Unix, the path is escaped to be safe for POSIX shells + fn try_shell_safe(&self) -> anyhow::Result; } impl> PathExt for T { @@ -162,16 +164,24 @@ impl> PathExt for T { Some(parts.into_iter().join(".")) } - fn try_shell_safe(&self, shell_kind: ShellKind) -> anyhow::Result { - 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") + fn try_shell_safe(&self) -> anyhow::Result { + #[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()) + } } } diff --git a/crates/util/src/shell.rs b/crates/util/src/shell.rs index d81946b8ad207596cfdaf1ec714de94a9b3f71d6..22e07acf25b46161138a297e6de701f74b483861 100644 --- a/crates/util/src/shell.rs +++ b/crates/util/src/shell.rs @@ -1,53 +1,6 @@ -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, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema)] -#[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, - /// An optional string to override the title of the terminal tab - title_override: Option, - }, -} - -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] @@ -232,20 +185,32 @@ impl ShellKind { .unwrap_or_else(|| program.as_os_str()) .to_string_lossy(); - 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" => ShellKind::Posix, - _ if is_windows => ShellKind::PowerShell, - // Some other shell detected, the user might install and use a - // unix-like shell. - _ => ShellKind::Posix, + 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 + } } } @@ -398,132 +363,44 @@ impl ShellKind { match self { ShellKind::PowerShell => Some('&'), ShellKind::Nushell => Some('^'), - ShellKind::Posix - | ShellKind::Csh - | ShellKind::Tcsh - | ShellKind::Rc - | ShellKind::Fish - | ShellKind::Cmd - | ShellKind::Xonsh => None, + _ => 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 => ';', + _ => ';', } } pub fn try_quote<'a>(&self, arg: &'a str) -> Option> { - // 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 :( shlex::try_quote(arg).ok().map(|arg| match self { - 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, + // 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, }) } - pub fn split(&self, input: &str) -> Option> { - shlex::split(input) - } - pub const fn activate_keyword(&self) -> &'static str { match self { ShellKind::Cmd => "", ShellKind::Nushell => "overlay use", ShellKind::PowerShell => ".", - ShellKind::Fish - | ShellKind::Csh - | ShellKind::Tcsh - | ShellKind::Posix - | ShellKind::Rc - | ShellKind::Xonsh => "source", + ShellKind::Fish => "source", + ShellKind::Csh => "source", + ShellKind::Tcsh => "source", + ShellKind::Posix | ShellKind::Rc => "source", + ShellKind::Xonsh => "source", } } pub const fn clear_screen_command(&self) -> &'static str { match self { ShellKind::Cmd => "cls", - 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, + _ => "clear", } } } - -#[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() - ); - } -} diff --git a/crates/util/src/shell_env.rs b/crates/util/src/shell_env.rs index b3c9e3bef390b945314ba79fcc34ff2669a349a6..a82bea154ec5cb16153b70499eaf7e34c0464995 100644 --- a/crates/util/src/shell_env.rs +++ b/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); diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 3a78ef3d41e557d33d5af77021464ee1dcadf5e4..f725167724f82b8c4479eca53a5e8f48927b4f8b 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -9,7 +9,6 @@ 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"))] @@ -296,12 +295,12 @@ fn load_shell_from_passwd() -> Result<()> { } /// Returns a shell escaped path for the current zed executable -pub fn get_shell_safe_zed_path(shell_kind: shell::ShellKind) -> anyhow::Result { +pub fn get_shell_safe_zed_path() -> anyhow::Result { let zed_path = std::env::current_exe().context("Failed to determine current zed executable path.")?; zed_path - .try_shell_safe(shell_kind) + .try_shell_safe() .context("Failed to shell-escape Zed executable path.") }