bug: fix incorrect default config definitions for chart legends (#1594)

I had changed how this was parsed in-code but I forgot to update the default configs. This also adds some e2e tests to hopefully catch this all for real in the future, since the schema ones don't catch this stuff and the constants test doesn't actually run the binary for a proper e2e test.

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.lock

@@ -196,6 +196,7 @@ dependencies = [
  "strum",
  "sysctl",
  "sysinfo",
+ "tempfile",
  "time",
  "toml_edit",
  "unicode-ellipsis",
@@ -568,6 +569,12 @@ dependencies = [
  "windows-sys 0.52.0",
 ]
 
+[[package]]
+name = "fastrand"
+version = "2.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e8c02a5121d4ea3eb16a80748c74f5549a5665e4c21333c6098f283870fbdea6"
+
 [[package]]
 name = "fern"
 version = "0.6.2"
@@ -1469,6 +1476,19 @@ dependencies = [
  "windows 0.52.0",
 ]
 
+[[package]]
+name = "tempfile"
+version = "3.12.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "04cbcdd0c794ebb0d4cf35e88edd2f7d2c4c3e9a5a6dab322839b321c6a87a64"
+dependencies = [
+ "cfg-if",
+ "fastrand",
+ "once_cell",
+ "rustix",
+ "windows-sys 0.59.0",
+]
+
 [[package]]
 name = "terminal_size"
 version = "0.3.0"

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"]);