From ba362f81c97b877e61ba1be7e33bb42b94e82418 Mon Sep 17 00:00:00 2001
From: ClementTsang <cjhtsang@uwaterloo.ca>
Date: Sun, 15 May 2022 05:01:19 -0400
Subject: [PATCH] bug: fix issues with macos and windows during refactor

---
 src/app.rs                                  |  4 ++--
 src/app/data_harvester.rs                   | 22 ++++++++++++++-----
 src/app/data_harvester/network/heim.rs      |  2 +-
 src/app/data_harvester/processes/macos.rs   | 20 ++++++++++++-----
 src/app/data_harvester/processes/mod.rs     |  4 +---
 src/app/data_harvester/processes/windows.rs |  4 ++--
 src/app/states/table_state.rs               |  4 +++-
 src/app/widgets/process_widget.rs           | 24 +++++++++++++++------
 src/canvas.rs                               |  4 ++--
 src/clap.rs                                 |  2 +-
 src/lib.rs                                  |  2 +-
 src/utils/gen_util.rs                       | 21 ++++++++++++++++++
 12 files changed, 84 insertions(+), 29 deletions(-)

diff --git a/src/app.rs b/src/app.rs
index 6cceef0d..b226b3f5 100644
--- a/src/app.rs
+++ b/src/app.rs
@@ -11,7 +11,7 @@ use unicode_width::{UnicodeWidthChar, UnicodeWidthStr};
 use typed_builder::*;
 
 use data_farmer::*;
-use data_harvester::{processes, temperature};
+use data_harvester::temperature;
 use layout_manager::*;
 pub use states::*;
 
