Move task centering code closer to user input (#22082)

Follow-up of #22004 

* Reuse center terminals for tasks, when requested
* Extend task templates with `RevealTarget`, moving it from
`TaskSpawnTarget` into the core library
* Use `reveal_target` instead of `target` to avoid misinterpretations in
the task template context
* Do not expose `SpawnInTerminal` to user interface, avoid it
implementing `Serialize` and `Deserialize`
* Remove `NewCenterTask` action, extending `task::Spawn` interface
instead
* Do not require any extra unrelated parameters during task resolution,
instead, use task overrides on the resolved tasks on the modal side
* Add keybindings for opening the task modal in the
`RevealTarget::Center` mode

Release Notes:

- N/A

assets/keymaps/default-linux.json

@@ -426,7 +426,10 @@
       "ctrl-shift-r": "task::Rerun",
       "ctrl-alt-r": "task::Rerun",
       "alt-t": "task::Rerun",
-      "alt-shift-t": "task::Spawn"
+      "alt-shift-t": "task::Spawn",
+      "alt-shift-r": ["task::Spawn", { "reveal_target": "center" }]
+      // also possible to spawn tasks by name:
+      // "foo-bar": ["task::Spawn", { "task_name": "MyTask", "reveal_target": "dock" }]
     }
   },
   // Bindings from Sublime Text

assets/keymaps/default-macos.json

@@ -495,8 +495,9 @@
     "bindings": {
       "cmd-shift-r": "task::Spawn",
       "cmd-alt-r": "task::Rerun",
-      "alt-t": "task::Spawn",
-      "alt-shift-t": "task::Spawn"
+      "ctrl-alt-shift-r": ["task::Spawn", { "reveal_target": "center" }]
+      // also possible to spawn tasks by name:
+      // "foo-bar": ["task_name::Spawn", { "task_name": "MyTask", "reveal_target": "dock" }]
     }
   },
   // Bindings from Sublime Text

assets/settings/initial_tasks.json

@@ -15,10 +15,14 @@
     // Whether to allow multiple instances of the same task to be run, or rather wait for the existing ones to finish, defaults to `false`.
     "allow_concurrent_runs": false,
     // What to do with the terminal pane and tab, after the command was started:
-    // * `always` — always show the terminal pane, add and focus the corresponding task's tab in it (default)
-    // * `no_focus` — always show the terminal pane, add/reuse the task's tab there, but don't focus it
-    // * `never` — avoid changing current terminal pane focus, but still add/reuse the task's tab there
+    // * `always` — always show the task's pane, and focus the corresponding tab in it (default)
+    // * `no_focus` — always show the task's pane, add the task's tab in it, but don't focus it
+    // * `never` — do not alter focus, but still add/reuse the task's tab in its pane
     "reveal": "always",
+    // Where to place the task's terminal item after starting the task:
+    // * `dock` — in the terminal dock, "regular" terminal items' place (default)
+    // * `center` — in the central pane group, "main" editor area
+    "reveal_target": "dock",
     // What to do with the terminal pane and tab, after the command had finished:
     // * `never` — Do nothing when the command finishes (default)
     // * `always` — always hide the terminal tab, hide the pane also if it was the last tab in it

crates/editor/src/editor.rs

@@ -522,7 +522,7 @@ impl RunnableTasks {
     ) -> impl Iterator<Item = (TaskSourceKind, ResolvedTask)> + 'a {
         self.templates.iter().filter_map(|(kind, template)| {
             template
-                .resolve_task(&kind.to_id_base(), Default::default(), cx)
+                .resolve_task(&kind.to_id_base(), cx)
                 .map(|task| (kind.clone(), task))
         })
     }

crates/project/src/task_inventory.rs

@@ -184,7 +184,7 @@ impl Inventory {
                 let id_base = kind.to_id_base();
                 Some((
                     kind,
-                    task.resolve_task(&id_base, Default::default(), task_context)?,
+                    task.resolve_task(&id_base, task_context)?,
                     not_used_score,
                 ))
             })
