From 72b151e3aa5bec144be0fa9e4f59d069f0ccdfd2 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Fri, 30 Jan 2026 16:26:13 -0500 Subject: [PATCH] Revert "Allow always_allow patterns for Nushell, Elvish, and Rc shells" (#48050) Reverts zed-industries/zed#47908 This PR inadvertently caused a regression: - https://github.com/zed-industries/zed/issues/48047 --- crates/acp_thread/src/acp_thread.rs | 10 +- crates/acp_thread/src/terminal.rs | 3 +- crates/agent/src/thread.rs | 4 +- crates/agent/src/tool_permissions.rs | 193 +++------- crates/agent_servers/src/acp.rs | 2 +- crates/askpass/src/askpass.rs | 13 +- .../src/transport/stdio_transport.rs | 2 +- crates/dap_adapters/src/javascript.rs | 10 +- crates/debugger_ui/src/new_process_modal.rs | 12 +- crates/debugger_ui/src/session/running.rs | 3 +- crates/language/src/toolchain.rs | 2 +- crates/languages/src/python.rs | 68 ++-- crates/project/src/debugger/locators/cargo.rs | 2 +- .../project/src/lsp_store/lsp_ext_command.rs | 5 +- crates/project/src/terminals.rs | 34 +- .../tests/integration/project_tests.rs | 2 +- crates/prompt_store/src/prompts.rs | 8 +- crates/remote/src/transport/docker.rs | 5 +- crates/remote/src/transport/ssh.rs | 26 +- crates/remote/src/transport/wsl.rs | 10 +- crates/task/src/task.rs | 2 +- crates/terminal/src/terminal.rs | 13 +- crates/terminal_view/src/terminal_panel.rs | 15 +- crates/util/src/paths.rs | 14 +- crates/util/src/shell.rs | 334 +++++------------- crates/util/src/shell_builder.rs | 174 ++++----- crates/util/src/shell_env.rs | 44 ++- crates/util/src/util.rs | 2 +- 28 files changed, 318 insertions(+), 694 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index b61d869243ac87e06becc0cf29fa88a6799c625b..840c443b11a679cae349a33ffe0a2323445dceea 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -2421,6 +2421,7 @@ impl AcpThread { let project = self.project.clone(); let language_registry = project.read(cx).languages().clone(); + let is_windows = project.read(cx).path_style(cx).is_windows(); let terminal_id = acp::TerminalId::new(Uuid::new_v4().to_string()); let terminal_task = cx.spawn({ @@ -2434,9 +2435,10 @@ impl AcpThread { .and_then(|r| r.read(cx).default_system_shell()) }) .unwrap_or_else(|| get_default_system_shell_preferring_bash()); - let (task_command, task_args) = ShellBuilder::new(&Shell::Program(shell)) - .redirect_stdin_to_dev_null() - .build(Some(command.clone()), &args); + let (task_command, task_args) = + ShellBuilder::new(&Shell::Program(shell), is_windows) + .redirect_stdin_to_dev_null() + .build(Some(command.clone()), &args); let terminal = project .update(cx, |project, cx| { project.create_terminal_task( @@ -2790,7 +2792,7 @@ mod tests { // Create a real PTY terminal that runs a command which prints output then sleeps // We use printf instead of echo and chain with && sleep to ensure proper execution let (completion_tx, _completion_rx) = smol::channel::unbounded(); - let (program, args) = ShellBuilder::new(&Shell::System).build( + let (program, args) = ShellBuilder::new(&Shell::System, false).build( Some("printf 'output_before_kill\\n' && sleep 60".to_owned()), &[], ); diff --git a/crates/acp_thread/src/terminal.rs b/crates/acp_thread/src/terminal.rs index 21b01bb665119333c4b1b65464d49210290ad906..61da3c27b551cbdb156a86f120a8a7af4799af96 100644 --- a/crates/acp_thread/src/terminal.rs +++ b/crates/acp_thread/src/terminal.rs @@ -227,7 +227,8 @@ pub async fn create_terminal_entity( .map(Shell::Program) }) .unwrap_or_else(|| Shell::Program(get_default_system_shell_preferring_bash())); - let (task_command, task_args) = task::ShellBuilder::new(&shell) + let is_windows = project.read_with(cx, |project, cx| project.path_style(cx).is_windows()); + let (task_command, task_args) = task::ShellBuilder::new(&shell, is_windows) .redirect_stdin_to_dev_null() .build(Some(command.clone()), &args); diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 6c7c532a7984350fba2f4e6079f851482365c2e8..29f59720e7dd1da0a48cb3d8c69686ca19a55c0e 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -665,9 +665,7 @@ impl ToolPermissionContext { // Check if the user's shell supports POSIX-like command chaining. // See the doc comment above for the full explanation of why this is needed. let shell_supports_always_allow = if tool_name == TerminalTool::name() { - ShellKind::system() - .map(|k| k.supports_posix_chaining()) - .unwrap_or(false) + ShellKind::system().supports_posix_chaining() } else { true }; diff --git a/crates/agent/src/tool_permissions.rs b/crates/agent/src/tool_permissions.rs index bf30c750ed56ad740fc3010632ca4c8913545859..f747aa56eb36f3b73ff8c8ba2450e7f6e843290a 100644 --- a/crates/agent/src/tool_permissions.rs +++ b/crates/agent/src/tool_permissions.rs @@ -30,22 +30,15 @@ impl ToolPermissionDecision { /// /// # Shell Compatibility (Terminal Tool Only) /// - /// For the terminal tool, commands are parsed using brush-parser to extract sub-commands - /// for security. This ensures that patterns like `^cargo\b` cannot be bypassed with - /// shell injection like `cargo && rm -rf /`. + /// For the terminal tool, commands are parsed to extract sub-commands for security. + /// This parsing only works for shells with POSIX-like `&&` / `||` / `;` / `|` syntax: /// - /// The parser handles `;` (sequential execution), `|` (piping), `&&` and `||` - /// (conditional execution), `$()` and backticks (command substitution), and process - /// substitution. + /// **Compatible shells:** Posix (sh, bash, dash, zsh), Fish 3.0+, PowerShell 7+/Pwsh, + /// Cmd, Xonsh, Csh, Tcsh /// - /// # Shell Notes + /// **Incompatible shells:** Nushell, Elvish, Rc (Plan 9) /// - /// - **Nushell**: Uses `;` for sequential execution. The `and`/`or` keywords are - /// boolean operators on values (e.g., `$true and $false`), not command chaining. - /// - **Elvish**: Uses `;` to separate pipelines, which brush-parser handles. Elvish - /// does not have `&&` or `||` operators. Its `and`/`or` are special commands that - /// operate on values, not command chaining (e.g., `and $true $false`). - /// - **Rc (Plan 9)**: Uses `;` for sequential execution. Does not have `&&`/`||`. + /// For incompatible shells, `always_allow` patterns are disabled for safety. /// /// # Pattern Matching Tips /// @@ -64,7 +57,7 @@ impl ToolPermissionDecision { input: &str, permissions: &ToolPermissions, always_allow_tool_actions: bool, - shell_kind: Option, + shell_kind: ShellKind, ) -> ToolPermissionDecision { // If always_allow_tool_actions is enabled, bypass all permission checks. // This is intentionally placed first - it's a global override that the user @@ -93,28 +86,22 @@ impl ToolPermissionDecision { // If parsing fails or the shell syntax is unsupported, always_allow is // disabled for this command (we set allow_enabled to false to signal this). if tool_name == TerminalTool::name() { - // Check if this shell's syntax can be parsed by brush-parser. - // See the doc comment above for shell compatibility notes. - let supports_chaining = shell_kind - .map(|k| k.supports_posix_chaining()) - .unwrap_or(false); - if !supports_chaining { + // Our shell parser (brush-parser) only supports POSIX-like shell syntax. + // See the doc comment above for the list of compatible/incompatible shells. + if !shell_kind.supports_posix_chaining() { // For shells with incompatible syntax, we can't reliably parse // the command to extract sub-commands. if !rules.always_allow.is_empty() { // If the user has configured always_allow patterns, we must deny // because we can't safely verify the command doesn't contain // hidden sub-commands that bypass the allow patterns. - const SUFFIX: &str = " does not support \"always allow\" patterns for the \ - terminal tool because Zed cannot parse its command chaining syntax. \ - Please remove the always_allow patterns from your tool_permissions \ - settings, or switch to a supported shell."; - let message = if let Some(name) = shell_kind.map(|k| k.name()) { - format!("The {} shell{}", name, SUFFIX) - } else { - format!("This shell is unrecognized, and{}", SUFFIX) - }; - return ToolPermissionDecision::Deny(message); + return ToolPermissionDecision::Deny(format!( + "The {} shell does not support \"always allow\" patterns for the terminal \ + tool because Zed cannot parse its command chaining syntax. Please remove \ + the always_allow patterns from your tool_permissions settings, or switch \ + to a POSIX-conforming shell.", + shell_kind + )); } // No always_allow rules, so we can still check deny/confirm patterns. return check_commands(std::iter::once(input.to_string()), rules, tool_name, false); @@ -242,7 +229,6 @@ mod tests { use crate::pattern_extraction::extract_terminal_pattern; use agent_settings::{CompiledRegex, InvalidRegexPattern, ToolRules}; use std::sync::Arc; - use util::shell::PosixShell; fn pattern(command: &str) -> &'static str { Box::leak( @@ -260,7 +246,7 @@ mod tests { deny: Vec<(&'static str, bool)>, confirm: Vec<(&'static str, bool)>, global: bool, - shell: Option, + shell: ShellKind, } impl PermTest { @@ -273,7 +259,7 @@ mod tests { deny: vec![], confirm: vec![], global: false, - shell: Some(ShellKind::Posix(PosixShell::Sh)), + shell: ShellKind::Posix, } } @@ -309,7 +295,7 @@ mod tests { self.global = g; self } - fn shell(mut self, s: Option) -> Self { + fn shell(mut self, s: ShellKind) -> Self { self.shell = s; self } @@ -384,7 +370,7 @@ mod tests { tools: collections::HashMap::default(), }, global, - Some(ShellKind::Posix(PosixShell::Sh)), + ShellKind::Posix, ) } @@ -619,34 +605,16 @@ mod tests { let p = ToolPermissions { tools }; // With always_allow_tool_actions=true, even default_mode: Deny is overridden assert_eq!( - ToolPermissionDecision::from_input( - "terminal", - "x", - &p, - true, - Some(ShellKind::Posix(PosixShell::Sh)) - ), + ToolPermissionDecision::from_input("terminal", "x", &p, true, ShellKind::Posix), ToolPermissionDecision::Allow ); // With always_allow_tool_actions=false, default_mode: Deny is respected assert!(matches!( - ToolPermissionDecision::from_input( - "terminal", - "x", - &p, - false, - Some(ShellKind::Posix(PosixShell::Sh)) - ), + ToolPermissionDecision::from_input("terminal", "x", &p, false, ShellKind::Posix), ToolPermissionDecision::Deny(_) )); assert_eq!( - ToolPermissionDecision::from_input( - "edit_file", - "x", - &p, - false, - Some(ShellKind::Posix(PosixShell::Sh)) - ), + ToolPermissionDecision::from_input("edit_file", "x", &p, false, ShellKind::Posix), ToolPermissionDecision::Allow ); } @@ -667,13 +635,7 @@ mod tests { let p = ToolPermissions { tools }; // "terminal" should not match "term" rules, so falls back to Confirm (no rules) assert_eq!( - ToolPermissionDecision::from_input( - "terminal", - "x", - &p, - false, - Some(ShellKind::Posix(PosixShell::Sh)) - ), + ToolPermissionDecision::from_input("terminal", "x", &p, false, ShellKind::Posix), ToolPermissionDecision::Confirm ); } @@ -701,24 +663,12 @@ mod tests { }; // With global=true, all checks are bypassed including invalid pattern check assert!(matches!( - ToolPermissionDecision::from_input( - "terminal", - "echo hi", - &p, - true, - Some(ShellKind::Posix(PosixShell::Sh)) - ), + ToolPermissionDecision::from_input("terminal", "echo hi", &p, true, ShellKind::Posix), ToolPermissionDecision::Allow )); // With global=false, invalid patterns block the tool assert!(matches!( - ToolPermissionDecision::from_input( - "terminal", - "echo hi", - &p, - false, - Some(ShellKind::Posix(PosixShell::Sh)) - ), + ToolPermissionDecision::from_input("terminal", "echo hi", &p, false, ShellKind::Posix), ToolPermissionDecision::Deny(_) )); } @@ -907,13 +857,7 @@ mod tests { ); let p = ToolPermissions { tools }; assert!(matches!( - ToolPermissionDecision::from_input( - "terminal", - "x", - &p, - false, - Some(ShellKind::Posix(PosixShell::Sh)) - ), + ToolPermissionDecision::from_input("terminal", "x", &p, false, ShellKind::Posix), ToolPermissionDecision::Deny(_) )); assert_eq!( @@ -922,7 +866,7 @@ mod tests { "x", &p, false, - Some(ShellKind::Posix(PosixShell::Sh)) + ShellKind::Posix ), ToolPermissionDecision::Allow ); @@ -956,88 +900,38 @@ mod tests { } #[test] - fn nushell_allows_with_allow_pattern() { - // Nushell uses `;` for sequential execution, which brush-parser handles. - // The `and`/`or` keywords are boolean operators, not command chaining. - t("ls") - .allow(&["^ls"]) - .shell(Some(ShellKind::Nushell)) - .is_allow(); + fn nushell_denies_when_always_allow_configured() { + t("ls").allow(&["^ls"]).shell(ShellKind::Nushell).is_deny(); } #[test] - fn nushell_denies_with_deny_pattern() { + fn nushell_allows_deny_patterns() { t("rm -rf /") .deny(&["rm\\s+-rf"]) - .shell(Some(ShellKind::Nushell)) + .shell(ShellKind::Nushell) .is_deny(); } #[test] - fn nushell_confirms_with_confirm_pattern() { + fn nushell_allows_confirm_patterns() { t("sudo reboot") .confirm(&["sudo"]) - .shell(Some(ShellKind::Nushell)) + .shell(ShellKind::Nushell) .is_confirm(); } #[test] - fn nushell_falls_through_to_default() { + fn nushell_no_allow_patterns_uses_default() { t("ls") .deny(&["rm"]) .mode(ToolPermissionMode::Allow) - .shell(Some(ShellKind::Nushell)) - .is_allow(); - } - - #[test] - fn elvish_allows_with_allow_pattern() { - // Elvish uses `;` to separate pipelines, which brush-parser handles. - // The `and`/`or` special commands require parentheses around expressions. - t("ls") - .allow(&["^ls"]) - .shell(Some(ShellKind::Elvish)) - .is_allow(); - } - - #[test] - fn rc_allows_with_allow_pattern() { - // Rc (Plan 9) uses `;` for sequential execution, which brush-parser handles. - // Rc does not have `&&`/`||` operators. - t("ls") - .allow(&["^ls"]) - .shell(Some(ShellKind::Rc)) + .shell(ShellKind::Nushell) .is_allow(); } #[test] - fn unknown_shell_denies_when_always_allow_configured() { - // Unknown shells have unrecognized syntax, so we cannot safely parse them. - // For security, always_allow patterns are disabled. - t("ls").allow(&["^ls"]).shell(None).is_deny(); - } - - #[test] - fn unknown_shell_allows_deny_patterns() { - // Deny patterns still work for unknown shells since they're checked - // against the raw input string. - t("rm -rf /").deny(&["rm\\s+-rf"]).shell(None).is_deny(); - } - - #[test] - fn unknown_shell_allows_confirm_patterns() { - // Confirm patterns still work for unknown shells. - t("sudo reboot").confirm(&["sudo"]).shell(None).is_confirm(); - } - - #[test] - fn unknown_shell_falls_through_to_default() { - // With no always_allow patterns, unknown shells use the default mode. - t("ls") - .deny(&["rm"]) - .mode(ToolPermissionMode::Allow) - .shell(None) - .is_allow(); + fn elvish_denies_when_always_allow_configured() { + t("ls").allow(&["^ls"]).shell(ShellKind::Elvish).is_deny(); } #[test] @@ -1066,13 +960,8 @@ mod tests { ); let p = ToolPermissions { tools }; - let result = ToolPermissionDecision::from_input( - "terminal", - "x", - &p, - false, - Some(ShellKind::Posix(PosixShell::Sh)), - ); + let result = + ToolPermissionDecision::from_input("terminal", "x", &p, false, ShellKind::Posix); match result { ToolPermissionDecision::Deny(msg) => { assert!(msg.contains("2 regex patterns"), "Expected plural: {}", msg); diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index 1d3adb0d846afa915cc003be8b3b1a76a53635a4..d4a30c539dc6e0879c2604cbdea1a670e58db36f 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -195,7 +195,7 @@ impl AcpConnection { cx: &mut AsyncApp, ) -> Result { let shell = cx.update(|cx| TerminalSettings::get(None, cx).shell.clone()); - let builder = ShellBuilder::new(&shell).non_interactive(); + let builder = ShellBuilder::new(&shell, cfg!(windows)).non_interactive(); let mut child = builder.build_std_command(Some(command.path.display().to_string()), &command.args); child.envs(command.env.iter().flatten()); diff --git a/crates/askpass/src/askpass.rs b/crates/askpass/src/askpass.rs index 97308dc3335e00b47b3258f35a75fda6c4918e66..917b0efc1599112e44d23199cd832d75c4ba30b1 100644 --- a/crates/askpass/src/askpass.rs +++ b/crates/askpass/src/askpass.rs @@ -20,11 +20,7 @@ use futures::{ }; use gpui::{AsyncApp, BackgroundExecutor, Task}; use smol::fs; -use util::{ - ResultExt as _, debug_panic, maybe, - paths::PathExt, - shell::{PosixShell, ShellKind}, -}; +use util::{ResultExt as _, debug_panic, maybe, paths::PathExt, shell::ShellKind}; /// Path to the program used for askpass /// @@ -212,8 +208,7 @@ impl PasswordProxy { let shell_kind = if cfg!(windows) { ShellKind::PowerShell } else { - // TODO: Consider using the user's actual shell instead of hardcoding "sh" - ShellKind::Posix(PosixShell::Sh) + ShellKind::Posix }; let askpass_program = ASKPASS_PROGRAM.get_or_init(|| current_exec); // Create an askpass script that communicates back to this process. @@ -359,7 +354,7 @@ fn generate_askpass_script( .try_quote_prefix_aware(&askpass_program) .context("Failed to shell-escape Askpass program path")?; let askpass_socket = askpass_socket - .try_shell_safe(Some(&shell_kind)) + .try_shell_safe(shell_kind) .context("Failed to shell-escape Askpass socket path")?; let print_args = "printf '%s\\0' \"$@\""; let shebang = "#!/bin/sh"; @@ -384,7 +379,7 @@ fn generate_askpass_script( .try_quote_prefix_aware(&askpass_program) .context("Failed to shell-escape Askpass program path")?; let askpass_socket = askpass_socket - .try_shell_safe(Some(&shell_kind)) + .try_shell_safe(shell_kind) .context("Failed to shell-escape Askpass socket path")?; Ok(format!( r#" diff --git a/crates/context_server/src/transport/stdio_transport.rs b/crates/context_server/src/transport/stdio_transport.rs index c9f48d69e089c98356507423989aea633f1b4125..c3af1aa8745a074ad545cad0518d2ffea2822b65 100644 --- a/crates/context_server/src/transport/stdio_transport.rs +++ b/crates/context_server/src/transport/stdio_transport.rs @@ -32,7 +32,7 @@ impl StdioTransport { cx: &AsyncApp, ) -> Result { let shell = cx.update(|cx| TerminalSettings::get(None, cx).shell.clone()); - let builder = ShellBuilder::new(&shell).non_interactive(); + let builder = ShellBuilder::new(&shell, cfg!(windows)).non_interactive(); let mut command = builder.build_smol_command(Some(binary.executable.display().to_string()), &binary.args); diff --git a/crates/dap_adapters/src/javascript.rs b/crates/dap_adapters/src/javascript.rs index b5a2e39ecd1fd9fcf24cd70af62914a3f11b9b15..68f5ca7e7976640c5b3e44ec5e2e2b880a6c2407 100644 --- a/crates/dap_adapters/src/javascript.rs +++ b/crates/dap_adapters/src/javascript.rs @@ -6,10 +6,7 @@ use gpui::AsyncApp; use serde_json::Value; use std::{path::PathBuf, sync::OnceLock}; use task::DebugRequest; -use util::{ - ResultExt, maybe, - shell::{PosixShell, ShellKind}, -}; +use util::{ResultExt, maybe, shell::ShellKind}; use crate::*; @@ -70,10 +67,7 @@ impl JsDebugAdapter { .get("type") .filter(|value| value == &"node-terminal")?; let command = configuration.get("command")?.as_str()?.to_owned(); - // TODO: Consider using the user's actual shell instead of hardcoding "sh" - let mut args = ShellKind::Posix(PosixShell::Sh) - .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( diff --git a/crates/debugger_ui/src/new_process_modal.rs b/crates/debugger_ui/src/new_process_modal.rs index 4ac49d5bb0cc382221f27d5e3b748a65f9603cbe..982edd7feca720666cf90b3b771e272f628076bd 100644 --- a/crates/debugger_ui/src/new_process_modal.rs +++ b/crates/debugger_ui/src/new_process_modal.rs @@ -28,11 +28,7 @@ use ui::{ prelude::*, }; use ui_input::InputField; -use util::{ - ResultExt, debug_panic, - rel_path::RelPath, - shell::{PosixShell, ShellKind}, -}; +use util::{ResultExt, debug_panic, rel_path::RelPath, shell::ShellKind}; use workspace::{ModalView, Workspace, notifications::DetachAndPromptErr, pane}; use crate::{ @@ -874,8 +870,7 @@ impl ConfigureMode { }; } let command = self.program.read(cx).text(cx); - // TODO: Consider using the user's actual shell instead of hardcoding "sh" - let mut args = ShellKind::Posix(PosixShell::Sh) + let mut args = ShellKind::Posix .split(&command) .into_iter() .flatten() @@ -1303,8 +1298,7 @@ impl PickerDelegate for DebugDelegate { }) .unwrap_or_default(); - // TODO: Consider using the user's actual shell instead of hardcoding "sh" - let mut args = ShellKind::Posix(PosixShell::Sh) + let mut args = ShellKind::Posix .split(&text) .into_iter() .flatten() diff --git a/crates/debugger_ui/src/session/running.rs b/crates/debugger_ui/src/session/running.rs index 2d7088870590736c3c24d1aa28321b947c70a40b..94878c3f0c0ffdc910b15297cb6573e5c8cd252e 100644 --- a/crates/debugger_ui/src/session/running.rs +++ b/crates/debugger_ui/src/session/running.rs @@ -971,6 +971,7 @@ impl RunningState { let task_store = project.read(cx).task_store().downgrade(); let weak_project = project.downgrade(); let weak_workspace = workspace.downgrade(); + let is_windows = project.read(cx).path_style(cx).is_windows(); let remote_shell = project .read(cx) .remote_client() @@ -1088,7 +1089,7 @@ impl RunningState { task.resolved.shell = Shell::Program(remote_shell); } - let builder = ShellBuilder::new(&task.resolved.shell); + let builder = ShellBuilder::new(&task.resolved.shell, is_windows); let command_label = builder.command_label(task.resolved.command.as_deref().unwrap_or("")); let (command, args) = builder.build(task.resolved.command.clone(), &task.resolved.args); diff --git a/crates/language/src/toolchain.rs b/crates/language/src/toolchain.rs index 33ac65caf342e59b484595ec5564427ccd67605b..0d80f84e7ec1dc330db823a0938421a1f5ad85c9 100644 --- a/crates/language/src/toolchain.rs +++ b/crates/language/src/toolchain.rs @@ -117,7 +117,7 @@ pub trait ToolchainLister: Send + Sync + 'static { fn activation_script( &self, toolchain: &Toolchain, - shell: Option, + shell: ShellKind, cx: &App, ) -> BoxFuture<'static, Vec>; diff --git a/crates/languages/src/python.rs b/crates/languages/src/python.rs index d0402f96d91b9a00791b0707d6e96b583dce7312..0efaa3f0964a28711ccb3e55a3c75825cd3a0b83 100644 --- a/crates/languages/src/python.rs +++ b/crates/languages/src/python.rs @@ -46,7 +46,7 @@ use std::{ path::{Path, PathBuf}, sync::Arc, }; -use task::{PosixShell, ShellKind, TaskTemplate, TaskTemplates, VariableName}; +use task::{ShellKind, TaskTemplate, TaskTemplates, VariableName}; use util::{ResultExt, maybe}; #[derive(Debug, Serialize, Deserialize)] @@ -1168,15 +1168,13 @@ fn wr_distance( } } -fn micromamba_shell_name(kind: Option) -> &'static str { +fn micromamba_shell_name(kind: ShellKind) -> &'static str { match kind { - Some(ShellKind::Csh) => "csh", - Some(ShellKind::Fish) => "fish", - Some(ShellKind::Nushell) => "nu", - Some(ShellKind::PowerShell) | Some(ShellKind::Pwsh) => "powershell", - Some(ShellKind::Cmd) => "cmd.exe", - #[cfg(windows)] - None => "powershell", + ShellKind::Csh => "csh", + ShellKind::Fish => "fish", + ShellKind::Nushell => "nu", + ShellKind::PowerShell => "powershell", + ShellKind::Cmd => "cmd.exe", // default / catch-all: _ => "posix", } @@ -1341,7 +1339,7 @@ impl ToolchainLister for PythonToolchainProvider { fn activation_script( &self, toolchain: &Toolchain, - shell: Option, + shell: ShellKind, cx: &App, ) -> BoxFuture<'static, Vec> { let settings = TerminalSettings::get_global(cx); @@ -1408,21 +1406,10 @@ impl ToolchainLister for PythonToolchainProvider { | PythonEnvironmentKind::Poetry, ) => { if let Some(activation_scripts) = &toolchain.activation_scripts { - // For unknown shells, fall back to platform-appropriate defaults: - // POSIX (sh) on Unix, PowerShell on Windows - #[cfg(unix)] - let fallback_shell = ShellKind::Posix(PosixShell::Sh); - #[cfg(windows)] - let fallback_shell = ShellKind::PowerShell; - - let shell_kind = shell.unwrap_or(fallback_shell); - // Use activation_script_key() to normalize POSIX shells (e.g., Bash, Zsh) - // to Posix(Sh) for lookup, since activation scripts are stored with Posix(Sh) key - let lookup_key = shell_kind.activation_script_key(); - if let Some(activate_script_path) = activation_scripts.get(&lookup_key) { - let activate_keyword = shell_kind.activate_keyword(); + if let Some(activate_script_path) = activation_scripts.get(&shell) { + let activate_keyword = shell.activate_keyword(); if let Some(quoted) = - shell_kind.try_quote(&activate_script_path.to_string_lossy()) + shell.try_quote(&activate_script_path.to_string_lossy()) { activation_script.push(format!("{activate_keyword} {quoted}")); } @@ -1436,27 +1423,17 @@ impl ToolchainLister for PythonToolchainProvider { let version = toolchain.environment.version.as_deref().unwrap_or("system"); let pyenv = &manager.executable; let pyenv = pyenv.display(); - let pyenv_sh_activation = format!("\"{pyenv}\" shell - sh {version}"); activation_script.extend(match shell { - Some(ShellKind::Fish) => { - Some(format!("\"{pyenv}\" shell - fish {version}")) - } - Some(ShellKind::Posix(_)) => Some(pyenv_sh_activation), - #[cfg(unix)] - None => Some(pyenv_sh_activation), - Some(ShellKind::Nushell) => { - Some(format!("^\"{pyenv}\" shell - nu {version}")) - } - Some(ShellKind::PowerShell) - | Some(ShellKind::Pwsh) - | Some(ShellKind::Csh) - | Some(ShellKind::Tcsh) - | Some(ShellKind::Cmd) - | Some(ShellKind::Rc) - | Some(ShellKind::Xonsh) - | Some(ShellKind::Elvish) => None, - #[cfg(windows)] - None => None, + ShellKind::Fish => Some(format!("\"{pyenv}\" shell - fish {version}")), + ShellKind::Posix => Some(format!("\"{pyenv}\" shell - sh {version}")), + ShellKind::Nushell => Some(format!("^\"{pyenv}\" shell - nu {version}")), + ShellKind::PowerShell | ShellKind::Pwsh => None, + ShellKind::Csh => None, + ShellKind::Tcsh => None, + ShellKind::Cmd => None, + ShellKind::Rc => None, + ShellKind::Xonsh => None, + ShellKind::Elvish => None, }) } _ => {} @@ -1520,9 +1497,8 @@ async fn resolve_venv_activation_scripts( ) { log::debug!("(Python) Resolving activation scripts for venv toolchain {venv:?}"); if let Some(prefix) = &venv.prefix { - // TODO: Consider using the user's actual shell instead of hardcoding "sh" for (shell_kind, script_name) in &[ - (ShellKind::Posix(PosixShell::Sh), "activate"), + (ShellKind::Posix, "activate"), (ShellKind::Rc, "activate"), (ShellKind::Csh, "activate.csh"), (ShellKind::Tcsh, "activate.csh"), diff --git a/crates/project/src/debugger/locators/cargo.rs b/crates/project/src/debugger/locators/cargo.rs index 063480120ccfd514344a59efbf67b6673384dad3..66af7a2fe62fa06478680d23dd35575a64fd3c0f 100644 --- a/crates/project/src/debugger/locators/cargo.rs +++ b/crates/project/src/debugger/locators/cargo.rs @@ -122,7 +122,7 @@ impl DapLocator for CargoLocator { .cwd .clone() .context("Couldn't get cwd from debug config which is needed for locators")?; - let builder = ShellBuilder::new(&build_config.shell).non_interactive(); + let builder = ShellBuilder::new(&build_config.shell, cfg!(windows)).non_interactive(); let mut child = builder .build_smol_command( Some("cargo".into()), diff --git a/crates/project/src/lsp_store/lsp_ext_command.rs b/crates/project/src/lsp_store/lsp_ext_command.rs index 9bd02c318a2f3742079ee759ee1a70f72460f571..270db67576f0a02155997757a01d489d44ef1766 100644 --- a/crates/project/src/lsp_store/lsp_ext_command.rs +++ b/crates/project/src/lsp_store/lsp_ext_command.rs @@ -657,7 +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(); + let shell_kind = task_template.shell.shell_kind(cfg!(windows)); task_template.args.push("--".to_string()); task_template.args.extend( cargo @@ -683,8 +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| { - util::shell::try_quote_option(shell_kind, &extra_arg) - .map(|s| s.to_string()) + shell_kind.try_quote(&extra_arg).map(|s| s.to_string()) }), ); } diff --git a/crates/project/src/terminals.rs b/crates/project/src/terminals.rs index 0a1f6ed266ab4cbbca47da26ea744e869d93cfaf..5ab3c4c36214d744aff7d24f3a3877ee91b1e7d4 100644 --- a/crates/project/src/terminals.rs +++ b/crates/project/src/terminals.rs @@ -12,12 +12,11 @@ use std::{ path::{Path, PathBuf}, sync::Arc, }; -use task::{Shell, ShellBuilder, SpawnInTerminal}; +use task::{Shell, ShellBuilder, ShellKind, SpawnInTerminal}; use terminal::{ TaskState, TaskStatus, Terminal, TerminalBuilder, insert_zed_terminal_env, terminal_settings::TerminalSettings, }; -use util::shell::ShellKind; use util::{command::new_std_command, get_default_system_shell, maybe, rel_path::RelPath}; use crate::{Project, ProjectPath}; @@ -93,7 +92,7 @@ impl Project { None => settings.shell.program(), }; let path_style = self.path_style(cx); - let shell_kind = ShellKind::new(&shell); + let shell_kind = ShellKind::new(&shell, path_style.is_windows()); // Prepare a task for resolving the environment let env_task = @@ -143,14 +142,12 @@ impl Project { .update(cx, move |_, cx| { let format_to_run = || { if let Some(command) = &spawn_task.command { - let command = - util::shell::prepend_command_prefix_option(shell_kind, command); - let command = - util::shell::try_quote_prefix_aware_option(shell_kind, &command); + let command = shell_kind.prepend_command_prefix(command); + let command = shell_kind.try_quote_prefix_aware(&command); let args = spawn_task .args .iter() - .filter_map(|arg| util::shell::try_quote_option(shell_kind, arg)); + .filter_map(|arg| shell_kind.try_quote(&arg)); command.into_iter().chain(args).join(" ") } else { @@ -164,17 +161,13 @@ impl Project { match remote_client { Some(remote_client) => match activation_script.clone() { activation_script if !activation_script.is_empty() => { - let separator = - util::shell::sequential_commands_separator_option( - shell_kind, - ); + let separator = shell_kind.sequential_commands_separator(); let activation_script = activation_script.join(&format!("{separator} ")); let to_run = format_to_run(); let arg = format!("{activation_script}{separator} {to_run}"); - let args = - util::shell::args_for_shell_option(shell_kind, false, arg); + let args = shell_kind.args_for_shell(false, arg); let shell = remote_client .read(cx) .shell() @@ -201,17 +194,13 @@ impl Project { }, None => match activation_script.clone() { activation_script if !activation_script.is_empty() => { - let separator = - util::shell::sequential_commands_separator_option( - shell_kind, - ); + let separator = shell_kind.sequential_commands_separator(); let activation_script = activation_script.join(&format!("{separator} ")); let to_run = format_to_run(); let arg = format!("{activation_script}{separator} {to_run}"); - let args = - util::shell::args_for_shell_option(shell_kind, false, arg); + let args = shell_kind.args_for_shell(false, arg); ( Shell::WithArguments { @@ -369,7 +358,7 @@ impl Project { let lang_registry = self.languages.clone(); cx.spawn(async move |project, cx| { - let shell_kind = ShellKind::new(&shell); + let shell_kind = ShellKind::new(&shell, path_style.is_windows()); let mut env = env_task.await.unwrap_or_default(); env.extend(settings.env); @@ -524,7 +513,8 @@ impl Project { .and_then(|remote_client| remote_client.read(cx).shell()) .map(Shell::Program) .unwrap_or_else(|| settings.shell.clone()); - let builder = ShellBuilder::new(&shell).non_interactive(); + let is_windows = self.path_style(cx).is_windows(); + let builder = ShellBuilder::new(&shell, is_windows).non_interactive(); let (command, args) = builder.build(Some(command), &Vec::new()); let env_task = self.resolve_directory_environment( diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index dd6ca5cc046d491d69ea4b4296e3111677d2eb03..dabd4282dee96a5952bb3e903bdfb54bf54d36f8 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -11266,7 +11266,7 @@ fn python_lang(fs: Arc) -> Arc { fn activation_script( &self, _: &Toolchain, - _: Option, + _: ShellKind, _: &gpui::App, ) -> futures::future::BoxFuture<'static, Vec> { Box::pin(async { vec![] }) diff --git a/crates/prompt_store/src/prompts.rs b/crates/prompt_store/src/prompts.rs index b85b97bccbe00fb272619cd8b358445017ac40c4..6a845bb8dd394f8a1ff26a8a0e130156a2a158bd 100644 --- a/crates/prompt_store/src/prompts.rs +++ b/crates/prompt_store/src/prompts.rs @@ -57,12 +57,8 @@ impl ProjectContext { user_rules: default_user_rules, os: std::env::consts::OS.to_string(), arch: std::env::consts::ARCH.to_string(), - shell: ShellKind::new_with_fallback( - &get_default_system_shell_preferring_bash(), - cfg!(windows), - ) - .name() - .to_string(), + shell: ShellKind::new(&get_default_system_shell_preferring_bash(), cfg!(windows)) + .to_string(), } } } diff --git a/crates/remote/src/transport/docker.rs b/crates/remote/src/transport/docker.rs index d709c619765b37894735d09812fa4c7e23d18a29..713e532704063a244b94a7f2fb0fbd5a47727f23 100644 --- a/crates/remote/src/transport/docker.rs +++ b/crates/remote/src/transport/docker.rs @@ -13,7 +13,7 @@ use std::{ sync::Arc, }; use util::ResultExt; -use util::shell::{PosixShell, ShellKind}; +use util::shell::ShellKind; use util::{ paths::{PathStyle, RemotePathBuf}, rel_path::RelPath, @@ -301,8 +301,7 @@ impl DockerExecConnection { delegate.set_status(Some("Extracting remote development server"), cx); let server_mode = 0o755; - // TODO: Consider using the remote's actual shell instead of hardcoding "sh" - let shell_kind = ShellKind::Posix(PosixShell::Sh); + 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 diff --git a/crates/remote/src/transport/ssh.rs b/crates/remote/src/transport/ssh.rs index 8924f499a0257184287a24afb2d58f38405d023c..e93ed18892be6bb5b4515b67ffb902fb6244301d 100644 --- a/crates/remote/src/transport/ssh.rs +++ b/crates/remote/src/transport/ssh.rs @@ -32,7 +32,7 @@ use tempfile::TempDir; use util::{ paths::{PathStyle, RemotePathBuf}, rel_path::RelPath, - shell::{PosixShell, ShellKind}, + shell::ShellKind, }; pub(crate) struct SshRemoteConnection { @@ -591,7 +591,7 @@ impl SshRemoteConnection { let ssh_shell = socket.shell(is_windows).await; log::info!("Remote shell discovered: {}", ssh_shell); - let ssh_shell_kind = ShellKind::new_with_fallback(&ssh_shell, is_windows); + let ssh_shell_kind = ShellKind::new(&ssh_shell, is_windows); let ssh_platform = socket.platform(ssh_shell_kind, is_windows).await?; log::info!("Remote platform discovered: {:?}", ssh_platform); @@ -918,7 +918,7 @@ impl SshRemoteConnection { dst_path: &RelPath, tmp_path: &RelPath, ) -> Result<()> { - let shell_kind = self.ssh_shell_kind; + let shell_kind = ShellKind::Posix; let server_mode = 0o755; let orig_tmp_path = tmp_path.display(self.path_style()); let server_mode = format!("{:o}", server_mode); @@ -1138,6 +1138,7 @@ impl SshSocket { .expect("shell quoting") .into_owned(); for arg in args { + // We're trying to work with: sh, bash, zsh, fish, tcsh, ...? debug_assert!( !arg.as_ref().contains('\n'), "multiline arguments do not work in all shells" @@ -1290,14 +1291,8 @@ impl SshSocket { async fn shell_posix(&self) -> String { const DEFAULT_SHELL: &str = "sh"; - // TODO: Consider using the user's actual shell instead of hardcoding "sh" match self - .run_command( - ShellKind::Posix(PosixShell::Sh), - "sh", - &["-c", "echo $SHELL"], - false, - ) + .run_command(ShellKind::Posix, "sh", &["-c", "echo $SHELL"], false) .await { Ok(output) => parse_shell(&output, DEFAULT_SHELL), @@ -1391,8 +1386,7 @@ impl SshConnectionOptions { "-w", ]; - // TODO: Consider using the user's actual shell instead of hardcoding "sh" - let mut tokens = ShellKind::Posix(PosixShell::Sh) + let mut tokens = ShellKind::Posix .split(input) .context("invalid input")? .into_iter(); @@ -1630,7 +1624,7 @@ fn build_command_posix( .context("shell quoting")? )?; for arg in input_args { - let arg = ssh_shell_kind.try_quote(arg).context("shell quoting")?; + let arg = ssh_shell_kind.try_quote(&arg).context("shell quoting")?; write!(exec, " {}", &arg)?; } } else { @@ -1672,7 +1666,7 @@ fn build_command_windows( ssh_env: HashMap, ssh_path_style: PathStyle, ssh_shell: &str, - ssh_shell_kind: ShellKind, + _ssh_shell_kind: ShellKind, ssh_args: Vec, interactive: Interactive, ) -> Result { @@ -1691,7 +1685,7 @@ fn build_command_windows( shell_kind .try_quote(&working_dir) .context("shell quoting")?, - ssh_shell_kind.sequential_and_commands_separator() + shell_kind.sequential_and_commands_separator() )?; } @@ -1778,7 +1772,7 @@ mod tests { env.clone(), PathStyle::Posix, "/bin/bash", - ShellKind::Posix(PosixShell::Bash), + ShellKind::Posix, vec!["-o".to_string(), "ControlMaster=auto".to_string()], Interactive::No, )?; diff --git a/crates/remote/src/transport/wsl.rs b/crates/remote/src/transport/wsl.rs index cad8e43e0427698b7e5f1f41f2767bb9d05f771d..2a1b5d5ce144421af8516154fe7869914041a743 100644 --- a/crates/remote/src/transport/wsl.rs +++ b/crates/remote/src/transport/wsl.rs @@ -23,7 +23,7 @@ use std::{ use util::{ paths::{PathStyle, RemotePathBuf}, rel_path::RelPath, - shell::{PosixShell, Shell, ShellKind}, + shell::{Shell, ShellKind}, shell_builder::ShellBuilder, }; @@ -75,8 +75,7 @@ impl WslRemoteConnection { arch: RemoteArch::X86_64, }, shell: String::new(), - // TODO: Consider using the user's actual shell instead of hardcoding "sh" - shell_kind: ShellKind::Posix(PosixShell::Sh), + shell_kind: ShellKind::Posix, default_system_shell: String::from("/bin/sh"), has_wsl_interop: false, }; @@ -86,8 +85,7 @@ impl WslRemoteConnection { .await .context("failed detecting shell")?; log::info!("Remote shell discovered: {}", this.shell); - // WSL is always Linux, so is_windows = false - this.shell_kind = ShellKind::new_with_fallback(&this.shell, false); + this.shell_kind = ShellKind::new(&this.shell, false); this.has_wsl_interop = this.detect_has_wsl_interop().await.unwrap_or_default(); log::info!( "Remote has wsl interop {}", @@ -470,7 +468,7 @@ impl RemoteConnection for WslRemoteConnection { write!(&mut exec, "{} -l", self.shell)?; } let (command, args) = - ShellBuilder::new(&Shell::Program(self.shell.clone())).build(Some(exec), &[]); + ShellBuilder::new(&Shell::Program(self.shell.clone()), false).build(Some(exec), &[]); let mut wsl_args = if let Some(user) = &self.connection_options.user { vec![ diff --git a/crates/task/src/task.rs b/crates/task/src/task.rs index a90cd2b8bdeec359ea2d6ced586e00c73833bdbd..92d59094190de9924327722bb659058569869269 100644 --- a/crates/task/src/task.rs +++ b/crates/task/src/task.rs @@ -25,7 +25,7 @@ pub use task_template::{ DebugArgsRequest, HideStrategy, RevealStrategy, TaskTemplate, TaskTemplates, substitute_variables_in_map, substitute_variables_in_str, }; -pub use util::shell::{PosixShell, Shell, ShellKind}; +pub use util::shell::{Shell, ShellKind}; pub use util::shell_builder::ShellBuilder; pub use vscode_debug_format::VsCodeDebugTaskFile; pub use vscode_format::VsCodeTaskFile; diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index 1f4ea3cbb8eb7869c21e8cde4444a9bbbcf0922f..0fa3b37e1501ed6407d18b07e0b2188ce5e77cf7 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -516,7 +516,7 @@ impl TerminalBuilder { // the compilation target. This is fine right now due to the restricted // way we use the return value, but would become incorrect if we // supported remoting into windows. - let shell_kind = shell.shell_kind(); + let shell_kind = shell.shell_kind(cfg!(windows)); let pty_options = { let alac_shell = shell_params.as_ref().map(|params| { @@ -532,7 +532,7 @@ impl TerminalBuilder { drain_on_exit: true, env: env.clone().into_iter().collect(), #[cfg(windows)] - escape_args: shell_kind.map(|k| k.tty_escape_args()).unwrap_or(true), + escape_args: shell_kind.tty_escape_args(), } }; @@ -664,10 +664,7 @@ impl TerminalBuilder { // and while we have sent the activation script to the pty, it will be executed asynchronously. // Therefore, we somehow need to wait for the activation script to finish executing before we // can proceed with clearing the screen. - let clear_cmd = shell_kind - .map(|k| k.clear_screen_command()) - .unwrap_or("clear"); - terminal.write_to_pty(clear_cmd.as_bytes()); + terminal.write_to_pty(shell_kind.clear_screen_command().as_bytes()); // Simulate enter key press terminal.write_to_pty(b"\x0d"); } @@ -2562,7 +2559,7 @@ mod tests { let (completion_tx, completion_rx) = smol::channel::unbounded(); let args: Vec = args.iter().map(|s| s.to_string()).collect(); let (program, args) = - ShellBuilder::new(&Shell::System).build(Some(command.to_owned()), &args); + ShellBuilder::new(&Shell::System, false).build(Some(command.to_owned()), &args); let builder = cx .update(|cx| { TerminalBuilder::new( @@ -2783,7 +2780,7 @@ mod tests { cx.executor().allow_parking(); let (completion_tx, completion_rx) = smol::channel::unbounded(); - let (program, args) = ShellBuilder::new(&Shell::System) + let (program, args) = ShellBuilder::new(&Shell::System, false) .build(Some("asdasdasdasd".to_owned()), &["@@@@@".to_owned()]); let builder = cx .update(|cx| { diff --git a/crates/terminal_view/src/terminal_panel.rs b/crates/terminal_view/src/terminal_panel.rs index d9cfabb1da9b7405e5e316322156f7b0a0062f55..9d6049b6944dbf7a574b5c721b01b59bc7cf84f7 100644 --- a/crates/terminal_view/src/terminal_panel.rs +++ b/crates/terminal_view/src/terminal_panel.rs @@ -562,6 +562,7 @@ impl TerminalPanel { } let remote_client = project.remote_client(); + let is_windows = project.path_style(cx).is_windows(); let remote_shell = remote_client .as_ref() .and_then(|remote_client| remote_client.read(cx).shell()); @@ -574,7 +575,7 @@ impl TerminalPanel { task.shell.clone() }; - let task = prepare_task_for_spawn(task, &shell); + let task = prepare_task_for_spawn(task, &shell, is_windows); if task.allow_concurrent_runs && task.use_new_terminal { return self.spawn_in_new_terminal(task, window, cx); @@ -1161,8 +1162,12 @@ impl TerminalPanel { /// Prepares a `SpawnInTerminal` by computing the command, args, and command_label /// based on the shell configuration. This is a pure function that can be tested /// without spawning actual terminals. -pub fn prepare_task_for_spawn(task: &SpawnInTerminal, shell: &Shell) -> SpawnInTerminal { - let builder = ShellBuilder::new(shell); +pub fn prepare_task_for_spawn( + task: &SpawnInTerminal, + shell: &Shell, + is_windows: bool, +) -> SpawnInTerminal { + let builder = ShellBuilder::new(shell, is_windows); let command_label = builder.command_label(task.command.as_deref().unwrap_or("")); let (command, args) = builder.build_no_quote(task.command.clone(), &task.args); @@ -1854,7 +1859,7 @@ mod tests { let input = SpawnInTerminal::default(); let shell = Shell::System; - let result = prepare_task_for_spawn(&input, &shell); + let result = prepare_task_for_spawn(&input, &shell, false); let expected_shell = util::get_system_shell(); assert_eq!(result.env, HashMap::default()); @@ -1926,7 +1931,7 @@ mod tests { }; let shell = Shell::System; - let result = prepare_task_for_spawn(&input, &shell); + let result = prepare_task_for_spawn(&input, &shell, false); let system_shell = util::get_system_shell(); assert_eq!(result.env, HashMap::default()); diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index 17be735cf7638d3130ce848976411084abde5f3a..f010ff57c636f40de50a06baefd99c9ee43728f9 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -86,7 +86,7 @@ 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: Option<&ShellKind>) -> anyhow::Result; + fn try_shell_safe(&self, shell_kind: ShellKind) -> anyhow::Result; } impl> PathExt for T { @@ -164,19 +164,13 @@ impl> PathExt for T { Some(parts.into_iter().join(".")) } - fn try_shell_safe(&self, shell_kind: Option<&ShellKind>) -> anyhow::Result { + 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")?; - let quoted = match shell_kind { - Some(kind) => kind.try_quote(path_str), - #[cfg(windows)] - None => Some(ShellKind::quote_powershell(path_str)), - #[cfg(unix)] - None => shlex::try_quote(path_str).ok(), - }; - quoted + shell_kind + .try_quote(path_str) .as_deref() .map(ToOwned::to_owned) .context("Failed to quote path") diff --git a/crates/util/src/shell.rs b/crates/util/src/shell.rs index 2d6156d6b0f5c072b115829bfa88ef6704ce192d..ecac8eaf6aa7d13148d6a5c0770bcedc351c14cf 100644 --- a/crates/util/src/shell.rs +++ b/crates/util/src/shell.rs @@ -1,6 +1,6 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use std::{borrow::Cow, ffi::OsStr, path::Path, sync::LazyLock}; +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)] @@ -39,45 +39,19 @@ impl Shell { } } - pub fn shell_kind(&self) -> Option { + pub fn shell_kind(&self, is_windows: bool) -> ShellKind { match self { - Shell::Program(program) => ShellKind::new(program), - Shell::WithArguments { program, .. } => ShellKind::new(program), + Shell::Program(program) => ShellKind::new(program, is_windows), + Shell::WithArguments { program, .. } => ShellKind::new(program, is_windows), Shell::System => ShellKind::system(), } } } -/// Specific POSIX-compatible shell variants. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub enum PosixShell { - #[default] - Sh, - Bash, - Zsh, - Dash, - Ksh, - Mksh, - Ash, -} - -impl PosixShell { - pub fn name(&self) -> &'static str { - match self { - PosixShell::Sh => "sh", - PosixShell::Bash => "bash", - PosixShell::Zsh => "zsh", - PosixShell::Dash => "dash", - PosixShell::Ksh => "ksh", - PosixShell::Mksh => "mksh", - PosixShell::Ash => "ash", - } - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] pub enum ShellKind { - Posix(PosixShell), + #[default] + Posix, Csh, Tcsh, Rc, @@ -259,127 +233,84 @@ pub fn get_windows_system_shell() -> String { (*SYSTEM_SHELL).clone() } -impl ShellKind { - /// Returns the canonical shell kind for activation script lookups. - /// - /// This normalizes all POSIX shell variants to `Posix(Sh)` so that - /// activation scripts stored with a single POSIX key can be found - /// regardless of which specific POSIX shell the user has. - pub fn activation_script_key(&self) -> ShellKind { - match self { - ShellKind::Posix(_) => ShellKind::Posix(PosixShell::Sh), - other => *other, - } - } - - /// Creates a ShellKind from a program path, with a platform-aware fallback for unknown shells. - /// - /// Unlike `new()` which returns `None` for unrecognized shells, this method - /// falls back to `PowerShell` when `is_windows` is true, or `Posix(Sh)` otherwise. - /// This is useful for remote connections where we know the target platform but - /// may not recognize the specific shell. - pub fn new_with_fallback(program: impl AsRef, is_windows: bool) -> Self { - Self::new(program).unwrap_or_else(|| { - if is_windows { - ShellKind::PowerShell - } else { - ShellKind::Posix(PosixShell::Sh) - } - }) - } - - /// Returns the name of the shell. - pub fn name(&self) -> &'static str { +impl fmt::Display for ShellKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - ShellKind::Posix(posix) => posix.name(), - ShellKind::Csh => "csh", - ShellKind::Tcsh => "tcsh", - ShellKind::Fish => "fish", - ShellKind::PowerShell => "powershell", - ShellKind::Pwsh => "pwsh", - ShellKind::Nushell => "nu", - ShellKind::Cmd => "cmd", - ShellKind::Rc => "rc", - ShellKind::Xonsh => "xonsh", - ShellKind::Elvish => "elvish", + ShellKind::Posix => write!(f, "sh"), + ShellKind::Csh => write!(f, "csh"), + ShellKind::Tcsh => write!(f, "tcsh"), + ShellKind::Fish => write!(f, "fish"), + ShellKind::PowerShell => write!(f, "powershell"), + ShellKind::Pwsh => write!(f, "pwsh"), + ShellKind::Nushell => write!(f, "nu"), + ShellKind::Cmd => write!(f, "cmd"), + ShellKind::Rc => write!(f, "rc"), + ShellKind::Xonsh => write!(f, "xonsh"), + ShellKind::Elvish => write!(f, "elvish"), } } +} - pub fn system() -> Option { - Self::new(&get_system_shell()) +impl ShellKind { + pub fn system() -> Self { + Self::new(&get_system_shell(), cfg!(windows)) } - /// Returns whether this shell's command chaining syntax can be parsed by brush-parser. + /// Returns whether this shell uses POSIX-like command chaining syntax (`&&`, `||`, `;`, `|`). /// /// This is used to determine if we can safely parse shell commands to extract sub-commands /// for security purposes (e.g., preventing shell injection in "always allow" patterns). /// - /// The brush-parser handles `;` (sequential execution) and `|` (piping), which are - /// supported by all common shells. It also handles `&&` and `||` for conditional - /// execution, `$()` and backticks for command substitution, and process substitution. - /// - /// # Security Note - /// - /// Only explicitly recognized shells return `true`. Unknown shells (None) return `false` to - /// prevent security bypasses - if we don't know a shell's syntax, we can't safely - /// parse it for `always_allow` patterns. - /// - /// # Shell Notes + /// **Compatible shells:** Posix (sh, bash, dash, zsh), Fish 3.0+, PowerShell 7+/Pwsh, + /// Cmd, Xonsh, Csh, Tcsh /// - /// - **Nushell**: Uses `;` for sequential execution. The `and`/`or` keywords are boolean - /// operators on values (e.g., `$true and $false`), not command chaining operators. - /// - **Elvish**: Uses `;` to separate pipelines, which brush-parser handles. Elvish does - /// not have `&&` or `||` operators. Its `and`/`or` are special commands that operate - /// on values, not command chaining (e.g., `and $true $false`). - /// - **Rc (Plan 9)**: Uses `;` for sequential execution and `|` for piping. Does not - /// have `&&`/`||` operators for conditional chaining. + /// **Incompatible shells:** Nushell (uses `and`/`or` keywords), Elvish (uses `and`/`or` + /// keywords), Rc (Plan 9 shell - no `&&`/`||` operators) pub fn supports_posix_chaining(&self) -> bool { - match self { - ShellKind::Posix(_) - | ShellKind::Fish - | ShellKind::PowerShell - | ShellKind::Pwsh - | ShellKind::Cmd - | ShellKind::Xonsh - | ShellKind::Csh - | ShellKind::Tcsh - | ShellKind::Nushell - | ShellKind::Elvish - | ShellKind::Rc => true, - } - } - - pub fn new(program: impl AsRef) -> Option { - match program.as_ref().file_stem().and_then(OsStr::to_str)? { - "powershell" => Some(ShellKind::PowerShell), - "pwsh" => Some(ShellKind::Pwsh), - "cmd" => Some(ShellKind::Cmd), - "nu" => Some(ShellKind::Nushell), - "fish" => Some(ShellKind::Fish), - "csh" => Some(ShellKind::Csh), - "tcsh" => Some(ShellKind::Tcsh), - "rc" => Some(ShellKind::Rc), - "xonsh" => Some(ShellKind::Xonsh), - "elvish" => Some(ShellKind::Elvish), - "sh" => Some(ShellKind::Posix(PosixShell::Sh)), - "bash" => Some(ShellKind::Posix(PosixShell::Bash)), - "zsh" => Some(ShellKind::Posix(PosixShell::Zsh)), - "dash" => Some(ShellKind::Posix(PosixShell::Dash)), - "ksh" => Some(ShellKind::Posix(PosixShell::Ksh)), - "mksh" => Some(ShellKind::Posix(PosixShell::Mksh)), - "ash" => Some(ShellKind::Posix(PosixShell::Ash)), - // Unrecognized shell - we cannot safely parse its syntax for - // `always_allow` patterns, so they will be disabled. - // Fall back to platform-specific behavior for non-security purposes. - _ => None, - } - } - - pub fn to_shell_variable(&self, input: &str) -> String { + matches!( + self, + ShellKind::Posix + | ShellKind::Fish + | ShellKind::PowerShell + | ShellKind::Pwsh + | ShellKind::Cmd + | ShellKind::Xonsh + | ShellKind::Csh + | ShellKind::Tcsh + ) + } + + pub fn new(program: impl AsRef, is_windows: bool) -> Self { + let program = program.as_ref(); + let program = program + .file_stem() + .unwrap_or_else(|| program.as_os_str()) + .to_string_lossy(); + + match &*program { + "powershell" => ShellKind::PowerShell, + "pwsh" => ShellKind::Pwsh, + "cmd" => ShellKind::Cmd, + "nu" => ShellKind::Nushell, + "fish" => ShellKind::Fish, + "csh" => ShellKind::Csh, + "tcsh" => ShellKind::Tcsh, + "rc" => ShellKind::Rc, + "xonsh" => ShellKind::Xonsh, + "elvish" => ShellKind::Elvish, + "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, + } + } + + pub fn to_shell_variable(self, input: &str) -> String { match self { Self::PowerShell | Self::Pwsh => Self::to_powershell_variable(input), Self::Cmd => Self::to_cmd_variable(input), - Self::Posix(_) => input.to_owned(), + Self::Posix => input.to_owned(), Self::Fish => input.to_owned(), Self::Csh => input.to_owned(), Self::Tcsh => input.to_owned(), @@ -409,7 +340,7 @@ impl ShellKind { } } - pub fn to_powershell_variable(input: &str) -> String { + fn to_powershell_variable(input: &str) -> String { if let Some(var_str) = input.strip_prefix("${") { if var_str.find(':').is_none() { // If the input starts with "${", remove the trailing "}" @@ -505,15 +436,13 @@ impl ShellKind { pub fn args_for_shell(&self, interactive: bool, combined_command: String) -> Vec { match self { - ShellKind::PowerShell | ShellKind::Pwsh => { - vec!["-C".to_owned(), combined_command] - } + ShellKind::PowerShell | ShellKind::Pwsh => vec!["-C".to_owned(), combined_command], ShellKind::Cmd => vec![ "/S".to_owned(), "/C".to_owned(), format!("\"{combined_command}\""), ], - ShellKind::Posix(_) + ShellKind::Posix | ShellKind::Nushell | ShellKind::Fish | ShellKind::Csh @@ -532,7 +461,7 @@ impl ShellKind { match self { ShellKind::PowerShell | ShellKind::Pwsh => Some('&'), ShellKind::Nushell => Some('^'), - ShellKind::Posix(_) + ShellKind::Posix | ShellKind::Csh | ShellKind::Tcsh | ShellKind::Rc @@ -555,7 +484,7 @@ impl ShellKind { pub const fn sequential_commands_separator(&self) -> char { match self { ShellKind::Cmd => '&', - ShellKind::Posix(_) + ShellKind::Posix | ShellKind::Csh | ShellKind::Tcsh | ShellKind::Rc @@ -571,7 +500,7 @@ impl ShellKind { pub const fn sequential_and_commands_separator(&self) -> &'static str { match self { ShellKind::Cmd - | ShellKind::Posix(_) + | ShellKind::Posix | ShellKind::Csh | ShellKind::Tcsh | ShellKind::Rc @@ -587,7 +516,7 @@ impl ShellKind { ShellKind::PowerShell => Some(Self::quote_powershell(arg)), ShellKind::Pwsh => Some(Self::quote_pwsh(arg)), ShellKind::Cmd => Some(Self::quote_cmd(arg)), - ShellKind::Posix(_) + ShellKind::Posix | ShellKind::Csh | ShellKind::Tcsh | ShellKind::Rc @@ -598,7 +527,7 @@ impl ShellKind { } } - pub fn quote_windows(arg: &str, enclose: bool) -> Cow<'_, str> { + fn quote_windows(arg: &str, enclose: bool) -> Cow<'_, str> { if arg.is_empty() { return Cow::Borrowed("\"\""); } @@ -810,7 +739,7 @@ impl ShellKind { ShellKind::Fish | ShellKind::Csh | ShellKind::Tcsh - | ShellKind::Posix(_) + | ShellKind::Posix | ShellKind::Rc | ShellKind::Xonsh | ShellKind::Elvish => "source", @@ -820,7 +749,7 @@ impl ShellKind { pub const fn clear_screen_command(&self) -> &'static str { match self { ShellKind::Cmd => "cls", - ShellKind::Posix(_) + ShellKind::Posix | ShellKind::Csh | ShellKind::Tcsh | ShellKind::Rc @@ -839,7 +768,7 @@ impl ShellKind { pub const fn tty_escape_args(&self) -> bool { match self { ShellKind::Cmd => false, - ShellKind::Posix(_) + ShellKind::Posix | ShellKind::Csh | ShellKind::Tcsh | ShellKind::Rc @@ -853,109 +782,6 @@ impl ShellKind { } } -// Helper functions for `Option` that provide platform-specific defaults for unknown shells. -// These should be used when you have an `Option` and need to handle the `None` case -// with appropriate fallback behavior (POSIX-like on Unix, PowerShell-like on Windows). - -/// Quote an argument for the shell, with platform-specific fallback for unknown shells. -pub fn try_quote_option(shell_kind: Option, arg: &str) -> Option> { - match shell_kind { - Some(kind) => kind.try_quote(arg), - #[cfg(windows)] - None => Some(ShellKind::quote_powershell(arg)), - #[cfg(unix)] - None => shlex::try_quote(arg).ok(), - } -} - -/// Quote an argument for the shell (prefix-aware), with platform-specific fallback for unknown shells. -pub fn try_quote_prefix_aware_option( - shell_kind: Option, - arg: &str, -) -> Option> { - match shell_kind { - Some(kind) => kind.try_quote_prefix_aware(arg), - #[cfg(windows)] - None => Some(ShellKind::quote_powershell(arg)), - #[cfg(unix)] - None => shlex::try_quote(arg).ok(), - } -} - -/// Get the command prefix for the shell, with platform-specific fallback for unknown shells. -pub fn command_prefix_option(shell_kind: Option) -> Option { - match shell_kind { - Some(kind) => kind.command_prefix(), - #[cfg(windows)] - None => Some('&'), - #[cfg(unix)] - None => None, - } -} - -/// Prepend the command prefix if needed, with platform-specific fallback for unknown shells. -pub fn prepend_command_prefix_option<'a>( - shell_kind: Option, - command: &'a str, -) -> Cow<'a, str> { - match command_prefix_option(shell_kind) { - Some(prefix) if !command.starts_with(prefix) => Cow::Owned(format!("{prefix}{command}")), - _ => Cow::Borrowed(command), - } -} - -/// Get the sequential commands separator, with platform-specific fallback for unknown shells. -pub fn sequential_commands_separator_option(shell_kind: Option) -> char { - match shell_kind { - Some(kind) => kind.sequential_commands_separator(), - #[cfg(windows)] - None => ';', - #[cfg(unix)] - None => ';', - } -} - -/// Get the sequential-and commands separator, with platform-specific fallback for unknown shells. -pub fn sequential_and_commands_separator_option(shell_kind: Option) -> &'static str { - match shell_kind { - Some(kind) => kind.sequential_and_commands_separator(), - #[cfg(windows)] - None => ";", - #[cfg(unix)] - None => ";", - } -} - -/// Get shell arguments for running a command, with platform-specific fallback for unknown shells. -pub fn args_for_shell_option( - shell_kind: Option, - interactive: bool, - combined_command: String, -) -> Vec { - match shell_kind { - Some(kind) => kind.args_for_shell(interactive, combined_command), - #[cfg(windows)] - None => vec!["-C".to_owned(), combined_command], - #[cfg(unix)] - None => interactive - .then(|| "-i".to_owned()) - .into_iter() - .chain(["-c".to_owned(), combined_command]) - .collect(), - } -} - -/// Convert a variable to shell syntax, with platform-specific fallback for unknown shells. -pub fn to_shell_variable_option(shell_kind: Option, input: &str) -> String { - match shell_kind { - Some(kind) => kind.to_shell_variable(input), - #[cfg(windows)] - None => ShellKind::to_powershell_variable(input), - #[cfg(unix)] - None => input.to_owned(), - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/util/src/shell_builder.rs b/crates/util/src/shell_builder.rs index a54b93658bc42e6804d8cc65f363b177167a02ab..86a588d44afa3bf4841a83f2700bd371dce4bd42 100644 --- a/crates/util/src/shell_builder.rs +++ b/crates/util/src/shell_builder.rs @@ -1,15 +1,7 @@ use std::borrow::Cow; use crate::shell::get_system_shell; -use crate::shell::{ - Shell, ShellKind, args_for_shell_option, to_shell_variable_option, try_quote_option, - try_quote_prefix_aware_option, -}; - -const POSIX_STDIN_REDIRECT_PREFIX: char = '('; -const POSIX_STDIN_REDIRECT_SUFFIX: &str = ") , + kind: ShellKind, } impl ShellBuilder { /// Create a new ShellBuilder as configured. - pub fn new(shell: &Shell) -> Self { + pub fn new(shell: &Shell, is_windows: bool) -> Self { let (program, args) = match shell { Shell::System => (get_system_shell(), Vec::new()), Shell::Program(shell) => (shell.clone(), Vec::new()), Shell::WithArguments { program, args, .. } => (program.clone(), args.clone()), }; - let kind = ShellKind::new(&program); + let kind = ShellKind::new(&program, is_windows); Self { program, args, @@ -52,34 +44,20 @@ impl ShellBuilder { self.program.clone() } else { match self.kind { - Some(ShellKind::PowerShell) | Some(ShellKind::Pwsh) => { - format!("{} -C '{}'", self.program, command_to_use_in_label) - } - #[cfg(windows)] - None => { + ShellKind::PowerShell | ShellKind::Pwsh => { format!("{} -C '{}'", self.program, command_to_use_in_label) } - Some(ShellKind::Cmd) => { + ShellKind::Cmd => { format!("{} /C \"{}\"", self.program, command_to_use_in_label) } - Some( - ShellKind::Posix(_) - | ShellKind::Nushell - | ShellKind::Fish - | ShellKind::Csh - | ShellKind::Tcsh - | ShellKind::Rc - | ShellKind::Xonsh - | ShellKind::Elvish, - ) => { - let interactivity = self.interactive.then_some("-i ").unwrap_or_default(); - format!( - "{PROGRAM} {interactivity}-c '{command_to_use_in_label}'", - PROGRAM = self.program - ) - } - #[cfg(unix)] - None => { + ShellKind::Posix + | ShellKind::Nushell + | ShellKind::Fish + | ShellKind::Csh + | ShellKind::Tcsh + | ShellKind::Rc + | ShellKind::Xonsh + | ShellKind::Elvish => { let interactivity = self.interactive.then_some("-i ").unwrap_or_default(); format!( "{PROGRAM} {interactivity}-c '{command_to_use_in_label}'", @@ -95,44 +73,6 @@ impl ShellBuilder { self } - fn apply_stdin_redirect(&self, combined_command: &mut String) { - match self.kind { - Some(ShellKind::Fish) => { - combined_command.insert_str(0, "begin; "); - combined_command.push_str("; end { - combined_command.insert(0, POSIX_STDIN_REDIRECT_PREFIX); - combined_command.push_str(POSIX_STDIN_REDIRECT_SUFFIX); - } - #[cfg(unix)] - None => { - combined_command.insert(0, POSIX_STDIN_REDIRECT_PREFIX); - combined_command.push_str(POSIX_STDIN_REDIRECT_SUFFIX); - } - Some(ShellKind::PowerShell) | Some(ShellKind::Pwsh) => { - combined_command.insert_str(0, POWERSHELL_STDIN_REDIRECT_PREFIX); - combined_command.push_str(POWERSHELL_STDIN_REDIRECT_SUFFIX); - } - #[cfg(windows)] - None => { - combined_command.insert_str(0, POWERSHELL_STDIN_REDIRECT_PREFIX); - combined_command.push_str(POWERSHELL_STDIN_REDIRECT_SUFFIX); - } - Some(ShellKind::Cmd) => { - combined_command.push_str("< NUL"); - } - } - } - /// Returns the program and arguments to run this task in a shell. pub fn build( mut self, @@ -141,7 +81,7 @@ impl ShellBuilder { ) -> (String, Vec) { if let Some(task_command) = task_command { let task_command = if !task_args.is_empty() { - match try_quote_prefix_aware_option(self.kind, &task_command) { + match self.kind.try_quote_prefix_aware(&task_command) { Some(task_command) => task_command.into_owned(), None => task_command, } @@ -150,22 +90,41 @@ impl ShellBuilder { }; let mut combined_command = task_args.iter().fold(task_command, |mut command, arg| { command.push(' '); - let shell_variable = to_shell_variable_option(self.kind, arg); - command.push_str(&match try_quote_option(self.kind, &shell_variable) { + let shell_variable = self.kind.to_shell_variable(arg); + command.push_str(&match self.kind.try_quote(&shell_variable) { Some(shell_variable) => shell_variable, None => Cow::Owned(shell_variable), }); command }); if self.redirect_stdin { - self.apply_stdin_redirect(&mut combined_command); + match self.kind { + ShellKind::Fish => { + combined_command.insert_str(0, "begin; "); + combined_command.push_str("; end { + combined_command.insert(0, '('); + combined_command.push_str(") { + combined_command.insert_str(0, "$null | & {"); + combined_command.push_str("}"); + } + ShellKind::Cmd => { + combined_command.push_str("< NUL"); + } + } } - self.args.extend(args_for_shell_option( - self.kind, - self.interactive, - combined_command, - )); + self.args + .extend(self.kind.args_for_shell(self.interactive, combined_command)); } (self.program, self.args) @@ -181,18 +140,37 @@ impl ShellBuilder { if let Some(task_command) = task_command { let mut combined_command = task_args.iter().fold(task_command, |mut command, arg| { command.push(' '); - command.push_str(&to_shell_variable_option(self.kind, arg)); + command.push_str(&self.kind.to_shell_variable(arg)); command }); if self.redirect_stdin { - self.apply_stdin_redirect(&mut combined_command); + match self.kind { + ShellKind::Fish => { + combined_command.insert_str(0, "begin; "); + combined_command.push_str("; end { + combined_command.insert(0, '('); + combined_command.push_str(") { + combined_command.insert_str(0, "$null | & {"); + combined_command.push_str("}"); + } + ShellKind::Cmd => { + combined_command.push_str("< NUL"); + } + } } - self.args.extend(args_for_shell_option( - self.kind, - self.interactive, - combined_command, - )); + self.args + .extend(self.kind.args_for_shell(self.interactive, combined_command)); } (self.program, self.args) @@ -224,7 +202,7 @@ impl ShellBuilder { if task_args.is_empty() { task_command = task_command .as_ref() - .map(|cmd| try_quote_prefix_aware_option(self.kind, cmd).map(Cow::into_owned)) + .map(|cmd| self.kind.try_quote_prefix_aware(&cmd).map(Cow::into_owned)) .unwrap_or(task_command); } let (program, args) = self.build(task_command, task_args); @@ -232,7 +210,7 @@ impl ShellBuilder { let mut child = crate::command::new_std_command(program); #[cfg(windows)] - if kind == Some(ShellKind::Cmd) { + if kind == ShellKind::Cmd { use std::os::windows::process::CommandExt; for arg in args { @@ -248,7 +226,7 @@ impl ShellBuilder { child } - pub fn kind(&self) -> Option { + pub fn kind(&self) -> ShellKind { self.kind } } @@ -260,7 +238,7 @@ mod test { #[test] fn test_nu_shell_variable_substitution() { let shell = Shell::Program("nu".to_owned()); - let shell_builder = ShellBuilder::new(&shell); + let shell_builder = ShellBuilder::new(&shell, false); let (program, args) = shell_builder.build( Some("echo".into()), @@ -288,7 +266,7 @@ mod test { #[test] fn redirect_stdin_to_dev_null_precedence() { let shell = Shell::Program("nu".to_owned()); - let shell_builder = ShellBuilder::new(&shell); + let shell_builder = ShellBuilder::new(&shell, false); let (program, args) = shell_builder .redirect_stdin_to_dev_null() @@ -301,7 +279,7 @@ mod test { #[test] fn redirect_stdin_to_dev_null_fish() { let shell = Shell::Program("fish".to_owned()); - let shell_builder = ShellBuilder::new(&shell); + let shell_builder = ShellBuilder::new(&shell, false); let (program, args) = shell_builder .redirect_stdin_to_dev_null() @@ -314,7 +292,7 @@ mod test { #[test] fn does_not_quote_sole_command_only() { let shell = Shell::Program("fish".to_owned()); - let shell_builder = ShellBuilder::new(&shell); + let shell_builder = ShellBuilder::new(&shell, false); let (program, args) = shell_builder.build(Some("echo".into()), &[]); @@ -322,7 +300,7 @@ mod test { assert_eq!(args, vec!["-i", "-c", "echo"]); let shell = Shell::Program("fish".to_owned()); - let shell_builder = ShellBuilder::new(&shell); + let shell_builder = ShellBuilder::new(&shell, false); let (program, args) = shell_builder.build(Some("echo oo".into()), &[]); diff --git a/crates/util/src/shell_env.rs b/crates/util/src/shell_env.rs index c8f9259467f443458002dc83e9f6fcdef5a2e57b..c41a28b46953bdb87f319bfbbb84b7ebd22be245 100644 --- a/crates/util/src/shell_env.rs +++ b/crates/util/src/shell_env.rs @@ -36,8 +36,8 @@ async fn capture_unix( use crate::command::new_std_command; - let shell_kind = ShellKind::new(shell_path); - let zed_path = super::get_shell_safe_zed_path(shell_kind.as_ref())?; + 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 = new_std_command(shell_path); @@ -50,21 +50,21 @@ async fn capture_unix( const FD_STDERR: std::os::fd::RawFd = 2; let (fd_num, redir) = match shell_kind { - Some(ShellKind::Rc) => (FD_STDIN, format!(">[1={}]", FD_STDIN)), // `[1=0]` - Some(ShellKind::Nushell) | Some(ShellKind::Tcsh) => (FD_STDOUT, "".to_string()), + ShellKind::Rc => (FD_STDIN, format!(">[1={}]", FD_STDIN)), // `[1=0]` + ShellKind::Nushell | ShellKind::Tcsh => (FD_STDOUT, "".to_string()), // xonsh doesn't support redirecting to stdin, and control sequences are printed to // stdout on startup - Some(ShellKind::Xonsh) => (FD_STDERR, "o>e".to_string()), - Some(ShellKind::PowerShell) => (FD_STDIN, format!(">{}", FD_STDIN)), + ShellKind::Xonsh => (FD_STDERR, "o>e".to_string()), + ShellKind::PowerShell => (FD_STDIN, format!(">{}", FD_STDIN)), _ => (FD_STDIN, format!(">&{}", FD_STDIN)), // `>&0` }; match shell_kind { - Some(ShellKind::Csh) | Some(ShellKind::Tcsh) => { + ShellKind::Csh | ShellKind::Tcsh => { // For csh/tcsh, login shell requires passing `-` as 0th argument (instead of `-l`) command.arg0("-"); } - Some(ShellKind::Fish) => { + ShellKind::Fish => { // in fish, asdf, direnv attach to the `fish_prompt` event command_string.push_str("emit fish_prompt;"); command.arg("-l"); @@ -75,7 +75,7 @@ async fn capture_unix( } // cd into the directory, triggering directory specific side-effects (asdf, direnv, etc) command_string.push_str(&format!("cd '{}';", directory.display())); - if let Some(prefix) = shell_kind.and_then(|k| k.command_prefix()) { + if let Some(prefix) = shell_kind.command_prefix() { command_string.push(prefix); } command_string.push_str(&format!("{} --printenv {}", zed_path, redir)); @@ -140,18 +140,16 @@ async fn capture_windows( let zed_path = std::env::current_exe().context("Failed to determine current zed executable path.")?; - let shell_kind = ShellKind::new(shell_path); + let shell_kind = ShellKind::new(shell_path, true); let mut cmd = crate::command::new_smol_command(shell_path); cmd.args(args); let cmd = match shell_kind { - Some( - ShellKind::Csh - | ShellKind::Tcsh - | ShellKind::Rc - | ShellKind::Fish - | ShellKind::Xonsh - | ShellKind::Posix(_), - ) => cmd.args([ + ShellKind::Csh + | ShellKind::Tcsh + | ShellKind::Rc + | ShellKind::Fish + | ShellKind::Xonsh + | ShellKind::Posix => cmd.args([ "-l", "-i", "-c", @@ -161,7 +159,7 @@ async fn capture_windows( zed_path.display() ), ]), - Some(ShellKind::PowerShell) | Some(ShellKind::Pwsh) | None => cmd.args([ + ShellKind::PowerShell | ShellKind::Pwsh => cmd.args([ "-NonInteractive", "-NoProfile", "-Command", @@ -171,7 +169,7 @@ async fn capture_windows( zed_path.display() ), ]), - Some(ShellKind::Elvish) => cmd.args([ + ShellKind::Elvish => cmd.args([ "-c", &format!( "cd '{}'; '{}' --printenv", @@ -179,19 +177,19 @@ async fn capture_windows( zed_path.display() ), ]), - Some(ShellKind::Nushell) => cmd.args([ + ShellKind::Nushell => cmd.args([ "-c", &format!( "cd '{}'; {}'{}' --printenv", directory.display(), - ShellKind::Nushell + shell_kind .command_prefix() .map(|prefix| prefix.to_string()) .unwrap_or_default(), zed_path.display() ), ]), - Some(ShellKind::Cmd) => cmd.args([ + ShellKind::Cmd => cmd.args([ "/c", "cd", &directory.display().to_string(), diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 4d5e2dd073d1e59a3078fe090f7d6051a818189b..87b8a1615d86c74fa4b3b1cb7c89fc94939558a0 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -303,7 +303,7 @@ 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: Option<&shell::ShellKind>) -> anyhow::Result { +pub fn get_shell_safe_zed_path(shell_kind: shell::ShellKind) -> anyhow::Result { let mut zed_path = std::env::current_exe().context("Failed to determine current zed executable path.")?; if cfg!(target_os = "linux")