@@ -132,7 +132,7 @@ pub struct App {
 
     #[cfg(target_family = "unix")]
     #[builder(default, setter(skip))]
-    pub user_table: processes::UserTable,
+    pub user_table: data_harvester::processes::UserTable,
 
     pub cpu_state: CpuState,
     pub mem_state: MemState,
diff --git a/src/app/data_harvester.rs b/src/app/data_harvester.rs
index 7d3500dc..979bdadb 100644
--- a/src/app/data_harvester.rs
+++ b/src/app/data_harvester.rs
@@ -280,11 +280,23 @@ impl DataCollector {
                 }
                 #[cfg(not(target_os = "linux"))]
                 {
-                    processes::get_process_data(
-                        &self.sys,
-                        self.use_current_cpu_total,
-                        self.mem_total_kb,
-                    )
+                    #[cfg(target_family = "unix")]
+                    {
+                        processes::get_process_data(
+                            &self.sys,
+                            self.use_current_cpu_total,
+                            self.mem_total_kb,
+                            &mut self.user_table,
+                        )
+                    }
+                    #[cfg(not(target_family = "unix"))]
+                    {
+                        processes::get_process_data(
+                            &self.sys,
+                            self.use_current_cpu_total,
+                            self.mem_total_kb,
+                        )
+                    }
                 }
             } {
                 self.data.list_of_processes = Some(process_list);
diff --git a/src/app/data_harvester/network/heim.rs b/src/app/data_harvester/network/heim.rs
index 59a6ac69..b7980ddb 100644
--- a/src/app/data_harvester/network/heim.rs
+++ b/src/app/data_harvester/network/heim.rs
@@ -3,7 +3,7 @@
 use super::NetworkHarvest;
 use std::time::Instant;
 
-// FIXME: Eventually make it so that this thing also takes individual usage into account, so we can show per-interface!
+// TODO: Eventually make it so that this thing also takes individual usage into account, so we can show per-interface!
 pub async fn get_network_data(
     prev_net_access_time: Instant, prev_net_rx: &mut u64, prev_net_tx: &mut u64,
     curr_time: Instant, actually_get: bool, filter: &Option<crate::app::Filter>,
diff --git a/src/app/data_harvester/processes/macos.rs b/src/app/data_harvester/processes/macos.rs
index ba75687f..0c401b0a 100644
--- a/src/app/data_harvester/processes/macos.rs
+++ b/src/app/data_harvester/processes/macos.rs
@@ -3,6 +3,8 @@
 use super::ProcessHarvest;
 use sysinfo::{PidExt, ProcessExt, ProcessStatus, ProcessorExt, System, SystemExt};
 
+use crate::data_harvester::processes::UserTable;
+
 fn get_macos_process_cpu_usage(
     pids: &[i32],
 ) -> std::io::Result<std::collections::HashMap<i32, f64>> {
@@ -35,7 +37,7 @@ fn get_macos_process_cpu_usage(
 }
 
 pub fn get_process_data(
-    sys: &System, use_current_cpu_total: bool, mem_total_kb: u64,
+    sys: &System, use_current_cpu_total: bool, mem_total_kb: u64, user_table: &mut UserTable,
 ) -> crate::utils::error::Result<Vec<ProcessHarvest>> {
     let mut process_vector: Vec<ProcessHarvest> = Vec::new();
     let process_hashmap = sys.processes();
@@ -86,6 +88,11 @@ pub fn get_process_data(
         };
 
         let disk_usage = process_val.disk_usage();
+        let process_state = {
+            let ps = process_val.status();
+            (ps.to_string(), convert_process_status_to_char(ps))
+        };
+        let uid = process_val.uid;
         process_vector.push(ProcessHarvest {
             pid: process_val.pid().as_u32() as _,
             parent_pid: process_val.parent().map(|p| p.as_u32() as _),
@@ -102,16 +109,19 @@ pub fn get_process_data(
             write_bytes_per_sec: disk_usage.written_bytes,
             total_read_bytes: disk_usage.total_read_bytes,
             total_write_bytes: disk_usage.total_written_bytes,
-            process_state: process_val.status().to_string(),
-            process_state_char: convert_process_status_to_char(process_val.status()),
-            uid: process_val.uid,
+            process_state,
+            uid,
+            user: user_table
+                .get_uid_to_username_mapping(uid)
+                .map(Into::into)
+                .unwrap_or_else(|_| "N/A".into()),
         });
     }
 
     let unknown_state = ProcessStatus::Unknown(0).to_string();
     let cpu_usage_unknown_pids: Vec<i32> = process_vector
         .iter()
-        .filter(|process| process.process_state == unknown_state)
+        .filter(|process| process.process_state.0 == unknown_state)
         .map(|process| process.pid)
         .collect();
     let cpu_usages = get_macos_process_cpu_usage(&cpu_usage_unknown_pids)?;
diff --git a/src/app/data_harvester/processes/mod.rs b/src/app/data_harvester/processes/mod.rs
index a34dbf8a..40662d7a 100644
--- a/src/app/data_harvester/processes/mod.rs
+++ b/src/app/data_harvester/processes/mod.rs
@@ -23,8 +23,6 @@ cfg_if::cfg_if! {
     }
 }
 
-use std::borrow::Cow;
-
 use crate::Pid;
 
 #[derive(Debug, Clone, Default)]
@@ -71,7 +69,7 @@ pub struct ProcessHarvest {
 
     /// This is the process' user. This is only used on Unix platforms.
     #[cfg(target_family = "unix")]
-    pub user: Cow<'static, str>,
+    pub user: std::borrow::Cow<'static, str>,
     // TODO: Additional fields
     // pub rss_kb: u64,
     // pub virt_kb: u64,
diff --git a/src/app/data_harvester/processes/windows.rs b/src/app/data_harvester/processes/windows.rs
index af9ead5f..a3564874 100644
--- a/src/app/data_harvester/processes/windows.rs
+++ b/src/app/data_harvester/processes/windows.rs
@@ -55,6 +55,7 @@ pub fn get_process_data(
         };
 
         let disk_usage = process_val.disk_usage();
+        let process_state = (process_val.status().to_string(), 'R');
         process_vector.push(ProcessHarvest {
             pid: process_val.pid().as_u32() as _,
             parent_pid: process_val.parent().map(|p| p.as_u32() as _),
@@ -71,8 +72,7 @@ pub fn get_process_data(
             write_bytes_per_sec: disk_usage.written_bytes,
             total_read_bytes: disk_usage.total_read_bytes,
             total_write_bytes: disk_usage.total_written_bytes,
-            process_state: process_val.status().to_string(),
-            process_state_char: 'R',
+            process_state,
         });
     }
 
diff --git a/src/app/states/table_state.rs b/src/app/states/table_state.rs
index 38ade467..b7bc9dab 100644
--- a/src/app/states/table_state.rs
+++ b/src/app/states/table_state.rs
@@ -626,5 +626,7 @@ mod test {
     }
 
     #[test]
-    fn test_row_width_boundary_creation() {}
+    fn test_row_width_boundary_creation() {
+        // FIXME: [TEST] finish this
+    }
 }
diff --git a/src/app/widgets/process_widget.rs b/src/app/widgets/process_widget.rs
index 74d4c3bf..bf449e85 100644
--- a/src/app/widgets/process_widget.rs
+++ b/src/app/widgets/process_widget.rs
@@ -235,11 +235,14 @@ impl ProcWidgetColumn {
                 }
             }
             ProcWidgetColumn::User => {
-                data.sort_by_cached_key(|p| p.name.to_lowercase());
-                if sort_descending {
-                    data.sort_by_cached_key(|p| Reverse(p.user.to_lowercase()));
-                } else {
-                    data.sort_by_cached_key(|p| p.user.to_lowercase());
+                #[cfg(target_family = "unix")]
+                {
+                    data.sort_by_cached_key(|p| p.name.to_lowercase());
+                    if sort_descending {
+                        data.sort_by_cached_key(|p| Reverse(p.user.to_lowercase()));
+                    } else {
+                        data.sort_by_cached_key(|p| p.user.to_lowercase());
+                    }
                 }
             }
         }
@@ -785,7 +788,16 @@ impl ProcWidget {
                             main: process.process_state.0.clone().into(),
                             alt: process.process_state.1.to_string().into(),
                         },
-                        ProcWidgetColumn::User => process.user.clone().into(),
+                        ProcWidgetColumn::User => {
+                            #[cfg(target_family = "unix")]
+                            {
+                                process.user.clone().into()
+                            }
+                            #[cfg(not(target_family = "unix"))]
+                            {
+                                "".into()
+                            }
+                        }
                     };
 
                     if let Some(curr) = col_widths.get_mut(itx) {
diff --git a/src/canvas.rs b/src/canvas.rs
index 6736c96a..e69c5053 100644
--- a/src/canvas.rs
+++ b/src/canvas.rs
@@ -68,9 +68,9 @@ pub struct Painter {
     height: u16,
     width: u16,
     styled_help_text: Vec<Spans<'static>>,
-    is_mac_os: bool, // FIXME: This feels out of place...
+    is_mac_os: bool, // TODO: This feels out of place...
 
-    // FIXME: Redo this entire thing.
+    // TODO: Redo this entire thing.
     row_constraints: Vec<Constraint>,
     col_constraints: Vec<Vec<Constraint>>,
     col_row_constraints: Vec<Vec<Vec<Constraint>>>,
diff --git a/src/clap.rs b/src/clap.rs
index 0fbd3223..c667088a 100644
--- a/src/clap.rs
+++ b/src/clap.rs
@@ -148,7 +148,7 @@ pub fn build_app() -> Command<'static> {
         .help("Uses a dot marker for graphs.")
         .long_help("Uses a dot marker for graphs as opposed to the default braille marker.");
 
-    let group = Arg::new("group") // FIXME: Rename this to something like "group_process", would be "breaking" though.
+    let group = Arg::new("group") // TODO: Rename this to something like "group_process", would be "breaking" though.
         .short('g')
         .long("group")
         .help("Groups processes with the same name by default.")
diff --git a/src/lib.rs b/src/lib.rs
index 84e5b2e6..4042226b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -316,7 +316,7 @@ pub fn handle_force_redraws(app: &mut App) {
         app.cpu_state.force_update = None;
     }
 
-    // FIXME: [OPT] Prefer reassignment over new vectors?
+    // TODO: [OPT] Prefer reassignment over new vectors?
     if app.mem_state.force_update.is_some() {
         app.converted_data.mem_data = convert_mem_data_points(&app.data_collection);
         app.converted_data.swap_data = convert_swap_data_points(&app.data_collection);
diff --git a/src/utils/gen_util.rs b/src/utils/gen_util.rs
index ed3edcd3..c1dcef99 100644
--- a/src/utils/gen_util.rs
+++ b/src/utils/gen_util.rs
@@ -116,6 +116,7 @@ pub fn partial_ordering_rev<T: std::cmp::PartialOrd>(a: T, b: T) -> Ordering {
 pub fn get_ordering<T: std::cmp::PartialOrd>(
     a_val: T, b_val: T, reverse_order: bool,
 ) -> std::cmp::Ordering {
+    // FIXME: Maybe we can just delete this entirely and change references to use partial_ordering...
     match a_val.partial_cmp(&b_val) {
         Some(x) => match x {
             Ordering::Greater => {
@@ -137,3 +138,23 @@ pub fn get_ordering<T: std::cmp::PartialOrd>(
         None => Ordering::Equal,
     }
 }
+
+#[cfg(test)]
+mod test {
+    // use super::*;
+
+    #[test]
+    fn test_sort_partial_fn() {
+        // FIXME: Do this
+    }
+
+    #[test]
+    fn test_partial_ordering() {
+        // FIXME: Do this
+    }
+
+    #[test]
+    fn test_reverse_partial_ordering() {
+        // FIXME: Do this
+    }
+}