From b9c8b1519cc710053f8d104f08d3dd3162815f23 Mon Sep 17 00:00:00 2001 From: thomasschafer Date: Thu, 19 Dec 2024 20:45:47 +0000 Subject: [PATCH] Make replacement async --- src/app.rs | 144 +++++++++++++++++++++++++++++++++++---------------- src/event.rs | 4 +- src/ui.rs | 11 ++-- tests/app.rs | 14 +++-- 4 files changed, 111 insertions(+), 62 deletions(-) diff --git a/src/app.rs b/src/app.rs index 9426833..43a41a6 100644 --- a/src/app.rs +++ b/src/app.rs @@ -143,12 +143,38 @@ impl SearchInProgressState { } } +#[derive(Debug)] +pub struct PerformingReplacementState { + handle: Option>, + #[allow(dead_code)] + processing_sender: UnboundedSender, + processing_receiver: UnboundedReceiver, +} + +impl PerformingReplacementState { + pub fn new( + handle: Option>, + processing_sender: UnboundedSender, + processing_receiver: UnboundedReceiver, + ) -> Self { + Self { + handle, + processing_sender, + processing_receiver, + } + } + + fn set_handle(&mut self, handle: JoinHandle<()>) { + self.handle = Some(handle); + } +} + #[derive(Debug)] pub enum Screen { SearchFields, SearchProgressing(SearchInProgressState), SearchComplete(SearchState), - PerformingReplacement, + PerformingReplacement(PerformingReplacementState), Results(ReplaceState), } @@ -400,14 +426,16 @@ impl App { } pub async fn background_processing_recv(&mut self) -> Option { - if let Screen::SearchProgressing(SearchInProgressState { - processing_receiver, - .. - }) = &mut self.current_screen - { - processing_receiver.recv().await - } else { - None + match &mut self.current_screen { + Screen::SearchProgressing(SearchInProgressState { + processing_receiver, + .. + }) => processing_receiver.recv().await, + Screen::PerformingReplacement(PerformingReplacementState { + processing_receiver, + .. + }) => processing_receiver.recv().await, + _ => None, } } @@ -432,9 +460,6 @@ impl App { rerender: true, }, AppEvent::PerformSearch => self.perform_search_if_valid(), - AppEvent::PerformReplacement(mut search_state) => { - self.perform_replacement(&mut search_state) - } } } @@ -468,28 +493,60 @@ impl App { } } - pub fn perform_replacement(&mut self, search_state: &mut SearchState) -> EventHandlingResult { - for (path, results) in &search_state - .results - .iter_mut() - .filter(|res| res.included) - .chunk_by(|res| res.path.clone()) - { - let mut results = results.collect::>(); - if let Err(file_err) = Self::replace_in_file(path, &mut results) { - results.iter_mut().for_each(|res| { - res.replace_result = Some(ReplaceResult::Error(file_err.to_string())) - }); + pub fn trigger_replacement(&mut self) { + let (background_processing_sender, background_processing_receiver) = + mpsc::unbounded_channel(); + + match mem::replace( + &mut self.current_screen, + Screen::PerformingReplacement(PerformingReplacementState::new( + None, + background_processing_sender.clone(), + background_processing_receiver, + )), + ) { + Screen::SearchComplete(search_state) => { + let handle = Self::perform_replacement(search_state, background_processing_sender); + if let Screen::PerformingReplacement(ref mut state) = &mut self.current_screen { + state.set_handle(handle); + } else { + panic!( + "Expected screen to be PerformingReplacement, found {:?}", + self.current_screen + ); + } + } + screen => { + self.current_screen = screen; } } + } + pub fn perform_replacement( + mut search_state: SearchState, + background_processing_sender: UnboundedSender, + ) -> JoinHandle<()> { + tokio::spawn(async move { + for (path, results) in &search_state + .results + .iter_mut() + .filter(|res| res.included) + .chunk_by(|res| res.path.clone()) + { + let mut results = results.collect::>(); + if let Err(file_err) = Self::replace_in_file(path, &mut results) { + results.iter_mut().for_each(|res| { + res.replace_result = Some(ReplaceResult::Error(file_err.to_string())) + }); + } + } - let replace_state = self.calculate_statistics(&search_state.results); + let replace_state = Self::calculate_statistics(&search_state.results); - self.current_screen = Screen::Results(replace_state); - EventHandlingResult { - exit: false, - rerender: true, - } + // Ignore error: we may have gone back to the previous screen + let _ = background_processing_sender.send( + BackgroundProcessingEvent::ReplacementCompleted(replace_state), + ); + }) } pub fn handle_background_processing_event( @@ -526,6 +583,13 @@ impl App { rerender: true, } } + BackgroundProcessingEvent::ReplacementCompleted(replace_state) => { + self.current_screen = Screen::Results(replace_state); + EventHandlingResult { + exit: false, + rerender: true, + } + } } } @@ -581,17 +645,7 @@ impl App { .toggle_all_selected(); } (KeyCode::Enter, _) => { - if matches!(self.current_screen, Screen::SearchComplete(_)) { - if let Screen::SearchComplete(search_state) = - mem::replace(&mut self.current_screen, Screen::PerformingReplacement) - { - self.app_event_sender - .send(AppEvent::PerformReplacement(search_state)) - .unwrap(); - } else { - panic!("Expected SearchComplete, found {:?}", self.current_screen); - } - } + self.trigger_replacement(); } (KeyCode::Char('o'), KeyModifiers::CONTROL) => { self.cancel_search(); @@ -635,7 +689,7 @@ impl App { Screen::SearchProgressing(_) | Screen::SearchComplete(_) => { self.handle_key_confirmation(key) } - Screen::PerformingReplacement => false, + Screen::PerformingReplacement(_) => false, // TODO: handle keys here Screen::Results(replace_state) => replace_state.handle_key_results(key), }; Ok(EventHandlingResult { @@ -741,7 +795,7 @@ impl App { false } - fn calculate_statistics(&self, results: &[SearchResult]) -> ReplaceState { + fn calculate_statistics(results: &[SearchResult]) -> ReplaceState { let mut num_successes = 0; let mut num_ignored = 0; let mut errors = vec![]; @@ -964,7 +1018,7 @@ mod tests { async fn test_calculate_statistics_all_success() { let app = build_test_app(vec![success_result(), success_result(), success_result()]); let stats = if let Screen::SearchComplete(search_state) = &app.current_screen { - app.calculate_statistics(&search_state.results) + App::calculate_statistics(&search_state.results) } else { panic!("Expected SearchComplete"); }; @@ -991,7 +1045,7 @@ mod tests { ignored_result(), ]); let stats = if let Screen::SearchComplete(search_state) = &app.current_screen { - app.calculate_statistics(&search_state.results) + App::calculate_statistics(&search_state.results) } else { panic!("Expected SearchComplete"); }; diff --git a/src/event.rs b/src/event.rs index b8d4c5d..1c62f29 100644 --- a/src/event.rs +++ b/src/event.rs @@ -3,7 +3,7 @@ use futures::StreamExt; use std::path::PathBuf; use tokio::sync::mpsc; -use crate::app::SearchState; +use crate::app::ReplaceState; #[derive(Clone, Debug, PartialEq, Eq)] pub enum ReplaceResult { @@ -25,13 +25,13 @@ pub struct SearchResult { pub enum AppEvent { Rerender, PerformSearch, - PerformReplacement(SearchState), } #[derive(Debug)] pub enum BackgroundProcessingEvent { AddSearchResult(SearchResult), SearchCompleted, + ReplacementCompleted(ReplaceState), } #[derive(Debug)] diff --git a/src/ui.rs b/src/ui.rs index d813b36..e17654f 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -434,7 +434,7 @@ pub fn render(app: &App, frame: &mut Frame<'_>) { Screen::SearchProgressing(_) | Screen::SearchComplete(_) => { Box::new(render_confirmation_view) } - Screen::PerformingReplacement => { + Screen::PerformingReplacement(_) => { Box::new(render_loading_view("Performing replacement...".to_owned())) } Screen::Results(ref replace_state) => Box::new(render_results_view(replace_state)), @@ -460,7 +460,7 @@ pub fn render(app: &App, frame: &mut Frame<'_>) { ]); keys } - Screen::PerformingReplacement => vec![], + Screen::PerformingReplacement(_) => vec![], Screen::Results(ref replace_state) => { if !replace_state.errors.is_empty() { vec![" down", " up"] @@ -470,11 +470,8 @@ pub fn render(app: &App, frame: &mut Frame<'_>) { } }; - let additional_keys = if matches!(app.current_screen, Screen::PerformingReplacement) { - vec![] - } else { - vec![" reset", " quit"] - }; + let additional_keys = [" reset", " quit"]; + let all_keys = current_keys .iter() .chain(additional_keys.iter()) diff --git a/tests/app.rs b/tests/app.rs index 8d7447b..5377334 100644 --- a/tests/app.rs +++ b/tests/app.rs @@ -5,7 +5,6 @@ use scooter::{ use serial_test::serial; use std::cmp::max; use std::fs::{self, create_dir_all}; -use std::mem; use std::path::{Path, PathBuf}; use std::thread::sleep; use std::time::{Duration, Instant}; @@ -289,6 +288,7 @@ fn setup_app(temp_dir: &TempDir, search_fields: SearchFields, include_hidden: bo app } +// TODO: simplify this test - it is somewhat tied to the current implementation async fn search_and_replace_test( temp_dir: &TempDir, search_fields: SearchFields, @@ -307,10 +307,7 @@ async fn search_and_replace_test( process_bp_events(&mut app).await; assert!(wait_for_screen!(&app, Screen::SearchComplete)); - // TODO: this mem::replace needs to be kept in sync with the same action that happens in app.rs - can we fix this? - let mut search_state = if let Screen::SearchComplete(search_state) = - mem::replace(&mut app.current_screen, Screen::PerformingReplacement) - { + if let Screen::SearchComplete(search_state) = &mut app.current_screen { for (file_path, num_matches) in &expected_matches { assert_eq!( search_state @@ -327,8 +324,6 @@ async fn search_and_replace_test( } assert_eq!(search_state.results.len(), num_expected_matches); - - search_state } else { panic!( "Expected SearchComplete results, found {:?}", @@ -336,7 +331,10 @@ async fn search_and_replace_test( ); }; - app.perform_replacement(&mut search_state); + app.trigger_replacement(); + + process_bp_events(&mut app).await; + assert!(wait_for_screen!(&app, Screen::Results)); if let Screen::Results(search_state) = &app.current_screen { assert_eq!(search_state.num_successes, num_expected_matches);