other: show N/A for Nvidia GPUs if we detect one but can't get temps (#1557)

* other: show N/A for Nvidia GPUs if we detect one but can't get the temperature

* refactor: driveby refactor of filter system and code for temp

* missed one

src/app/filter.rs

@@ -1,21 +1,32 @@
+use regex::Regex;
+
 /// Filters used by widgets to filter out certain entries.
 /// TODO: Move this out maybe?
 #[derive(Debug, Clone)]
 pub struct Filter {
     /// Whether the filter _accepts_ all entries that match `list`,
     /// or _denies_ any entries that match it.
-    pub is_list_ignored: bool, // TODO: Maybe change to "ignore_matches"?
+    is_list_ignored: bool, // TODO: Maybe change to "ignore_matches"?
 
     /// The list of regexes to match against. Whether it goes through
     /// the filter or not depends on `is_list_ignored`.
-    pub list: Vec<regex::Regex>,
+    list: Vec<Regex>,
 }
 
 impl Filter {
+    /// Create a new filter.
+    #[inline]
+    pub(crate) fn new(ignore_matches: bool, list: Vec<Regex>) -> Self {
+        Self {
+            is_list_ignored: ignore_matches,
+            list,
+        }
+    }
+
     /// Whether the filter should keep the entry or reject it.
     #[inline]
-    pub(crate) fn keep_entry(&self, value: &str) -> bool {
+    pub(crate) fn should_keep(&self, entry: &str) -> bool {
-        if self.has_match(value) {
+        if self.has_match(entry) {
             // If a match is found, then if we wanted to ignore if we match, return false.
             // If we want to keep if we match, return true. Thus, return the
             // inverse of `is_list_ignored`.
@@ -30,6 +41,21 @@ impl Filter {
     pub(crate) fn has_match(&self, value: &str) -> bool {
         self.list.iter().any(|regex| regex.is_match(value))
     }
+
+    /// Whether entries matching the list should be ignored or kept.
+    #[inline]
+    pub(crate) fn ignore_matches(&self) -> bool {
+        self.is_list_ignored
+    }
+
+    /// Check a filter if it exists, otherwise accept if it is [`None`].
+    #[inline]
+    pub(crate) fn optional_should_keep(filter: &Option<Self>, entry: &str) -> bool {
+        filter
+            .as_ref()
+            .map(|f| f.should_keep(entry))
+            .unwrap_or(true)
+    }
 }
 
 #[cfg(test)]
@@ -56,7 +82,7 @@ mod test {
         assert_eq!(
             results
                 .into_iter()
-                .filter(|r| ignore_true.keep_entry(r))
+                .filter(|r| ignore_true.should_keep(r))
                 .collect::<Vec<_>>(),
             vec!["wifi_0", "amd gpu"]
         );
@@ -69,7 +95,7 @@ mod test {
         assert_eq!(
             results
                 .into_iter()
-                .filter(|r| ignore_false.keep_entry(r))
+                .filter(|r| ignore_false.should_keep(r))
                 .collect::<Vec<_>>(),
             vec!["CPU socket temperature", "motherboard temperature"]
         );
@@ -85,7 +111,7 @@ mod test {
         assert_eq!(
             results
                 .into_iter()
-                .filter(|r| multi_true.keep_entry(r))
+                .filter(|r| multi_true.should_keep(r))
                 .collect::<Vec<_>>(),
             vec!["wifi_0", "amd gpu"]
         );
@@ -101,7 +127,7 @@ mod test {
         assert_eq!(
             results
                 .into_iter()
-                .filter(|r| multi_false.keep_entry(r))
+                .filter(|r| multi_false.should_keep(r))
                 .collect::<Vec<_>>(),
             vec!["CPU socket temperature", "motherboard temperature"]
         );

src/data_collection/disks.rs

@@ -103,26 +103,26 @@ pub fn keep_disk_entry(
     disk_name: &str, mount_point: &str, disk_filter: &Option<Filter>, mount_filter: &Option<Filter>,
 ) -> bool {
     match (disk_filter, mount_filter) {
-        (Some(d), Some(m)) => match (d.is_list_ignored, m.is_list_ignored) {
+        (Some(d), Some(m)) => match (d.ignore_matches(), m.ignore_matches()) {
             (true, true) => !(d.has_match(disk_name) || m.has_match(mount_point)),
             (true, false) => {
                 if m.has_match(mount_point) {
                     true
                 } else {
-                    d.keep_entry(disk_name)
+                    d.should_keep(disk_name)
                 }
             }
             (false, true) => {
                 if d.has_match(disk_name) {
                     true
                 } else {
-                    m.keep_entry(mount_point)
+                    m.should_keep(mount_point)
                 }
             }
             (false, false) => d.has_match(disk_name) || m.has_match(mount_point),
         },
-        (Some(d), None) => d.keep_entry(disk_name),
+        (Some(d), None) => d.should_keep(disk_name),
-        (None, Some(m)) => m.keep_entry(mount_point),
+        (None, Some(m)) => m.should_keep(mount_point),
         (None, None) => true,
     }
 }
@@ -158,25 +158,10 @@ mod test {
 
     #[test]
     fn test_keeping_disk_entry() {
-        let disk_ignore = Some(Filter {
+        let disk_ignore = Some(Filter::new(true, vec![Regex::new("nvme").unwrap()]));
-            is_list_ignored: true,
+        let disk_keep = Some(Filter::new(false, vec![Regex::new("nvme").unwrap()]));
-            list: vec![Regex::new("nvme").unwrap()],
+        let mount_ignore = Some(Filter::new(true, vec![Regex::new("boot").unwrap()]));
-        });
+        let mount_keep = Some(Filter::new(false, vec![Regex::new("boot").unwrap()]));
-
-        let disk_keep = Some(Filter {
-            is_list_ignored: false,
-            list: vec![Regex::new("nvme").unwrap()],
-        });
-
-        let mount_ignore = Some(Filter {
-            is_list_ignored: true,
-            list: vec![Regex::new("boot").unwrap()],
-        });
-
-        let mount_keep = Some(Filter {
-            is_list_ignored: false,
-            list: vec![Regex::new("boot").unwrap()],
-        });
 
         assert_eq!(run_filter(&None, &None), vec![0, 1, 2, 3, 4]);
 

src/data_collection/network/sysinfo.rs

@@ -18,7 +18,7 @@ pub fn get_network_data(
 
     for (name, network) in networks {
         let to_keep = if let Some(filter) = filter {
-            filter.keep_entry(name)
+            filter.should_keep(name)
         } else {
             true
         };

src/data_collection/nvidia.rs

@@ -9,7 +9,7 @@ use crate::{
     app::{filter::Filter, layout_manager::UsedWidgets},
     data_collection::{
         memory::MemHarvest,
-        temperature::{is_temp_filtered, TempHarvest, TemperatureType},
+        temperature::{TempHarvest, TemperatureType},
     },
 };
 
@@ -32,6 +32,7 @@ pub fn get_nvidia_vecs(
             let mut mem_vec = Vec::with_capacity(num_gpu as usize);
             let mut proc_vec = Vec::with_capacity(num_gpu as usize);
             let mut total_mem = 0;
+
             for i in 0..num_gpu {
                 if let Ok(device) = nvml.device_by_index(i) {
                     if let Ok(name) = device.name() {
@@ -51,17 +52,26 @@ pub fn get_nvidia_vecs(
                                 ));
                             }
                         }
-                        if widgets_to_harvest.use_temp && is_temp_filtered(filter, &name) {
+
+                        if widgets_to_harvest.use_temp
+                            && Filter::optional_should_keep(filter, &name)
+                        {
                             if let Ok(temperature) = device.temperature(TemperatureSensor::Gpu) {
                                 let temperature = temp_type.convert_temp_unit(temperature as f32);
 
                                 temp_vec.push(TempHarvest {
-                                    name: name.clone(),
+                                    name,
                                     temperature: Some(temperature),
                                 });
+                            } else {
+                                temp_vec.push(TempHarvest {
+                                    name,
+                                    temperature: None,
+                                });
                             }
                         }
                     }
+
                     if widgets_to_harvest.use_proc {
                         let mut procs = HashMap::new();
 
@@ -130,6 +140,7 @@ pub fn get_nvidia_vecs(
                     }
                 }
             }
+
             Some(GpusData {
                 memory: if !mem_vec.is_empty() {
                     Some(mem_vec)

src/data_collection/temperature.rs

@@ -15,8 +15,6 @@ cfg_if::cfg_if! {
 
 use std::str::FromStr;
 
-use crate::app::filter::Filter;
-
 #[derive(Default, Debug, Clone)]
 pub struct TempHarvest {
     pub name: String,
@@ -66,21 +64,6 @@ impl TemperatureType {
     }
 }
 
-pub fn is_temp_filtered(filter: &Option<Filter>, text: &str) -> bool {
-    if let Some(filter) = filter {
-        let mut ret = filter.is_list_ignored;
-        for r in &filter.list {
-            if r.is_match(text) {
-                ret = !filter.is_list_ignored;
-                break;
-            }
-        }
-        ret
-    } else {
-        true
-    }
-}
-
 #[cfg(test)]
 mod test {
     use crate::data_collection::temperature::TemperatureType;

src/data_collection/temperature/linux.rs

@@ -8,7 +8,7 @@ use std::{
 use anyhow::Result;
 use hashbrown::{HashMap, HashSet};
 
-use super::{is_temp_filtered, TempHarvest, TemperatureType};
+use super::{TempHarvest, TemperatureType};
 use crate::app::filter::Filter;
 
 const EMPTY_NAME: &str = "Unknown";
@@ -327,7 +327,7 @@ fn hwmon_temperatures(temp_type: &TemperatureType, filter: &Option<Filter>) -> H
 
                 // TODO: It's possible we may want to move the filter check further up to avoid
                 // probing hwmon if not needed?
-                if is_temp_filtered(filter, &name) {
+                if Filter::optional_should_keep(filter, &name) {
                     if let Ok(temp_celsius) = parse_temp(&temp_path) {
                         temperatures.push(TempHarvest {
                             name,
@@ -377,7 +377,7 @@ fn add_thermal_zone_temperatures(
                     name
                 };
 
-                if is_temp_filtered(filter, &name) {
+                if Filter::optional_should_keep(filter, &name) {
                     let temp_path = file_path.join("temp");
                     if let Ok(temp_celsius) = parse_temp(&temp_path) {
                         let name = counted_name(&mut seen_names, name);

src/data_collection/temperature/sysinfo.rs

@@ -2,7 +2,7 @@
 
 use anyhow::Result;
 
-use super::{is_temp_filtered, TempHarvest, TemperatureType};
+use super::{TempHarvest, TemperatureType};
 use crate::app::filter::Filter;
 
 pub fn get_temperature_data(
@@ -13,7 +13,7 @@ pub fn get_temperature_data(
     for component in components {
         let name = component.label().to_string();
 
-        if is_temp_filtered(filter, &name) {
+        if Filter::optional_should_keep(filter, &name) {
             temperature_vec.push(TempHarvest {
                 name,
                 temperature: Some(temp_type.convert_temp_unit(component.temperature())),

src/options.rs

@@ -868,10 +868,7 @@ fn get_ignore_list(ignore_list: &Option<IgnoreList>) -> OptionResult<Option<Filt
 
         let list = list.map_err(|err| OptionError::config(err.to_string()))?;
 
-        Ok(Some(Filter {
+        Ok(Some(Filter::new(ignore_list.is_list_ignored, list)))
-            list,
-            is_list_ignored: ignore_list.is_list_ignored,
-        }))
     } else {
         Ok(None)
     }