From 57b087e41e132878861b53956be50fefcd4e6932 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 16 Apr 2024 02:31:33 +0200 Subject: [PATCH] On label conflict, prefer resolved tasks with never templates (#10586) This way, static (and other) templates that change, e.g. env vars, will get the new template resolved task in the modal to run. Release Notes: - N/A --- crates/project/src/project_tests.rs | 161 +++++++++++++++++++++++++-- crates/project/src/task_inventory.rs | 59 ++++++++-- 2 files changed, 200 insertions(+), 20 deletions(-) diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index fed32ccf4e5f6420ac4bd90badcf6e8cc875d3b1..188fb50b5333d5119f1d5072948bd28dd9f2220e 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -14,6 +14,7 @@ use serde_json::json; #[cfg(not(windows))] use std::os; use std::task::Poll; +use task::{TaskContext, TaskSource, TaskTemplate, TaskTemplates}; use unindent::Unindent as _; use util::{assert_set_eq, paths::PathMatcher, test::temp_tree}; use worktree::WorktreeModelHandle as _; @@ -125,8 +126,19 @@ async fn test_managing_project_specific_settings(cx: &mut gpui::TestAppContext) let project = Project::test(fs.clone(), ["/the-root".as_ref()], cx).await; let worktree = project.update(cx, |project, _| project.worktrees().next().unwrap()); + let task_context = TaskContext::default(); cx.executor().run_until_parked(); + let workree_id = cx.update(|cx| { + project.update(cx, |project, cx| { + project.worktrees().next().unwrap().read(cx).id() + }) + }); + let global_task_source_kind = TaskSourceKind::Worktree { + id: workree_id, + abs_path: PathBuf::from("/the-root/.zed/tasks.json"), + id_base: "local_tasks_for_worktree", + }; cx.update(|cx| { let tree = worktree.read(cx); @@ -154,17 +166,29 @@ async fn test_managing_project_specific_settings(cx: &mut gpui::TestAppContext) assert_eq!(settings_a.tab_size.get(), 8); assert_eq!(settings_b.tab_size.get(), 2); - let workree_id = project.update(cx, |project, cx| { - project.worktrees().next().unwrap().read(cx).id() - }); let all_tasks = project .update(cx, |project, cx| { - project - .task_inventory() - .update(cx, |inventory, cx| inventory.list_tasks(None, None, cx)) + project.task_inventory().update(cx, |inventory, cx| { + let (mut old, new) = inventory.used_and_current_resolved_tasks( + None, + Some(workree_id), + &task_context, + cx, + ); + old.extend(new); + old + }) }) .into_iter() - .map(|(source_kind, task)| (source_kind, task.label)) + .map(|(source_kind, task)| { + let resolved = task.resolved.unwrap(); + ( + source_kind, + task.resolved_label, + resolved.args, + resolved.env, + ) + }) .collect::>(); assert_eq!( all_tasks, @@ -172,24 +196,141 @@ async fn test_managing_project_specific_settings(cx: &mut gpui::TestAppContext) ( TaskSourceKind::Worktree { id: workree_id, - abs_path: PathBuf::from("/the-root/.zed/tasks.json"), + abs_path: PathBuf::from("/the-root/b/.zed/tasks.json"), id_base: "local_tasks_for_worktree", }, - "cargo check".to_string() + "cargo check".to_string(), + vec!["check".to_string()], + HashMap::default(), + ), + ( + global_task_source_kind.clone(), + "cargo check".to_string(), + vec!["check".to_string(), "--all".to_string()], + HashMap::default(), ), + ] + ); + }); + + project.update(cx, |project, cx| { + let inventory = project.task_inventory(); + inventory.update(cx, |inventory, cx| { + let (mut old, new) = inventory.used_and_current_resolved_tasks( + None, + Some(workree_id), + &task_context, + cx, + ); + old.extend(new); + let (_, resolved_task) = old + .into_iter() + .find(|(source_kind, _)| source_kind == &global_task_source_kind) + .expect("should have one global task"); + inventory.task_scheduled(global_task_source_kind.clone(), resolved_task); + }) + }); + + cx.update(|cx| { + let all_tasks = project + .update(cx, |project, cx| { + project.task_inventory().update(cx, |inventory, cx| { + inventory.remove_local_static_source(Path::new("/the-root/.zed/tasks.json")); + inventory.add_source( + global_task_source_kind.clone(), + |cx| { + cx.new_model(|_| { + let source = TestTaskSource { + tasks: TaskTemplates(vec![TaskTemplate { + label: "cargo check".to_string(), + command: "cargo".to_string(), + args: vec![ + "check".to_string(), + "--all".to_string(), + "--all-targets".to_string(), + ], + env: HashMap::from_iter(Some(( + "RUSTFLAGS".to_string(), + "-Zunstable-options".to_string(), + ))), + ..TaskTemplate::default() + }]), + }; + Box::new(source) as Box<_> + }) + }, + cx, + ); + let (mut old, new) = inventory.used_and_current_resolved_tasks( + None, + Some(workree_id), + &task_context, + cx, + ); + old.extend(new); + old + }) + }) + .into_iter() + .map(|(source_kind, task)| { + let resolved = task.resolved.unwrap(); + ( + source_kind, + task.resolved_label, + resolved.args, + resolved.env, + ) + }) + .collect::>(); + assert_eq!( + all_tasks, + vec![ ( TaskSourceKind::Worktree { id: workree_id, abs_path: PathBuf::from("/the-root/b/.zed/tasks.json"), id_base: "local_tasks_for_worktree", }, - "cargo check".to_string() + "cargo check".to_string(), + vec!["check".to_string()], + HashMap::default(), + ), + ( + TaskSourceKind::Worktree { + id: workree_id, + abs_path: PathBuf::from("/the-root/.zed/tasks.json"), + id_base: "local_tasks_for_worktree", + }, + "cargo check".to_string(), + vec![ + "check".to_string(), + "--all".to_string(), + "--all-targets".to_string() + ], + HashMap::from_iter(Some(( + "RUSTFLAGS".to_string(), + "-Zunstable-options".to_string() + ))), ), ] ); }); } +struct TestTaskSource { + tasks: TaskTemplates, +} + +impl TaskSource for TestTaskSource { + fn as_any(&mut self) -> &mut dyn std::any::Any { + self + } + + fn tasks_to_schedule(&mut self, _: &mut ModelContext>) -> TaskTemplates { + self.tasks.clone() + } +} + #[gpui::test] async fn test_managing_language_servers(cx: &mut gpui::TestAppContext) { init_test(cx); diff --git a/crates/project/src/task_inventory.rs b/crates/project/src/task_inventory.rs index aeafca1a19fac401926f0b72eb0562cf3053ac45..58e2f9339afe393a08582ba2717e57c88aa515a1 100644 --- a/crates/project/src/task_inventory.rs +++ b/crates/project/src/task_inventory.rs @@ -7,7 +7,7 @@ use std::{ sync::Arc, }; -use collections::{HashMap, VecDeque}; +use collections::{hash_map, HashMap, VecDeque}; use gpui::{AppContext, Context, Model, ModelContext, Subscription}; use itertools::{Either, Itertools}; use language::Language; @@ -229,7 +229,7 @@ impl Inventory { }, ); let not_used_score = post_inc(&mut lru_score); - let current_resolved_tasks = self + let currently_resolved_tasks = self .sources .iter() .filter(|source| { @@ -257,16 +257,55 @@ impl Inventory { (kind.clone(), task, lru_score) }) .collect::>(); - let previous_resolved_tasks = task_usage + let previously_spawned_tasks = task_usage .into_iter() .map(|(_, (kind, task, lru_score))| (kind.clone(), task.clone(), lru_score)); - previous_resolved_tasks - .chain(current_resolved_tasks) + let mut tasks_by_label = HashMap::default(); + tasks_by_label = previously_spawned_tasks.into_iter().fold( + tasks_by_label, + |mut tasks_by_label, (source, task, lru_score)| { + match tasks_by_label.entry((source, task.resolved_label.clone())) { + hash_map::Entry::Occupied(mut o) => { + let (_, previous_lru_score) = o.get(); + if previous_lru_score >= &lru_score { + o.insert((task, lru_score)); + } + } + hash_map::Entry::Vacant(v) => { + v.insert((task, lru_score)); + } + } + tasks_by_label + }, + ); + tasks_by_label = currently_resolved_tasks.into_iter().fold( + tasks_by_label, + |mut tasks_by_label, (source, task, lru_score)| { + match tasks_by_label.entry((source, task.resolved_label.clone())) { + hash_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)); + } + } + hash_map::Entry::Vacant(v) => { + v.insert((task, lru_score)); + } + } + tasks_by_label + }, + ); + + tasks_by_label + .into_iter() + .map(|((kind, _), (task, lru_score))| (kind, task, lru_score)) .sorted_unstable_by(task_lru_comparator) - .unique_by(|(kind, task, _)| (kind.clone(), task.resolved_label.clone())) - .partition_map(|(kind, task, lru_index)| { - if lru_index < not_used_score { + .partition_map(|(kind, task, lru_score)| { + if lru_score < not_used_score { Either::Left((kind, task)) } else { Either::Right((kind, task)) @@ -705,7 +744,7 @@ mod tests { ( TaskSourceKind::AbsPath { id_base: "test source", - abs_path: path_1.to_path_buf(), + abs_path: path_2.to_path_buf(), }, common_name.to_string(), ), @@ -719,7 +758,7 @@ mod tests { ( TaskSourceKind::AbsPath { id_base: "test source", - abs_path: path_2.to_path_buf(), + abs_path: path_1.to_path_buf(), }, common_name.to_string(), ),