From 356ea5549bb4877e9893fe0e1053e73c5a62e806 Mon Sep 17 00:00:00 2001 From: Jack Wills <32690432+mrjackwills@users.noreply.github.com> Date: Tue, 3 Dec 2024 21:59:00 +0000 Subject: [PATCH] refactor: get_filter combine filter_term and term_by into a tuple, and insert into FrameData, to reduce .lock() calls --- src/app_data/mod.rs | 53 +++++++++++++++++++++++-------------------- src/ui/draw_blocks.rs | 26 ++++++++++----------- src/ui/mod.rs | 15 ++++++++---- 3 files changed, 51 insertions(+), 43 deletions(-) diff --git a/src/app_data/mod.rs b/src/app_data/mod.rs index 9cbd0c8..5e4ed06 100644 --- a/src/app_data/mod.rs +++ b/src/app_data/mod.rs @@ -160,14 +160,9 @@ impl AppData { } /// Filter related methods - /// Get the current filter term - pub const fn get_filter_term(&self) -> Option<&String> { - self.filter.term.as_ref() - } - - /// Get the current filter by choice - pub const fn get_filter_by(&self) -> FilterBy { - self.filter.by + /// Get the filterby and filter_term + pub const fn get_filter(&self) -> (FilterBy, Option<&String>) { + (self.filter.by, self.filter.term.as_ref()) } /// Check if a given container can be inserted into the "visible" list, based on current filter term and filter_by @@ -1646,13 +1641,13 @@ mod tests { let mut app_data = gen_appdata(&containers); - assert!(app_data.get_filter_term().is_none()); + assert!(app_data.get_filter().1.is_none()); let pre_len = app_data.containers.items.len(); app_data.filter_term_push('_'); app_data.filter_term_push('2'); - assert_eq!(app_data.get_filter_term(), Some(&"_2".to_string())); + assert_eq!(app_data.get_filter().1, Some(&"_2".to_string())); app_data.filter_containers(); let post_len = app_data.containers.items.len(); @@ -1672,7 +1667,7 @@ mod tests { let mut app_data = gen_appdata(&containers); - assert!(app_data.get_filter_term().is_none()); + assert!(app_data.get_filter().1.is_none()); let pre_len = app_data.containers.items.len(); for c in ['i', 'm', 'a', 'g', 'e', '_', '2'] { @@ -1681,8 +1676,10 @@ mod tests { // app_data.filter_term_push('2'); app_data.filter_by_next(); - assert_eq!(app_data.get_filter_by(), FilterBy::Image); - assert_eq!(app_data.get_filter_term(), Some(&"image_2".to_string())); + assert_eq!( + app_data.get_filter(), + (FilterBy::Image, Some(&"image_2".to_string())) + ); app_data.filter_containers(); let post_len = app_data.containers.items.len(); @@ -1701,7 +1698,7 @@ mod tests { ContainerStatus::from("Exited".to_owned()).clone_into(&mut containers[0].status); let mut app_data = gen_appdata(&containers); - assert!(app_data.get_filter_term().is_none()); + assert!(app_data.get_filter().1.is_none()); let pre_len = app_data.containers.items.len(); app_data.filter_term_push('x'); @@ -1709,8 +1706,10 @@ mod tests { app_data.filter_by_next(); app_data.filter_by_next(); - assert_eq!(app_data.get_filter_by(), FilterBy::Status); - assert_eq!(app_data.get_filter_term(), Some(&"x".to_string())); + assert_eq!( + app_data.get_filter(), + (FilterBy::Status, Some(&"x".to_string())) + ); app_data.filter_containers(); let post_len = app_data.containers.items.len(); @@ -1729,7 +1728,7 @@ mod tests { ContainerStatus::from("Exited".to_owned()).clone_into(&mut containers[0].status); let mut app_data = gen_appdata(&containers); - assert!(app_data.get_filter_term().is_none()); + assert!(app_data.get_filter().1.is_none()); let pre_len = app_data.containers.items.len(); app_data.filter_term_push('x'); @@ -1738,8 +1737,10 @@ mod tests { app_data.filter_by_next(); app_data.filter_by_next(); - assert_eq!(app_data.get_filter_by(), FilterBy::All); - assert_eq!(app_data.get_filter_term(), Some(&"x".to_string())); + assert_eq!( + app_data.get_filter(), + (FilterBy::All, Some(&"image_2".to_string())) + ); app_data.filter_containers(); let post_len = app_data.containers.items.len(); @@ -1758,7 +1759,7 @@ mod tests { ContainerStatus::from("Exited".to_owned()).clone_into(&mut containers[0].status); let mut app_data = gen_appdata(&containers); - assert!(app_data.get_filter_term().is_none()); + assert!(app_data.get_filter().1.is_none()); let pre_len = app_data.containers.items.len(); app_data.filter_term_push('x'); @@ -1766,8 +1767,10 @@ mod tests { app_data.filter_by_next(); app_data.filter_by_next(); - assert_eq!(app_data.get_filter_by(), FilterBy::Status); - assert_eq!(app_data.get_filter_term(), Some(&"x".to_string())); + assert_eq!( + app_data.get_filter(), + (FilterBy::Status, Some(&"x".to_string())) + ); app_data.filter_containers(); let post_len = app_data.containers.items.len(); @@ -1779,8 +1782,10 @@ mod tests { assert!(!app_data.can_insert(&containers[2])); app_data.filter_by_prev(); - assert_eq!(app_data.get_filter_by(), FilterBy::Image); - assert_eq!(app_data.get_filter_term(), Some(&"x".to_string())); + assert_eq!( + app_data.get_filter(), + (FilterBy::Image, Some(&"x".to_string())) + ); app_data.filter_containers(); let post_len = app_data.containers.items.len(); diff --git a/src/ui/draw_blocks.rs b/src/ui/draw_blocks.rs index 65e8702..8a6a646 100644 --- a/src/ui/draw_blocks.rs +++ b/src/ui/draw_blocks.rs @@ -229,7 +229,7 @@ pub fn containers( .collect::>(); if items.is_empty() { - let text = if app_data.lock().get_filter_term().is_some() { + let text = if fd.filter_term.is_some() { "no containers match filter" } else if gui_state.lock().is_loading() { &format!("loading {}", fd.loading_icon) @@ -423,16 +423,14 @@ fn make_chart<'a, T: Stats + Display>( } /// Create the filter_by by spans, coloured dependant on which one is selected -fn filter_by_spans(app_data: &Arc>) -> [Span; 4] { - let filter_by = app_data.lock().get_filter_by(); - +fn filter_by_spans(fd: &FrameData) -> [Span; 4] { let selected = Style::default().bg(Color::Gray).fg(Color::Black); let not_selected = Style::default().bg(Color::Reset).fg(Color::Reset); // This should be refactored somehow let name = [" Name ", " Image ", " Status ", " All "]; - match filter_by { + match fd.filter_by { FilterBy::Name => [ Span::styled(name[0], selected), Span::styled(name[1], not_selected), @@ -461,7 +459,7 @@ fn filter_by_spans(app_data: &Arc>) -> [Span; 4] { } /// Draw the filter bar -pub fn filter_bar(area: Rect, frame: &mut Frame, app_data: &Arc>) { +pub fn filter_bar(area: Rect, frame: &mut Frame, fd: &FrameData) { let style_but = Style::default().fg(Color::Black).bg(Color::Magenta); let style_desc = Style::default().fg(Color::Gray).bg(Color::Reset); @@ -471,7 +469,7 @@ pub fn filter_bar(area: Rect, frame: &mut Frame, app_data: &Arc>) Span::styled(" ← by → ", style_but), Span::from(" "), ]; - line.extend_from_slice(&filter_by_spans(app_data)); + line.extend_from_slice(&filter_by_spans(fd)); line.extend_from_slice(&[ Span::styled( " term: ", @@ -480,10 +478,9 @@ pub fn filter_bar(area: Rect, frame: &mut Frame, app_data: &Arc>) .add_modifier(Modifier::BOLD), ), Span::styled( - app_data - .lock() - .get_filter_term() - .map_or(String::new(), std::borrow::ToOwned::to_owned), + fd.filter_term + .as_ref() + .map_or(String::new(), std::clone::Clone::clone), Style::default().fg(Color::Gray), ), ]); @@ -2845,7 +2842,7 @@ mod tests { setup .terminal .draw(|f| { - super::filter_bar(setup.area, f, &setup.app_data); + super::filter_bar(setup.area, f, &setup.fd); }) .unwrap(); @@ -2890,7 +2887,7 @@ mod tests { setup .terminal .draw(|f| { - super::filter_bar(setup.area, f, &setup.app_data); + super::filter_bar(setup.area, f, &setup.fd); }) .unwrap(); @@ -2931,10 +2928,11 @@ mod tests { // Test when filter_by chances setup.app_data.lock().filter_by_next(); + let fd = FrameData::from((setup.app_data.lock(), setup.gui_state.lock())); setup .terminal .draw(|f| { - super::filter_bar(setup.area, f, &setup.app_data); + super::filter_bar(setup.area, f, &fd); }) .unwrap(); diff --git a/src/ui/mod.rs b/src/ui/mod.rs index 3a41c1d..31140a1 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -26,7 +26,7 @@ mod gui_state; pub use self::color_match::*; pub use self::gui_state::{DeleteButton, GuiState, SelectablePanel, Status}; use crate::{ - app_data::{AppData, Columns, ContainerId, Header, SortedOrder}, + app_data::{AppData, Columns, ContainerId, FilterBy, Header, SortedOrder}, app_error::AppError, exec::TerminalSize, input_handler::InputMessages, @@ -228,6 +228,8 @@ pub struct FrameData { log_title: String, delete_confirm: Option, has_containers: bool, + filter_by: FilterBy, + filter_term: Option, has_error: Option, height: u16, help_visible: bool, @@ -248,18 +250,21 @@ impl From<(MutexGuard<'_, AppData>, MutexGuard<'_, GuiState>)> for FrameData { 12 }; + let (filter_by, filter_term) = data.0.get_filter(); Self { + filter_by, + filter_term: filter_term.cloned(), + height, columns: data.0.get_width(), container_title: data.0.get_container_title(), - log_title: data.0.get_log_title(), delete_confirm: data.1.get_delete_container(), has_containers: data.0.get_container_len() > 0, has_error: data.0.get_error(), - height, help_visible: data.1.status_contains(&[Status::Help]), - init: data.1.status_contains(&[Status::Init]), info_text: data.1.info_box_text.clone(), + init: data.1.status_contains(&[Status::Init]), loading_icon: data.1.get_loading().to_string(), + log_title: data.0.get_log_title(), selected_panel: data.1.get_selected_panel(), sorted_by: data.0.get_sorted(), } @@ -319,7 +324,7 @@ fn draw_frame(f: &mut Frame, app_data: &Arc>, gui_state: &Arc