refactor: use struct for args instead of builder interface (#1472)

* start moving args * tmp * refactor config * port over ags * update changelog
ClementTsang · May 27, 2024 · ee2e1fe · ee2e1fe
1 parent 71f6136
commit ee2e1fe
Show file tree

Hide file tree

Showing 20 changed files with 792 additions and 759 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,9 +21,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [#1436](https://github.com/ClementTsang/bottom/pull/1436): Use actual "swap" value for Windows.
 - [#1441](https://github.com/ClementTsang/bottom/pull/1441): The following arguments have changed names:
   - `--left_legend/-l` is now `--cpu_left_legend`.
-- [#1441](https://github.com/ClementTsang/bottom/pull/1441): The following config arguments have changed names:
+- [#1441](https://github.com/ClementTsang/bottom/pull/1441): The following config fields have changed names:
   - `expanded_on_startup` is now `expanded`.
   - `left_legend` is now `cpu_left_legend`.
+- [#1472](https://github.com/ClementTsang/bottom/pull/1472): The following arguments have changed names:
+  - `mem_as_value` is now `process_memory_as_value`.
+- [#1472](https://github.com/ClementTsang/bottom/pull/1472): The following config fields have changed names:
+  - `mem_as_value` is now `process_memory_as_value`.
 
 ### Bug Fixes
 

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -19,7 +19,9 @@ exclude = [
     "desktop/",
     "docs/",
     "sample_configs/",
+    "schema",
     "scripts/",
+    "wix/",
     ".all-contributorsrc",
     ".cirrus.yml",
     ".gitignore",
@@ -29,6 +31,9 @@ exclude = [
     "codecov.yml",
     "CONTRIBUTING.md",
     "Cross.toml",
+    "debug.log",
+    "flamegraph.svg",
+    "perf.data",
     "rustfmt.toml",
 ]
 rust-version = "1.74.0" # The oldest version I've tested that should still build - note this is not an official MSRV!
@@ -75,7 +80,7 @@ default = ["deploy"]
 anyhow = "1.0.86"
 backtrace = "0.3.71"
 cfg-if = "1.0.0"
-clap = { version = "4.5.4", features = ["default", "cargo", "wrap_help"] }
+clap = { version = "4.5.4", features = ["default", "cargo", "wrap_help", "derive"] }
 concat-string = "1.0.1"
 crossterm = "0.27.0"
 ctrlc = { version = "3.4.4", features = ["termination"] }
@@ -135,7 +140,7 @@ predicates = "3.1.0"
 portable-pty = "0.8.1"
 
 [build-dependencies]
-clap = { version = "4.5.4", features = ["default", "cargo", "wrap_help"] }
+clap = { version = "4.5.4", features = ["default", "cargo", "wrap_help", "derive"] }
 clap_complete = "4.5.2"
 clap_complete_nushell = "4.5.1"
 clap_complete_fig = "4.5.0"

diff --git a/build.rs b/build.rs
@@ -7,12 +7,12 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use clap::Command;
+use clap::{Command, CommandFactory};
 use clap_complete::{generate_to, shells::Shell, Generator};
 use clap_complete_fig::Fig;
 use clap_complete_nushell::Nushell;
 
-use crate::args::build_app;
+use crate::args::BottomArgs;
 
 fn create_dir(dir: &Path) -> io::Result<()> {
     let res = fs::create_dir_all(dir);
@@ -48,7 +48,7 @@ fn btm_generate() -> io::Result<()> {
             create_dir(&manpage_out_dir)?;
 
             // Generate completions
-            let mut app = build_app();
+            let mut app = BottomArgs::command();
             generate_completions(Shell::Bash, &mut app, &completion_out_dir)?;
             generate_completions(Shell::Zsh, &mut app, &completion_out_dir)?;
             generate_completions(Shell::Fish, &mut app, &completion_out_dir)?;

diff --git a/docs/content/configuration/command-line-options.md b/docs/content/configuration/command-line-options.md
@@ -25,18 +25,18 @@ see information on these options by running `btm -h`, or run `btm --help` to dis
 
 ## Process Options
 
-| Option                    | Behaviour                                                                              |
-| ------------------------- | -------------------------------------------------------------------------------------- |
-| `-S, --case_sensitive`    | Enables case sensitivity by default.                                                   |
-| `-u, --current_usage`     | Calculates process CPU usage as a percentage of current usage rather than total usage. |
-| `--disable_advanced_kill` | Hides additional stopping options Unix-like systems.                                   |
-| `-g, --group_processes`   | Groups processes with the same name by default.                                        |
-| `--mem_as_value`          | Defaults to showing process memory usage by value.                                     |
-| `--process_command`       | Shows the full command name instead of the process name by default.                    |
-| `-R, --regex`             | Enables regex by default while searching.                                              |
-| `-T, --tree`              | Makes the process widget use tree mode by default.                                     |
-| `-n, --unnormalized_cpu`  | Show process CPU% usage without averaging over the number of CPU cores.                |
-| `-W, --whole_word`        | Enables whole-word matching by default while searching.                                |
+| Option                      | Behaviour                                                                              |
+| --------------------------- | -------------------------------------------------------------------------------------- |
+| `-S, --case_sensitive`      | Enables case sensitivity by default.                                                   |
+| `-u, --current_usage`       | Calculates process CPU usage as a percentage of current usage rather than total usage. |
+| `--disable_advanced_kill`   | Hides additional stopping options Unix-like systems.                                   |
+| `-g, --group_processes`     | Groups processes with the same name by default.                                        |
+| `--process_memory_as_value` | Defaults to showing process memory usage by value.                                     |
+| `--process_command`         | Shows the full command name instead of the process name by default.                    |
+| `-R, --regex`               | Enables regex by default while searching.                                              |
+| `-T, --tree`                | Makes the process widget use tree mode by default.                                     |
+| `-n, --unnormalized_cpu`    | Show process CPU% usage without averaging over the number of CPU cores.                |
+| `-W, --whole_word`          | Enables whole-word matching by default while searching.                                |
 
 ## Temperature Options
 

diff --git a/docs/content/configuration/config-file/flags.md b/docs/content/configuration/config-file/flags.md
@@ -18,7 +18,7 @@ each time:
 | ---------------------------- | ------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------- |
 | `hide_avg_cpu`               | Boolean                                                                                                            | Hides the average CPU usage.                                     |
 | `dot_marker`                 | Boolean                                                                                                            | Uses a dot marker for graphs.                                    |
-| `cpu_left_legend`                | Boolean                                                                                                            | Puts the CPU chart legend to the left side.                      |
+| `cpu_left_legend`            | Boolean                                                                                                            | Puts the CPU chart legend to the left side.                      |
 | `current_usage`              | Boolean                                                                                                            | Sets process CPU% to be based on current CPU%.                   |
 | `group_processes`            | Boolean                                                                                                            | Groups processes with the same name by default.                  |
 | `case_sensitive`             | Boolean                                                                                                            | Enables case sensitivity by default.                             |
@@ -37,7 +37,7 @@ each time:
 | `disable_click`              | Boolean                                                                                                            | Disables mouse clicks.                                           |
 | `color`                      | String (one of ["default", "default-light", "gruvbox", "gruvbox-light", "nord", "nord-light"])                     | Use a color scheme, use --help for supported values.             |
 | `enable_cache_memory`        | Boolean                                                                                                            | Enable cache and buffer memory stats (not available on Windows). |
-| `mem_as_value`               | Boolean                                                                                                            | Defaults to showing process memory usage by value.               |
+| `process_memory_as_value`    | Boolean                                                                                                            | Defaults to showing process memory usage by value.               |
 | `tree`                       | Boolean                                                                                                            | Defaults to showing the process widget in tree mode.             |
 | `show_table_scroll_position` | Boolean                                                                                                            | Shows the scroll position tracker in table widgets.              |
 | `process_command`            | Boolean                                                                                                            | Show processes as their commands by default.                     |
@@ -48,6 +48,6 @@ each time:
 | `enable_gpu`                 | Boolean                                                                                                            | Shows the GPU widgets.                                           |
 | `retention`                  | String (human readable time, such as "10m", "1h", etc.)                                                            | How much data is stored at once in terms of time.                |
 | `unnormalized_cpu`           | Boolean                                                                                                            | Show process CPU% without normalizing over the number of cores.  |
-| `expanded`        | Boolean                                                                                                            | Expand the default widget upon starting the app.                 |
+| `expanded`                   | Boolean                                                                                                            | Expand the default widget upon starting the app.                 |
 | `memory_legend`              | String (one of ["none", "top-left", "top", "top-right", "left", "right", "bottom-left", "bottom", "bottom-right"]) | Where to place the legend for the memory widget.                 |
 | `network_legend`             | String (one of ["none", "top-left", "top", "top-right", "left", "right", "bottom-left", "bottom", "bottom-right"]) | Where to place the legend for the network widget.                |
diff --git a/sample_configs/default_config.toml b/sample_configs/default_config.toml
@@ -58,7 +58,7 @@
 # Built-in themes.  Valid values are "default", "default-light", "gruvbox", "gruvbox-light", "nord", "nord-light"
 #color = "default"
 # Show memory values in the processes widget as values by default
-#mem_as_value = false
+#process_memory_as_value = false
 # Show tree mode by default in the processes widget.
 #tree = false
 # Shows an indicator in table widgets tracking where in the list you are.

diff --git a/schema/v1.0/bottom.json b/schema/v1.0/bottom.json
@@ -184,7 +184,7 @@
           "description": "Built-in themes",
           "type": "string"
         },
-        "mem_as_value": {
+        "process_memory_as_value": {
           "default": false,
           "description": "Show memory values in the processes widget as values by default",
           "type": "boolean"

diff --git a/src/bin/main.rs b/src/bin/main.rs
@@ -17,11 +17,10 @@ use bottom::{
     args,
     canvas::{self, styling::CanvasStyling},
     check_if_terminal, cleanup_terminal, create_collection_thread, create_input_thread,
-    create_or_get_config,
     data_conversion::*,
-    handle_key_event_or_break, handle_mouse_event,
-    options::{get_color_scheme, get_widget_layout, init_app},
-    panic_hook, read_config, try_drawing, update_data, BottomEvent,
+    get_or_create_config, handle_key_event_or_break, handle_mouse_event,
+    options::{get_color_scheme, init_app},
+    panic_hook, try_drawing, update_data, BottomEvent,
 };
 use crossterm::{
     event::{EnableBracketedPaste, EnableMouseCapture},
@@ -37,7 +36,7 @@ use tui::{backend::CrosstermBackend, Terminal};
 fn main() -> Result<()> {
     // let _profiler = dhat::Profiler::new_heap();
 
-    let matches = args::get_matches();
+    let args = args::get_args();
 
     #[cfg(feature = "logging")]
     {
@@ -50,34 +49,17 @@ fn main() -> Result<()> {
     }
 
     // Read from config file.
-    let config = {
-        let config_path = read_config(matches.get_one::<String>("config_location"))
-            .context("Unable to access the given config file location.")?;
-
-        create_or_get_config(&config_path)
-            .context("Unable to properly parse or create the config file.")?
-    };
-
-    // Get widget layout separately
-    let (widget_layout, default_widget_id, default_widget_type_option) =
-        get_widget_layout(&matches, &config)
-            .context("Found an issue while trying to build the widget layout.")?;
+    let config = get_or_create_config(args.general.config_location.as_deref())
+        .context("Unable to parse or create the config file.")?;
 
     // FIXME: Should move this into build app or config
     let styling = {
-        let colour_scheme = get_color_scheme(&matches, &config)?;
+        let colour_scheme = get_color_scheme(&args, &config)?;
         CanvasStyling::new(colour_scheme, &config)?
     };
 
     // Create an "app" struct, which will control most of the program and store settings/state
-    let mut app = init_app(
-        matches,
-        config,
-        &widget_layout,
-        default_widget_id,
-        &default_widget_type_option,
-        &styling,
-    )?;
+    let (mut app, widget_layout) = init_app(args, config, &styling)?;
 
     // Create painter and set colours.
     let mut painter = canvas::Painter::init(widget_layout, styling)?;

diff --git a/src/canvas/styling.rs b/src/canvas/styling.rs
@@ -5,7 +5,7 @@ use colour_utils::*;
 use tui::style::{Color, Style};
 
 use super::ColourScheme;
-pub use crate::options::Config;
+pub use crate::options::ConfigV1;
 use crate::{constants::*, options::colours::ConfigColours, utils::error};
 
 pub struct CanvasStyling {
@@ -124,7 +124,7 @@ macro_rules! try_set_colour_list {
 }
 
 impl CanvasStyling {
-    pub fn new(colour_scheme: ColourScheme, config: &Config) -> anyhow::Result<Self> {
+    pub fn new(colour_scheme: ColourScheme, config: &ConfigV1) -> anyhow::Result<Self> {
         let mut canvas_colours = Self::default();
 
         match colour_scheme {
@@ -236,7 +236,7 @@ mod test {
     use tui::style::{Color, Style};
 
     use super::{CanvasStyling, ColourScheme};
-    use crate::options::Config;
+    use crate::options::ConfigV1;
 
     #[test]
     fn default_selected_colour_works() {
@@ -282,7 +282,7 @@ mod test {
 
     #[test]
     fn built_in_colour_schemes_work() {
-        let config = Config::default();
+        let config = ConfigV1::default();
         CanvasStyling::new(ColourScheme::Default, &config).unwrap();
         CanvasStyling::new(ColourScheme::DefaultLight, &config).unwrap();
         CanvasStyling::new(ColourScheme::Gruvbox, &config).unwrap();

diff --git a/src/constants.rs b/src/constants.rs
@@ -576,7 +576,7 @@ pub const CONFIG_TEXT: &str = r#"# This is a default config file for bottom.  Al
 # Built-in themes. Valid values are "default", "default-light", "gruvbox", "gruvbox-light", "nord", "nord-light"
 #color = "default"
 # Show memory values in the processes widget as values by default
-#mem_as_value = false
+#process_memory_as_value = false
 # Show tree mode by default in the processes widget.
 #tree = false
 # Shows an indicator in table widgets tracking where in the list you are.

diff --git a/src/data_collection/temperature.rs b/src/data_collection/temperature.rs
@@ -13,6 +13,8 @@ cfg_if::cfg_if! {
     }
 }
 
+use std::str::FromStr;
+
 use crate::app::filter::Filter;
 
 #[derive(Default, Debug, Clone)]
@@ -29,6 +31,21 @@ pub enum TemperatureType {
     Fahrenheit,
 }
 
+impl FromStr for TemperatureType {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "fahrenheit" | "f" => Ok(TemperatureType::Fahrenheit),
+            "kelvin" | "k" => Ok(TemperatureType::Kelvin),
+            "celsius" | "c" => Ok(TemperatureType::Celsius),
+            _ => Err(format!(
+                "\"{s}\" is an invalid temperature type, use \"<kelvin|k|celsius|c|fahrenheit|f>\"."
+            )),
+        }
+    }
+}
+
 impl TemperatureType {
     /// Given a temperature in Celsius, covert it if necessary for a different unit.
     pub fn convert_temp_unit(&self, temp_celsius: f32) -> f32 {