From a4ddd649e140e1bc7cab29a53662d4f345f4f1ff Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Mon, 31 Aug 2020 23:59:33 -0400 Subject: [PATCH] refactor: Update error messages w/ anyhow and thiserror (#216) Refactoring and updating of error messages + tests to be more useful. --- .cargo-husky/hooks/pre-push | 2 + .vscode/settings.json | 1 + Cargo.lock | 28 +++++++ Cargo.toml | 12 +-- README.md | 2 +- src/app/data_harvester/processes.rs | 6 +- src/app/layout_manager.rs | 20 ++++- src/bin/main.rs | 13 +++- src/canvas/canvas_colours.rs | 6 +- src/canvas/canvas_colours/colour_utils.rs | 32 +++++--- src/clap.rs | 2 +- src/lib.rs | 95 ++++++++++++++++------- src/options.rs | 55 +++++++------ src/utils/error.rs | 47 ++++------- tests/arg_tests.rs | 16 ++-- tests/invalid_config_tests.rs | 31 +++++--- tests/invalid_configs/empty_battery.toml | 2 + 17 files changed, 240 insertions(+), 130 deletions(-) create mode 100755 .cargo-husky/hooks/pre-push create mode 100644 tests/invalid_configs/empty_battery.toml diff --git a/.cargo-husky/hooks/pre-push b/.cargo-husky/hooks/pre-push new file mode 100755 index 00000000..46fb32d2 --- /dev/null +++ b/.cargo-husky/hooks/pre-push @@ -0,0 +1,2 @@ +echo "Running pre-push hook: cargo +nightly clippy -- -D clippy::all" +cargo +nightly clippy -- -D clippy::all \ No newline at end of file diff --git a/.vscode/settings.json b/.vscode/settings.json index d3f49cfa..b5030dc0 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -38,6 +38,7 @@ "curr", "czvf", "fpath", + "fract", "gotop", "gtop", "haase", diff --git a/Cargo.lock b/Cargo.lock index 9c458943..20f2221f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -27,6 +27,12 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "anyhow" +version = "1.0.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6b602bfe940d21c130f3895acd65221e8a61270debe89d628b9cb4e3ccb8569b" + [[package]] name = "arc-swap" version = "0.4.6" @@ -131,6 +137,7 @@ dependencies = [ name = "bottom" version = "0.4.7" dependencies = [ + "anyhow", "assert_cmd", "backtrace", "battery", @@ -151,6 +158,7 @@ dependencies = [ "regex", "serde", "sysinfo", + "thiserror", "toml", "tui", "typed-builder", @@ -1273,6 +1281,26 @@ dependencies = [ "unicode-width", ] +[[package]] +name = "thiserror" +version = "1.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7dfdd070ccd8ccb78f4ad66bf1982dc37f620ef696c6b5028fe2ed83dd3d0d08" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bd80fc12f73063ac132ac92aceea36734f04a1d93c1240c6944e23a3b8841793" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "thread_local" version = "1.0.1" diff --git a/Cargo.toml b/Cargo.toml index ce60333a..cdac5df4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,14 +24,17 @@ lto = "fat" codegen-units = 1 [dependencies] +anyhow = "1.0.32" battery = "0.7.6" -crossterm = "0.17" chrono = "0.4.15" +crossterm = "0.17" +ctrlc = {version = "3.1", features = ["termination"]} clap = "2.33" dirs = "3.0.1" futures = "0.3.5" heim = "0.0.10" itertools = "0.9.0" +libc = "0.2" regex = "1.3" sysinfo = "0.15.1" toml = "0.5.6" @@ -41,8 +44,7 @@ backtrace = "0.3" serde = {version = "1.0", features = ["derive"] } unicode-segmentation = "1.6.0" unicode-width = "0.1" -libc = "0.2" -ctrlc = {version = "3.1", features = ["termination"]} +thiserror = "1.0.20" tui = {version = "0.9.5", features = ["crossterm"], default-features = false } # For debugging only... @@ -82,5 +84,5 @@ output = "bottom_x86_64_installer.msi" [dev-dependencies.cargo-husky] version = "1" -default-features = false -features = ["prepush-hook", "run-cargo-clippy"] \ No newline at end of file +default-features = false +features = ["user-hooks"] \ No newline at end of file diff --git a/README.md b/README.md index 0083f3f7..cbf5b55e 100644 --- a/README.md +++ b/README.md @@ -509,7 +509,7 @@ Supported named colours are one of the following strings: `Reset, Black, Red, Gr | Cursor colour | The cursor's colour | `cursor_color="#ffffff"` | | Selected text colour | The colour of text that is selected | `scroll_entry_text_color="#ffffff"` | | Selected text background colour | The background colour of text that is selected | `scroll_entry_bg_color="#ffffff"` | -| Battery bar colours | Colour used is based on percentage and no. of colours | `battery_colours=["green", "yellow", "red"]` | +| Battery bar colours | Colour used is based on percentage and no. of colours | `battery_colors=["green", "yellow", "red"]` | #### Layout diff --git a/src/app/data_harvester/processes.rs b/src/app/data_harvester/processes.rs index bdd6a609..dcd6d647 100644 --- a/src/app/data_harvester/processes.rs +++ b/src/app/data_harvester/processes.rs @@ -253,11 +253,11 @@ fn read_proc( .splitn(2, '(') .collect::>() .last() - .ok_or(BottomError::MinorError())? + .ok_or(BottomError::MinorError)? .rsplitn(2, ')') .collect::>() .last() - .ok_or(BottomError::MinorError())? + .ok_or(BottomError::MinorError)? .to_string(); let command = { let cmd = read_path_contents(&pid_stat.proc_cmdline_path)?; @@ -271,7 +271,7 @@ fn read_proc( .split(')') .collect::>() .last() - .ok_or(BottomError::MinorError())? + .ok_or(BottomError::MinorError)? .split_whitespace() .collect::>(); let (process_state_char, process_state) = get_linux_process_state(&stat); diff --git a/src/app/layout_manager.rs b/src/app/layout_manager.rs index 35c6e02f..6036c2a5 100644 --- a/src/app/layout_manager.rs +++ b/src/app/layout_manager.rs @@ -943,7 +943,25 @@ impl std::str::FromStr for BottomWidgetType { "empty" => Ok(BottomWidgetType::Empty), "battery" | "batt" => Ok(BottomWidgetType::Battery), _ => Err(BottomError::ConfigError(format!( - "invalid widget type: {}", // FIXME: Make this more helpful, specify valid widget types (just go through the list) + "\"{}\" is an invalid widget name. + +Supported widget names: ++--------------------------+ +| cpu | ++--------------------------+ +| mem, memory | ++--------------------------+ +| net, network | ++--------------------------+ +| proc, process, processes | ++--------------------------+ +| temp, temperature | ++--------------------------+ +| disk | ++--------------------------+ +| batt, battery | ++--------------------------+ + ", s ))), } diff --git a/src/bin/main.rs b/src/bin/main.rs index 95444491..7f8b9900 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -3,7 +3,7 @@ #[macro_use] extern crate log; -use bottom::{canvas, constants::*, data_conversion::*, options::*, utils::error, *}; +use bottom::{canvas, constants::*, data_conversion::*, options::*, *}; use std::{ boxed::Box, @@ -17,6 +17,7 @@ use std::{ time::Duration, }; +use anyhow::{Context, Result}; use crossterm::{ event::EnableMouseCapture, execute, @@ -24,18 +25,22 @@ use crossterm::{ }; use tui::{backend::CrosstermBackend, Terminal}; -fn main() -> error::Result<()> { +fn main() -> Result<()> { #[cfg(debug_assertions)] { utils::logging::init_logger()?; } let matches = clap::get_matches(); - let config: Config = create_config(matches.value_of("CONFIG_LOCATION"))?; + let config_path = read_config(matches.value_of("CONFIG_LOCATION")) + .context("Unable to access the given config file location.")?; + let config: Config = 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)?; + get_widget_layout(&matches, &config) + .context("Found an issue while trying to build the widget layout.")?; // Create "app" struct, which will control most of the program and store settings/state let mut app = build_app( diff --git a/src/canvas/canvas_colours.rs b/src/canvas/canvas_colours.rs index 86c96f4b..98932d5d 100644 --- a/src/canvas/canvas_colours.rs +++ b/src/canvas/canvas_colours.rs @@ -1,5 +1,4 @@ use tui::style::{Color, Style}; -// use tui::style::Modifier; use colour_utils::*; @@ -175,11 +174,10 @@ impl CanvasColours { Ok(()) } - pub fn set_battery_colours(&mut self, colours: &[String]) -> error::Result<()> { + pub fn set_battery_colors(&mut self, colours: &[String]) -> error::Result<()> { if colours.is_empty() { Err(error::BottomError::ConfigError( - "invalid colour config: battery colour list must have at least one colour!" - .to_string(), + "battery colour list must have at least one colour.".to_string(), )) } else { let generated_colours: Result, _> = colours diff --git a/src/canvas/canvas_colours/colour_utils.rs b/src/canvas/canvas_colours/colour_utils.rs index 8b2f0594..f94b9bd9 100644 --- a/src/canvas/canvas_colours/colour_utils.rs +++ b/src/canvas/canvas_colours/colour_utils.rs @@ -100,7 +100,7 @@ pub fn convert_hex_to_color(hex: &str) -> error::Result { fn hex_err(hex: &str) -> error::Result { Err( error::BottomError::ConfigError(format!( - "invalid color hex: error when parsing hex value {}. It must be a valid 7 character hex string of the (ie: \"#112233\")." + "\"{}\" is an invalid hex colour. It must be a valid 7 character hex string of the (ie: \"#112233\")." , hex)) ) } @@ -124,7 +124,7 @@ pub fn convert_hex_to_color(hex: &str) -> error::Result { } Err(error::BottomError::ConfigError(format!( - "invalid color hex: value {} is not of valid length. It must be a 7 character string of the form \"#112233\".", + "\"{}\" is an invalid hex colour. It must be a 7 character string of the form \"#112233\".", hex ))) } @@ -144,7 +144,7 @@ pub fn get_style_from_config(input_val: &str) -> error::Result