@@ -378,7 +378,7 @@ mod test_inventory {
 
     use crate::Inventory;
 
-    use super::{task_source_kind_preference, TaskSourceKind};
+    use super::TaskSourceKind;
 
     pub(super) fn task_template_names(
         inventory: &Model<Inventory>,
@@ -409,7 +409,7 @@ mod test_inventory {
             let id_base = task_source_kind.to_id_base();
             inventory.task_scheduled(
                 task_source_kind.clone(),
-                task.resolve_task(&id_base, Default::default(), &TaskContext::default())
+                task.resolve_task(&id_base, &TaskContext::default())
                     .unwrap_or_else(|| panic!("Failed to resolve task with name {task_name}")),
             );
         });
@@ -427,31 +427,12 @@ 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, Default::default(), task_context)?,
-                    ))
+                    Some((source_kind, task.resolve_task(&id_base, task_context)?))
                 })
                 .map(|(source_kind, resolved_task)| (source_kind, resolved_task.resolved_label))
                 .collect()
         })
     }
-
-    pub(super) async fn list_tasks_sorted_by_last_used(
-        inventory: &Model<Inventory>,
-        worktree: Option<WorktreeId>,
-        cx: &mut TestAppContext,
-    ) -> Vec<(TaskSourceKind, String)> {
-        let (used, current) = inventory.update(cx, |inventory, cx| {
-            inventory.used_and_current_resolved_tasks(worktree, None, &TaskContext::default(), cx)
-        });
-        let mut all = used;
-        all.extend(current);
-        all.into_iter()
-            .map(|(source_kind, task)| (source_kind, task.resolved_label))
-            .sorted_by_key(|(kind, label)| (task_source_kind_preference(kind), label.clone()))
-            .collect()
-    }
 }
 
 /// A context provided that tries to provide values for all non-custom [`VariableName`] variants for a currently opened file.
@@ -877,7 +858,7 @@ mod tests {
         TaskStore::init(None);
     }
 
-    pub(super) async fn resolved_task_names(
+    async fn resolved_task_names(
         inventory: &Model<Inventory>,
         worktree: Option<WorktreeId>,
         cx: &mut TestAppContext,
@@ -905,4 +886,20 @@ mod tests {
         ))
         .unwrap()
     }
+
+    async fn list_tasks_sorted_by_last_used(
+        inventory: &Model<Inventory>,
+        worktree: Option<WorktreeId>,
+        cx: &mut TestAppContext,
+    ) -> Vec<(TaskSourceKind, String)> {
+        let (used, current) = inventory.update(cx, |inventory, cx| {
+            inventory.used_and_current_resolved_tasks(worktree, None, &TaskContext::default(), cx)
+        });
+        let mut all = used;
+        all.extend(current);
+        all.into_iter()
+            .map(|(source_kind, task)| (source_kind, task.resolved_label))
+            .sorted_by_key(|(kind, label)| (task_source_kind_preference(kind), label.clone()))
+            .collect()
+    }
 }

crates/task/src/lib.rs

@@ -15,14 +15,15 @@ use std::str::FromStr;
 
 pub use task_template::{HideStrategy, RevealStrategy, TaskTemplate, TaskTemplates};
 pub use vscode_format::VsCodeTaskFile;
+pub use zed_actions::RevealTarget;
 
 /// Task identifier, unique within the application.
 /// Based on it, task reruns and terminal tabs are managed.
