From d26d1c31b1d70c27e892e1c98a3415fd8f3d4d87 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Tue, 6 Aug 2024 11:05:09 -0400 Subject: [PATCH 1/3] fix(watch): declare tasks to be rerun --- Cargo.lock | 1 + crates/turborepo-lib/src/run/watch.rs | 7 ++ crates/turborepo-ui/Cargo.toml | 1 + crates/turborepo-ui/src/tui/app.rs | 95 +++++++++++++++++++++++---- crates/turborepo-ui/src/tui/event.rs | 3 + crates/turborepo-ui/src/tui/handle.rs | 5 ++ crates/turborepo-ui/src/tui/task.rs | 47 ++++++++++++- 7 files changed, 146 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 663eef320bd18..2e84a5b614e18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5669,6 +5669,7 @@ dependencies = [ "dialoguer", "indicatif", "indoc", + "itertools 0.10.5", "lazy_static", "nix 0.26.2", "ratatui", diff --git a/crates/turborepo-lib/src/run/watch.rs b/crates/turborepo-lib/src/run/watch.rs index ffec65a896362..3cf7ea0ba6558 100644 --- a/crates/turborepo-lib/src/run/watch.rs +++ b/crates/turborepo-lib/src/run/watch.rs @@ -275,6 +275,13 @@ impl WatchClient { .build(&signal_handler, telemetry) .await?; + if let Some(sender) = &self.ui_sender { + let task_names = run.engine.tasks_with_command(&run.pkg_dep_graph); + sender + .restart_tasks(task_names) + .map_err(|err| Error::UISend(err.to_string()))?; + } + Ok(run.run(self.ui_sender.clone(), true).await?) } ChangedPackages::All => { diff --git a/crates/turborepo-ui/Cargo.toml b/crates/turborepo-ui/Cargo.toml index bc20ec39440a3..560cad0b447bd 100644 --- a/crates/turborepo-ui/Cargo.toml +++ b/crates/turborepo-ui/Cargo.toml @@ -22,6 +22,7 @@ console = { workspace = true } crossterm = "0.27.0" dialoguer = { workspace = true } indicatif = { workspace = true } +itertools = { workspace = true } lazy_static = { workspace = true } nix = { version = "0.26.2", features = ["signal"] } ratatui = { workspace = true } diff --git a/crates/turborepo-ui/src/tui/app.rs b/crates/turborepo-ui/src/tui/app.rs index 07463932f275a..4d2b14376cb92 100644 --- a/crates/turborepo-ui/src/tui/app.rs +++ b/crates/turborepo-ui/src/tui/app.rs @@ -178,18 +178,6 @@ impl App { let running = planned.start(); self.tasks_by_status.running.push(running); - found_task = true; - } else if let Some(finished_idx) = self - .tasks_by_status - .finished - .iter() - .position(|finished| finished.name() == task) - { - let _finished = self.tasks_by_status.finished.remove(finished_idx); - self.tasks_by_status - .running - .push(Task::new(task.to_owned()).start()); - found_task = true; } @@ -305,6 +293,25 @@ impl App { }; } + #[tracing::instrument(skip(self))] + pub fn restart_tasks(&mut self, tasks: Vec) { + debug!("tasks to reset: {tasks:?}"); + // Make sure all tasks have a terminal output + for task in &tasks { + self.tasks + .entry(task.clone()) + .or_insert_with(|| TerminalOutput::new(self.pane_rows, self.pane_cols, None)); + } + + let new_selection_index = self + .tasks_by_status + .restart_tasks(tasks.iter().map(|s| s.as_str()), self.selected_task_index); + if self.has_user_scrolled { + self.selected_task_index = new_selection_index; + self.scroll.select(Some(new_selection_index)); + } + } + /// Persist all task output to the after closing the TUI pub fn persist_tasks(&mut self, started_tasks: Vec) -> std::io::Result<()> { for (task_name, task) in started_tasks.into_iter().filter_map(|started_task| { @@ -579,6 +586,9 @@ fn update( Event::CopySelection => { app.copy_selection(); } + Event::RestartTasks { tasks } => { + app.update_tasks(tasks); + } } Ok(None) } @@ -692,6 +702,7 @@ mod test { ); // Restart b + app.restart_tasks(vec!["b".to_string()]); app.start_task("b", OutputLogs::Full).unwrap(); assert_eq!( ( @@ -703,6 +714,7 @@ mod test { ); // Restart a + app.restart_tasks(vec!["a".to_string()]); app.start_task("a", OutputLogs::Full).unwrap(); assert_eq!( ( @@ -822,4 +834,63 @@ mod test { ); assert!(app.tasks.get("b").unwrap().status.is_none()); } + + #[test] + fn test_restarting_task_no_scroll() { + let mut app: App<()> = App::new( + 100, + 100, + vec!["a".to_string(), "b".to_string(), "c".to_string()], + ); + assert_eq!(app.scroll.selected(), Some(0), "selected a"); + assert_eq!(app.tasks_by_status.task_name(0), "a", "selected a"); + app.start_task("a", OutputLogs::None).unwrap(); + app.start_task("b", OutputLogs::None).unwrap(); + app.start_task("c", OutputLogs::None).unwrap(); + app.finish_task("b", TaskResult::Success).unwrap(); + app.finish_task("c", TaskResult::Success).unwrap(); + app.finish_task("a", TaskResult::Success).unwrap(); + + assert_eq!(app.scroll.selected(), Some(0), "selected b"); + assert_eq!(app.tasks_by_status.task_name(0), "b", "selected b"); + + app.restart_tasks(vec!["c".to_string()]); + + assert_eq!( + app.tasks_by_status + .task_name(app.scroll.selected().unwrap()), + "c", + "selected c" + ); + } + + #[test] + fn test_restarting_task() { + let mut app: App<()> = App::new( + 100, + 100, + vec!["a".to_string(), "b".to_string(), "c".to_string()], + ); + app.next(); + assert_eq!(app.scroll.selected(), Some(1), "selected b"); + assert_eq!(app.tasks_by_status.task_name(1), "b", "selected b"); + app.start_task("a", OutputLogs::None).unwrap(); + app.start_task("b", OutputLogs::None).unwrap(); + app.start_task("c", OutputLogs::None).unwrap(); + app.finish_task("b", TaskResult::Success).unwrap(); + app.finish_task("c", TaskResult::Success).unwrap(); + app.finish_task("a", TaskResult::Success).unwrap(); + + assert_eq!(app.scroll.selected(), Some(0), "selected b"); + assert_eq!(app.tasks_by_status.task_name(0), "b", "selected b"); + + app.restart_tasks(vec!["c".to_string()]); + + assert_eq!( + app.tasks_by_status + .task_name(app.scroll.selected().unwrap()), + "b", + "selected b" + ); + } } diff --git a/crates/turborepo-ui/src/tui/event.rs b/crates/turborepo-ui/src/tui/event.rs index c2a45931c2225..66670dd222eb6 100644 --- a/crates/turborepo-ui/src/tui/event.rs +++ b/crates/turborepo-ui/src/tui/event.rs @@ -38,6 +38,9 @@ pub enum Event { }, Mouse(crossterm::event::MouseEvent), CopySelection, + RestartTasks { + tasks: Vec, + }, } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)] diff --git a/crates/turborepo-ui/src/tui/handle.rs b/crates/turborepo-ui/src/tui/handle.rs index fe5ac0aa3a0f7..be54b89b96516 100644 --- a/crates/turborepo-ui/src/tui/handle.rs +++ b/crates/turborepo-ui/src/tui/handle.rs @@ -67,6 +67,11 @@ impl AppSender { pub fn update_tasks(&self, tasks: Vec) -> Result<(), mpsc::SendError> { self.primary.send(Event::UpdateTasks { tasks }) } + + /// Restart the list of tasks displayed in the TUI + pub fn restart_tasks(&self, tasks: Vec) -> Result<(), mpsc::SendError> { + self.primary.send(Event::RestartTasks { tasks }) + } } impl AppReceiver { diff --git a/crates/turborepo-ui/src/tui/task.rs b/crates/turborepo-ui/src/tui/task.rs index 44f67a117e522..164fda7a43f74 100644 --- a/crates/turborepo-ui/src/tui/task.rs +++ b/crates/turborepo-ui/src/tui/task.rs @@ -1,5 +1,7 @@ #![allow(dead_code)] -use std::time::Instant; +use std::{collections::HashSet, mem, time::Instant}; + +use itertools::Itertools; use super::event::TaskResult; @@ -73,6 +75,13 @@ impl Task { pub fn start(&self) -> Instant { self.state.start } + + pub fn restart(self) -> Task { + Task { + name: self.name, + state: Planned, + } + } } impl Task { @@ -87,6 +96,13 @@ impl Task { pub fn result(&self) -> TaskResult { self.state.result } + + pub fn restart(self) -> Task { + Task { + name: self.name, + state: Planned, + } + } } pub struct TaskNamesByStatus { @@ -138,4 +154,33 @@ impl TasksByStatus { .map(|task| task.to_string()) .collect() } + + pub fn restart_tasks<'a>( + &mut self, + tasks: impl Iterator, + current_selection: usize, + ) -> usize { + let tasks_to_restart = tasks.collect::>(); + + // check running & finished vecs & remove any where name matches tasks iter + let (restarted_running, keep_running): (Vec<_>, Vec<_>) = mem::take(&mut self.running) + .into_iter() + .partition(|task| tasks_to_restart.contains(task.name())); + self.running = keep_running; + + let (restarted_finished, keep_finished): (Vec<_>, Vec<_>) = mem::take(&mut self.finished) + .into_iter() + .partition(|task| tasks_to_restart.contains(task.name())); + self.finished = keep_finished; + self.planned.extend( + restarted_running + .into_iter() + .map(|task| task.restart()) + .chain(restarted_finished.into_iter().map(|task| task.restart())) + .sorted(), + ); + + // TODO: return new index? + current_selection + } } From c1aa1f4286637208b22e5d58b0e2efa057fe33a8 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Tue, 6 Aug 2024 14:52:44 -0400 Subject: [PATCH 2/3] fix(watch): follow task on restart --- Cargo.lock | 1 - crates/turborepo-ui/Cargo.toml | 1 - crates/turborepo-ui/src/tui/app.rs | 19 ++++++++++++++----- crates/turborepo-ui/src/tui/task.rs | 16 +++------------- 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2e84a5b614e18..663eef320bd18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5669,7 +5669,6 @@ dependencies = [ "dialoguer", "indicatif", "indoc", - "itertools 0.10.5", "lazy_static", "nix 0.26.2", "ratatui", diff --git a/crates/turborepo-ui/Cargo.toml b/crates/turborepo-ui/Cargo.toml index 560cad0b447bd..bc20ec39440a3 100644 --- a/crates/turborepo-ui/Cargo.toml +++ b/crates/turborepo-ui/Cargo.toml @@ -22,7 +22,6 @@ console = { workspace = true } crossterm = "0.27.0" dialoguer = { workspace = true } indicatif = { workspace = true } -itertools = { workspace = true } lazy_static = { workspace = true } nix = { version = "0.26.2", features = ["signal"] } ratatui = { workspace = true } diff --git a/crates/turborepo-ui/src/tui/app.rs b/crates/turborepo-ui/src/tui/app.rs index 4d2b14376cb92..097bec0d149ae 100644 --- a/crates/turborepo-ui/src/tui/app.rs +++ b/crates/turborepo-ui/src/tui/app.rs @@ -296,6 +296,7 @@ impl App { #[tracing::instrument(skip(self))] pub fn restart_tasks(&mut self, tasks: Vec) { debug!("tasks to reset: {tasks:?}"); + let highlighted_task = self.active_task().to_owned(); // Make sure all tasks have a terminal output for task in &tasks { self.tasks @@ -303,12 +304,20 @@ impl App { .or_insert_with(|| TerminalOutput::new(self.pane_rows, self.pane_cols, None)); } - let new_selection_index = self + self.tasks_by_status + .restart_tasks(tasks.iter().map(|s| s.as_str())); + + if !self.has_user_scrolled { + return; + } + + if let Some(new_index_to_highlight) = self .tasks_by_status - .restart_tasks(tasks.iter().map(|s| s.as_str()), self.selected_task_index); - if self.has_user_scrolled { - self.selected_task_index = new_selection_index; - self.scroll.select(Some(new_selection_index)); + .task_names_in_displayed_order() + .position(|running| running == highlighted_task.as_str()) + { + self.selected_task_index = new_index_to_highlight; + self.scroll.select(Some(new_index_to_highlight)); } } diff --git a/crates/turborepo-ui/src/tui/task.rs b/crates/turborepo-ui/src/tui/task.rs index 164fda7a43f74..30857a48503fb 100644 --- a/crates/turborepo-ui/src/tui/task.rs +++ b/crates/turborepo-ui/src/tui/task.rs @@ -1,8 +1,6 @@ #![allow(dead_code)] use std::{collections::HashSet, mem, time::Instant}; -use itertools::Itertools; - use super::event::TaskResult; #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)] @@ -155,14 +153,9 @@ impl TasksByStatus { .collect() } - pub fn restart_tasks<'a>( - &mut self, - tasks: impl Iterator, - current_selection: usize, - ) -> usize { + pub fn restart_tasks<'a>(&mut self, tasks: impl Iterator) { let tasks_to_restart = tasks.collect::>(); - // check running & finished vecs & remove any where name matches tasks iter let (restarted_running, keep_running): (Vec<_>, Vec<_>) = mem::take(&mut self.running) .into_iter() .partition(|task| tasks_to_restart.contains(task.name())); @@ -176,11 +169,8 @@ impl TasksByStatus { restarted_running .into_iter() .map(|task| task.restart()) - .chain(restarted_finished.into_iter().map(|task| task.restart())) - .sorted(), + .chain(restarted_finished.into_iter().map(|task| task.restart())), ); - - // TODO: return new index? - current_selection + self.planned.sort_unstable(); } } From 8203e0f925f2ede4828e17219daebb974cd9e44e Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Wed, 7 Aug 2024 11:55:26 -0400 Subject: [PATCH 3/3] fix(tui): update scroll on task list update --- crates/turborepo-ui/src/tui/app.rs | 83 ++++++++++++++---------------- 1 file changed, 40 insertions(+), 43 deletions(-) diff --git a/crates/turborepo-ui/src/tui/app.rs b/crates/turborepo-ui/src/tui/app.rs index 097bec0d149ae..2303dd9dadbdc 100644 --- a/crates/turborepo-ui/src/tui/app.rs +++ b/crates/turborepo-ui/src/tui/app.rs @@ -11,7 +11,7 @@ use ratatui::{ widgets::TableState, Frame, Terminal, }; -use tracing::debug; +use tracing::{debug, trace}; const PANE_SIZE_RATIO: f32 = 3.0 / 4.0; const FRAMERATE: Duration = Duration::from_millis(3); @@ -190,18 +190,7 @@ impl App { .output_logs = Some(output_logs); // If user hasn't interacted, keep highlighting top-most task in list. - if !self.has_user_scrolled { - return Ok(()); - } - - if let Some(new_index_to_highlight) = self - .tasks_by_status - .task_names_in_displayed_order() - .position(|running| running == highlighted_task) - { - self.selected_task_index = new_index_to_highlight; - self.scroll.select(Some(new_index_to_highlight)); - } + self.select_task(&highlighted_task)?; Ok(()) } @@ -223,11 +212,7 @@ impl App { .running .iter() .position(|running| running.name() == task) - .ok_or_else(|| { - debug!("could not find '{task}' to finish"); - println!("{:#?}", highlighted_task); - Error::TaskNotFound { name: task.into() } - })?; + .ok_or_else(|| Error::TaskNotFound { name: task.into() })?; let running = self.tasks_by_status.running.remove(running_idx); self.tasks_by_status.finished.push(running.finish(result)); @@ -237,20 +222,8 @@ impl App { .ok_or_else(|| Error::TaskNotFound { name: task.into() })? .task_result = Some(result); - // If user hasn't interacted, keep highlighting top-most task in list. - if !self.has_user_scrolled { - return Ok(()); - } - // Find the highlighted task from before the list movement in the new list. - if let Some(new_index_to_highlight) = self - .tasks_by_status - .task_names_in_displayed_order() - .position(|running| running == highlighted_task.as_str()) - { - self.selected_task_index = new_index_to_highlight; - self.scroll.select(Some(new_index_to_highlight)); - } + self.select_task(&highlighted_task)?; Ok(()) } @@ -274,6 +247,7 @@ impl App { #[tracing::instrument(skip(self))] pub fn update_tasks(&mut self, tasks: Vec) { debug!("updating task list: {tasks:?}"); + let highlighted_task = self.active_task().to_owned(); // Make sure all tasks have a terminal output for task in &tasks { self.tasks @@ -291,6 +265,12 @@ impl App { running: Default::default(), finished: Default::default(), }; + + // Task that was selected may have been removed, go back to top if this happens + if self.select_task(&highlighted_task).is_err() { + trace!("{highlighted_task} was removed from list"); + self.reset_scroll(); + } } #[tracing::instrument(skip(self))] @@ -307,18 +287,8 @@ impl App { self.tasks_by_status .restart_tasks(tasks.iter().map(|s| s.as_str())); - if !self.has_user_scrolled { - return; - } - - if let Some(new_index_to_highlight) = self - .tasks_by_status - .task_names_in_displayed_order() - .position(|running| running == highlighted_task.as_str()) - { - self.selected_task_index = new_index_to_highlight; - self.scroll.select(Some(new_index_to_highlight)); - } + self.select_task(&highlighted_task) + .expect("should find task after restart"); } /// Persist all task output to the after closing the TUI @@ -378,6 +348,33 @@ impl App { }; super::copy_to_clipboard(&text); } + + fn select_task(&mut self, task_name: &str) -> Result<(), Error> { + if !self.has_user_scrolled { + return Ok(()); + } + + let Some(new_index_to_highlight) = self + .tasks_by_status + .task_names_in_displayed_order() + .position(|task| task == task_name) + else { + return Err(Error::TaskNotFound { + name: task_name.to_owned(), + }); + }; + self.selected_task_index = new_index_to_highlight; + self.scroll.select(Some(new_index_to_highlight)); + + Ok(()) + } + + /// Resets scroll state + pub fn reset_scroll(&mut self) { + self.has_user_scrolled = false; + self.scroll.select(Some(0)); + self.selected_task_index = 0; + } } impl App {