refactor: some string-related code cleanup/refactor (#1463)

* other: organize some utility function files * deps: remove kstring * refactor: some naming changes * refactor: some more small refactoring/naming changes * simplify to_cell to return a Cow * enable lints
ClementTsang · May 7, 2024 · 398bf59 · 398bf59
1 parent bcc8917
commit 398bf59
Show file tree

Hide file tree

Showing 23 changed files with 696 additions and 720 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -86,7 +86,6 @@ humantime = "2.1.0"
 indexmap = "2.2.6"
 indoc = "2.0.5"
 itertools = "0.12.1"
-kstring = { version = "2.0.0", features = ["arc"] }
 log = { version = "0.4.21", optional = true }
 nvml-wrapper = { version = "0.10.0", optional = true, features = ["legacy-functions"] }
 regex = "1.10.4"
@@ -208,17 +207,15 @@ assets = [
     { source = "desktop/bottom.desktop", dest = "/usr/share/applications/bottom.desktop", mode = "644" },
 ]
 
-# Activate whenever we bump the unofficial MSRV to 1.74, I guess?
-# [lints.rust]
-# rust_2018_idioms = "deny"
+[lints.rust]
+rust_2018_idioms = "deny"
 # missing_docs = "deny"
-# unused_extern_crates = "deny"
 
-# [lints.rustdoc]
-# broken_intra_doc_links = "deny"
-# missing_crate_level_docs = "deny"
+[lints.rustdoc]
+broken_intra_doc_links = "deny"
+missing_crate_level_docs = "deny"
 
-# [lints.clippy]
-# todo = "deny"
-# unimplemented = "deny"
-# missing_safety_doc = "deny"
+[lints.clippy]
+todo = "deny"
+unimplemented = "deny"
+missing_safety_doc = "deny"
diff --git a/src/app/data_farmer.rs b/src/app/data_farmer.rs
@@ -21,7 +21,7 @@ use hashbrown::HashMap;
 use crate::data_collection::batteries;
 use crate::{
     data_collection::{cpu, disks, memory, network, processes::ProcessHarvest, temperature, Data},
-    utils::{data_prefixes::*, general::get_decimal_bytes},
+    utils::data_prefixes::*,
     Pid,
 };
 

diff --git a/src/app/states.rs b/src/app/states.rs
@@ -7,7 +7,7 @@ use unicode_segmentation::{GraphemeCursor, GraphemeIncomplete, UnicodeSegmentati
 use crate::{
     app::{layout_manager::BottomWidgetType, query::*},
     constants,
-    utils::general::str_width,
+    utils::strings::str_width,
     widgets::{
         BatteryWidgetState, CpuWidgetState, DiskTableWidget, MemWidgetState, NetWidgetState,
         ProcWidgetState, TempWidgetState,

diff --git a/src/bin/main.rs b/src/bin/main.rs
@@ -208,11 +208,10 @@ fn main() -> Result<()> {
                     }
 
                     if !app.frozen_state.is_frozen() {
-                        // Convert all data into tui-compliant components
+                        // Convert all data into data for the displayed widgets.
 
-                        // Network
                         if app.used_widgets.use_net {
-                            let network_data = convert_network_data_points(
+                            let network_data = convert_network_points(
                                 &app.data_collection,
                                 app.app_config_fields.use_basic_mode
                                     || app.app_config_fields.use_old_network_legend,
@@ -232,18 +231,16 @@ fn main() -> Result<()> {
                             }
                         }
 
-                        // Disk
                         if app.used_widgets.use_disk {
-                            app.converted_data.ingest_disk_data(&app.data_collection);
+                            app.converted_data.convert_disk_data(&app.data_collection);
 
                             for disk in app.states.disk_state.widget_states.values_mut() {
                                 disk.force_data_update();
                             }
                         }
 
-                        // Temperatures
                         if app.used_widgets.use_temp {
-                            app.converted_data.ingest_temp_data(
+                            app.converted_data.convert_temp_data(
                                 &app.data_collection,
                                 app.app_config_fields.temperature_type,
                             );
@@ -253,22 +250,25 @@ fn main() -> Result<()> {
                             }
                         }
 
-                        // Memory
                         if app.used_widgets.use_mem {
                             app.converted_data.mem_data =
                                 convert_mem_data_points(&app.data_collection);
+
                             #[cfg(not(target_os = "windows"))]
                             {
                                 app.converted_data.cache_data =
                                     convert_cache_data_points(&app.data_collection);
                             }
+
                             app.converted_data.swap_data =
                                 convert_swap_data_points(&app.data_collection);
+
                             #[cfg(feature = "zfs")]
                             {
                                 app.converted_data.arc_data =
                                     convert_arc_data_points(&app.data_collection);
                             }
+
                             #[cfg(feature = "gpu")]
                             {
                                 app.converted_data.gpu_data =
@@ -277,8 +277,10 @@ fn main() -> Result<()> {
 
                             app.converted_data.mem_labels =
                                 convert_mem_label(&app.data_collection.memory_harvest);
+
                             app.converted_data.swap_labels =
                                 convert_mem_label(&app.data_collection.swap_harvest);
+
                             #[cfg(not(target_os = "windows"))]
                             {
                                 app.converted_data.cache_labels =
@@ -287,26 +289,22 @@ fn main() -> Result<()> {
 
                             #[cfg(feature = "zfs")]
                             {
-                                let arc_labels =
+                                app.converted_data.arc_labels =
                                     convert_mem_label(&app.data_collection.arc_harvest);
-                                app.converted_data.arc_labels = arc_labels;
                             }
                         }
 
-                        // CPU
                         if app.used_widgets.use_cpu {
-                            app.converted_data.ingest_cpu_data(&app.data_collection);
+                            app.converted_data.convert_cpu_data(&app.data_collection);
                             app.converted_data.load_avg_data = app.data_collection.load_avg_harvest;
                         }
 
-                        // Processes
                         if app.used_widgets.use_proc {
                             for proc in app.states.proc_state.widget_states.values_mut() {
                                 proc.force_data_update();
                             }
                         }
 
-                        // Battery
                         #[cfg(feature = "battery")]
                         {
                             if app.used_widgets.use_battery {

diff --git a/src/canvas/components/data_table.rs b/src/canvas/components/data_table.rs
@@ -152,7 +152,7 @@ impl<DataType: DataToCell<H>, H: ColumnHeader, S: SortType, C: DataTableColumn<H
 
 #[cfg(test)]
 mod test {
-    use std::num::NonZeroU16;
+    use std::{borrow::Cow, num::NonZeroU16};
 
     use super::*;
 
@@ -164,7 +164,7 @@ mod test {
     impl DataToCell<&'static str> for TestType {
         fn to_cell(
             &self, _column: &&'static str, _calculated_width: NonZeroU16,
-        ) -> Option<tui::text::Text<'_>> {
+        ) -> Option<Cow<'static, str>> {
             None
         }
 

diff --git a/src/canvas/components/data_table/data_type.rs b/src/canvas/components/data_table/data_type.rs
@@ -1,6 +1,6 @@
-use std::num::NonZeroU16;
+use std::{borrow::Cow, num::NonZeroU16};
 
-use tui::{text::Text, widgets::Row};
+use tui::widgets::Row;
 
 use super::{ColumnHeader, DataTableColumn};
 use crate::canvas::Painter;
@@ -9,8 +9,9 @@ pub trait DataToCell<H>
 where
     H: ColumnHeader,
 {
-    /// Given data, a column, and its corresponding width, return what should be displayed in the [`DataTable`](super::DataTable).
-    fn to_cell(&self, column: &H, calculated_width: NonZeroU16) -> Option<Text<'_>>;
+    /// Given data, a column, and its corresponding width, return the string in the cell that will
+    /// be displayed in the [`DataTable`](super::DataTable).
+    fn to_cell(&self, column: &H, calculated_width: NonZeroU16) -> Option<Cow<'static, str>>;
 
     /// Apply styling to the generated [`Row`] of cells.
     ///

diff --git a/src/canvas/components/data_table/draw.rs b/src/canvas/components/data_table/draw.rs
@@ -20,6 +20,7 @@ use crate::{
     app::layout_manager::BottomWidget,
     canvas::Painter,
     constants::{SIDE_BORDERS, TABLE_GAP_HEIGHT_LIMIT},
+    utils::strings::truncate_to_text,
 };
 
 pub enum SelectionState {
@@ -225,7 +226,9 @@ where
                                 .iter()
                                 .zip(&self.state.calculated_widths)
                                 .filter_map(|(column, &width)| {
-                                    data_row.to_cell(column.inner(), width)
+                                    data_row
+                                        .to_cell(column.inner(), width)
+                                        .map(|content| truncate_to_text(&content, width.get()))
                                 }),
                         );
 

diff --git a/src/canvas/components/data_table/sortable.rs b/src/canvas/components/data_table/sortable.rs
@@ -8,7 +8,7 @@ use super::{
     ColumnHeader, ColumnWidthBounds, DataTable, DataTableColumn, DataTableProps, DataTableState,
     DataTableStyling, DataToCell,
 };
-use crate::utils::general::truncate_to_text;
+use crate::utils::strings::truncate_to_text;
 
 /// Denotes the sort order.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -99,6 +99,7 @@ impl SortType for Sortable {
                         };
                         // TODO: I think I can get away with removing the truncate_to_text call since
                         // I almost always bind to at least the header size...
+                        // TODO: Or should we instead truncate but ALWAYS leave the arrow at the end?
                         truncate_to_text(&concat_string!(c.header(), arrow), width.get())
                     } else {
                         truncate_to_text(&c.header(), width.get())
@@ -361,7 +362,7 @@ mod test {
     impl DataToCell<ColumnType> for TestType {
         fn to_cell(
             &self, _column: &ColumnType, _calculated_width: NonZeroU16,
-        ) -> Option<tui::text::Text<'_>> {
+        ) -> Option<Cow<'static, str>> {
             None
         }
 

diff --git a/src/canvas/components/tui_widget/time_chart/points.rs b/src/canvas/components/tui_widget/time_chart/points.rs
@@ -6,9 +6,8 @@ use tui::{
     },
 };
 
-use crate::utils::general::partial_ordering;
-
 use super::{Context, Dataset, Point, TimeChart};
+use crate::utils::general::partial_ordering;
 
 impl TimeChart<'_> {
     pub(crate) fn draw_points(&self, ctx: &mut Context<'_>) {