From cff9ad19f83dc6509124b885a1e1329f104d8ad5 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Fri, 29 Mar 2024 18:41:14 +0100 Subject: [PATCH] Add spawning of tasks without saving them in the task stack (#9951) These tasks are not considered for reruns with `task::Rerun`. This PR tears a bunch of stuff up around tasks: - `menu::SecondaryConfirm` for tasks is gonna spawn a task without storing it in history instead of being occupied by oneshot tasks. This is done so that cmd-clicking on the menu item actually does something meaningful. - `menu::UseSelectedQuery` got moved into picker, as tasks are it's only user (and it doesn't really make sense as a menu action). TODO: - [x] add release note - [x] Actually implement the core of this feature, which is spawning a task without saving it in history, lol. Fixes #9804 Release Notes: - Added "fire-and-forget" task spawning; `menu::SecondaryConfirm` in tasks modal now spawns a task without registering it as the last spawned task for the purposes of `task::Rerun`. By default you can spawn a task in this fashion with cmd+enter or by holding cmd and clicking on a task entry in a list. Spawning oneshots has been rebound to `option-enter` (under a `picker::ConfirmInput` name). Fixes #9804 (breaking change) - Moved `menu::UseSelectedQuery` action to `picker` namespace (breaking change). --- Cargo.lock | 1 + assets/keymaps/default-linux.json | 4 +- assets/keymaps/default-macos.json | 4 +- crates/picker/Cargo.toml | 1 + crates/picker/src/picker.rs | 28 ++++++++-- crates/tasks_ui/src/lib.rs | 17 +++--- crates/tasks_ui/src/modal.rs | 87 ++++++++++++++++++++++--------- 7 files changed, 104 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bdade5e85c..e1b2f8711b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6884,6 +6884,7 @@ dependencies = [ "env_logger", "gpui", "menu", + "serde", "serde_json", "ui", "workspace", diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index e58aaca3b8..e063606c87 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -16,7 +16,9 @@ "escape": "menu::Cancel", "ctrl-escape": "menu::Cancel", "ctrl-c": "menu::Cancel", - "shift-enter": "menu::UseSelectedQuery", + "shift-enter": "picker::UseSelectedQuery", + "alt-enter": ["picker::ConfirmInput", { "secondary": false }], + "ctrl-alt-enter": ["picker::ConfirmInput", { "secondary": true }], "ctrl-shift-w": "workspace::CloseWindow", "shift-escape": "workspace::ToggleZoom", "ctrl-o": "workspace::Open", diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 450238d2f9..e73bb217ee 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -19,7 +19,9 @@ "cmd-escape": "menu::Cancel", "ctrl-escape": "menu::Cancel", "ctrl-c": "menu::Cancel", - "shift-enter": "menu::UseSelectedQuery", + "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", diff --git a/crates/picker/Cargo.toml b/crates/picker/Cargo.toml index 435e4459e9..cd83d1d805 100644 --- a/crates/picker/Cargo.toml +++ b/crates/picker/Cargo.toml @@ -17,6 +17,7 @@ anyhow.workspace = true editor.workspace = true gpui.workspace = true menu.workspace = true +serde.workspace = true ui.workspace = true workspace.workspace = true diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index dc2f894ab3..5b1d2c43f3 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -1,11 +1,12 @@ use anyhow::Result; use editor::{scroll::Autoscroll, Editor}; use gpui::{ - div, list, prelude::*, uniform_list, AnyElement, AppContext, ClickEvent, DismissEvent, - EventEmitter, FocusHandle, FocusableView, Length, ListState, MouseButton, MouseUpEvent, Render, - Task, UniformListScrollHandle, View, ViewContext, WindowContext, + actions, div, impl_actions, list, prelude::*, uniform_list, AnyElement, AppContext, ClickEvent, + DismissEvent, EventEmitter, FocusHandle, FocusableView, Length, ListState, MouseButton, + MouseUpEvent, Render, Task, UniformListScrollHandle, View, ViewContext, WindowContext, }; use head::Head; +use serde::Deserialize; use std::{sync::Arc, time::Duration}; use ui::{prelude::*, v_flex, Color, Divider, Label, ListItem, ListItemSpacing}; use workspace::ModalView; @@ -18,6 +19,17 @@ enum ElementContainer { UniformList(UniformListScrollHandle), } +actions!(picker, [UseSelectedQuery]); + +/// ConfirmInput is an alternative editor action which - instead of selecting active picker entry - treats pickers editor input literally, +/// performing some kind of action on it. +#[derive(PartialEq, Clone, Deserialize, Default)] +pub struct ConfirmInput { + pub secondary: bool, +} + +impl_actions!(picker, [ConfirmInput]); + struct PendingUpdateMatches { delegate_update_matches: Option>, _task: Task>, @@ -65,6 +77,9 @@ pub trait PickerDelegate: Sized + 'static { } fn confirm(&mut self, secondary: bool, cx: &mut ViewContext>); + /// Instead of interacting with currently selected entry, treats editor input literally, + /// performing some kind of action on it. + fn confirm_input(&mut self, _secondary: bool, _: &mut ViewContext>) {} fn dismissed(&mut self, cx: &mut ViewContext>); fn selected_as_query(&self) -> Option { None @@ -278,7 +293,11 @@ impl Picker { } } - fn use_selected_query(&mut self, _: &menu::UseSelectedQuery, cx: &mut ViewContext) { + fn confirm_input(&mut self, input: &ConfirmInput, cx: &mut ViewContext) { + self.delegate.confirm_input(input.secondary, cx); + } + + fn use_selected_query(&mut self, _: &UseSelectedQuery, cx: &mut ViewContext) { if let Some(new_query) = self.delegate.selected_as_query() { self.set_query(new_query, cx); cx.stop_propagation(); @@ -472,6 +491,7 @@ impl Render for Picker { .on_action(cx.listener(Self::confirm)) .on_action(cx.listener(Self::secondary_confirm)) .on_action(cx.listener(Self::use_selected_query)) + .on_action(cx.listener(Self::confirm_input)) .child(match &self.head { Head::Editor(editor) => v_flex() .child( diff --git a/crates/tasks_ui/src/lib.rs b/crates/tasks_ui/src/lib.rs index a6f26d7bc9..99be570a9a 100644 --- a/crates/tasks_ui/src/lib.rs +++ b/crates/tasks_ui/src/lib.rs @@ -31,7 +31,7 @@ pub fn init(cx: &mut AppContext) { old_context }; - schedule_task(workspace, task.as_ref(), task_context, cx) + schedule_task(workspace, task.as_ref(), task_context, false, cx) }; }); }, @@ -70,7 +70,7 @@ fn spawn_task_with_name(name: String, cx: &mut ViewContext) { let (_, target_task) = tasks.into_iter().find(|(_, task)| task.name() == name)?; let cwd = task_cwd(this, cx).log_err().flatten(); let task_context = task_context(this, cwd, cx); - schedule_task(this, target_task.as_ref(), task_context, cx); + schedule_task(this, target_task.as_ref(), task_context, false, cx); Some(()) }) .ok() @@ -195,15 +195,18 @@ fn schedule_task( workspace: &Workspace, task: &dyn Task, task_cx: TaskContext, + omit_history: bool, cx: &mut ViewContext<'_, Workspace>, ) { let spawn_in_terminal = task.exec(task_cx.clone()); if let Some(spawn_in_terminal) = spawn_in_terminal { - workspace.project().update(cx, |project, cx| { - project.task_inventory().update(cx, |inventory, _| { - inventory.task_scheduled(task.id().clone(), task_cx); - }) - }); + if !omit_history { + workspace.project().update(cx, |project, cx| { + project.task_inventory().update(cx, |inventory, _| { + inventory.task_scheduled(task.id().clone(), task_cx); + }) + }); + } cx.emit(workspace::Event::SpawnTask(spawn_in_terminal)); } } diff --git a/crates/tasks_ui/src/modal.rs b/crates/tasks_ui/src/modal.rs index c8d5df4c0c..31a3e9158b 100644 --- a/crates/tasks_ui/src/modal.rs +++ b/crates/tasks_ui/src/modal.rs @@ -169,8 +169,8 @@ impl PickerDelegate for TasksModalDelegate { fn placeholder_text(&self, cx: &mut WindowContext) -> Arc { Arc::from(format!( "{} use task name as prompt, {} spawns a bash-like task from the prompt, {} runs the selected task", - cx.keystroke_text_for(&menu::UseSelectedQuery), - cx.keystroke_text_for(&menu::SecondaryConfirm), + cx.keystroke_text_for(&picker::UseSelectedQuery), + cx.keystroke_text_for(&picker::ConfirmInput {secondary: false}), cx.keystroke_text_for(&menu::Confirm), )) } @@ -236,32 +236,30 @@ impl PickerDelegate for TasksModalDelegate { }) } - fn confirm(&mut self, secondary: bool, cx: &mut ViewContext>) { + fn confirm(&mut self, omit_history_entry: bool, cx: &mut ViewContext>) { let current_match_index = self.selected_index(); - let task = if secondary { - if !self.prompt.trim().is_empty() { - self.spawn_oneshot(cx) - } else { - None - } - } else { - self.matches - .get(current_match_index) - .and_then(|current_match| { - let ix = current_match.candidate_id; - self.candidates - .as_ref() - .map(|candidates| candidates[ix].1.clone()) - }) - }; - + let task = self + .matches + .get(current_match_index) + .and_then(|current_match| { + let ix = current_match.candidate_id; + self.candidates + .as_ref() + .map(|candidates| candidates[ix].1.clone()) + }); let Some(task) = task else { return; }; self.workspace .update(cx, |workspace, cx| { - schedule_task(workspace, task.as_ref(), self.task_context.clone(), cx); + schedule_task( + workspace, + task.as_ref(), + self.task_context.clone(), + omit_history_entry, + cx, + ); }) .ok(); cx.emit(DismissEvent); @@ -325,6 +323,23 @@ impl PickerDelegate for TasksModalDelegate { } Some(spawn_prompt.command) } + fn confirm_input(&mut self, omit_history_entry: bool, cx: &mut ViewContext>) { + let Some(task) = self.spawn_oneshot(cx) else { + return; + }; + self.workspace + .update(cx, |workspace, cx| { + schedule_task( + workspace, + task.as_ref(), + self.task_context.clone(), + omit_history_entry, + cx, + ); + }) + .ok(); + cx.emit(DismissEvent); + } } #[cfg(test)] @@ -391,7 +406,7 @@ mod tests { "Only one task should match the query {query_str}" ); - cx.dispatch_action(menu::UseSelectedQuery); + cx.dispatch_action(picker::UseSelectedQuery); assert_eq!( query(&tasks_picker, cx), "echo 4", @@ -402,7 +417,7 @@ mod tests { Vec::::new(), "No task should be listed" ); - cx.dispatch_action(menu::SecondaryConfirm); + cx.dispatch_action(picker::ConfirmInput { secondary: false }); let tasks_picker = open_spawn_tasks(&workspace, cx); assert_eq!( @@ -425,7 +440,7 @@ mod tests { "New oneshot should match custom command query" ); - cx.dispatch_action(menu::SecondaryConfirm); + cx.dispatch_action(picker::ConfirmInput { secondary: false }); let tasks_picker = open_spawn_tasks(&workspace, cx); assert_eq!( query(&tasks_picker, cx), @@ -438,7 +453,7 @@ mod tests { "Last recently used one show task should be listed first" ); - cx.dispatch_action(menu::UseSelectedQuery); + cx.dispatch_action(picker::UseSelectedQuery); assert_eq!( query(&tasks_picker, cx), query_str, @@ -449,6 +464,28 @@ mod tests { vec![query_str], "Only custom task should be listed" ); + + let query_str = "0"; + cx.simulate_input(query_str); + assert_eq!(query(&tasks_picker, cx), "echo 40"); + assert_eq!( + task_names(&tasks_picker, cx), + Vec::::new(), + "New oneshot should not match any command query" + ); + + cx.dispatch_action(picker::ConfirmInput { secondary: true }); + let tasks_picker = open_spawn_tasks(&workspace, cx); + assert_eq!( + query(&tasks_picker, cx), + "", + "Query should be reset after confirming" + ); + assert_eq!( + task_names(&tasks_picker, cx), + vec!["echo 4", "another one", "example task", "echo 40"], + "Last recently used one show task should be listed last, as it is a fire-and-forget task" + ); } fn open_spawn_tasks(