-#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Deserialize, Serialize)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Deserialize)]
 pub struct TaskId(pub String);
 
 /// Contains all information needed by Zed to spawn a new terminal tab for the given task.
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct SpawnInTerminal {
     /// Id of the task to use when determining task tab affinity.
     pub id: TaskId,
@@ -47,6 +48,8 @@ pub struct SpawnInTerminal {
     pub allow_concurrent_runs: bool,
     /// What to do with the terminal pane and tab, after the command was started.
     pub reveal: RevealStrategy,
+    /// Where to show tasks' terminal output.
+    pub reveal_target: RevealTarget,
     /// What to do with the terminal pane and tab, after the command had finished.
     pub hide: HideStrategy,
     /// Which shell to use when spawning the task.
@@ -57,15 +60,6 @@ pub struct SpawnInTerminal {
     pub show_command: bool,
 }
 
-/// An action for spawning a specific task
-#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
-pub struct NewCenterTask {
-    /// The specification of the task to spawn.
-    pub action: SpawnInTerminal,
-}
-
-gpui::impl_actions!(tasks, [NewCenterTask]);
-
 /// A final form of the [`TaskTemplate`], that got resolved with a particualar [`TaskContext`] and now is ready to spawn the actual task.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ResolvedTask {
@@ -84,9 +78,6 @@ pub struct ResolvedTask {
     /// Further actions that need to take place after the resolved task is spawned,
     /// with all task variables resolved.
     pub resolved: Option<SpawnInTerminal>,
-
-    /// where to sawn the task in the UI, either in the terminal panel or in the center pane
-    pub target: zed_actions::TaskSpawnTarget,
 }
 
 impl ResolvedTask {

crates/task/src/task_template.rs

@@ -9,7 +9,7 @@ use sha2::{Digest, Sha256};
 use util::{truncate_and_remove_front, ResultExt};
 
 use crate::{
-    ResolvedTask, Shell, SpawnInTerminal, TaskContext, TaskId, VariableName,
+    ResolvedTask, RevealTarget, Shell, SpawnInTerminal, TaskContext, TaskId, VariableName,
     ZED_VARIABLE_NAME_PREFIX,
 };
 
@@ -42,10 +42,16 @@ pub struct TaskTemplate {
     #[serde(default)]
     pub allow_concurrent_runs: bool,
     /// What to do with the terminal pane and tab, after the command was started:
-    /// * `always` — always show the terminal pane, add and focus the corresponding task's tab in it (default)
-    /// * `never` — avoid changing current terminal pane focus, but still add/reuse the task's tab there
+    /// * `always` — always show the task's pane, and focus the corresponding tab in it (default)
+    // * `no_focus` — always show the task's pane, add the task's tab in it, but don't focus it
+    // * `never` — do not alter focus, but still add/reuse the task's tab in its pane
     #[serde(default)]
     pub reveal: RevealStrategy,
+    /// Where to place the task's terminal item after starting the task.
+    /// * `dock` — in the terminal dock, "regular" terminal items' place (default).
+    /// * `center` — in the central pane group, "main" editor area.
+    #[serde(default)]
+    pub reveal_target: RevealTarget,
     /// What to do with the terminal pane and tab, after the command had finished:
     /// * `never` — do nothing when the command finishes (default)
     /// * `always` — always hide the terminal tab, hide the pane also if it was the last tab in it
@@ -70,12 +76,12 @@ pub struct TaskTemplate {
 #[derive(Default, Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
 #[serde(rename_all = "snake_case")]
 pub enum RevealStrategy {
-    /// Always show the terminal pane, add and focus the corresponding task's tab in it.
+    /// Always show the task's pane, and focus the corresponding tab in it.
     #[default]
     Always,
-    /// Always show the terminal pane, add the task's tab in it, but don't focus it.
+    /// Always show the task's pane, add the task's tab in it, but don't focus it.
     NoFocus,
-    /// Do not change terminal pane focus, but still add/reuse the task's tab there.
+    /// Do not alter focus, but still add/reuse the task's tab in its pane.
     Never,
 }
 
@@ -115,12 +121,7 @@ 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,
-        target: zed_actions::TaskSpawnTarget,
-        cx: &TaskContext,
-    ) -> Option<ResolvedTask> {
+    pub fn resolve_task(&self, id_base: &str, cx: &TaskContext) -> Option<ResolvedTask> {
         if self.label.trim().is_empty() || self.command.trim().is_empty() {
             return None;
         }
@@ -219,7 +220,6 @@ impl TaskTemplate {
         Some(ResolvedTask {
             id: id.clone(),
             substituted_variables,
-            target,
             original_task: self.clone(),
             resolved_label: full_label.clone(),
             resolved: Some(SpawnInTerminal {
@@ -241,6 +241,7 @@ impl TaskTemplate {
                 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(),
                 show_summary: self.show_summary,
@@ -388,7 +389,7 @@ mod tests {
             },
         ] {
             assert_eq!(
-                task_with_blank_property.resolve_task(TEST_ID_BASE, Default::default(), &TaskContext::default()),
+                task_with_blank_property.resolve_task(TEST_ID_BASE, &TaskContext::default()),
                 None,
                 "should not resolve task with blank label and/or command: {task_with_blank_property:?}"
             );
@@ -406,7 +407,7 @@ mod tests {
 
         let resolved_task = |task_template: &TaskTemplate, task_cx| {
             let resolved_task = task_template
-                .resolve_task(TEST_ID_BASE, Default::default(), task_cx)
+                .resolve_task(TEST_ID_BASE, task_cx)
                 .unwrap_or_else(|| panic!("failed to resolve task {task_without_cwd:?}"));
             assert_substituted_variables(&resolved_task, Vec::new());
             resolved_task
@@ -532,7 +533,6 @@ mod tests {
         for i in 0..15 {
             let resolved_task = task_with_all_variables.resolve_task(
                 TEST_ID_BASE,
-                Default::default(),
                 &TaskContext {
                     cwd: None,
                     task_variables: TaskVariables::from_iter(all_variables.clone()),
@@ -621,7 +621,6 @@ mod tests {
             let removed_variable = not_all_variables.remove(i);
             let resolved_task_attempt = task_with_all_variables.resolve_task(
                 TEST_ID_BASE,
-                Default::default(),
                 &TaskContext {
                     cwd: None,
                     task_variables: TaskVariables::from_iter(not_all_variables),
@@ -638,10 +637,10 @@ mod tests {
             label: "My task".into(),
             command: "echo".into(),
             args: vec!["$PATH".into()],
-            ..Default::default()
+            ..TaskTemplate::default()
         };
         let resolved_task = task
-            .resolve_task(TEST_ID_BASE, Default::default(), &TaskContext::default())
+            .resolve_task(TEST_ID_BASE, &TaskContext::default())
             .unwrap();
         assert_substituted_variables(&resolved_task, Vec::new());
         let resolved = resolved_task.resolved.unwrap();
@@ -656,10 +655,10 @@ mod tests {
             label: "My task".into(),
             command: "echo".into(),
             args: vec!["$ZED_VARIABLE".into()],
-            ..Default::default()
+            ..TaskTemplate::default()
         };
         assert!(task
-            .resolve_task(TEST_ID_BASE, Default::default(), &TaskContext::default())
+            .resolve_task(TEST_ID_BASE, &TaskContext::default())
             .is_none());
     }
 
@@ -709,7 +708,7 @@ mod tests {
         .enumerate()
         {
             let resolved = symbol_dependent_task
-                .resolve_task(TEST_ID_BASE, Default::default(), &cx)
+                .resolve_task(TEST_ID_BASE, &cx)
                 .unwrap_or_else(|| panic!("Failed to resolve task {symbol_dependent_task:?}"));
             assert_eq!(
                 resolved.substituted_variables,
@@ -751,9 +750,7 @@ mod tests {
         context
             .task_variables
             .insert(VariableName::Symbol, "my-symbol".to_string());
-        assert!(faulty_go_test
-            .resolve_task("base", Default::default(), &context)
-            .is_some());
+        assert!(faulty_go_test.resolve_task("base", &context).is_some());
     }
 
     #[test]
@@ -812,7 +809,7 @@ mod tests {
         };
 
         let resolved = template
-            .resolve_task(TEST_ID_BASE, Default::default(), &context)
+            .resolve_task(TEST_ID_BASE, &context)
             .unwrap()
             .resolved
             .unwrap();