feature: Add mount filtering, rework filter priority logic (#455)

This PR accomplishes two things:

1. This PR aims to add mount_filter to the config file. This allows a user to filter their disk widget entries by the mount name as well; this was particularly a problem in trying to address #431.
2. A slight rework of how the filter system works due to the need of being able to manage two potentially conflicting filter sources, since the disk widget will now potentially filter on both the disk name and the mount name.

In regards to the second point, the new behaviour is as such:

1. Is the entry allowed through any filter? That is, does it match an entry in a filter where is_list_ignored is false? If so, we always keep this entry.
2. Is the entry denied through any filter? That is, does it match an entry in a filter where is_list_ignored is true? If so, we always deny this entry.
3. Anything else is allowed.

This main (breaking) change is really the third point. This would mean that temp_filter and net_filter, when set to allow listed entries with is_list_ignored = false, are kinda... useless, as a whitelist in the scenario of being the only filter is kinda pointless. But hopefully this shouldn't be a problem...?

CHANGELOG.md

@@ -53,6 +53,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - [#451](https://github.com/ClementTsang/bottom/pull/451): Add decimal place to disk values larger than 1GB for total read/write in process widgets, and read/write per second in process widgets and disk widgets.
 
+- [#455](https://github.com/ClementTsang/bottom/pull/455): Added a mount point filter for the disk widget. Also tweaked how the filter system works - see the PR for details.
+
 ## Bug Fixes
 
 - [#416](https://github.com/ClementTsang/bottom/pull/416): Fixes grouped vs ungrouped modes in the processes widget having inconsistent spacing.

README.md

@@ -720,59 +720,46 @@ and get the following CPU donut:
 
 #### Disk, temperature, and network filtering
 
-You can hide specific disks, temperature sensors, and networks by name in the config file via `disk_filter`, `temp_filter`, and `net_filter` respectively. Regex (`regex = true`), case-sensitivity (`case_sensitive = true`), and matching only the entire word (`whole_word = true`) are supported, but are off by default.
+You can hide specific disks, temperature sensors, and networks by name in the config file via `disk_filter` and `mount_filter`, `temp_filter`, and `net_filter` respectively. Regex (`regex = true`), case-sensitivity (`case_sensitive = true`), and matching only if the entire word matches (`whole_word = true`) are supported, but are off by default. Filters default to denying entries that match and can be toggled by setting `is_list_ignored` to `false` in the config file.
 
-For example, let's say , given this disk list:
+For example, here's the disk widget with no filter:
 
-![Disk filter not ignoring list](./assets/disk_filter_pre.png)
+![Disk no filter](./assets/disk_no_filter.png)
 
-I wish to _only_ show disks that follow the form `/dev/sda\d+`, or `/dev/nvme0n1p2`:
+The following in the config file would filter out some entries by disk name:
+
+```toml
+[disk_filter]
+is_list_ignored = true
+list = ["/dev/sda"]
+regex = true
+case_sensitive = false
+whole_word = false
+```
+
+![Disk widget with just disk name filter](./assets/disk_name_filter.png)
+
+If there are two potentially conflicting filters (i.e. when you are using both a disk and mount filter), the filter that explicitly allows an entry takes precedence over a filter that explicitly denies one. So for example, let's say I set a disk filter accepting anything with `/dev/sda`, but deny anything with `/mnt/.*` or `/`. So to do so, I write in the config file:
 
 ```toml
 [disk_filter]
 is_list_ignored = false
-list = ["/dev/sda\\d+", "/dev/nvme0n1p2"]
+list = ["/dev/sda"]
 regex = true
-```
+case_sensitive = false
+whole_word = false
 
-![Disk filter not ignoring list](./assets/disk_filter_post.png)
-
-This would ignore anything that does not match either of these two conditions. If I instead wish to ignore anything that matches this list, then I can set `is_list_ignored = true` instead:
-
-![Disk filter ignoring list](./assets/disk_filter_post2.png)
-
-Likewise, I can do something similar for `temp_filter`:
-
-![Temp filter before](./assets/temp_filter_pre.png)
-
-If I, say, only wanted to see any entry with the words "cpu" or "wifi" in it, case sensitive:
-
-```toml
-[temp_filter]
-is_list_ignored = false
-list = ["cpu", "wifi"]
-case_sensitive = true
-```
-
-![Temp filter after](./assets/temp_filter_post.png)
-
-Now, flipping to `case_sensitive = false` would instead show:
-
-![Temp filter after with case sensitivity off](./assets/temp_filter_post2.png)
-
-Lastly, let's say I want to filter out _exactly_ "iwlwifi_1" from my results. I could do:
-
-```toml
-[temp_filter]
+[mount_filter]
 is_list_ignored = true
-list = ["iwlwifi_1"]
-case_sensitive = true
+list = ["/mnt/.*", "/"]
+regex = true
+case_sensitive = false
 whole_word = true
 ```
 
-This will match the entire word, "iwlwifi_1", and ignore any result that exactly matches it:
+Which gives me:
 
-![Temp filter after with whole_word](./assets/temp_filter_post3.png)
+![Disk widget with disk name and mount filter](./assets/disk_name_mount_filter.png)
 
 ### Battery
 

src/app.rs

@@ -72,6 +72,7 @@ pub struct AppConfigFields {
 #[derive(Debug, Clone)]
 pub struct DataFilters {
     pub disk_filter: Option<Filter>,
+    pub mount_filter: Option<Filter>,
     pub temp_filter: Option<Filter>,
     pub net_filter: Option<Filter>,
 }

src/app/data_harvester.rs

@@ -325,8 +325,11 @@ impl DataCollector {
             }
         };
         let mem_data_fut = mem::get_mem_data(self.widgets_to_harvest.use_mem);
-        let disk_data_fut =
-            disks::get_disk_usage(self.widgets_to_harvest.use_disk, &self.filters.disk_filter);
+        let disk_data_fut = disks::get_disk_usage(
+            self.widgets_to_harvest.use_disk,
+            &self.filters.disk_filter,
+            &self.filters.mount_filter,
+        );
         let disk_io_usage_fut = disks::get_io_usage(self.widgets_to_harvest.use_disk);
         let temp_data_fut = {
             #[cfg(not(target_os = "linux"))]

src/app/data_harvester/disks.rs

@@ -34,7 +34,6 @@ pub async fn get_io_usage(actually_get: bool) -> crate::utils::error::Result<Opt
         if let Ok(io) = io {
             let mount_point = io.device_name().to_str().unwrap_or("Name Unavailable");
 
-            // FIXME: [MOUNT POINT] Add the filter here I guess?
             io_hash.insert(
                 mount_point.to_string(),
                 Some(IoData {
@@ -49,7 +48,7 @@ pub async fn get_io_usage(actually_get: bool) -> crate::utils::error::Result<Opt
 }
 
 pub async fn get_disk_usage(
-    actually_get: bool, name_filter: &Option<Filter>,
+    actually_get: bool, disk_filter: &Option<Filter>, mount_filter: &Option<Filter>,
 ) -> crate::utils::error::Result<Option<Vec<DiskHarvest>>> {
     if !actually_get {
         return Ok(None);
@@ -108,21 +107,53 @@ pub async fn get_disk_usage(
                 .unwrap_or("Name Unavailable"))
             .to_string();
 
-            let to_keep = if let Some(filter) = name_filter {
-                let mut ret = filter.is_list_ignored;
-                for r in &filter.list {
-                    if r.is_match(&name) {
-                        ret = !filter.is_list_ignored;
-                        break;
+            // Precedence ordering in the case where name and mount filters disagree, "allow" takes precedence over "deny".
+            //
+            // For implementation, we do this as follows:
+            // 1. Is the entry allowed through any filter? That is, does it match an entry in a filter where `is_list_ignored` is `false`? If so, we always keep this entry.
+            // 2. Is the entry denied through any filter? That is, does it match an entry in a filter where `is_list_ignored` is `true`? If so, we always deny this entry.
+            // 3. Anything else is allowed.
+
+            let filter_check_map = [(disk_filter, &name), (mount_filter, &mount_point)];
+
+            // This represents case 1.  That is, if there is a match in an allowing list - if there is, then
+            // immediately allow it!
+            let matches_allow_list = filter_check_map.iter().any(|(filter, text)| {
+                if let Some(filter) = filter {
+                    if !filter.is_list_ignored {
+                        for r in &filter.list {
+                            if r.is_match(text) {
+                                return true;
+                            }
+                        }
                     }
                 }
-                ret
-            } else {
+                false
+            });
+
+            let to_keep = if matches_allow_list {
                 true
+            } else {
+                // If it doesn't match an allow list, then check if it is denied.
+                // That is, if it matches in a reject filter, then reject.  Otherwise, we always keep it.
+                !filter_check_map.iter().any(|(filter, text)| {
+                    if let Some(filter) = filter {
+                        if filter.is_list_ignored {
+                            for r in &filter.list {
+                                if r.is_match(text) {
+                                    return true;
+                                }
+                            }
+                        }
+                    }
+                    false
+                })
             };
 
             if to_keep {
-                // The usage line fails in some cases (Void linux + LUKS, see https://github.com/ClementTsang/bottom/issues/419)
+                // The usage line can fail in some cases (for example, if you use Void Linux + LUKS,
+                // see https://github.com/ClementTsang/bottom/issues/419 for details).  As such, check
+                // it like this instead.
                 if let Ok(usage) = heim::disk::usage(partition.mount_point().to_path_buf()).await {
                     vec_disks.push(DiskHarvest {
                         free_space: Some(usage.free().get::<heim::units::information::byte>()),

src/app/data_harvester/network.rs

@@ -94,14 +94,18 @@ pub async fn get_network_data(
     while let Some(io) = io_data.next().await {
         if let Ok(io) = io {
             let to_keep = if let Some(filter) = filter {
-                let mut ret = filter.is_list_ignored;
-                for r in &filter.list {
-                    if r.is_match(&io.interface()) {
-                        ret = !filter.is_list_ignored;
-                        break;
+                if filter.is_list_ignored {
+                    let mut ret = true;
+                    for r in &filter.list {
+                        if r.is_match(&io.interface()) {
+                            ret = false;
+                            break;
+                        }
                     }
+                    ret
+                } else {
+                    true
                 }
-                ret
             } else {
                 true
             };

src/app/data_harvester/temperature.rs

@@ -21,6 +21,25 @@ impl Default for TemperatureType {
     }
 }
 
+fn is_temp_filtered(filter: &Option<Filter>, text: &str) -> bool {
+    if let Some(filter) = filter {
+        if filter.is_list_ignored {
+            let mut ret = true;
+            for r in &filter.list {
+                if r.is_match(text) {
+                    ret = false;
+                    break;
+                }
+            }
+            ret
+        } else {
+            true
+        }
+    } else {
+        true
+    }
+}
+
 #[cfg(not(target_os = "linux"))]
 pub async fn get_temperature_data(
     sys: &sysinfo::System, temp_type: &TemperatureType, actually_get: bool, filter: &Option<Filter>,
@@ -45,20 +64,7 @@ pub async fn get_temperature_data(
     for component in sensor_data {
         let name = component.get_label().to_string();
 
-        let to_keep = if let Some(filter) = filter {
-            let mut ret = filter.is_list_ignored;
-            for r in &filter.list {
-                if r.is_match(&name) {
-                    ret = !filter.is_list_ignored;
-                    break;
-                }
-            }
-            ret
-        } else {
-            true
-        };
-
-        if to_keep {
+        if is_temp_filtered(filter, &name) {
             temperature_vec.push(TempHarvest {
                 name,
                 temperature: match temp_type {
@@ -104,20 +110,7 @@ pub async fn get_temperature_data(
                 (None, None) => String::default(),
             };
 
-            let to_keep = if let Some(filter) = filter {
-                let mut ret = filter.is_list_ignored;
-                for r in &filter.list {
-                    if r.is_match(&name) {
-                        ret = !filter.is_list_ignored;
-                        break;
-                    }
-                }
-                ret
-            } else {
-                true
-            };
-
-            if to_keep {
+            if is_temp_filtered(filter, &name) {
                 temperature_vec.push(TempHarvest {
                     name,
                     temperature: match temp_type {

src/constants.rs

@@ -565,24 +565,31 @@ pub const OLD_CONFIG_TEXT: &str = r##"# This is a default config file for bottom
 #    default=true
 
 
-# Filters - you can hide specific temperature and disks using filters.  This is admittedly a bit
-# hard to use as of now, and there is a planned interface for managing this in the future:
+# Filters - you can hide specific temperature sensors, network interfaces, and disks using filters.  This is admittedly
+# a bit hard to use as of now, and there is a planned in-app interface for managing this in the future:
 #[disk_filter]
-#is_list_ignored = false
+#is_list_ignored = true
 #list = ["/dev/sda\\d+", "/dev/nvme0n1p2"]
 #regex = true
 #case_sensitive = false
 #whole_word = false
 
+#[mount_filter]
+#is_list_ignored = true
+#list = ["/mnt/.*", "/boot"]
+#regex = true
+#case_sensitive = false
+#whole_word = false
+
 #[temp_filter]
-#is_list_ignored = false
+#is_list_ignored = true
 #list = ["cpu", "wifi"]
 #regex = false
 #case_sensitive = false
 #whole_word = false
 
 #[net_filter]
-#is_list_ignored = false
+#is_list_ignored = true
 #list = ["virbr0.*"]
 #regex = true
 #case_sensitive = false

src/options.rs

@@ -30,6 +30,7 @@ pub struct Config {
     pub colors: Option<ConfigColours>,
     pub row: Option<Vec<Row>>,
     pub disk_filter: Option<IgnoreList>,
+    pub mount_filter: Option<IgnoreList>,
     pub temp_filter: Option<IgnoreList>,
     pub net_filter: Option<IgnoreList>,
 }
@@ -224,13 +225,24 @@ impl ConfigColours {
     }
 }
 
+/// Workaround as per https://github.com/serde-rs/serde/issues/1030
+fn default_as_true() -> bool {
+    true
+}
+
 #[derive(Clone, Debug, Default, Deserialize, Serialize)]
 pub struct IgnoreList {
+    #[serde(default = "default_as_true")]
+    // TODO: Deprecate and/or rename, current name sounds awful.
+    // Maybe to something like "deny_entries"?  Currently it defaults to a denylist anyways, so maybe "allow_entries"?
     pub is_list_ignored: bool,
     pub list: Vec<String>,
-    pub regex: Option<bool>,
-    pub case_sensitive: Option<bool>,
-    pub whole_word: Option<bool>,
+    #[serde(default = "bool::default")]
+    pub regex: bool,
+    #[serde(default = "bool::default")]
+    pub case_sensitive: bool,
+    #[serde(default = "bool::default")]
+    pub whole_word: bool,
 }
 
 pub fn build_app(
@@ -440,6 +452,8 @@ pub fn build_app(
 
     let disk_filter =
         get_ignore_list(&config.disk_filter).context("Update 'disk_filter' in your config file")?;
+    let mount_filter = get_ignore_list(&config.mount_filter)
+        .context("Update 'mount_filter' in your config file")?;
     let temp_filter =
         get_ignore_list(&config.temp_filter).context("Update 'temp_filter' in your config file")?;
     let net_filter =
@@ -502,6 +516,7 @@ pub fn build_app(
         .used_widgets(used_widgets)
         .filters(DataFilters {
             disk_filter,
+            mount_filter,
             temp_filter,
             net_filter,
         })
@@ -927,34 +942,22 @@ fn get_ignore_list(ignore_list: &Option<IgnoreList>) -> error::Result<Option<Fil
             .list
             .iter()
             .map(|name| {
-                let use_regex = if let Some(use_regex) = ignore_list.regex {
-                    use_regex
-                } else {
-                    false
-                };
-                let use_cs = if let Some(use_cs) = ignore_list.case_sensitive {
-                    use_cs
-                } else {
-                    false
-                };
-                let whole_word = if let Some(whole_word) = ignore_list.whole_word {
-                    whole_word
-                } else {
-                    false
-                };
-
                 let escaped_string: String;
                 let res = format!(
                     "{}{}{}{}",
-                    if whole_word { "^" } else { "" },
-                    if use_cs { "" } else { "(?i)" },
-                    if use_regex {
+                    if ignore_list.whole_word { "^" } else { "" },
+                    if ignore_list.case_sensitive {
+                        ""
+                    } else {
+                        "(?i)"
+                    },
+                    if ignore_list.regex {
                         name
                     } else {
                         escaped_string = regex::escape(name);
                         &escaped_string
                     },
-                    if whole_word { "$" } else { "" },
+                    if ignore_list.whole_word { "$" } else { "" },
                 );
 
                 Regex::new(&res)