other: flatten process config struct and clean up help text (#1395)

* refactor: flatten process config field

* other: clean up some doc formatting using indoc and breaklines

* fix broken test

* remove default as that breaks things for now

* add test

* more tests

Cargo.lock

@@ -170,6 +170,7 @@ dependencies = [
  "hashbrown",
  "humantime",
  "indexmap",
+ "indoc",
  "itertools 0.12.0",
  "kstring",
  "libc",
@@ -240,18 +241,18 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"
 
 [[package]]
 name = "clap"
-version = "4.4.16"
+version = "4.4.18"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "58e54881c004cec7895b0068a0a954cd5d62da01aef83fa35b1e594497bf5445"
+checksum = "1e578d6ec4194633722ccf9544794b71b1385c3c027efe0c55db226fc880865c"
 dependencies = [
  "clap_builder",
 ]
 
 [[package]]
 name = "clap_builder"
-version = "4.4.16"
+version = "4.4.18"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "59cb82d7f531603d2fd1f507441cdd35184fa81beff7bd489570de7f773460bb"
+checksum = "4df4df40ec50c46000231c914968278b1eb05098cf8f1b3a518a95030e71d1c7"
 dependencies = [
  "anstream",
  "anstyle",
@@ -262,9 +263,9 @@ dependencies = [
 
 [[package]]
 name = "clap_complete"
-version = "4.4.6"
+version = "4.4.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "97aeaa95557bd02f23fbb662f981670c3d20c5a26e69f7354b28f57092437fcd"
+checksum = "eaf7dcb7c21d8ca1a2482ee0f1d341f437c9a7af6ca6da359dc5e1b164e98215"
 dependencies = [
  "clap",
 ]

Cargo.toml

@@ -75,7 +75,7 @@ default = ["deploy"]
 anyhow = "1.0.79"
 backtrace = "0.3.69"
 cfg-if = "1.0.0"
-clap = { version = "4.4.16", features = ["default", "cargo", "wrap_help"] }
+clap = { version = "4.4.18", features = ["default", "cargo", "wrap_help"] }
 concat-string = "1.0.1"
 crossterm = "0.27.0"
 ctrlc = { version = "3.4.2", features = ["termination"] }
@@ -85,6 +85,7 @@ fern = { version = "0.6.2", optional = true }
 hashbrown = "0.14.3"
 humantime = "2.1.0"
 indexmap = "2.1.0"
+indoc = "2.0.4"
 itertools = "0.12.0"
 kstring = { version = "2.0.0", features = ["arc"] }
 log = { version = "0.4.20", optional = true }
@@ -139,11 +140,12 @@ predicates = "3.0.4"
 portable-pty = "0.8.1"
 
 [build-dependencies]
-clap = { version = "4.4.16", features = ["default", "cargo", "wrap_help"] }
+clap = { version = "4.4.18", features = ["default", "cargo", "wrap_help"] }
-clap_complete = "4.4.6"
+clap_complete = "4.4.8"
 clap_complete_fig = "4.4.2"
 clap_complete_nushell = "4.4.2"
 clap_mangen = "0.2.17"
+indoc = "2.0.4"
 
 [package.metadata.deb]
 section = "utility"

src/canvas/styling/colour_utils.rs

@@ -147,6 +147,21 @@ The following are supported strings:
 mod test {
     use super::*;
 
+    #[test]
+    fn general_str_to_colour() {
+        assert_eq!(str_to_colour("red").unwrap(), Color::Red);
+        assert!(str_to_colour("r ed").is_err());
+
+        assert_eq!(str_to_colour("#ffffff").unwrap(), Color::Rgb(255, 255, 255));
+        assert!(str_to_colour("#fff fff").is_err());
+
+        assert_eq!(
+            str_to_colour("255, 255, 255").unwrap(),
+            Color::Rgb(255, 255, 255)
+        );
+        assert!(str_to_colour("255, 256, 255").is_err());
+    }
+
     #[test]
     fn invalid_colour_names() {
         // Test invalid spacing in single word.
@@ -265,4 +280,21 @@ mod test {
 
         assert!(convert_hex_to_color("#हिन्दी").is_err());
     }
+
+    #[test]
+    fn test_rgb_colours() {
+        assert_eq!(
+            convert_rgb_to_color("0, 0, 0").unwrap(),
+            Color::Rgb(0, 0, 0)
+        );
+        assert_eq!(
+            convert_rgb_to_color("255, 255, 255").unwrap(),
+            Color::Rgb(255, 255, 255)
+        );
+        assert!(convert_rgb_to_color("255, 256, 255").is_err());
+        assert!(convert_rgb_to_color("256, 0, 256").is_err());
+        assert!(convert_rgb_to_color("1, -1, 1").is_err());
+        assert!(convert_rgb_to_color("1, -100000, 1").is_err());
+        assert!(convert_rgb_to_color("1, -100000, 100000").is_err());
+    }
 }

src/options.rs

@@ -112,10 +112,7 @@ pub fn init_app(
     let network_use_binary_prefix = is_flag_enabled!(network_use_binary_prefix, matches, config);
 
     let proc_columns: Option<IndexSet<ProcWidgetColumn>> = {
-        let columns = config
+        let columns = config.processes.as_ref().map(|cfg| cfg.columns.clone());
-            .processes
-            .as_ref()
-            .and_then(|cfg| cfg.columns.clone());
 
         match columns {
             Some(columns) => {
@@ -410,8 +407,6 @@ pub fn get_widget_layout(
         // Confirm that we have at least ONE widget left - if not, error out!
         if iter_id > 0 {
             ret_bottom_layout.get_movement_mappings();
-            // debug!("Bottom layout: {ret_bottom_layout:#?}");
-
             ret_bottom_layout
         } else {
             return Err(BottomError::ConfigError(

src/options/args.rs

@@ -3,80 +3,12 @@
 //! Note that you probably want to keep this as a single file so the build script doesn't
 //! trip all over itself.
 
+// TODO: New sections are misaligned! See if we can get that fixed.
+
 use std::cmp::Ordering;
 
-use clap::{builder::PossibleValuesParser, *};
+use clap::*;
-
+use indoc::indoc;
-const DEFAULT_WIDGET_TYPE_STR: &str = {
-    #[cfg(feature = "battery")]
-    {
-        "\
-Sets which widget type to use as the default widget.
-For the default layout, this defaults to the 'process' widget.
-For a custom layout, it defaults to the first widget it sees.
-
-For example, suppose we have a layout that looks like:
-+-------------------+-----------------------+
-|      CPU (1)      |        CPU (2)        |
-+---------+---------+-------------+---------+
-| Process | CPU (3) | Temperature | CPU (4) |
-+---------+---------+-------------+---------+
-
-Setting '--default_widget_type Temp' will make the Temperature
-widget selected by default.
-
-Supported widget names:
-+--------------------------+
-|            cpu           |
-+--------------------------+
-|        mem, memory       |
-+--------------------------+
-|       net, network       |
-+--------------------------+
-| proc, process, processes |
-+--------------------------+
-|     temp, temperature    |
-+--------------------------+
-|           disk           |
-+--------------------------+
-|       batt, battery      |
-+--------------------------+
-"
-    }
-    #[cfg(not(feature = "battery"))]
-    {
-        "\
-Sets which widget type to use as the default widget.
-For the default layout, this defaults to the 'process' widget.
-For a custom layout, it defaults to the first widget it sees.
-
-For example, suppose we have a layout that looks like:
-+-------------------+-----------------------+
-|      CPU (1)      |        CPU (2)        |
-+---------+---------+-------------+---------+
-| Process | CPU (3) | Temperature | CPU (4) |
-+---------+---------+-------------+---------+
-
-Setting '--default_widget_type Temp' will make the Temperature
-widget selected by default.
-
-Supported widget names:
-+--------------------------+
-|            cpu           |
-+--------------------------+
-|        mem, memory       |
-+--------------------------+
-|       net, network       |
-+--------------------------+
-| proc, process, processes |
-+--------------------------+
-|     temp, temperature    |
-+--------------------------+
-|           disk           |
-+--------------------------+
-"
-    }
-};
 
 pub fn get_matches() -> ArgMatches {
     build_app().get_matches()
@@ -175,16 +107,22 @@ fn general_args(cmd: Command) -> Command {
         .action(ArgAction::Set)
         .value_name("CONFIG PATH")
         .help("Sets the location of the config file.")
-        .long_help("Sets the location of the config file. Expects a config file in the TOML format. If it doesn't exist, one is created.")
+        .long_help(
+            "Sets the location of the config file. Expects a config file in the TOML format.\
+            If it doesn't exist, one is created.",
+        )
         .value_hint(ValueHint::AnyPath);
 
     let default_time_value = Arg::new("default_time_value")
-            .short('t')
+        .short('t')
-            .long("default_time_value")
+        .long("default_time_value")
-            .action(ArgAction::Set)
+        .action(ArgAction::Set)
-            .value_name("TIME")
+        .value_name("TIME")
-            .help("Default time value for graphs.")
+        .help("Default time value for graphs.")
-            .long_help("Default time value for graphs. Takes a number in milliseconds or a human duration (e.g. 60s). The minimum time is 30s, and the default is 60s.");
+        .long_help(
+            "Default time value for graphs. Takes a number in milliseconds or a human \
+            duration (e.g. 60s). The minimum time is 30s, and the default is 60s.",
+        );
 
     // TODO: Charts are broken in the manpage
     let default_widget_count = Arg::new("default_widget_count")
@@ -193,39 +131,71 @@ fn general_args(cmd: Command) -> Command {
         .requires_all(["default_widget_type"])
         .value_name("INT")
         .help("Sets the n'th selected widget type as the default.")
-        .long_help(
+        .long_help(indoc! {
-            "\
+            "Sets the n'th selected widget type to use as the default widget.
-Sets the n'th selected widget type to use as the default widget.
+            Requires 'default_widget_type' to also be set, and defaults to 1.
-Requires 'default_widget_type' to also be set, and defaults to 1.
 
-This reads from left to right, top to bottom. For example, suppose
+            This reads from left to right, top to bottom. For example, suppose
-we have a layout that looks like:
+            we have a layout that looks like:
-+-------------------+-----------------------+
+            +-------------------+-----------------------+
-|      CPU (1)      |        CPU (2)        |
+            |      CPU (1)      |        CPU (2)        |
-+---------+---------+-------------+---------+
+            +---------+---------+-------------+---------+
-| Process | CPU (3) | Temperature | CPU (4) |
+            | Process | CPU (3) | Temperature | CPU (4) |
-+---------+---------+-------------+---------+
+            +---------+---------+-------------+---------+
 
-And we set our default widget type to 'CPU'. If we set
+            And we set our default widget type to 'CPU'. If we set
-'--default_widget_count 1', then it would use the CPU (1) as
+            '--default_widget_count 1', then it would use the CPU (1) as
-the default widget. If we set '--default_widget_count 3', it would
+            the default widget. If we set '--default_widget_count 3', it would
-use CPU (3) as the default instead.
+            use CPU (3) as the default instead."
-    ",
+        });
-        );
 
     let default_widget_type = Arg::new("default_widget_type")
         .long("default_widget_type")
         .action(ArgAction::Set)
         .value_name("WIDGET TYPE")
         .help("Sets the default widget type, use --help for info.")
-        .long_help(DEFAULT_WIDGET_TYPE_STR);
+        .long_help(indoc!{
+            "Sets which widget type to use as the default widget. For the default \
+            layout, this defaults to the 'process' widget. For a custom layout, it defaults \
+            to the first widget it sees.
+
+            For example, suppose we have a layout that looks like:
+            +-------------------+-----------------------+
+            |      CPU (1)      |        CPU (2)        |
+            +---------+---------+-------------+---------+
+            | Process | CPU (3) | Temperature | CPU (4) |
+            +---------+---------+-------------+---------+
+
+            Setting '--default_widget_type Temp' will make the temperature widget selected by default."
+        })
+        .value_parser([
+            "cpu",
+            "mem",
+            "net",
+            "network",
+            "proc",
+            "process",
+            "processes",
+            "temp",
+            "temperature",
+            "disk",
+            #[cfg(not(feature = "battery"))]
+            "batt",
+            #[cfg(not(feature = "battery"))]
+            "battery",
+        ]);
 
     let expanded_on_startup = Arg::new("expanded_on_startup")
         .short('e')
         .long("expanded")
         .action(ArgAction::SetTrue)
         .help("Expand the default widget upon starting the app.")
-        .long_help("Expand the default widget upon starting the app. Same as pressing \"e\" inside the app. Use with \"default_widget_type\" and \"default_widget_count\" to select desired expanded widget. This flag has no effect in basic mode (--basic).");
+        .long_help(
+            "Expand the default widget upon starting the app. \
+            Same as pressing \"e\" inside the app. Use with \"default_widget_type\" \
+            and \"default_widget_count\" to select the desired expanded widget. This \
+            flag has no effect in basic mode (--basic).",
+        );
 
     let rate = Arg::new("rate")
         .short('r')
@@ -233,7 +203,11 @@ use CPU (3) as the default instead.
         .action(ArgAction::Set)
         .value_name("TIME")
         .help("Sets the data refresh rate.")
-        .long_help("Sets the data refresh rate. Takes a number in milliseconds or a human duration (e.g. 5s). The minimum is 250ms, and defaults to 1000ms. Smaller values may take more computer resources.");
+        .long_help(
+            "Sets the data refresh rate. Takes a number in milliseconds or a human\
+            duration (e.g. 5s). The minimum is 250ms, and defaults to 1000ms. Smaller \
+            values may take more computer resources.",
+        );
 
     let time_delta = Arg::new("time_delta")
         .short('d')
@@ -241,14 +215,23 @@ use CPU (3) as the default instead.
         .action(ArgAction::Set)
         .value_name("TIME")
         .help("The amount of time changed upon zooming.")
-        .long_help("The amount of time changed when zooming in/out. Takes a number in milliseconds or a human duration (e.g. 30s). The minimum is 1s, and defaults to 15s.");
+        .long_help(
+            "The amount of time changed when zooming in/out. Takes a number in \
+            milliseconds or a human duration (e.g. 30s). The minimum is 1s, and \
+            defaults to 15s.",
+        );
 
+    // TODO: Unify how we do defaults.
     let retention = Arg::new("retention")
         .long("retention")
         .action(ArgAction::Set)
         .value_name("TIME")
         .help("The timespan of data stored.")
-        .long_help("How much data is stored at once in terms of time. Takes a number in milliseconds or a human duration (e.g. 20m), with a minimum of 1 minute. Note higher values will take up more memory. Defaults to 10 minutes.");
+        .long_help(
+            "How much data is stored at once in terms of time. Takes a number \
+            in milliseconds or a human duration (e.g. 20m), with a minimum of 1 minute. \
+            Note that higher values will take up more memory. Defaults to 10 minutes.",
+        );
 
     cmd.args(args![
         autohide_time,
@@ -278,35 +261,28 @@ fn style_args(cmd: Command) -> Command {
         .long("color")
         .action(ArgAction::Set)
         .value_name("COLOR SCHEME")
-        .value_parser(PossibleValuesParser::new([
+        .value_parser([
             "default",
             "default-light",
             "gruvbox",
             "gruvbox-light",
             "nord",
             "nord-light",
-        ]))
+        ])
         .hide_possible_values(true)
-        .help("Use a color scheme, use --help for info.")
+        .help(
-        .long_help(
+            "Use a color scheme, use --help for info on the colors. \
-            "\
+            [possible values: default, default-light, gruvbox, gruvbox-light, nord, nord-light]",
-Use a pre-defined color scheme. Currently supported values are:
+        )
-+------------------------------------------------------------+
+        .long_help(indoc! {
-| default                                                    |
+            "Use a pre-defined color scheme. Currently supported values are:
-+------------------------------------------------------------+
+            - default
-| default-light (default but for use with light backgrounds) |
+            - default-light (default but adjusted for lighter backgrounds)
-+------------------------------------------------------------+
+            - gruvbox       (a bright theme with 'retro groove' colors)
-| gruvbox (a bright theme with 'retro groove' colors)        |
+            - gruvbox-light (gruvbox but adjusted for lighter backgrounds)
-+------------------------------------------------------------+
+            - nord          (an arctic, north-bluish color palette)
-| gruvbox-light (gruvbox but for use with light backgrounds) |
+            - nord-light    (nord but adjusted for lighter backgrounds)"
-+------------------------------------------------------------+
+        });
-| nord (an arctic, north-bluish color palette)               |
-+------------------------------------------------------------+
-| nord-light (nord but for use with light backgrounds)       |
-+------------------------------------------------------------+
-Defaults to \"default\".
-    ",
-        );
 
     cmd.arg(color)
 }
@@ -358,14 +334,20 @@ fn process_args(cmd: Command) -> Command {
         .long("current_usage")
         .action(ArgAction::SetTrue)
         .help("Sets process CPU% to be based on current CPU%.")
-        .long_help("Sets process CPU% usage to be based on the current system CPU% usage rather than total CPU usage.");
+        .long_help(
+            "Sets process CPU% usage to be based on the current system CPU% usage rather \
+            than total CPU usage.",
+        );
 
     let unnormalized_cpu = Arg::new("unnormalized_cpu")
         .short('n')
         .long("unnormalized_cpu")
         .action(ArgAction::SetTrue)
         .help("Show process CPU% usage without normalizing over the number of cores.")
-        .long_help("Shows all process CPU% usage without averaging over the number of CPU cores in the system.");
+        .long_help(
+            "Shows all process CPU% usage without averaging over the number of CPU cores \
+            in the system.",
+        );
 
     let group_processes = Arg::new("group_processes")
         .short('g')
@@ -391,7 +373,10 @@ fn process_args(cmd: Command) -> Command {
         .long("disable_advanced_kill")
         .action(ArgAction::SetTrue)
         .help("Hides advanced process killing.")
-        .long_help("Hides advanced options to stop a process on Unix-like systems. The only option shown is 15 (TERM).");
+        .long_help(
+            "Hides advanced options to stop a process on Unix-like systems. The only \
+            option shown is 15 (TERM).",
+        );
 
     let whole_word = Arg::new("whole_word")
         .short('W')
@@ -446,7 +431,10 @@ fn mem_args(cmd: Command) -> Command {
         .long("mem_as_value")
         .action(ArgAction::SetTrue)
         .help("Defaults to showing process memory usage by value.")
-        .long_help("Defaults to showing process memory usage by value. Otherwise, it defaults to showing it by percentage.");
+        .long_help(
+            "Defaults to showing process memory usage by value. Otherwise, it defaults \
+            to showing it by percentage.",
+        );
 
     #[cfg(not(target_os = "windows"))]
     {
@@ -466,11 +454,15 @@ fn mem_args(cmd: Command) -> Command {
 fn network_args(cmd: Command) -> Command {
     let cmd = cmd.next_help_heading("Network Options");
 
+    // TODO: Change this to be configured as network graph type?
     let use_old_network_legend = Arg::new("use_old_network_legend")
         .long("use_old_network_legend")
         .action(ArgAction::SetTrue)
         .help("DEPRECATED - uses a separate network legend.")
-        .long_help("DEPRECATED - uses an older (pre-0.4), separate network widget legend. This display is not tested anymore and may be broken.");
+        .long_help(
+            "DEPRECATED - uses an older (pre-0.4), separate network widget legend. This \
+            display is not tested anymore and may be broken.",
+        );
 
     let network_use_bytes = Arg::new("network_use_bytes")
         .long("network_use_bytes")
@@ -488,7 +480,10 @@ fn network_args(cmd: Command) -> Command {
         .long("network_use_binary_prefix")
         .action(ArgAction::SetTrue)
         .help("Displays the network widget with binary prefixes.")
-        .long_help("Displays the network widget with binary prefixes (i.e. kibibits, mebibits) rather than a decimal prefix (i.e. kilobits, megabits). Defaults to decimal prefixes.");
+        .long_help(
+            "Displays the network widget with binary prefixes (i.e. kibibits, mebibits) \
+            rather than a decimal prefix (i.e. kilobits, megabits). Defaults to decimal prefixes.",
+        );
 
     cmd.args(args![
         use_old_network_legend,
@@ -595,7 +590,8 @@ mod test {
 
         assert!(
             !help_str.to_string().contains("\nOptions:\n"),
-            "the default 'Options' heading should not exist; if it does then an argument is missing a help heading."
+            "the default 'Options' heading should not exist; if it does then an argument is \
+            missing a help heading."
         );
     }
 }

src/options/config/process_columns.rs

@@ -5,7 +5,8 @@ use crate::widgets::ProcWidgetColumn;
 /// Process column settings.
 #[derive(Clone, Debug, Default, Deserialize)]
 pub struct ProcessConfig {
-    pub columns: Option<Vec<ProcWidgetColumn>>,
+    #[serde(default)]
+    pub columns: Vec<ProcWidgetColumn>,
 }
 
 #[cfg(test)]
@@ -17,7 +18,7 @@ mod test {
     fn empty_column_setting() {
         let config = "";
         let generated: ProcessConfig = toml_edit::de::from_str(config).unwrap();
-        assert!(generated.columns.is_none());
+        assert!(generated.columns.is_empty());
     }
 
     #[test]
@@ -29,7 +30,7 @@ mod test {
         let generated: ProcessConfig = toml_edit::de::from_str(config).unwrap();
         assert_eq!(
             generated.columns,
-            Some(vec![
+            vec![
                 ProcWidgetColumn::Cpu,
                 ProcWidgetColumn::PidOrCount,
                 ProcWidgetColumn::User,
@@ -41,7 +42,7 @@ mod test {
                 ProcWidgetColumn::Time,
                 ProcWidgetColumn::User,
                 ProcWidgetColumn::State,
-            ]),
+            ],
         );
     }
 
@@ -55,30 +56,18 @@ mod test {
     fn process_column_settings_3() {
         let config = r#"columns = ["Twrite", "T.Write"]"#;
         let generated: ProcessConfig = toml_edit::de::from_str(config).unwrap();
-        assert_eq!(
+        assert_eq!(generated.columns, vec![ProcWidgetColumn::TotalWrite; 2]);
-            generated.columns,
-            Some(vec![ProcWidgetColumn::TotalWrite; 2])
-        );
 
         let config = r#"columns = ["Tread", "T.read"]"#;
         let generated: ProcessConfig = toml_edit::de::from_str(config).unwrap();
-        assert_eq!(
+        assert_eq!(generated.columns, vec![ProcWidgetColumn::TotalRead; 2]);
-            generated.columns,
-            Some(vec![ProcWidgetColumn::TotalRead; 2])
-        );
 
         let config = r#"columns = ["read", "rps", "r/s"]"#;
         let generated: ProcessConfig = toml_edit::de::from_str(config).unwrap();
-        assert_eq!(
+        assert_eq!(generated.columns, vec![ProcWidgetColumn::ReadPerSecond; 3]);
-            generated.columns,
-            Some(vec![ProcWidgetColumn::ReadPerSecond; 3])
-        );
 
         let config = r#"columns = ["write", "wps", "w/s"]"#;
         let generated: ProcessConfig = toml_edit::de::from_str(config).unwrap();
-        assert_eq!(
+        assert_eq!(generated.columns, vec![ProcWidgetColumn::WritePerSecond; 3]);
-            generated.columns,
-            Some(vec![ProcWidgetColumn::WritePerSecond; 3])
-        );
     }
 }

tests/integration/arg_tests.rs

@@ -109,7 +109,7 @@ fn test_invalid_default_widget_1() {
         .arg("fake_widget")
         .assert()
         .failure()
-        .stderr(predicate::str::contains("invalid widget name"));
+        .stderr(predicate::str::contains("invalid value"));
 }
 
 #[test]