tasks: Reorganize task modal (#11752)

![image](https://github.com/zed-industries/zed/assets/24362066/bc7cc3d3-d9fc-4be6-b9b6-e3d8edf5b533)

Release Notes:
- Improved tasks modal by highlighting a distinction between a task
template and concrete task instance and surfacing available keybindings
more prominently. Task templates are now always available in the modal,
even if there's already a history entry with the same label.
- Changed default key binding for "picker::UseSelectedQuery" to `opt-e`.

Cargo.lock

@@ -10088,6 +10088,7 @@ dependencies = [
  "fuzzy",
  "gpui",
  "language",
+ "menu",
  "picker",
  "project",
  "schemars",

assets/icons/history_rerun.svg

@@ -0,0 +1,5 @@
+<svg width="12" height="12" viewBox="0 0 12 12" fill="none" xmlns="http://www.w3.org/2000/svg">
+<path d="M1.5 6C1.5 6.89002 1.76392 7.76004 2.25839 8.50007C2.75285 9.24009 3.45566 9.81686 4.27792 10.1575C5.10019 10.4981 6.00499 10.5872 6.87791 10.4135C7.75082 10.2399 8.55264 9.81132 9.18198 9.18198C9.81132 8.55264 10.2399 7.75082 10.4135 6.87791C10.5872 6.00499 10.4981 5.10019 10.1575 4.27792C9.81686 3.45566 9.24009 2.75285 8.50007 2.25839C7.76004 1.76392 6.89002 1.5 6 1.5C4.74198 1.50473 3.53448 1.99561 2.63 2.87L1.5 4" stroke="#919081" stroke-linecap="round" stroke-linejoin="round"/>
+<path d="M1.5 1.5V4H4" stroke="#919081" stroke-linecap="round" stroke-linejoin="round"/>
+<path d="M6 3.5V6L8 7" stroke="#919081" stroke-linecap="round" stroke-linejoin="round"/>
+</svg>

assets/keymaps/default-macos.json

@@ -19,9 +19,6 @@
       "cmd-escape": "menu::Cancel",
       "ctrl-escape": "menu::Cancel",
       "ctrl-c": "menu::Cancel",
-      "shift-enter": "picker::UseSelectedQuery",
-      "alt-enter": ["picker::ConfirmInput", { "secondary": false }],
-      "cmd-alt-enter": ["picker::ConfirmInput", { "secondary": true }],
       "cmd-shift-w": "workspace::CloseWindow",
       "shift-escape": "workspace::ToggleZoom",
       "cmd-o": "workspace::Open",
@@ -630,6 +627,14 @@
       "ctrl-backspace": "tab_switcher::CloseSelectedItem"
     }
   },
+  {
+    "context": "Picker",
+    "bindings": {
+      "alt-e": "picker::UseSelectedQuery",
+      "alt-enter": ["picker::ConfirmInput", { "secondary": false }],
+      "cmd-alt-enter": ["picker::ConfirmInput", { "secondary": true }]
+    }
+  },
   {
     "context": "Terminal",
     "bindings": {

crates/languages/src/bash.rs

@@ -6,7 +6,6 @@ pub(super) fn bash_task_context() -> ContextProviderWithTasks {
         TaskTemplate {
             label: "execute selection".to_owned(),
             command: VariableName::SelectedText.template_value(),
-            ignore_previously_resolved: true,
             ..TaskTemplate::default()
         },
         TaskTemplate {

crates/languages/src/python.rs

@@ -187,7 +187,6 @@ pub(super) fn python_task_context() -> ContextProviderWithTasks {
             label: "execute selection".to_owned(),
             command: "python3".to_owned(),
             args: vec!["-c".to_owned(), VariableName::SelectedText.template_value()],
-            ignore_previously_resolved: true,
             ..TaskTemplate::default()
         },
         TaskTemplate {

crates/project/src/task_inventory.rs

@@ -8,7 +8,7 @@ use std::{
 
 use collections::{btree_map, BTreeMap, VecDeque};
 use gpui::{AppContext, Context, Model, ModelContext};
-use itertools::{Either, Itertools};
+use itertools::Itertools;
 use language::Language;
 use task::{
     static_source::StaticSource, ResolvedTask, TaskContext, TaskId, TaskTemplate, VariableName,
@@ -188,7 +188,6 @@ impl Inventory {
             .last_scheduled_tasks
             .iter()
             .rev()
-            .filter(|(_, task)| !task.original_task().ignore_previously_resolved)
             .filter(|(task_kind, _)| {
                 if matches!(task_kind, TaskSourceKind::Language { .. }) {
                     Some(task_kind) == task_source_kind.as_ref()
@@ -256,38 +255,46 @@ impl Inventory {
                 tasks_by_label
             },
         );
-        tasks_by_label = currently_resolved_tasks.into_iter().fold(
+        tasks_by_label = currently_resolved_tasks.iter().fold(
             tasks_by_label,
             |mut tasks_by_label, (source, task, lru_score)| {
-                match tasks_by_label.entry((source, task.resolved_label.clone())) {
+                match tasks_by_label.entry((source.clone(), task.resolved_label.clone())) {
                     btree_map::Entry::Occupied(mut o) => {
                         let (previous_task, _) = o.get();
                         let new_template = task.original_task();
-                        if new_template.ignore_previously_resolved
-                            || new_template != previous_task.original_task()
-                        {
-                            o.insert((task, lru_score));
+                        if new_template != previous_task.original_task() {
+                            o.insert((task.clone(), *lru_score));
                         }
                     }
                     btree_map::Entry::Vacant(v) => {
-                        v.insert((task, lru_score));
+                        v.insert((task.clone(), *lru_score));
                     }
                 }
                 tasks_by_label
             },
         );
 
-        tasks_by_label
+        let resolved = tasks_by_label
             .into_iter()
             .map(|((kind, _), (task, lru_score))| (kind, task, lru_score))
-            .sorted_unstable_by(task_lru_comparator)
-            .partition_map(|(kind, task, lru_score)| {
+            .sorted_by(task_lru_comparator)
+            .filter_map(|(kind, task, lru_score)| {
                 if lru_score < not_used_score {
-                    Either::Left((kind, task))
+                    Some((kind, task))
                 } else {
-                    Either::Right((kind, task))
+                    None
                 }
             })
+            .collect();
+
+        (
+            resolved,
+            currently_resolved_tasks
+                .into_iter()
+                .sorted_unstable_by(task_lru_comparator)
+                .map(|(kind, task, _)| (kind, task))
+                .collect(),
+        )
     }
 
     /// Returns the last scheduled task, if any of the sources contains one with the matching id.
@@ -334,6 +341,7 @@ fn task_lru_comparator(
                     &task_b.resolved_label,
                 ))
                 .then(task_a.resolved_label.cmp(&task_b.resolved_label))
+                .then(kind_a.cmp(kind_b))
         })
 }
 
@@ -530,6 +538,7 @@ mod tests {
         assert_eq!(
             resolved_task_names(&inventory, None, cx),
             vec![
+                "2_task".to_string(),
                 "2_task".to_string(),
                 "1_a_task".to_string(),
                 "1_task".to_string(),
@@ -548,6 +557,9 @@ mod tests {
         assert_eq!(
             resolved_task_names(&inventory, None, cx),
             vec![
+                "3_task".to_string(),
+                "1_task".to_string(),
+                "2_task".to_string(),
                 "3_task".to_string(),
                 "1_task".to_string(),
                 "2_task".to_string(),
@@ -578,6 +590,9 @@ mod tests {
         assert_eq!(
             resolved_task_names(&inventory, None, cx),
             vec![
+                "3_task".to_string(),
+                "1_task".to_string(),
+                "2_task".to_string(),
                 "3_task".to_string(),
                 "1_task".to_string(),
                 "2_task".to_string(),
@@ -595,6 +610,10 @@ mod tests {
         assert_eq!(
             resolved_task_names(&inventory, None, cx),
             vec![
+                "11_hello".to_string(),
+                "3_task".to_string(),
+                "1_task".to_string(),
+                "2_task".to_string(),
                 "11_hello".to_string(),
                 "3_task".to_string(),
                 "1_task".to_string(),

crates/task/src/task_template.rs

@@ -39,20 +39,6 @@ pub struct TaskTemplate {
     /// Whether to allow multiple instances of the same task to be run, or rather wait for the existing ones to finish.
     #[serde(default)]
     pub allow_concurrent_runs: bool,
-    // Tasks like "execute the selection" better have the constant labels (to avoid polluting the history with temporary tasks),
-    // and always use the latest context with the latest selection.
-    //
-    // Current impl will always pick previously spawned tasks on full label conflict in the tasks modal and terminal tabs, never
-    // getting the latest selection for them.
-    // This flag inverts the behavior, effectively removing all previously spawned tasks from history,
-    // if their full labels are the same as the labels of the newly resolved tasks.
-    // Such tasks are still re-runnable, and will use the old context in that case (unless the rerun task forces this).
-    //
-    // Current approach is relatively hacky, a better way is understand when the new resolved tasks needs a rerun,
-    // and replace the historic task accordingly.
-    #[doc(hidden)]
-    #[serde(default)]
-    pub ignore_previously_resolved: 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

crates/tasks_ui/Cargo.toml

@@ -13,6 +13,7 @@ editor.workspace = true
 file_icons.workspace = true
 fuzzy.workspace = true
 gpui.workspace = true
+menu.workspace = true
 picker.workspace = true
 project.workspace = true
 task.workspace = true

crates/tasks_ui/src/modal.rs

@@ -3,17 +3,18 @@ use std::sync::Arc;
 use crate::active_item_selection_properties;
 use fuzzy::{StringMatch, StringMatchCandidate};
 use gpui::{
-    impl_actions, rems, AppContext, DismissEvent, EventEmitter, FocusableView, InteractiveElement,
-    Model, ParentElement, Render, SharedString, Styled, Subscription, View, ViewContext,
-    VisualContext, WeakView,
+    impl_actions, rems, AnyElement, AppContext, DismissEvent, EventEmitter, FocusableView,
+    InteractiveElement, Model, ParentElement, Render, SharedString, Styled, Subscription, View,
+    ViewContext, VisualContext, WeakView,
 };
 use picker::{highlighted_match_with_paths::HighlightedText, Picker, PickerDelegate};
 use project::{Inventory, TaskSourceKind};
 use task::{ResolvedTask, TaskContext, TaskTemplate};
 use ui::{
-    div, v_flex, ButtonCommon, ButtonSize, Clickable, Color, FluentBuilder as _, Icon, IconButton,
-    IconButtonShape, IconName, IconSize, ListItem, ListItemSpacing, RenderOnce, Selectable,
-    Tooltip, WindowContext,
+    div, h_flex, v_flex, ActiveTheme, Button, ButtonCommon, ButtonSize, Clickable, Color,
+    FluentBuilder as _, Icon, IconButton, IconButtonShape, IconName, IconSize, IntoElement,
+    KeyBinding, LabelSize, ListItem, ListItemSpacing, RenderOnce, Selectable, Tooltip,
+    WindowContext,
 };
 use util::ResultExt;
 use workspace::{tasks::schedule_resolved_task, ModalView, Workspace};
@@ -87,7 +88,7 @@ impl TasksModalDelegate {
             selected_index: 0,
             prompt: String::default(),
             task_context,
-            placeholder_text: Arc::from("Run a task..."),
+            placeholder_text: Arc::from("Find a task, or run a command"),
         }
     }
 
@@ -352,12 +353,37 @@ impl PickerDelegate for TasksModalDelegate {
             TaskSourceKind::Language { name } => file_icons::FileIcons::get(cx)
                 .get_type_icon(&name.to_lowercase())
                 .map(|icon_path| Icon::from_path(icon_path)),
+        }
+        .map(|icon| icon.color(Color::Muted).size(IconSize::Small));
+        let history_run_icon = if Some(ix) <= self.divider_index {
+            Some(
+                Icon::new(IconName::HistoryRerun)
+                    .color(Color::Muted)
+                    .size(IconSize::Small)
+                    .into_any_element(),
+            )
+        } else {
+            Some(
+                v_flex()
+                    .flex_none()
+                    .size(IconSize::Small.rems())
+                    .into_any_element(),
+            )
         };
 
         Some(
             ListItem::new(SharedString::from(format!("tasks-modal-{ix}")))
-                .inset(true)
+                .inset(false)
+                .start_slot::<Icon>(icon)
+                .end_slot::<AnyElement>(history_run_icon)
                 .spacing(ListItemSpacing::Sparse)
+                // .map(|this| {
+                //     if Some(ix) <= self.divider_index {
+                //         this.start_slot(Icon::new(IconName::HistoryRerun).size(IconSize::Small))
+                //     } else {
+                //         this.start_slot(v_flex().flex_none().size(IconSize::Small.rems()))
+                //     }
+                // })
                 .when_some(tooltip_label, |list_item, item_label| {
                     list_item.tooltip(move |_| item_label.clone())
                 })
@@ -392,11 +418,7 @@ impl PickerDelegate for TasksModalDelegate {
                     } else {
                         item
                     };
-                    if let Some(icon) = icon {
-                        item.end_slot(icon)
-                    } else {
-                        item
-                    }
+                    item
                 })
                 .selected(selected)
                 .child(highlighted_location.render(cx)),
@@ -429,6 +451,80 @@ impl PickerDelegate for TasksModalDelegate {
             Vec::new()
         }
     }
+    fn render_footer(&self, cx: &mut ViewContext<Picker<Self>>) -> Option<gpui::AnyElement> {
+        let is_recent_selected = self.divider_index >= Some(self.selected_index);
+        let current_modifiers = cx.modifiers();
+        Some(
+            h_flex()
+                .w_full()
+                .h_8()
+                .p_2()
+                .justify_between()
+                .rounded_b_md()
+                .bg(cx.theme().colors().ghost_element_selected)
+                .children(
+                    KeyBinding::for_action(&picker::UseSelectedQuery, cx).map(|keybind| {
+                        let edit_entry_label = if is_recent_selected {
+                            "Edit task"
+                        } else if !self.matches.is_empty() {
+                            "Edit template"
+                        } else {
+                            "Rerun last task"
+                        };
+
+                        Button::new("edit-current-task", edit_entry_label)
+                            .label_size(LabelSize::Small)
+                            .key_binding(keybind)
+                    }),
+                )
+                .map(|this| {
+                    if current_modifiers.alt || self.matches.is_empty() {
+                        this.children(
+                            KeyBinding::for_action(
+                                &picker::ConfirmInput {
+                                    secondary: current_modifiers.secondary(),
+                                },
+                                cx,
+                            )
+                            .map(|keybind| {
+                                let spawn_oneshot_label = if current_modifiers.secondary() {
+                                    "Spawn oneshot without history"
+                                } else {
+                                    "Spawn oneshot"
+                                };
+
+                                Button::new("spawn-onehshot", spawn_oneshot_label)
+                                    .label_size(LabelSize::Small)
+                                    .key_binding(keybind)
+                            }),
+                        )
+                    } else if current_modifiers.secondary() {
+                        this.children(KeyBinding::for_action(&menu::SecondaryConfirm, cx).map(
+                            |keybind| {
+                                let label = if is_recent_selected {
+                                    "Rerun without history"
+                                } else {
+                                    "Spawn without history"
+                                };
+                                Button::new("spawn", label)
+                                    .label_size(LabelSize::Small)
+                                    .key_binding(keybind)
+                            },
+                        ))
+                    } else {
+                        this.children(KeyBinding::for_action(&menu::Confirm, cx).map(|keybind| {
+                            let run_entry_label =
+                                if is_recent_selected { "Rerun" } else { "Spawn" };
+
+                            Button::new("spawn", run_entry_label)
+                                .label_size(LabelSize::Small)
+                                .key_binding(keybind)
+                        }))
+                    }
+                })
+                .into_any_element(),
+        )
+    }
 }
 
 #[cfg(test)]
@@ -787,7 +883,7 @@ mod tests {
         let tasks_picker = open_spawn_tasks(&workspace, cx);
         assert_eq!(
             task_names(&tasks_picker, cx),
-            vec!["TypeScript task from file /dir/a1.ts", "Another task from file /dir/a1.ts", "Task without variables"],
+            vec!["TypeScript task from file /dir/a1.ts", "TypeScript task from file /dir/a1.ts", "Another task from file /dir/a1.ts", "Task without variables"],
             "After spawning the task and getting it into the history, it should be up in the sort as recently used"
         );
         tasks_picker.update(cx, |_, cx| {

crates/ui/src/components/icon.rs

@@ -182,6 +182,7 @@ pub enum IconName {
     ZedXCopilot,
     ZedAssistant,
     PullRequest,
+    HistoryRerun,
 }
 
 impl IconName {
@@ -295,6 +296,7 @@ impl IconName {
             IconName::ZedXCopilot => "icons/zed_x_copilot.svg",
             IconName::ZedAssistant => "icons/zed_assistant.svg",
             IconName::PullRequest => "icons/pull_request.svg",
+            IconName::HistoryRerun => "icons/history_rerun.svg",
         }
     }
 }