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

@@ -160,14 +160,9 @@ impl AppData {
     }
 
     /// Filter related methods
-    /// Get the current filter term
+    /// Get the filterby and filter_term
-    pub const fn get_filter_term(&self) -> Option<&String> {
+    pub const fn get_filter(&self) -> (FilterBy, Option<&String>) {
-        self.filter.term.as_ref()
+        (self.filter.by, self.filter.term.as_ref())
-    }
-
-    /// Get the current filter by choice
-    pub const fn get_filter_by(&self) -> FilterBy {
-        self.filter.by
     }
 
     /// 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!(
-        assert_eq!(app_data.get_filter_term(), Some(&"image_2".to_string()));
+            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!(
-        assert_eq!(app_data.get_filter_term(), Some(&"x".to_string()));
+            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!(
-        assert_eq!(app_data.get_filter_term(), Some(&"x".to_string()));
+            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!(
-        assert_eq!(app_data.get_filter_term(), Some(&"x".to_string()));
+            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!(
-        assert_eq!(app_data.get_filter_term(), Some(&"x".to_string()));
+            app_data.get_filter(),
+            (FilterBy::Image, Some(&"x".to_string()))
+        );
 
         app_data.filter_containers();
         let post_len = app_data.containers.items.len();

src/ui/draw_blocks.rs

@@ -229,7 +229,7 @@ pub fn containers(
         .collect::<Vec<_>>();
 
     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<Mutex<AppData>>) -> [Span; 4] {
+fn filter_by_spans(fd: &FrameData) -> [Span; 4] {
-    let filter_by = app_data.lock().get_filter_by();
-
     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<Mutex<AppData>>) -> [Span; 4] {
 }
 
 /// Draw the filter bar
-pub fn filter_bar(area: Rect, frame: &mut Frame, app_data: &Arc<Mutex<AppData>>) {
+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<Mutex<AppData>>)
         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<Mutex<AppData>>)
                 .add_modifier(Modifier::BOLD),
         ),
         Span::styled(
-            app_data
+            fd.filter_term
-                .lock()
+                .as_ref()
-                .get_filter_term()
+                .map_or(String::new(), std::clone::Clone::clone),
-                .map_or(String::new(), std::borrow::ToOwned::to_owned),
             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();
 

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<ContainerId>,
     has_containers: bool,
+    filter_by: FilterBy,
+    filter_term: Option<String>,
     has_error: Option<AppError>,
     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<Mutex<AppData>>, gui_state: &Arc<Mut
 
     // Draw filter bar
     if let Some(rect) = whole_layout.get(2) {
-        draw_blocks::filter_bar(*rect, f, app_data);
+        draw_blocks::filter_bar(*rect, f, &fd);
     }
 
     if let Some(id) = fd.delete_confirm.as_ref() {