refactor: clean up help drawing code (ClementTsang#1374)

* refactor: clean up the help drawing

* a bit cleaner

* add test

* some fmt

src/canvas.rs

@@ -11,7 +11,7 @@ use styling::*;
 use tui::{
     backend::Backend,
     layout::{Constraint, Direction, Layout, Rect},
-    text::{Line, Span},
+    text::Span,
     widgets::Paragraph,
     Frame, Terminal,
 };
@@ -58,9 +58,8 @@ impl FromStr for ColourScheme {
 /// Handles the canvas' state.
 pub struct Painter {
     pub colours: CanvasStyling,
-    height: u16,
-    width: u16,
-    styled_help_text: Vec<Line<'static>>,
+    previous_height: u16,
+    previous_width: u16,
 
     // TODO: Redo this entire thing.
     row_constraints: Vec<LayoutConstraint>,
@@ -151,11 +150,10 @@ impl Painter {
             col_constraints.push(new_col_constraints);
         });
 
-        let mut painter = Painter {
+        let painter = Painter {
             colours: styling,
-            height: 0,
-            width: 0,
-            styled_help_text: Vec::default(),
+            previous_height: 0,
+            previous_width: 0,
             row_constraints,
             col_constraints,
             col_row_constraints,
@@ -164,8 +162,6 @@ impl Painter {
             derived_widget_draw_locs: Vec::default(),
         };
 
-        painter.complete_painter_init();
-
         Ok(painter)
     }
 
@@ -179,40 +175,6 @@ impl Painter {
         }
     }
 
-    /// Must be run once before drawing, but after setting colours.
-    /// This is to set some remaining styles and text.
-    fn complete_painter_init(&mut self) {
-        let mut styled_help_spans = Vec::new();
-
-        // Init help text:
-        HELP_TEXT.iter().enumerate().for_each(|(itx, section)| {
-            if itx == 0 {
-                styled_help_spans.extend(
-                    section
-                        .iter()
-                        .map(|&text| Span::styled(text, self.colours.text_style))
-                        .collect::<Vec<_>>(),
-                );
-            } else {
-                // Not required check but it runs only a few times... so whatever ig, prevents me from
-                // being dumb and leaving a help text section only one line long.
-                if section.len() > 1 {
-                    styled_help_spans.push(Span::raw(""));
-                    styled_help_spans
-                        .push(Span::styled(section[0], self.colours.table_header_style));
-                    styled_help_spans.extend(
-                        section[1..]
-                            .iter()
-                            .map(|&text| Span::styled(text, self.colours.text_style))
-                            .collect::<Vec<_>>(),
-                    );
-                }
-            }
-        });
-
-        self.styled_help_text = styled_help_spans.into_iter().map(Line::from).collect();
-    }
-
     fn draw_frozen_indicator(&self, f: &mut Frame<'_>, draw_loc: Rect) {
         f.render_widget(
             Paragraph::new(Span::styled(
@@ -244,12 +206,13 @@ impl Painter {
             let terminal_height = terminal_size.height;
             let terminal_width = terminal_size.width;
 
-            if (self.height == 0 && self.width == 0)
-                || (self.height != terminal_height || self.width != terminal_width)
+            if (self.previous_height == 0 && self.previous_width == 0)
+                || (self.previous_height != terminal_height
+                    || self.previous_width != terminal_width)
             {
                 app_state.is_force_redraw = true;
-                self.height = terminal_height;
-                self.width = terminal_width;
+                self.previous_height = terminal_height;
+                self.previous_width = terminal_width;
             }
 
             if app_state.should_get_widget_bounds() {
@@ -389,9 +352,9 @@ impl Painter {
                                 _ => 0,
                             };
 
-                        self.draw_process_widget(f, app_state, rect[0], true, widget_id);
+                        self.draw_process(f, app_state, rect[0], true, widget_id);
                     }
-                    Battery => self.draw_battery_display(
+                    Battery => self.draw_battery(
                         f,
                         app_state,
                         rect[0],
@@ -495,18 +458,12 @@ impl Painter {
                                         ProcSort => 2,
                                         _ => 0,
                                     };
-                                self.draw_process_widget(
-                                    f,
-                                    app_state,
-                                    vertical_chunks[3],
-                                    false,
-                                    wid,
-                                );
+                                self.draw_process(f, app_state, vertical_chunks[3], false, wid);
                             }
                             Temp => {
                                 self.draw_temp_table(f, app_state, vertical_chunks[3], widget_id)
                             }
-                            Battery => self.draw_battery_display(
+                            Battery => self.draw_battery(
                                 f,
                                 app_state,
                                 vertical_chunks[3],
@@ -775,29 +732,16 @@ impl Painter {
         widget_draw_locs: &[Rect],
     ) {
         use BottomWidgetType::*;
-        for (widget, widget_draw_loc) in widgets.children.iter().zip(widget_draw_locs) {
-            if widget_draw_loc.width >= 2 && widget_draw_loc.height >= 2 {
+        for (widget, draw_loc) in widgets.children.iter().zip(widget_draw_locs) {
+            if draw_loc.width >= 2 && draw_loc.height >= 2 {
                 match &widget.widget_type {
-                    Empty => {}
-                    Cpu => self.draw_cpu(f, app_state, *widget_draw_loc, widget.widget_id),
-                    Mem => self.draw_memory_graph(f, app_state, *widget_draw_loc, widget.widget_id),
-                    Net => self.draw_network(f, app_state, *widget_draw_loc, widget.widget_id),
-                    Temp => self.draw_temp_table(f, app_state, *widget_draw_loc, widget.widget_id),
-                    Disk => self.draw_disk_table(f, app_state, *widget_draw_loc, widget.widget_id),
-                    Proc => self.draw_process_widget(
-                        f,
-                        app_state,
-                        *widget_draw_loc,
-                        true,
-                        widget.widget_id,
-                    ),
-                    Battery => self.draw_battery_display(
-                        f,
-                        app_state,
-                        *widget_draw_loc,
-                        true,
-                        widget.widget_id,
-                    ),
+                    Cpu => self.draw_cpu(f, app_state, *draw_loc, widget.widget_id),
+                    Mem => self.draw_memory_graph(f, app_state, *draw_loc, widget.widget_id),
+                    Net => self.draw_network(f, app_state, *draw_loc, widget.widget_id),
+                    Temp => self.draw_temp_table(f, app_state, *draw_loc, widget.widget_id),
+                    Disk => self.draw_disk_table(f, app_state, *draw_loc, widget.widget_id),
+                    Proc => self.draw_process(f, app_state, *draw_loc, true, widget.widget_id),
+                    Battery => self.draw_battery(f, app_state, *draw_loc, true, widget.widget_id),
                     _ => {}
                 }
             }

src/canvas/dialogs/help_dialog.rs

@@ -8,13 +8,41 @@ use tui::{
 };
 use unicode_width::UnicodeWidthStr;
 
-use crate::{app::App, canvas::Painter, constants};
+use crate::{
+    app::App,
+    canvas::Painter,
+    constants::{self, HELP_TEXT},
+};
 
 const HELP_BASE: &str = " Help ── Esc to close ";
 
 // TODO: [REFACTOR] Make generic dialog boxes to build off of instead?
 impl Painter {
+    fn help_text_lines(&self) -> Vec<Line<'_>> {
+        let mut styled_help_spans = Vec::new();
+
+        // Init help text:
+        HELP_TEXT.iter().enumerate().for_each(|(itx, section)| {
+            let mut section = section.iter();
+
+            if itx > 0 {
+                if let Some(header) = section.next() {
+                    styled_help_spans.push(Span::default());
+                    styled_help_spans.push(Span::styled(*header, self.colours.table_header_style));
+                }
+            }
+
+            section.for_each(|&text| {
+                styled_help_spans.push(Span::styled(text, self.colours.text_style))
+            });
+        });
+
+        styled_help_spans.into_iter().map(Line::from).collect()
+    }
+
     pub fn draw_help_dialog(&self, f: &mut Frame<'_>, app_state: &mut App, draw_loc: Rect) {
+        let styled_help_text = self.help_text_lines();
+
         let help_title = Line::from(vec![
             Span::styled(" Help ", self.colours.widget_title_style),
             Span::styled(
@@ -35,11 +63,11 @@ impl Painter {
             .border_style(self.colours.border_style);
 
         if app_state.should_get_widget_bounds() {
-            app_state.help_dialog_state.height = block.inner(draw_loc).height;
-
             // We must also recalculate how many lines are wrapping to properly get scrolling to work on
             // small terminal sizes... oh joy.
 
+            app_state.help_dialog_state.height = block.inner(draw_loc).height;
+
             let mut overflow_buffer = 0;
             let paragraph_width = max(draw_loc.width.saturating_sub(2), 1);
             let mut prev_section_len = 0;
@@ -73,10 +101,10 @@ impl Painter {
                 });
 
             let max_scroll_index = &mut app_state.help_dialog_state.scroll_state.max_scroll_index;
-            *max_scroll_index = (self.styled_help_text.len() as u16 + 3 + overflow_buffer)
+            *max_scroll_index = (styled_help_text.len() as u16 + 3 + overflow_buffer)
                 .saturating_sub(draw_loc.height + 1);
 
-            // Fix if over-scrolled
+            // Fix the scroll index if it is over-scrolled
             let index = &mut app_state
                 .help_dialog_state
                 .scroll_state
@@ -86,7 +114,7 @@ impl Painter {
         }
 
         f.render_widget(
-            Paragraph::new(self.styled_help_text.clone())
+            Paragraph::new(styled_help_text.clone())
                 .block(block)
                 .style(self.colours.text_style)
                 .alignment(Alignment::Left)

src/canvas/widgets/battery_display.rs

@@ -15,7 +15,7 @@ use crate::{
 };
 
 impl Painter {
-    pub fn draw_battery_display(
+    pub fn draw_battery(
         &self, f: &mut Frame<'_>, app_state: &mut App, draw_loc: Rect, draw_border: bool,
         widget_id: u64,
     ) {

src/canvas/widgets/process_table.rs

@@ -21,7 +21,7 @@ const SORT_MENU_WIDTH: u16 = 7;
 impl Painter {
     /// Draws and handles all process-related drawing.  Use this.
     /// - `widget_id` here represents the widget ID of the process widget itself!
-    pub fn draw_process_widget(
+    pub fn draw_process(
         &self, f: &mut Frame<'_>, app_state: &mut App, draw_loc: Rect, draw_border: bool,
         widget_id: u64,
     ) {

src/constants.rs

@@ -752,13 +752,24 @@ mod test {
 
     #[test]
     fn help_menu_matches_entry_len() {
+        // The two match since HELP_TEXT contains HELP_CONTENTS_TEXT as an entry
         assert_eq!(
             HELP_CONTENTS_TEXT.len(),
             HELP_TEXT.len(),
             "the two should be equal, or this test should be updated"
         )
     }
 
+    #[test]
+    fn help_menu_text_has_sections() {
+        for (itx, line) in HELP_TEXT.iter().enumerate() {
+            if itx > 0 {
+                assert!(line.len() >= 2, "each section should be at least 2 lines");
+                assert!(line[0].contains(" - "), "each section should have a header");
+            }
+        }
+    }
+
     /// This test exists because previously, [`SIDE_BORDERS`] was set incorrectly after I moved from
     /// tui-rs to ratatui.
     #[test]