bug: fix incorrect default config definitions for chart legends

CHANGELOG.md

@@ -28,6 +28,7 @@ That said, these are more guidelines rather than hardset rules, though the proje
 - [#1552](https://github.com/ClementTsang/bottom/pull/1552): Fix typo in default config.
 - [#1578](https://github.com/ClementTsang/bottom/pull/1578): Fix missing selected text background colour in `default-light` theme.
 - [#1593](https://github.com/ClementTsang/bottom/pull/1593): Fix using `"none"` for chart legend position in configs.
+- [#1594](https://github.com/ClementTsang/bottom/pull/1594): Fix incorrect default config definitions for chart legends.
 
 ### Changes
 

Cargo.toml

@@ -143,6 +143,7 @@ filedescriptor = "0.8.2"
 assert_cmd = "2.0.16"
 cargo-husky = { version = "1.5.0", default-features = false, features = ["user-hooks"] }
 predicates = "3.1.2"
+tempfile = "3.12.0"
 
 [target.'cfg(all(target_arch = "x86_64", target_os = "linux"))'.dev-dependencies]
 portable-pty = "0.8.1"

sample_configs/default_config.toml

@@ -77,13 +77,13 @@
 # How much data is stored at once in terms of time.
 #retention = "10m"
 # Where to place the legend for the memory widget. One of "none", "top-left", "top", "top-right", "left", "right", "bottom-left", "bottom", "bottom-right".
-#memory_legend = "TopRight"
+#memory_legend = "top-right"
 # Where to place the legend for the network widget. One of "none", "top-left", "top", "top-right", "left", "right", "bottom-left", "bottom", "bottom-right".
-#network_legend = "TopRight"
+#network_legend = "top-right"
 
 # Processes widget configuration
 #[processes]
-# The columns shown by the process widget. The following columns are supported:
+# The columns shown by the process widget. The following columns are supported (the GPU columns are only available if the GPU feature is enabled when built):
 # PID, Name, CPU%, Mem%, R/s, W/s, T.Read, T.Write, User, State, Time, GMem%, GPU%
 #columns = ["PID", "Name", "CPU%", "Mem%", "R/s", "W/s", "T.Read", "T.Write", "User", "State", "GMem%", "GPU%"]
 
@@ -94,36 +94,64 @@
 
 # Disk widget configuration
 #[disk]
+
+# By default, there are no disk name filters enabled. These can be turned on to filter out specific data entries if you
+# don't want to see them. An example use case is provided below.
 #[disk.name_filter]
+# Whether to ignore any matches. Defaults to true.
 #is_list_ignored = true
+# A list of filters to try and match.
 #list = ["/dev/sda\\d+", "/dev/nvme0n1p2"]
+# Whether to use regex. Defaults to false.
 #regex = true
+# Whether to be case-sensitive. Defaults to false.
 #case_sensitive = false
+# Whether to be require matching the whole word. Defaults to false.
 #whole_word = false
 
+# By default, there are no mount name filters enabled. An example use case is provided below.
 #[disk.mount_filter]
+# Whether to ignore any matches. Defaults to true.
 #is_list_ignored = true
+# A list of filters to try and match.
 #list = ["/mnt/.*", "/boot"]
+# Whether to use regex. Defaults to false.
 #regex = true
+# Whether to be case-sensitive. Defaults to false.
 #case_sensitive = false
+# Whether to be require matching the whole word. Defaults to false.
 #whole_word = false
 
 # Temperature widget configuration
 #[temperature]
+
+# By default, there are no temperature sensor filters enabled. An example use case is provided below.
 #[temperature.sensor_filter]
+# Whether to ignore any matches. Defaults to true.
 #is_list_ignored = true
+# A list of filters to try and match.
 #list = ["cpu", "wifi"]
+# Whether to use regex. Defaults to false.
 #regex = false
+# Whether to be case-sensitive. Defaults to false.
 #case_sensitive = false
+# Whether to be require matching the whole word. Defaults to false.
 #whole_word = false
 
 # Network widget configuration
 #[network]
+
+# By default, there are no network interface filters enabled. An example use case is provided below.
 #[network.interface_filter]
+# Whether to ignore any matches. Defaults to true.
 #is_list_ignored = true
+# A list of filters to try and match.
 #list = ["virbr0.*"]
+# Whether to use regex. Defaults to false.
 #regex = true
+# Whether to be case-sensitive. Defaults to false.
 #case_sensitive = false
+# Whether to be require matching the whole word. Defaults to false.
 #whole_word = false
 
 # These are all the components that support custom theming.  Note that colour support

src/constants.rs

@@ -337,13 +337,13 @@ pub(crate) const CONFIG_TEXT: &str = r#"# This is a default config file for bott
 # How much data is stored at once in terms of time.
 #retention = "10m"
 # Where to place the legend for the memory widget. One of "none", "top-left", "top", "top-right", "left", "right", "bottom-left", "bottom", "bottom-right".
-#memory_legend = "TopRight"
+#memory_legend = "top-right"
 # Where to place the legend for the network widget. One of "none", "top-left", "top", "top-right", "left", "right", "bottom-left", "bottom", "bottom-right".
-#network_legend = "TopRight"
+#network_legend = "top-right"
 
 # Processes widget configuration
 #[processes]
-# The columns shown by the process widget. The following columns are supported:
+# The columns shown by the process widget. The following columns are supported (the GPU columns are only available if the GPU feature is enabled when built):
 # PID, Name, CPU%, Mem%, R/s, W/s, T.Read, T.Write, User, State, Time, GMem%, GPU%
 #columns = ["PID", "Name", "CPU%", "Mem%", "R/s", "W/s", "T.Read", "T.Write", "User", "State", "GMem%", "GPU%"]
 
@@ -354,36 +354,64 @@ pub(crate) const CONFIG_TEXT: &str = r#"# This is a default config file for bott
 
 # Disk widget configuration
 #[disk]
+
+# By default, there are no disk name filters enabled. These can be turned on to filter out specific data entries if you
+# don't want to see them. An example use case is provided below.
 #[disk.name_filter]
+# Whether to ignore any matches. Defaults to true.
 #is_list_ignored = true
+# A list of filters to try and match.
 #list = ["/dev/sda\\d+", "/dev/nvme0n1p2"]
+# Whether to use regex. Defaults to false.
 #regex = true
+# Whether to be case-sensitive. Defaults to false.
 #case_sensitive = false
+# Whether to be require matching the whole word. Defaults to false.
 #whole_word = false
 
+# By default, there are no mount name filters enabled. An example use case is provided below.
 #[disk.mount_filter]
+# Whether to ignore any matches. Defaults to true.
 #is_list_ignored = true
+# A list of filters to try and match.
 #list = ["/mnt/.*", "/boot"]
+# Whether to use regex. Defaults to false.
 #regex = true
+# Whether to be case-sensitive. Defaults to false.
 #case_sensitive = false
+# Whether to be require matching the whole word. Defaults to false.
 #whole_word = false
 
 # Temperature widget configuration
 #[temperature]
+
+# By default, there are no temperature sensor filters enabled. An example use case is provided below.
 #[temperature.sensor_filter]
+# Whether to ignore any matches. Defaults to true.
 #is_list_ignored = true
+# A list of filters to try and match.
 #list = ["cpu", "wifi"]
+# Whether to use regex. Defaults to false.
 #regex = false
+# Whether to be case-sensitive. Defaults to false.
 #case_sensitive = false
+# Whether to be require matching the whole word. Defaults to false.
 #whole_word = false
 
 # Network widget configuration
 #[network]
+
+# By default, there are no network interface filters enabled. An example use case is provided below.
 #[network.interface_filter]
+# Whether to ignore any matches. Defaults to true.
 #is_list_ignored = true
+# A list of filters to try and match.
 #list = ["virbr0.*"]
+# Whether to use regex. Defaults to false.
 #regex = true
+# Whether to be case-sensitive. Defaults to false.
 #case_sensitive = false
+# Whether to be require matching the whole word. Defaults to false.
 #whole_word = false
 
 # These are all the components that support custom theming.  Note that colour support

src/options.rs

@@ -957,13 +957,13 @@ fn get_network_legend_position(
     let result = if let Some(s) = &args.network.network_legend {
         match s.to_ascii_lowercase().trim() {
             "none" => None,
-            position => Some(parse_config_value!(position.parse(), "network_legend")?),
+            position => Some(parse_arg_value!(position.parse(), "network_legend")?),
         }
     } else if let Some(flags) = &config.flags {
         if let Some(s) = &flags.network_legend {
             match s.to_ascii_lowercase().trim() {
                 "none" => None,
-                position => Some(parse_arg_value!(position.parse(), "network_legend")?),
+                position => Some(parse_config_value!(position.parse(), "network_legend")?),
             }
         } else {
             Some(LegendPosition::default())
@@ -981,13 +981,13 @@ fn get_memory_legend_position(
     let result = if let Some(s) = &args.memory.memory_legend {
         match s.to_ascii_lowercase().trim() {
             "none" => None,
-            position => Some(parse_config_value!(position.parse(), "memory_legend")?),
+            position => Some(parse_arg_value!(position.parse(), "memory_legend")?),
         }
     } else if let Some(flags) = &config.flags {
         if let Some(s) = &flags.memory_legend {
             match s.to_ascii_lowercase().trim() {
                 "none" => None,
-                position => Some(parse_arg_value!(position.parse(), "memory_legend")?),
+                position => Some(parse_config_value!(position.parse(), "memory_legend")?),
             }
         } else {
             Some(LegendPosition::default())

tests/integration/util.rs

@@ -5,7 +5,14 @@ use hashbrown::HashMap;
 use portable_pty::{native_pty_system, Child, CommandBuilder, MasterPty, PtySize};
 
 pub fn abs_path(path: &str) -> OsString {
-    Path::new(path).canonicalize().unwrap().into_os_string()
+    let path = Path::new(path);
+
+    if path.exists() {
+        path.canonicalize().unwrap().into_os_string()
+    } else {
+        // We are going to trust that the path given is valid...
+        path.to_owned().into_os_string()
+    }
 }
 
 /// Returns a QEMU runner target given an architecture.

tests/integration/valid_config_tests.rs

@@ -2,6 +2,9 @@
 
 use std::{io::Read, thread, time::Duration};
 
+#[cfg(feature = "default")]
+use std::{io::Write, path::Path};
+
 use crate::util::spawn_btm_in_pty;
 
 fn reader_to_string(mut reader: Box<dyn Read>) -> String {
@@ -53,6 +56,101 @@ fn test_empty() {
     run_and_kill(&["-C", "./tests/valid_configs/empty_config.toml"]);
 }
 
+#[cfg(feature = "default")]
+fn test_uncommented_default_config(original: &Path, test_name: &str) {
+    use regex::Regex;
+
+    // Take the default config file and uncomment everything.
+    let default_config = match std::fs::File::open(original) {
+        Ok(mut default_config_file) => {
+            let mut buf = String::new();
+            default_config_file
+                .read_to_string(&mut buf)
+                .expect("can read file");
+
+            buf
+        }
+        Err(err) => {
+            println!("Could not open default config, skipping {test_name}. Error: {err:?}");
+            return;
+        }
+    };
+
+    let default_config = Regex::new(r"(?m)^#([a-zA-Z\[])")
+        .unwrap()
+        .replace_all(&default_config, "$1");
+
+    let default_config = Regex::new(r"(?m)^#(\s\s+)([a-zA-Z\[])")
+        .unwrap()
+        .replace_all(&default_config, "$2");
+
+    let mut uncommented_config = match tempfile::NamedTempFile::new() {
+        Ok(tf) => tf,
+        Err(err) => {
+            println!("Could not create a temp file, skipping {test_name}. Error: {err:?}");
+            return;
+        }
+    };
+
+    if let Err(err) = uncommented_config.write_all(default_config.as_bytes()) {
+        println!("Could not write to temp file, skipping {test_name}. Error: {err:?}");
+        return;
+    }
+
+    run_and_kill(&["-C", &uncommented_config.path().to_string_lossy()]);
+
+    uncommented_config.close().unwrap();
+}
+
+#[cfg(feature = "default")]
+#[test]
+fn test_default() {
+    test_uncommented_default_config(
+        Path::new("./sample_configs/default_config.toml"),
+        "test_default",
+    );
+}
+
+#[cfg(feature = "default")]
+#[test]
+fn test_new_default() {
+    use tempfile::TempPath;
+
+    let new_temp_default_path = match tempfile::NamedTempFile::new() {
+        Ok(temp_file) => temp_file.into_temp_path(),
+        Err(err) => {
+            println!("Could not create a temp file, skipping test_new_default. Error: {err:?}");
+            return;
+        }
+    };
+
+    // This is a hack because we need a temp file that doesn't exist.
+    let actual_temp_default_path = new_temp_default_path.to_path_buf();
+    new_temp_default_path.close().unwrap();
+
+    if !actual_temp_default_path.exists() {
+        run_and_kill(&["-C", &(actual_temp_default_path.to_string_lossy())]);
+
+        // Re-take control over the temp path to ensure it gets deleted.
+        let actual_temp_default_path = TempPath::from_path(actual_temp_default_path);
+        test_uncommented_default_config(&actual_temp_default_path, "test_new_default");
+
+        actual_temp_default_path.close().unwrap();
+    } else {
+        println!("temp path we want to check exists, skip test_new_default test.");
+    }
+}
+
+#[test]
+fn test_demo() {
+    let path: &str = "./sample_configs/demo_config.toml";
+    if std::path::Path::new(path).exists() {
+        run_and_kill(&["-C", path]);
+    } else {
+        println!("Could not read demo config.");
+    }
+}
+
 #[test]
 fn test_many_proc() {
     run_and_kill(&["-C", "./tests/valid_configs/many_proc.toml"]);