From d800c984b44529034d1b044164f512ebacabb2b6 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 13 Oct 2025 12:41:10 +0200 Subject: [PATCH] task: Do not wrap tasks in extra shell invocations unless requested --- crates/agent_ui/src/acp/thread_view.rs | 9 +- crates/debugger_ui/src/new_process_modal.rs | 18 ++- crates/debugger_ui/src/session/running.rs | 4 +- crates/editor/src/editor.rs | 14 ++- crates/editor/src/lsp_ext.rs | 11 +- crates/project/src/debugger/locators/go.rs | 8 +- .../project/src/debugger/locators/python.rs | 2 +- .../project/src/lsp_store/lsp_ext_command.rs | 9 +- crates/project/src/project_tests.rs | 7 +- crates/project/src/task_inventory.rs | 26 ++-- crates/project/src/task_store.rs | 3 + crates/proto/proto/task.proto | 1 + crates/task/src/task.rs | 1 + crates/task/src/task_template.rs | 113 ++++++++++++++---- crates/tasks_ui/src/modal.rs | 39 +++++- crates/terminal_view/src/terminal_panel.rs | 78 +++--------- crates/workspace/src/tasks.rs | 11 +- 17 files changed, 226 insertions(+), 128 deletions(-) diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index 5c575de401daf26bd7815bc49d923072243ee980..929a0fafbe28301ca1e1b1fe1b94b3ecc8f8c74a 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/crates/agent_ui/src/acp/thread_view.rs @@ -1628,11 +1628,8 @@ impl AcpThreadView { window.spawn(cx, async move |cx| { let mut task = login.clone(); - task.shell = task::Shell::WithArguments { - program: task.command.take().expect("login command should be set"), - args: std::mem::take(&mut task.args), - title_override: None - }; + task.command = task.command.clone(); + task.args = task.args.clone(); task.full_label = task.label.clone(); task.id = task::TaskId(format!("external-agent-{}-login", task.label)); task.command_label = task.label.clone(); @@ -1641,7 +1638,7 @@ impl AcpThreadView { task.hide = task::HideStrategy::Always; let terminal = terminal_panel.update_in(cx, |terminal_panel, window, cx| { - terminal_panel.spawn_task(&task, window, cx) + terminal_panel.spawn_task(task, window, cx) })?; let terminal = terminal.await?; diff --git a/crates/debugger_ui/src/new_process_modal.rs b/crates/debugger_ui/src/new_process_modal.rs index e12c768e12b1e098e150027c89d05695c59c51f6..13d5a7b6f9bd741d61ade0bcd88ee4df018c1906 100644 --- a/crates/debugger_ui/src/new_process_modal.rs +++ b/crates/debugger_ui/src/new_process_modal.rs @@ -193,8 +193,22 @@ impl NewProcessModal { let (used_tasks, current_resolved_tasks) = task_inventory .update(cx, |task_inventory, cx| { - task_inventory - .used_and_current_resolved_tasks(task_contexts.clone(), cx) + let remote_shell = workspace + .read_with(cx, |workspace, cx| { + workspace + .project() + .read(cx) + .remote_client()? + .read(cx) + .shell() + }) + .ok() + .flatten(); + task_inventory.used_and_current_resolved_tasks( + task_contexts.clone(), + move || remote_shell.clone(), + cx, + ) })? .await; diff --git a/crates/debugger_ui/src/session/running.rs b/crates/debugger_ui/src/session/running.rs index 0e21ef1268412418c381fc14617a917f9529834d..6701183b1c1b0bb46c2a1735eab15eeea7b36c6a 100644 --- a/crates/debugger_ui/src/session/running.rs +++ b/crates/debugger_ui/src/session/running.rs @@ -989,7 +989,7 @@ impl RunningState { (task, None) } }; - let Some(mut task) = task_template.resolve_task("debug-build-task", &task_context) else { + let Some(mut task) = task_template.resolve_task("debug-build-task",&|| remote_shell.clone(), &task_context) else { anyhow::bail!("Could not resolve task variables within a debug scenario"); }; @@ -1026,7 +1026,7 @@ impl RunningState { None }; - if let Some(remote_shell) = remote_shell && task.resolved.shell == Shell::System { + if task.resolved.shell == Shell::System && let Some(remote_shell) = remote_shell.clone() { task.resolved.shell = Shell::Program(remote_shell); } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index ed6b8ec2eca4dcb558bc832ac56b92af8791712c..3a3754f7fd1284ca8275e0b6e2c76dd508621b5f 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -810,11 +810,12 @@ struct RunnableTasks { impl RunnableTasks { fn resolve<'a>( &'a self, + remote_shell: &'a dyn Fn() -> Option, cx: &'a task::TaskContext, ) -> impl Iterator + 'a { self.templates.iter().filter_map(|(kind, template)| { template - .resolve_task(&kind.to_id_base(), cx) + .resolve_task(&kind.to_id_base(), remote_shell, cx) .map(|task| (kind.clone(), task)) }) } @@ -6002,13 +6003,14 @@ impl Editor { Some(CodeActionSource::Indicator(_)) => Task::ready(Ok(Default::default())), _ => { let mut task_context_task = Task::ready(None); + let remote_shell = + maybe!({ project.as_ref()?.read(cx).remote_client()?.read(cx).shell() }); if let Some(tasks) = &tasks && let Some(project) = project { task_context_task = Self::build_tasks_context(&project, &buffer, buffer_row, tasks, cx); } - cx.spawn_in(window, { let buffer = buffer.clone(); async move |editor, cx| { @@ -6018,7 +6020,9 @@ impl Editor { tasks .zip(task_context.clone()) .map(|(tasks, task_context)| ResolvedTasks { - templates: tasks.resolve(&task_context).collect(), + templates: tasks + .resolve(&|| remote_shell.clone(), &task_context) + .collect(), position: snapshot.buffer_snapshot().anchor_before(Point::new( multibuffer_point.row, tasks.column, @@ -8320,9 +8324,11 @@ impl Editor { let reveal_strategy = action.reveal; let task_context = Self::build_tasks_context(&project, &buffer, buffer_row, &tasks, cx); + let remote_shell = maybe!({ project.read(cx).remote_client()?.read(cx).shell() }); cx.spawn_in(window, async move |_, cx| { let context = task_context.await?; - let (task_source_kind, mut resolved_task) = tasks.resolve(&context).next()?; + let (task_source_kind, mut resolved_task) = + tasks.resolve(&|| remote_shell.clone(), &context).next()?; let resolved = &mut resolved_task.resolved; resolved.reveal = reveal_strategy; diff --git a/crates/editor/src/lsp_ext.rs b/crates/editor/src/lsp_ext.rs index 0c4760f5684acf450b793a1deac54be983dcafd0..bd5a3977c6186118b1f4698b238ff347c62a5c3f 100644 --- a/crates/editor/src/lsp_ext.rs +++ b/crates/editor/src/lsp_ext.rs @@ -21,7 +21,7 @@ use task::ResolvedTask; use task::TaskContext; use text::BufferId; use ui::SharedString; -use util::ResultExt as _; +use util::{ResultExt as _, maybe}; pub(crate) fn find_specific_language_server_in_selection( editor: &Editor, @@ -114,7 +114,7 @@ pub fn lsp_tasks( server_id.zip(Some(buffers)) }) .collect::>(); - + let remote_shell = maybe!({ project.read(cx).remote_client()?.read(cx).shell() }); cx.spawn(async move |cx| { cx.spawn(async move |cx| { let mut lsp_tasks = HashMap::default(); @@ -151,8 +151,11 @@ pub fn lsp_tasks( { new_lsp_tasks.extend(new_runnables.runnables.into_iter().filter_map( |(location, runnable)| { - let resolved_task = - runnable.resolve_task(&id_base, &lsp_buffer_context)?; + let resolved_task = runnable.resolve_task( + &id_base, + &|| remote_shell.clone(), + &lsp_buffer_context, + )?; Some((location, resolved_task)) }, )); diff --git a/crates/project/src/debugger/locators/go.rs b/crates/project/src/debugger/locators/go.rs index eec06084ec78548e1a627080663d2afccc8a0aca..a9817438ab163cf76cea461d1225990ae666f15d 100644 --- a/crates/project/src/debugger/locators/go.rs +++ b/crates/project/src/debugger/locators/go.rs @@ -246,7 +246,7 @@ impl DapLocator for GoLocator { mod tests { use super::*; use gpui::TestAppContext; - use task::{HideStrategy, RevealStrategy, RevealTarget, Shell, TaskTemplate}; + use task::{HideStrategy, RevealStrategy, RevealTarget, TaskTemplate}; #[gpui::test] async fn test_create_scenario_for_go_build(_: &mut TestAppContext) { @@ -262,7 +262,7 @@ mod tests { reveal: RevealStrategy::Always, reveal_target: RevealTarget::Dock, hide: HideStrategy::Never, - shell: Shell::System, + shell: None, tags: vec![], show_summary: true, show_command: true, @@ -289,7 +289,7 @@ mod tests { reveal: RevealStrategy::Always, reveal_target: RevealTarget::Dock, hide: HideStrategy::Never, - shell: Shell::System, + shell: None, tags: vec![], show_summary: true, show_command: true, @@ -427,7 +427,7 @@ mod tests { reveal: RevealStrategy::Always, reveal_target: RevealTarget::Dock, hide: HideStrategy::Never, - shell: Shell::System, + shell: None, tags: vec![], show_summary: true, show_command: true, diff --git a/crates/project/src/debugger/locators/python.rs b/crates/project/src/debugger/locators/python.rs index c3754548d0676e76f08368828d554600ab700fc0..3b3478d8c2360db26cf3a48c2c953e4d57b17f38 100644 --- a/crates/project/src/debugger/locators/python.rs +++ b/crates/project/src/debugger/locators/python.rs @@ -116,7 +116,7 @@ mod test { reveal_target: task::RevealTarget::Dock, hide: task::HideStrategy::Never, tags: vec!["python-module-main-method".into()], - shell: task::Shell::System, + shell: None, show_summary: false, show_command: false, }; diff --git a/crates/project/src/lsp_store/lsp_ext_command.rs b/crates/project/src/lsp_store/lsp_ext_command.rs index 5066143244da890a63ead6650cb61fdb71d3964a..a94e5f8612d3a81c5d405553222fafc8d4487f88 100644 --- a/crates/project/src/lsp_store/lsp_ext_command.rs +++ b/crates/project/src/lsp_store/lsp_ext_command.rs @@ -23,7 +23,7 @@ use std::{ path::{Path, PathBuf}, sync::Arc, }; -use task::TaskTemplate; +use task::{ShellKind, TaskTemplate}; use text::{BufferId, PointUtf16, ToPointUtf16}; pub enum LspExtExpandMacro {} @@ -657,7 +657,12 @@ 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)); + let shell_kind = task_template + .shell + .as_ref() + .map_or_else(ShellKind::system, |shell| { + shell.shell_kind(cfg!(windows)) + }); task_template.args.push("--".to_string()); task_template.args.extend( cargo diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 3dc918d5a757af56038471e1a601d6f2cf7dbbe1..829612ee9afddf46d29acd5d05dd0c165dc8ee32 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -9812,9 +9812,14 @@ fn get_all_tasks( cx: &mut App, ) -> Task> { let new_tasks = project.update(cx, |project, cx| { + let remote_shell = project.remote_client().and_then(|it| it.read(cx).shell()); project.task_store.update(cx, |task_store, cx| { task_store.task_inventory().unwrap().update(cx, |this, cx| { - this.used_and_current_resolved_tasks(task_contexts, cx) + this.used_and_current_resolved_tasks( + task_contexts, + move || remote_shell.clone(), + cx, + ) }) }) }); diff --git a/crates/project/src/task_inventory.rs b/crates/project/src/task_inventory.rs index 4f4939491f19ebe6d32b82f780c4a9988a66c1d4..2fe632b4fbc14b1aa0190473cd2a799f6cd238a8 100644 --- a/crates/project/src/task_inventory.rs +++ b/crates/project/src/task_inventory.rs @@ -424,6 +424,7 @@ impl Inventory { pub fn used_and_current_resolved_tasks( &self, task_contexts: Arc, + remote_shell: impl 'static + Fn() -> Option + Send, cx: &mut Context, ) -> Task<( Vec<(TaskSourceKind, ResolvedTask)>, @@ -518,14 +519,14 @@ impl Inventory { task_contexts.active_item_context.as_ref().filter( |(worktree_id, _, _)| Some(id) == worktree_id.as_ref(), )?; - task.resolve_task(&id_base, item_context) + task.resolve_task(&id_base, &remote_shell, item_context) }) .or_else(|| { let (_, worktree_context) = task_contexts .active_worktree_context .as_ref() .filter(|(worktree_id, _)| id == worktree_id)?; - task.resolve_task(&id_base, worktree_context) + task.resolve_task(&id_base, &remote_shell, worktree_context) }) .or_else(|| { if let TaskSourceKind::Worktree { id, .. } = &kind { @@ -534,7 +535,7 @@ impl Inventory { .iter() .find(|(worktree_id, _)| worktree_id == id) .map(|(_, context)| context)?; - task.resolve_task(&id_base, worktree_context) + task.resolve_task(&id_base, &remote_shell, worktree_context) } else { None } @@ -543,15 +544,15 @@ impl Inventory { None.or_else(|| { let (_, _, item_context) = task_contexts.active_item_context.as_ref()?; - task.resolve_task(&id_base, item_context) + task.resolve_task(&id_base, &remote_shell, item_context) }) .or_else(|| { let (_, worktree_context) = task_contexts.active_worktree_context.as_ref()?; - task.resolve_task(&id_base, worktree_context) + task.resolve_task(&id_base, &remote_shell, worktree_context) }) } - .or_else(|| task.resolve_task(&id_base, &TaskContext::default())) + .or_else(|| task.resolve_task(&id_base, &remote_shell, &TaskContext::default())) .map(move |resolved_task| (kind.clone(), resolved_task, not_used_score)) }) .filter(|(_, resolved_task, _)| { @@ -896,7 +897,7 @@ mod test_inventory { .update(&mut cx, |inventory, _| { inventory.task_scheduled( task_source_kind.clone(), - task.resolve_task(&id_base, &TaskContext::default()) + task.resolve_task(&id_base, &|| None, &TaskContext::default()) .unwrap_or_else(|| { panic!("Failed to resolve task with name {task_name}") }), @@ -929,7 +930,7 @@ mod test_inventory { .update(&mut cx, |inventory, _| { inventory.task_scheduled( task_source_kind.clone(), - task.resolve_task(&id_base, &TaskContext::default()) + task.resolve_task(&id_base, &|| None, &TaskContext::default()) .unwrap_or_else(|| { panic!("Failed to resolve task with name {task_name}") }), @@ -953,7 +954,10 @@ mod test_inventory { .into_iter() .filter_map(|(source_kind, task)| { let id_base = source_kind.to_id_base(); - Some((source_kind, task.resolve_task(&id_base, task_context)?)) + Some(( + source_kind, + task.resolve_task(&id_base, &|| None, task_context)?, + )) }) .map(|(source_kind, resolved_task)| (source_kind, resolved_task.resolved_label)) .collect() @@ -1558,7 +1562,7 @@ mod tests { task_contexts.active_worktree_context = worktree.map(|worktree| (worktree, TaskContext::default())); - inventory.used_and_current_resolved_tasks(Arc::new(task_contexts), cx) + inventory.used_and_current_resolved_tasks(Arc::new(task_contexts), || None, cx) }); cx.background_spawn(async move { @@ -1597,7 +1601,7 @@ mod tests { task_contexts.active_worktree_context = worktree.map(|worktree| (worktree, TaskContext::default())); - inventory.used_and_current_resolved_tasks(Arc::new(task_contexts), cx) + inventory.used_and_current_resolved_tasks(Arc::new(task_contexts), || None, cx) }) .await; let mut all = used; diff --git a/crates/project/src/task_store.rs b/crates/project/src/task_store.rs index 0de5e239798c6a95078d79c2a25775c914a13611..db4f2d27fdd5f88056f8782854b49e5bf2af7910 100644 --- a/crates/project/src/task_store.rs +++ b/crates/project/src/task_store.rs @@ -155,6 +155,7 @@ impl TaskStore { .into_iter() .map(|(variable_name, variable_value)| (variable_name.to_string(), variable_value)) .collect(), + is_windows: cfg!(windows), }) } @@ -345,6 +346,7 @@ fn local_task_context_for_location( project_env: project_env.unwrap_or_default(), cwd: worktree_abs_path.map(|p| p.to_path_buf()), task_variables, + is_windows: cfg!(windows), }) }) } @@ -414,6 +416,7 @@ fn remote_task_context_for_location( ) .collect(), project_env: task_context.project_env.into_iter().collect(), + is_windows: task_context.is_windows, }) }) } diff --git a/crates/proto/proto/task.proto b/crates/proto/proto/task.proto index 1844087d623cc3eac0e5d7500a50dfb31028f304..2bf552ae73bdfd753855ed4f3aa9d4c5b46ef35b 100644 --- a/crates/proto/proto/task.proto +++ b/crates/proto/proto/task.proto @@ -13,6 +13,7 @@ message TaskContext { optional string cwd = 1; map task_variables = 2; map project_env = 3; + bool is_windows = 4; } message Shell { diff --git a/crates/task/src/task.rs b/crates/task/src/task.rs index 280bf5ccdb91271d7ff738654d507573c9d667d4..f9d9fdeeb889268e65ca1eb84649208cce60ef39 100644 --- a/crates/task/src/task.rs +++ b/crates/task/src/task.rs @@ -310,6 +310,7 @@ pub struct TaskContext { /// This is the environment one would get when `cd`ing in a terminal /// into the project's root directory. pub project_env: HashMap, + pub is_windows: bool, } /// This is a new type representing a 'tag' on a 'runnable symbol', typically a test of main() function, found via treesitter. diff --git a/crates/task/src/task_template.rs b/crates/task/src/task_template.rs index 33ff610b6e1ba509c75138ad4bf35478e69deaf1..f4e95dc2198b63437abd5ec7b5c9d2c2c945c5c2 100644 --- a/crates/task/src/task_template.rs +++ b/crates/task/src/task_template.rs @@ -8,6 +8,7 @@ use util::schemars::DefaultDenyUnknownFields; use util::serde::default_true; use util::{ResultExt, truncate_and_remove_front}; +use crate::ShellBuilder; use crate::{ AttachRequest, ResolvedTask, RevealTarget, Shell, SpawnInTerminal, TaskContext, TaskId, VariableName, ZED_VARIABLE_NAME_PREFIX, serde_helpers::non_empty_string_vec, @@ -65,7 +66,7 @@ pub struct TaskTemplate { pub tags: Vec, /// Which shell to use when spawning the task. #[serde(default)] - pub shell: Shell, + pub shell: Option, /// Whether to show the task line in the task output. #[serde(default = "default_true")] pub show_summary: bool, @@ -132,7 +133,12 @@ impl TaskTemplate { /// /// Every [`ResolvedTask`] gets a [`TaskId`], based on the `id_base` (to avoid collision with various task sources), /// and hashes of its template and [`TaskContext`], see [`ResolvedTask`] fields' documentation for more details. - pub fn resolve_task(&self, id_base: &str, cx: &TaskContext) -> Option { + pub fn resolve_task( + &self, + id_base: &str, + remote_shell: &dyn Fn() -> Option, + cx: &TaskContext, + ) -> Option { if self.label.trim().is_empty() || self.command.trim().is_empty() { return None; } @@ -241,6 +247,55 @@ impl TaskTemplate { env }; + let (shell, command, args, command_label) = match &self.shell { + // shell is specified, command + args may be a shell script so we force wrap it in another a shell execution + Some(shell) => { + let shell = if let Shell::System = shell + && let Some(remote_shell) = remote_shell() + { + Shell::Program(remote_shell) + } else { + shell.clone() + }; + + let builder = ShellBuilder::new(&shell, cx.is_windows); + let command_label = builder.command_label(&command); + let (command, args) = builder.build(Some(command), &args_with_substitutions); + + (shell, command, args, command_label) + } + // no shell specified but command contains whitespace, might be a shell script so spawn a shell for backwards compat + None if command.contains(char::is_whitespace) => { + let shell = match remote_shell() { + Some(remote_shell) => Shell::Program(remote_shell), + None => Shell::System, + }; + + let builder = ShellBuilder::new(&shell, cx.is_windows); + let command_label = builder.command_label(&command); + let (command, args) = builder.build(Some(command), &args_with_substitutions); + + (shell, command, args, command_label) + } + // no shell specified, interpret as direct process spawn + None => { + let command_label = args_with_substitutions.iter().fold( + command.clone(), + |mut command_label, arg| { + command_label.push(' '); + command_label.push_str(arg); + command_label + }, + ); + ( + Shell::System, + command, + args_with_substitutions, + command_label, + ) + } + }; + Some(ResolvedTask { id: id.clone(), substituted_variables, @@ -251,23 +306,16 @@ impl TaskTemplate { cwd, full_label, label: human_readable_label, - command_label: args_with_substitutions.iter().fold( - command.clone(), - |mut command_label, arg| { - command_label.push(' '); - command_label.push_str(arg); - command_label - }, - ), + command_label, command: Some(command), - args: args_with_substitutions, + args: args, env, use_new_terminal: self.use_new_terminal, allow_concurrent_runs: self.allow_concurrent_runs, reveal: self.reveal, reveal_target: self.reveal_target, hide: self.hide, - shell: self.shell.clone(), + shell, show_summary: self.show_summary, show_command: self.show_command, show_rerun: true, @@ -462,7 +510,11 @@ mod tests { }, ] { assert_eq!( - task_with_blank_property.resolve_task(TEST_ID_BASE, &TaskContext::default()), + task_with_blank_property.resolve_task( + TEST_ID_BASE, + &|| None, + &TaskContext::default() + ), None, "should not resolve task with blank label and/or command: {task_with_blank_property:?}" ); @@ -480,7 +532,7 @@ mod tests { let resolved_task = |task_template: &TaskTemplate, task_cx| { let resolved_task = task_template - .resolve_task(TEST_ID_BASE, task_cx) + .resolve_task(TEST_ID_BASE, &|| None, task_cx) .unwrap_or_else(|| panic!("failed to resolve task {task_without_cwd:?}")); assert_substituted_variables(&resolved_task, Vec::new()); resolved_task.resolved @@ -490,6 +542,7 @@ mod tests { cwd: None, task_variables: TaskVariables::default(), project_env: HashMap::default(), + is_windows: cfg!(windows), }; assert_eq!( resolved_task(&task_without_cwd, &cx).cwd, @@ -502,6 +555,7 @@ mod tests { cwd: Some(context_cwd.clone()), task_variables: TaskVariables::default(), project_env: HashMap::default(), + is_windows: cfg!(windows), }; assert_eq!( resolved_task(&task_without_cwd, &cx).cwd, @@ -518,6 +572,7 @@ mod tests { cwd: None, task_variables: TaskVariables::default(), project_env: HashMap::default(), + is_windows: cfg!(windows), }; assert_eq!( resolved_task(&task_with_cwd, &cx).cwd, @@ -529,6 +584,7 @@ mod tests { cwd: Some(context_cwd), task_variables: TaskVariables::default(), project_env: HashMap::default(), + is_windows: cfg!(windows), }; assert_eq!( resolved_task(&task_with_cwd, &cx).cwd, @@ -601,10 +657,11 @@ mod tests { for i in 0..15 { let resolved_task = task_with_all_variables.resolve_task( TEST_ID_BASE, - &TaskContext { + &|| None, &TaskContext { cwd: None, task_variables: TaskVariables::from_iter(all_variables.clone()), project_env: HashMap::default(), + is_windows: cfg!(windows), }, ).unwrap_or_else(|| panic!("Should successfully resolve task {task_with_all_variables:?} with variables {all_variables:?}")); @@ -689,10 +746,12 @@ mod tests { let removed_variable = not_all_variables.remove(i); let resolved_task_attempt = task_with_all_variables.resolve_task( TEST_ID_BASE, + &|| None, &TaskContext { cwd: None, task_variables: TaskVariables::from_iter(not_all_variables), project_env: HashMap::default(), + is_windows: cfg!(windows), }, ); assert_eq!( @@ -711,7 +770,7 @@ mod tests { ..TaskTemplate::default() }; let resolved_task = task - .resolve_task(TEST_ID_BASE, &TaskContext::default()) + .resolve_task(TEST_ID_BASE, &|| None, &TaskContext::default()) .unwrap(); assert_substituted_variables(&resolved_task, Vec::new()); let resolved = resolved_task.resolved; @@ -729,7 +788,7 @@ mod tests { ..TaskTemplate::default() }; assert!( - task.resolve_task(TEST_ID_BASE, &TaskContext::default()) + task.resolve_task(TEST_ID_BASE, &|| None, &TaskContext::default()) .is_none() ); } @@ -750,6 +809,7 @@ mod tests { "test_symbol".to_string(), ))), project_env: HashMap::default(), + is_windows: cfg!(windows), }; for (i, symbol_dependent_task) in [ @@ -780,7 +840,7 @@ mod tests { .enumerate() { let resolved = symbol_dependent_task - .resolve_task(TEST_ID_BASE, &cx) + .resolve_task(TEST_ID_BASE, &|| None, &cx) .unwrap_or_else(|| panic!("Failed to resolve task {symbol_dependent_task:?}")); assert_eq!( resolved.substituted_variables, @@ -822,7 +882,11 @@ mod tests { context .task_variables .insert(VariableName::Symbol, "my-symbol".to_string()); - assert!(faulty_go_test.resolve_task("base", &context).is_some()); + assert!( + faulty_go_test + .resolve_task("base", &|| None, &context) + .is_some() + ); } #[test] @@ -878,10 +942,11 @@ mod tests { cwd: None, task_variables: TaskVariables::from_iter(all_variables), project_env, + is_windows: cfg!(windows), }; let resolved = template - .resolve_task(TEST_ID_BASE, &context) + .resolve_task(TEST_ID_BASE, &|| None, &context) .unwrap() .resolved; @@ -917,10 +982,11 @@ mod tests { (VariableName::Row, "123".to_string()), ]), project_env: HashMap::default(), + is_windows: cfg!(windows), }; let resolved = task_with_defaults - .resolve_task(TEST_ID_BASE, &context_with_file) + .resolve_task(TEST_ID_BASE, &|| None, &context_with_file) .expect("Should resolve task with existing variables"); assert_eq!( @@ -939,10 +1005,11 @@ mod tests { cwd: None, task_variables: TaskVariables::from_iter(vec![(VariableName::Row, "456".to_string())]), project_env: HashMap::default(), + is_windows: cfg!(windows), }; let resolved = task_with_defaults - .resolve_task(TEST_ID_BASE, &context_without_file) + .resolve_task(TEST_ID_BASE, &|| None, &context_without_file) .expect("Should resolve task using default values"); assert_eq!( @@ -965,7 +1032,7 @@ mod tests { assert!( task_no_default - .resolve_task(TEST_ID_BASE, &TaskContext::default()) + .resolve_task(TEST_ID_BASE, &|| None, &TaskContext::default()) .is_none(), "Should fail when ZED variable has no default and doesn't exist" ); diff --git a/crates/tasks_ui/src/modal.rs b/crates/tasks_ui/src/modal.rs index f82321feeb245b4ee3b6d56627387c8594d5db8e..12df6a46ef1566b026b093ea842d5eacf0ecaa4a 100644 --- a/crates/tasks_ui/src/modal.rs +++ b/crates/tasks_ui/src/modal.rs @@ -74,7 +74,10 @@ impl TasksModalDelegate { } } - fn spawn_oneshot(&mut self) -> Option<(TaskSourceKind, ResolvedTask)> { + fn spawn_oneshot( + &mut self, + cx: &mut Context>, + ) -> Option<(TaskSourceKind, ResolvedTask)> { if self.prompt.trim().is_empty() { return None; } @@ -99,7 +102,23 @@ impl TasksModalDelegate { } Some(( source_kind, - new_oneshot.resolve_task(&id_base, active_context)?, + new_oneshot.resolve_task( + &id_base, + &|| { + self.workspace + .read_with(cx, |workspace, cx| { + workspace + .project() + .read(cx) + .remote_client()? + .read(cx) + .shell() + }) + .ok() + .flatten() + }, + active_context, + )?, )) } @@ -274,10 +293,20 @@ impl PickerDelegate for TasksModalDelegate { Some(candidates) => Task::ready(string_match_candidates(candidates)), None => { if let Some(task_inventory) = self.task_store.read(cx).task_inventory().cloned() { + let workspace = self.workspace.clone(); let task_list = task_inventory.update(cx, |this, cx| { - this.used_and_current_resolved_tasks(self.task_contexts.clone(), cx) + let remote_shell = workspace + .read_with(cx, |it, cx| { + it.project().read(cx).remote_client()?.read(cx).shell() + }) + .ok() + .flatten(); + this.used_and_current_resolved_tasks( + self.task_contexts.clone(), + move || remote_shell.clone(), + cx, + ) }); - let workspace = self.workspace.clone(); let lsp_task_sources = self.task_contexts.lsp_task_sources.clone(); let task_position = self.task_contexts.latest_selection; cx.spawn(async move |picker, cx| { @@ -603,7 +632,7 @@ impl PickerDelegate for TasksModalDelegate { window: &mut Window, cx: &mut Context>, ) { - let Some((task_source_kind, mut task)) = self.spawn_oneshot() else { + let Some((task_source_kind, mut task)) = self.spawn_oneshot(cx) else { return; }; diff --git a/crates/terminal_view/src/terminal_panel.rs b/crates/terminal_view/src/terminal_panel.rs index 6568eac324552a293d64060c07f6299d2edf9f8d..e23962d71988bdb5c71fca9bb532a5320bfe409a 100644 --- a/crates/terminal_view/src/terminal_panel.rs +++ b/crates/terminal_view/src/terminal_panel.rs @@ -19,7 +19,7 @@ use itertools::Itertools; use project::{Fs, Project, ProjectEntryId}; use search::{BufferSearchBar, buffer_search::DivRegistrar}; use settings::{Settings, TerminalDockPosition}; -use task::{RevealStrategy, RevealTarget, Shell, ShellBuilder, SpawnInTerminal, TaskId}; +use task::{RevealStrategy, RevealTarget, SpawnInTerminal, TaskId}; use terminal::{Terminal, terminal_settings::TerminalSettings}; use ui::{ ButtonLike, Clickable, ContextMenu, FluentBuilder, PopoverMenu, SplitButton, Toggleable, @@ -520,45 +520,10 @@ impl TerminalPanel { pub fn spawn_task( &mut self, - task: &SpawnInTerminal, + task: SpawnInTerminal, window: &mut Window, cx: &mut Context, ) -> Task>> { - let Some(workspace) = self.workspace.upgrade() else { - return Task::ready(Err(anyhow!("failed to read workspace"))); - }; - - let project = workspace.read(cx).project().read(cx); - - if project.is_via_collab() { - return Task::ready(Err(anyhow!("cannot spawn tasks as a guest"))); - } - - 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()); - - let shell = if let Some(remote_shell) = remote_shell - && task.shell == Shell::System - { - Shell::Program(remote_shell) - } else { - task.shell.clone() - }; - - let builder = ShellBuilder::new(&shell, is_windows); - let command_label = builder.command_label(task.command.as_deref().unwrap_or("")); - let (command, args) = builder.build(task.command.clone(), &task.args); - - let task = SpawnInTerminal { - command_label, - command: Some(command), - args, - ..task.clone() - }; - if task.allow_concurrent_runs && task.use_new_terminal { return self.spawn_in_new_terminal(task, window, cx); } @@ -1708,7 +1673,7 @@ impl workspace::TerminalProvider for TerminalProvider { window.spawn(cx, async move |cx| { let terminal = terminal_panel .update_in(cx, |terminal_panel, window, cx| { - terminal_panel.spawn_task(&task, window, cx) + terminal_panel.spawn_task(task, window, cx) }) .ok()? .await; @@ -1773,13 +1738,12 @@ mod tests { let task = window_handle .update(cx, |_, window, cx| { terminal_panel.update(cx, |terminal_panel, cx| { - terminal_panel.spawn_task(&SpawnInTerminal::default(), window, cx) + terminal_panel.spawn_task(SpawnInTerminal::default(), window, cx) }) }) .unwrap(); let terminal = task.await.unwrap(); - let expected_shell = util::get_system_shell(); terminal .update(cx, |terminal, _| { let task_metadata = terminal @@ -1791,15 +1755,11 @@ mod tests { assert_eq!(task_metadata.cwd, None); assert_eq!(task_metadata.shell, task::Shell::System); assert_eq!( - task_metadata.command, - Some(expected_shell.clone()), + task_metadata.command, None, "Empty tasks should spawn a -i shell" ); assert_eq!(task_metadata.args, Vec::::new()); - assert_eq!( - task_metadata.command_label, expected_shell, - "We show the shell launch for empty commands" - ); + assert_eq!(task_metadata.command_label, ""); }) .unwrap(); } @@ -1848,6 +1808,8 @@ mod tests { #[cfg(unix)] #[gpui::test] async fn test_spawn_script_like_task(cx: &mut TestAppContext) { + use task::{TaskContext, TaskTemplate}; + init_test(cx); let fs = FakeFs::new(cx.executor()); @@ -1862,21 +1824,19 @@ mod tests { }) .unwrap(); - let user_command = r#"REPO_URL=$(git remote get-url origin | sed -e \"s/^git@\\(.*\\):\\(.*\\)\\.git$/https:\\/\\/\\1\\/\\2/\"); COMMIT_SHA=$(git log -1 --format=\"%H\" -- \"${ZED_RELATIVE_FILE}\"); echo \"${REPO_URL}/blob/${COMMIT_SHA}/${ZED_RELATIVE_FILE}#L${ZED_ROW}-$(echo $(($(wc -l <<< \"$ZED_SELECTED_TEXT\") + $ZED_ROW - 1)))\" | xclip -selection clipboard"#.to_string(); - - let expected_cwd = PathBuf::from("/some/work"); + let user_command = r#"REPO_URL=$(git remote get-url origin | sed -e \"s/^git@\\(.*\\):\\(.*\\)\\.git$/https:\\/\\/\\1\\/\\2/\"); COMMIT_SHA=$(git log -1 --format=\"%H\" -- \"ZED_RELATIVE_FILE\"); echo \"REPO_URL/blob/COMMIT_SHA/ZED_RELATIVE_FILE#LZED_ROW-$(echo $(($(wc -l <<< \"ZED_SELECTED_TEXT\") + ZED_ROW - 1)))\" | xclip -selection clipboard"#.to_string(); + let template = TaskTemplate { + command: user_command.clone(), + cwd: Some("/some/work".to_owned()), + label: "some task".to_owned(), + ..TaskTemplate::default() + } + .resolve_task("id", &|| None, &TaskContext::default()) + .unwrap(); let task = window_handle .update(cx, |_, window, cx| { terminal_panel.update(cx, |terminal_panel, cx| { - terminal_panel.spawn_task( - &SpawnInTerminal { - command: Some(user_command.clone()), - cwd: Some(expected_cwd.clone()), - ..SpawnInTerminal::default() - }, - window, - cx, - ) + terminal_panel.spawn_task(template.resolved, window, cx) }) }) .unwrap(); @@ -1891,7 +1851,7 @@ mod tests { .spawned_task .clone(); assert_eq!(task_metadata.env, HashMap::default()); - assert_eq!(task_metadata.cwd, Some(expected_cwd)); + assert_eq!(task_metadata.cwd.as_deref(), Some("/some/work".as_ref())); assert_eq!(task_metadata.shell, task::Shell::System); assert_eq!(task_metadata.command, Some(shell.clone())); assert_eq!( diff --git a/crates/workspace/src/tasks.rs b/crates/workspace/src/tasks.rs index 5f52cb49e74d67619b9ba7c033a33fe8a7ad51c8..7ef86e92020a347a7093d3ff2ef404a95301fd79 100644 --- a/crates/workspace/src/tasks.rs +++ b/crates/workspace/src/tasks.rs @@ -20,7 +20,8 @@ impl Workspace { window: &mut Window, cx: &mut Context, ) { - match self.project.read(cx).remote_connection_state(cx) { + let project = self.project.read(cx); + match project.remote_connection_state(cx) { None | Some(ConnectionState::Connected) => {} Some( ConnectionState::Connecting @@ -33,9 +34,11 @@ impl Workspace { } } - if let Some(spawn_in_terminal) = - task_to_resolve.resolve_task(&task_source_kind.to_id_base(), task_cx) - { + if let Some(spawn_in_terminal) = task_to_resolve.resolve_task( + &task_source_kind.to_id_base(), + &|| project.remote_client()?.read(cx).shell(), + task_cx, + ) { self.schedule_resolved_task( task_source_kind, spawn_in_terminal,