Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Rework task modal (#10341)

Browse source 
New list (used tasks are above the separator line, sorted by the usage
recency), then all language tasks, then project-local and global tasks
are listed.
Note that there are two test tasks (for `test_name_1` and `test_name_2`
functions) that are created from the same task template:
<img width="563" alt="Screenshot 2024-04-10 at 01 00 46"
src="https://github.com/zed-industries/zed/assets/2690773/7455a82f-2af2-47bf-99bd-d9c5a36e64ab">

Tasks are deduplicated by labels, with the used tasks left in case of
the conflict with the new tasks from the template:
<img width="555" alt="Screenshot 2024-04-10 at 01 01 06"
src="https://github.com/zed-industries/zed/assets/2690773/8f5a249e-abec-46ef-a991-08c6d0348648">

Regular recent tasks can be now removed too:
<img width="565" alt="Screenshot 2024-04-10 at 01 00 55"
src="https://github.com/zed-industries/zed/assets/2690773/0976b8fe-b5d7-4d2a-953d-1d8b1f216192">

When the caret is in the place where no function symbol could be
retrieved, no cargo tests for function are listed in tasks:
<img width="556" alt="image"
src="https://github.com/zed-industries/zed/assets/2690773/df30feba-fe27-4645-8be9-02afc70f02da">


Part of https://github.com/zed-industries/zed/issues/10132
Reworks the task code to simplify it and enable proper task labels.

* removes `trait Task`, renames `Definition` into `TaskTemplate` and use
that instead of `Arc<dyn Task>` everywhere
* implement more generic `TaskId` generation that depends on the
`TaskContext` and `TaskTemplate`
* remove `TaskId` out of the template and only create it after
"resolving" the template into the `ResolvedTask`: this way, task
templates, task state (`TaskContext`) and task "result" (resolved state)
are clearly separated and are not mixed
* implement the logic for filtering out non-related language tasks and
tasks that have non-resolved Zed task variables
* rework Zed template-vs-resolved-task display in modal: now all reruns
and recently used tasks are resolved tasks with "fixed" context (unless
configured otherwise in the task json) that are always shown, and Zed
can add on top tasks with different context that are derived from the
same template as the used, resolved tasks
* sort the tasks list better, showing more specific and least recently
used tasks higher
* shows a separator between used and unused tasks, allow removing the
used tasks same as the oneshot ones
* remote the Oneshot task source as redundant: all oneshot tasks are now
stored in the inventory's history
* when reusing the tasks as query in the modal, paste the expanded task
label now, show trimmed resolved label in the modal
* adjusts Rust and Elixir task labels to be more descriptive and closer
to bash scripts

Release Notes:

- Improved task modal ordering, run and deletion capabilities
This commit is contained in:
Kirill Bulatov 2024-04-11 01:02:04 +02:00 • committed by GitHub
parent b0eda77d73
commit d1ad96782c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
21 changed files with 1103 additions and 671 deletions
Download patch file Download diff file Expand all files Collapse all files

82
crates/tasks_ui/src/lib.rs
View file

@ -5,8 +5,8 @@ use editor::Editor;
use gpui::{AppContext, ViewContext, WindowContext};
use language::{Language, Point};
use modal::{Spawn, TasksModal};
use project::{Location, WorktreeId};
use task::{Task, TaskContext, TaskVariables, VariableName};
use project::{Location, TaskSourceKind, WorktreeId};
use task::{ResolvedTask, TaskContext, TaskTemplate, TaskVariables, VariableName};
use util::ResultExt;
use workspace::Workspace;
@ -23,18 +23,32 @@ pub fn init(cx: &mut AppContext) {
workspace
.register_action(spawn_task_or_modal)
.register_action(move |workspace, action: &modal::Rerun, cx| {
if let Some((task, old_context)) =
if let Some((task_source_kind, last_scheduled_task)) =
workspace.project().update(cx, |project, cx| {
project.task_inventory().read(cx).last_scheduled_task()
})
{
let task_context = if action.reevaluate_context {
if action.reevaluate_context {
let original_task = last_scheduled_task.original_task;
let cwd = task_cwd(workspace, cx).log_err().flatten();
task_context(workspace, cwd, cx)
let task_context = task_context(workspace, cwd, cx);
schedule_task(
workspace,
task_source_kind,
&original_task,
task_context,
false,
cx,
)
} else {
old_context
};
schedule_task(workspace, &task, task_context, false, cx)
schedule_resolved_task(
workspace,
task_source_kind,
last_scheduled_task,
false,
cx,
);
}
};
});
},
@ -64,13 +78,21 @@ fn spawn_task_with_name(name: String, cx: &mut ViewContext<Workspace>) {
let (worktree, language) = active_item_selection_properties(workspace, cx);
let tasks = workspace.project().update(cx, |project, cx| {
project.task_inventory().update(cx, |inventory, cx| {
inventory.list_tasks(language, worktree, false, cx)
inventory.list_tasks(language, worktree, cx)
})
});
let (_, target_task) = tasks.into_iter().find(|(_, task)| task.name() == name)?;
let (task_source_kind, target_task) =
tasks.into_iter().find(|(_, task)| task.label == name)?;
let cwd = task_cwd(workspace, cx).log_err().flatten();
let task_context = task_context(workspace, cwd, cx);
schedule_task(workspace, &target_task, task_context, false, cx);
schedule_task(
workspace,
task_source_kind,
&target_task,
task_context,
false,
cx,
);
Some(())
})
.ok()
@ -214,17 +236,38 @@ fn task_context(
fn schedule_task(
workspace: &Workspace,
task: &Arc<dyn Task>,
task_source_kind: TaskSourceKind,
task_to_resolve: &TaskTemplate,
task_cx: TaskContext,
omit_history: bool,
cx: &mut ViewContext<'_, Workspace>,
) {
let spawn_in_terminal = task.prepare_exec(task_cx.clone());
if let Some(spawn_in_terminal) = spawn_in_terminal {
if let Some(spawn_in_terminal) =
task_to_resolve.resolve_task(&task_source_kind.to_id_base(), task_cx)
{
schedule_resolved_task(
workspace,
task_source_kind,
spawn_in_terminal,
omit_history,
cx,
);
}
}
fn schedule_resolved_task(
workspace: &Workspace,
task_source_kind: TaskSourceKind,
mut resolved_task: ResolvedTask,
omit_history: bool,
cx: &mut ViewContext<'_, Workspace>,
) {
if let Some(spawn_in_terminal) = resolved_task.resolved.take() {
if !omit_history {
resolved_task.resolved = Some(spawn_in_terminal.clone());
workspace.project().update(cx, |project, cx| {
project.task_inventory().update(cx, |inventory, _| {
inventory.task_scheduled(Arc::clone(task), task_cx);
inventory.task_scheduled(task_source_kind, resolved_task);
})
});
}
@ -274,9 +317,9 @@ mod tests {
use editor::Editor;
use gpui::{Entity, TestAppContext};
use language::{Language, LanguageConfig, SymbolContextProvider};
use project::{FakeFs, Project, TaskSourceKind};
use project::{FakeFs, Project};
use serde_json::json;
use task::{oneshot_source::OneshotSource, TaskContext, TaskVariables, VariableName};
use task::{TaskContext, TaskVariables, VariableName};
use ui::VisualContext;
use workspace::{AppState, Workspace};
@ -344,11 +387,6 @@ mod tests {
.with_context_provider(Some(Arc::new(SymbolContextProvider))),
);
let project = Project::test(fs, ["/dir".as_ref()], cx).await;
project.update(cx, |project, cx| {
project.task_inventory().update(cx, |inventory, cx| {
inventory.add_source(TaskSourceKind::UserInput, |cx| OneshotSource::new(cx), cx)
})
});
let worktree_id = project.update(cx, |project, cx| {
project.worktrees().next().unwrap().read(cx).id()
});

181
crates/tasks_ui/src/modal.rs
View file

@ -1,6 +1,6 @@
use std::sync::Arc;
use crate::{active_item_selection_properties, schedule_task};
use crate::{active_item_selection_properties, schedule_resolved_task};
use fuzzy::{StringMatch, StringMatchCandidate};
use gpui::{
impl_actions, rems, AppContext, DismissEvent, EventEmitter, FocusableView, Global,
@ -9,7 +9,7 @@ use gpui::{
};
use picker::{highlighted_match_with_paths::HighlightedText, Picker, PickerDelegate};
use project::{Inventory, TaskSourceKind};
use task::{oneshot_source::OneshotSource, Task, TaskContext};
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,
@ -51,7 +51,8 @@ impl_actions!(task, [Rerun, Spawn]);
/// A modal used to spawn new tasks.
pub(crate) struct TasksModalDelegate {
inventory: Model<Inventory>,
candidates: Option<Vec<(TaskSourceKind, Arc<dyn Task>)>>,
candidates: Option<Vec<(TaskSourceKind, ResolvedTask)>>,
last_used_candidate_index: Option<usize>,
matches: Vec<StringMatch>,
selected_index: usize,
workspace: WeakView<Workspace>,
@ -71,6 +72,7 @@ impl TasksModalDelegate {
workspace,
candidates: None,
matches: Vec::new(),
last_used_candidate_index: None,
selected_index: 0,
prompt: String::default(),
task_context,
@ -78,24 +80,25 @@ impl TasksModalDelegate {
}
}
fn spawn_oneshot(&mut self, cx: &mut AppContext) -> Option<Arc<dyn Task>> {
fn spawn_oneshot(&mut self) -> Option<(TaskSourceKind, ResolvedTask)> {
if self.prompt.trim().is_empty() {
return None;
}
self.inventory
.update(cx, |inventory, _| inventory.source::<OneshotSource>())?
.update(cx, |oneshot_source, _| {
Some(
oneshot_source
.as_any()
.downcast_mut::<OneshotSource>()?
.spawn(self.prompt.clone()),
)
})
let source_kind = TaskSourceKind::UserInput;
let id_base = source_kind.to_id_base();
let new_oneshot = TaskTemplate {
label: self.prompt.clone(),
command: self.prompt.clone(),
..TaskTemplate::default()
};
Some((
source_kind,
new_oneshot.resolve_task(&id_base, self.task_context.clone())?,
))
}
fn delete_oneshot(&mut self, ix: usize, cx: &mut AppContext) {
fn delete_previously_used(&mut self, ix: usize, cx: &mut AppContext) {
let Some(candidates) = self.candidates.as_mut() else {
return;
};
@ -106,16 +109,8 @@ impl TasksModalDelegate {
// it doesn't make sense to requery the inventory for new candidates, as that's potentially costly and more often than not it should just return back
// the original list without a removed entry.
candidates.remove(ix);
self.inventory.update(cx, |inventory, cx| {
let oneshot_source = inventory.source::<OneshotSource>()?;
let task_id = task.id();
oneshot_source.update(cx, |this, _| {
let oneshot_source = this.as_any().downcast_mut::<OneshotSource>()?;
oneshot_source.remove(task_id);
Some(())
});
Some(())
self.inventory.update(cx, |inventory, _| {
inventory.delete_previously_used(&task.id);
});
}
}
@ -194,26 +189,47 @@ impl PickerDelegate for TasksModalDelegate {
cx.spawn(move |picker, mut cx| async move {
let Some(candidates) = picker
.update(&mut cx, |picker, cx| {
let candidates = picker.delegate.candidates.get_or_insert_with(|| {
let Ok((worktree, language)) =
picker.delegate.workspace.update(cx, |workspace, cx| {
active_item_selection_properties(workspace, cx)
})
else {
return Vec::new();
};
picker.delegate.inventory.update(cx, |inventory, cx| {
inventory.list_tasks(language, worktree, true, cx)
})
});
let candidates = match &mut picker.delegate.candidates {
Some(candidates) => candidates,
None => {
let Ok((worktree, language)) =
picker.delegate.workspace.update(cx, |workspace, cx| {
active_item_selection_properties(workspace, cx)
})
else {
return Vec::new();
};
let (used, current) =
picker.delegate.inventory.update(cx, |inventory, cx| {
inventory.used_and_current_resolved_tasks(
language,
worktree,
picker.delegate.task_context.clone(),
cx,
)
});
picker.delegate.last_used_candidate_index = if used.is_empty() {
None
} else {
Some(used.len() - 1)
};
let mut new_candidates = used;
new_candidates.extend(current);
picker.delegate.candidates.insert(new_candidates)
}
};
candidates
.iter()
.enumerate()
.map(|(index, (_, candidate))| StringMatchCandidate {
id: index,
char_bag: candidate.name().chars().collect(),
string: candidate.name().into(),
char_bag: candidate.resolved_label.chars().collect(),
string: candidate
.resolved
.as_ref()
.map(|resolved| resolved.label.clone())
.unwrap_or_else(|| candidate.resolved_label.clone()),
})
.collect::<Vec<_>>()
})
@ -256,21 +272,15 @@ impl PickerDelegate for TasksModalDelegate {
let ix = current_match.candidate_id;
self.candidates
.as_ref()
.map(|candidates| candidates[ix].1.clone())
.map(|candidates| candidates[ix].clone())
});
let Some(task) = task else {
let Some((task_source_kind, task)) = task else {
return;
};
self.workspace
.update(cx, |workspace, cx| {
schedule_task(
workspace,
&task,
self.task_context.clone(),
omit_history_entry,
cx,
);
schedule_resolved_task(workspace, task_source_kind, task, omit_history_entry, cx);
})
.ok();
cx.emit(DismissEvent);
@ -288,16 +298,13 @@ impl PickerDelegate for TasksModalDelegate {
) -> Option<Self::ListItem> {
let candidates = self.candidates.as_ref()?;
let hit = &self.matches[ix];
let (source_kind, _) = &candidates[hit.candidate_id];
let (source_kind, _) = &candidates.get(hit.candidate_id)?;
let highlighted_location = HighlightedText {
text: hit.string.clone(),
highlight_positions: hit.positions.clone(),
char_count: hit.string.chars().count(),
};
let base_item = ListItem::new(SharedString::from(format!("tasks-modal-{ix}")))
.inset(true)
.spacing(ListItemSpacing::Sparse);
let icon = match source_kind {
TaskSourceKind::UserInput => Some(Icon::new(IconName::Terminal)),
TaskSourceKind::AbsPath { .. } => Some(Icon::new(IconName::Settings)),
@ -307,9 +314,13 @@ impl PickerDelegate for TasksModalDelegate {
.map(|icon_path| Icon::from_path(icon_path)),
};
Some(
base_item
ListItem::new(SharedString::from(format!("tasks-modal-{ix}")))
.inset(true)
.spacing(ListItemSpacing::Sparse)
.map(|item| {
let item = if matches!(source_kind, TaskSourceKind::UserInput) {
let item = if matches!(source_kind, TaskSourceKind::UserInput)
|| Some(ix) <= self.last_used_candidate_index
{
let task_index = hit.candidate_id;
let delete_button = div().child(
IconButton::new("delete", IconName::Close)
@ -317,14 +328,21 @@ impl PickerDelegate for TasksModalDelegate {
.icon_color(Color::Muted)
.size(ButtonSize::None)
.icon_size(IconSize::XSmall)
.on_click(cx.listener(move |this, _event, cx| {
.on_click(cx.listener(move |picker, _event, cx| {
cx.stop_propagation();
cx.prevent_default();
this.delegate.delete_oneshot(task_index, cx);
this.refresh(cx);
picker.delegate.delete_previously_used(task_index, cx);
picker.delegate.last_used_candidate_index = picker
.delegate
.last_used_candidate_index
.unwrap_or(0)
.checked_sub(1);
picker.refresh(cx);
}))
.tooltip(|cx| Tooltip::text("Delete an one-shot task", cx)),
.tooltip(|cx| {
Tooltip::text("Delete previously scheduled task", cx)
}),
);
item.end_hover_slot(delete_button)
} else {
@ -346,35 +364,38 @@ impl PickerDelegate for TasksModalDelegate {
let task_index = self.matches.get(self.selected_index())?.candidate_id;
let tasks = self.candidates.as_ref()?;
let (_, task) = tasks.get(task_index)?;
let mut spawn_prompt = task.prepare_exec(self.task_context.clone())?;
if !spawn_prompt.args.is_empty() {
spawn_prompt.command.push(' ');
spawn_prompt
.command
.extend(intersperse(spawn_prompt.args, " ".to_string()));
}
Some(spawn_prompt.command)
task.resolved.as_ref().map(|spawn_in_terminal| {
let mut command = spawn_in_terminal.command.clone();
if !spawn_in_terminal.args.is_empty() {
command.push(' ');
command.extend(intersperse(spawn_in_terminal.args.clone(), " ".to_string()));
}
command
})
}
fn confirm_input(&mut self, omit_history_entry: bool, cx: &mut ViewContext<Picker<Self>>) {
let Some(task) = self.spawn_oneshot(cx) else {
let Some((task_source_kind, task)) = self.spawn_oneshot() else {
return;
};
self.workspace
.update(cx, |workspace, cx| {
schedule_task(
workspace,
&task,
self.task_context.clone(),
omit_history_entry,
cx,
);
schedule_resolved_task(workspace, task_source_kind, task, omit_history_entry, cx);
})
.ok();
cx.emit(DismissEvent);
}
fn separators_after_indices(&self) -> Vec<usize> {
if let Some(i) = self.last_used_candidate_index {
vec![i]
} else {
Vec::new()
}
}
}
// TODO kb more tests on recent tasks from language templates
#[cfg(test)]
mod tests {
use gpui::{TestAppContext, VisualTestContext};
@ -412,12 +433,6 @@ mod tests {
.await;
let project = Project::test(fs, ["/dir".as_ref()], cx).await;
project.update(cx, |project, cx| {
project.task_inventory().update(cx, |inventory, cx| {
inventory.add_source(TaskSourceKind::UserInput, |cx| OneshotSource::new(cx), cx)
})
});
let (workspace, cx) = cx.add_window_view(|cx| Workspace::test_new(project, cx));
let tasks_picker = open_spawn_tasks(&workspace, cx);
@ -518,8 +533,8 @@ mod tests {
);
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"
vec!["echo 4", "another one", "example task"],
"No query should be added to the list, as it was submitted with secondary action (that maps to omit_history = true)"
);
cx.dispatch_action(Spawn {
@ -535,7 +550,7 @@ mod tests {
});
assert_eq!(
task_names(&tasks_picker, cx),
vec!["echo 4", "another one", "example task", "echo 40"],
vec!["echo 4", "another one", "example task"],
);
}