refactor: out with arg/config error, and make user messages more consistent (#1494)

* refactor: out with arg/config error, and make user messages more consistent

* finish up

* fix all the tests
This commit is contained in:
Clement Tsang 2024-07-19 02:51:50 -04:00 committed by GitHub
parent d97d75f797
commit 1ec4ca3f06
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 329 additions and 273 deletions

View File

@ -1,9 +1,6 @@
use std::collections::BTreeMap; use std::collections::BTreeMap;
use crate::{ use crate::{constants::DEFAULT_WIDGET_ID, options::OptionError};
constants::DEFAULT_WIDGET_ID,
error::{BottomError, Result},
};
/// Represents a more usable representation of the layout, derived from the /// Represents a more usable representation of the layout, derived from the
/// config. /// config.
@ -985,9 +982,9 @@ impl BottomWidgetType {
} }
impl std::str::FromStr for BottomWidgetType { impl std::str::FromStr for BottomWidgetType {
type Err = BottomError; type Err = OptionError;
fn from_str(s: &str) -> Result<Self> { fn from_str(s: &str) -> Result<Self, Self::Err> {
let lower_case = s.to_lowercase(); let lower_case = s.to_lowercase();
match lower_case.as_str() { match lower_case.as_str() {
"cpu" => Ok(BottomWidgetType::Cpu), "cpu" => Ok(BottomWidgetType::Cpu),
@ -1002,8 +999,8 @@ impl std::str::FromStr for BottomWidgetType {
_ => { _ => {
#[cfg(feature = "battery")] #[cfg(feature = "battery")]
{ {
Err(BottomError::ConfigError(format!( Err(OptionError::config(format!(
"\"{s}\" is an invalid widget name. "'{s}' is an invalid widget name.
Supported widget names: Supported widget names:
+--------------------------+ +--------------------------+
@ -1028,8 +1025,8 @@ Supported widget names:
} }
#[cfg(not(feature = "battery"))] #[cfg(not(feature = "battery"))]
{ {
Err(BottomError::ConfigError(format!( Err(OptionError::config(format!(
"\"{s}\" is an invalid widget name. "'{s}' is an invalid widget name.
Supported widget names: Supported widget names:
+--------------------------+ +--------------------------+

View File

@ -22,7 +22,8 @@ use crate::{
App, App,
}, },
constants::*, constants::*,
utils::{error, error::BottomError}, options::OptionError,
utils::error,
}; };
#[derive(Debug)] #[derive(Debug)]
@ -37,9 +38,9 @@ pub enum ColourScheme {
} }
impl FromStr for ColourScheme { impl FromStr for ColourScheme {
type Err = BottomError; type Err = OptionError;
fn from_str(s: &str) -> error::Result<Self> { fn from_str(s: &str) -> Result<Self, Self::Err> {
let lower_case = s.to_lowercase(); let lower_case = s.to_lowercase();
match lower_case.as_str() { match lower_case.as_str() {
"default" => Ok(ColourScheme::Default), "default" => Ok(ColourScheme::Default),
@ -48,8 +49,8 @@ impl FromStr for ColourScheme {
"gruvbox-light" => Ok(ColourScheme::GruvboxLight), "gruvbox-light" => Ok(ColourScheme::GruvboxLight),
"nord" => Ok(ColourScheme::Nord), "nord" => Ok(ColourScheme::Nord),
"nord-light" => Ok(ColourScheme::NordLight), "nord-light" => Ok(ColourScheme::NordLight),
_ => Err(BottomError::ConfigError(format!( _ => Err(OptionError::other(format!(
"`{s}` is an invalid built-in color scheme." "'{s}' is an invalid built-in color scheme."
))), ))),
} }
} }

View File

@ -182,19 +182,19 @@ impl Painter {
{ {
if to_kill_processes.1.len() != 1 { if to_kill_processes.1.len() != 1 {
Line::from(format!( Line::from(format!(
"Kill {} processes with the name \"{}\"? Press ENTER to confirm.", "Kill {} processes with the name '{}'? Press ENTER to confirm.",
to_kill_processes.1.len(), to_kill_processes.1.len(),
to_kill_processes.0 to_kill_processes.0
)) ))
} else { } else {
Line::from(format!( Line::from(format!(
"Kill 1 process with the name \"{}\"? Press ENTER to confirm.", "Kill 1 process with the name '{}'? Press ENTER to confirm.",
to_kill_processes.0 to_kill_processes.0
)) ))
} }
} else { } else {
Line::from(format!( Line::from(format!(
"Kill process \"{}\" with PID {}? Press ENTER to confirm.", "Kill process '{}' with PID {}? Press ENTER to confirm.",
to_kill_processes.0, first_pid to_kill_processes.0, first_pid
)) ))
}, },

View File

@ -1,12 +1,15 @@
mod colour_utils; mod colour_utils;
use anyhow::Context;
use colour_utils::*; use colour_utils::*;
use tui::style::{Color, Style}; use tui::style::{Color, Style};
use super::ColourScheme; use super::ColourScheme;
pub use crate::options::ConfigV1; pub use crate::options::ConfigV1;
use crate::{constants::*, options::colours::ColoursConfig, utils::error}; use crate::{
constants::*,
options::{colours::ColoursConfig, OptionError, OptionResult},
utils::error,
};
pub struct CanvasStyling { pub struct CanvasStyling {
pub currently_selected_text_colour: Color, pub currently_selected_text_colour: Color,
@ -98,11 +101,12 @@ impl Default for CanvasStyling {
macro_rules! try_set_colour { macro_rules! try_set_colour {
($field:expr, $colours:expr, $colour_field:ident) => { ($field:expr, $colours:expr, $colour_field:ident) => {
if let Some(colour_str) = &$colours.$colour_field { if let Some(colour_str) = &$colours.$colour_field {
$field = str_to_fg(colour_str).context(concat!( $field = str_to_fg(colour_str).map_err(|err| {
"update '", OptionError::config(format!(
stringify!($colour_field), "Please update 'colors.{}' in your config file. {err}",
"' in your config file" stringify!($colour_field)
))?; ))
})?;
} }
}; };
} }
@ -113,12 +117,13 @@ macro_rules! try_set_colour_list {
$field = colour_list $field = colour_list
.iter() .iter()
.map(|s| str_to_fg(s)) .map(|s| str_to_fg(s))
.collect::<error::Result<Vec<Style>>>() .collect::<Result<Vec<Style>, String>>()
.context(concat!( .map_err(|err| {
"update '", OptionError::config(format!(
stringify!($colour_field), "Please update 'colors.{}' in your config file. {err}",
"' in your config file" stringify!($colour_field)
))?; ))
})?;
} }
}; };
} }
@ -154,7 +159,7 @@ impl CanvasStyling {
Ok(canvas_colours) Ok(canvas_colours)
} }
pub fn set_colours_from_palette(&mut self, colours: &ColoursConfig) -> anyhow::Result<()> { pub fn set_colours_from_palette(&mut self, colours: &ColoursConfig) -> OptionResult<()> {
// CPU // CPU
try_set_colour!(self.avg_colour_style, colours, avg_cpu_color); try_set_colour!(self.avg_colour_style, colours, avg_cpu_color);
try_set_colour!(self.all_colour_style, colours, all_cpu_color); try_set_colour!(self.all_colour_style, colours, all_cpu_color);
@ -201,12 +206,12 @@ impl CanvasStyling {
if let Some(scroll_entry_text_color) = &colours.selected_text_color { if let Some(scroll_entry_text_color) = &colours.selected_text_color {
self.set_scroll_entry_text_color(scroll_entry_text_color) self.set_scroll_entry_text_color(scroll_entry_text_color)
.context("update 'selected_text_color' in your config file")?; .map_err(|_| OptionError::invalid_config_value("selected_text_color"))?
} }
if let Some(scroll_entry_bg_color) = &colours.selected_bg_color { if let Some(scroll_entry_bg_color) = &colours.selected_bg_color {
self.set_scroll_entry_bg_color(scroll_entry_bg_color) self.set_scroll_entry_bg_color(scroll_entry_bg_color)
.context("update 'selected_bg_color' in your config file")?; .map_err(|_| OptionError::invalid_config_value("selected_bg_color"))?
} }
Ok(()) Ok(())

View File

@ -3,8 +3,6 @@ use itertools::Itertools;
use tui::style::{Color, Style}; use tui::style::{Color, Style};
use unicode_segmentation::UnicodeSegmentation; use unicode_segmentation::UnicodeSegmentation;
use crate::utils::error;
pub const FIRST_COLOUR: Color = Color::LightMagenta; pub const FIRST_COLOUR: Color = Color::LightMagenta;
pub const SECOND_COLOUR: Color = Color::LightYellow; pub const SECOND_COLOUR: Color = Color::LightYellow;
pub const THIRD_COLOUR: Color = Color::LightCyan; pub const THIRD_COLOUR: Color = Color::LightCyan;
@ -16,19 +14,16 @@ pub const AVG_COLOUR: Color = Color::Red;
pub const ALL_COLOUR: Color = Color::Green; pub const ALL_COLOUR: Color = Color::Green;
/// Convert a hex string to a colour. /// Convert a hex string to a colour.
fn convert_hex_to_color(hex: &str) -> error::Result<Color> { fn convert_hex_to_color(hex: &str) -> Result<Color, String> {
fn hex_component_to_int(hex: &str, first: &str, second: &str) -> error::Result<u8> { fn hex_component_to_int(hex: &str, first: &str, second: &str) -> Result<u8, String> {
u8::from_str_radix(&concat_string!(first, second), 16).map_err(|_| { u8::from_str_radix(&concat_string!(first, second), 16)
error::BottomError::ConfigError(format!( .map_err(|_| format!("'{hex}' is an invalid hex color, could not decode."))
"\"{hex}\" is an invalid hex color, could not decode."
))
})
} }
fn invalid_hex_format(hex: &str) -> error::BottomError { fn invalid_hex_format(hex: &str) -> String {
error::BottomError::ConfigError(format!( format!(
"\"{hex}\" is an invalid hex color. It must be either a 7 character hex string of the form \"#12ab3c\" or a 3 character hex string of the form \"#1a2\".", "'{hex}' is an invalid hex color. It must be either a 7 character hex string of the form '#12ab3c' or a 3 character hex string of the form '#1a2'.",
)) )
} }
if !hex.starts_with('#') { if !hex.starts_with('#') {
@ -55,11 +50,11 @@ fn convert_hex_to_color(hex: &str) -> error::Result<Color> {
} }
} }
pub fn str_to_fg(input_val: &str) -> error::Result<Style> { pub fn str_to_fg(input_val: &str) -> Result<Style, String> {
Ok(Style::default().fg(str_to_colour(input_val)?)) Ok(Style::default().fg(str_to_colour(input_val)?))
} }
pub fn str_to_colour(input_val: &str) -> error::Result<Color> { pub fn str_to_colour(input_val: &str) -> Result<Color, String> {
if input_val.len() > 1 { if input_val.len() > 1 {
if input_val.starts_with('#') { if input_val.starts_with('#') {
convert_hex_to_color(input_val) convert_hex_to_color(input_val)
@ -69,18 +64,16 @@ pub fn str_to_colour(input_val: &str) -> error::Result<Color> {
convert_name_to_colour(input_val) convert_name_to_colour(input_val)
} }
} else { } else {
Err(error::BottomError::ConfigError(format!( Err(format!("Value '{input_val}' is not valid.",))
"value \"{input_val}\" is not valid.",
)))
} }
} }
fn convert_rgb_to_color(rgb_str: &str) -> error::Result<Color> { fn convert_rgb_to_color(rgb_str: &str) -> Result<Color, String> {
let rgb_list = rgb_str.split(',').collect::<Vec<&str>>(); let rgb_list = rgb_str.split(',').collect::<Vec<&str>>();
if rgb_list.len() != 3 { if rgb_list.len() != 3 {
return Err(error::BottomError::ConfigError(format!( return Err(format!(
"value \"{rgb_str}\" is an invalid RGB colour. It must be a comma separated value with 3 integers from 0 to 255 (ie: \"255, 0, 155\").", "Value '{rgb_str}' is an invalid RGB colour. It must be a comma separated value with 3 integers from 0 to 255 (ie: '255, 0, 155').",
))); ));
} }
let rgb = rgb_list let rgb = rgb_list
@ -93,16 +86,17 @@ fn convert_rgb_to_color(rgb_str: &str) -> error::Result<Color> {
} }
}) })
.collect::<Vec<_>>(); .collect::<Vec<_>>();
if rgb.len() == 3 { if rgb.len() == 3 {
Ok(Color::Rgb(rgb[0], rgb[1], rgb[2])) Ok(Color::Rgb(rgb[0], rgb[1], rgb[2]))
} else { } else {
Err(error::BottomError::ConfigError(format!( Err(format!(
"value \"{rgb_str}\" contained invalid RGB values. It must be a comma separated value with 3 integers from 0 to 255 (ie: \"255, 0, 155\").", "Value '{rgb_str}' contained invalid RGB values. It must be a comma separated value with 3 integers from 0 to 255 (ie: '255, 0, 155').",
))) ))
} }
} }
fn convert_name_to_colour(color_name: &str) -> error::Result<Color> { fn convert_name_to_colour(color_name: &str) -> Result<Color, String> {
match color_name.to_lowercase().trim() { match color_name.to_lowercase().trim() {
"reset" => Ok(Color::Reset), "reset" => Ok(Color::Reset),
"black" => Ok(Color::Black), "black" => Ok(Color::Black),
@ -121,8 +115,8 @@ fn convert_name_to_colour(color_name: &str) -> error::Result<Color> {
"lightmagenta" | "light magenta" => Ok(Color::LightMagenta), "lightmagenta" | "light magenta" => Ok(Color::LightMagenta),
"lightcyan" | "light cyan" => Ok(Color::LightCyan), "lightcyan" | "light cyan" => Ok(Color::LightCyan),
"white" => Ok(Color::White), "white" => Ok(Color::White),
_ => Err(error::BottomError::ConfigError(format!( _ => Err(format!(
"\"{color_name}\" is an invalid named color. "'{color_name}' is an invalid named color.
The following are supported strings: The following are supported strings:
+--------+-------------+---------------------+ +--------+-------------+---------------------+
@ -137,9 +131,8 @@ The following are supported strings:
| Yellow | Light Red | White | | Yellow | Light Red | White |
+--------+-------------+---------------------+ +--------+-------------+---------------------+
| Blue | Light Green | | | Blue | Light Green | |
+--------+-------------+---------------------+ +--------+-------------+---------------------+\n"
", )),
))),
} }
} }

View File

@ -36,7 +36,7 @@ impl UnixProcessExt for MacOSProcessExt {
.for_each(|chunk| { .for_each(|chunk| {
let chunk: Vec<&str> = chunk.collect(); let chunk: Vec<&str> = chunk.collect();
if chunk.len() != 2 { if chunk.len() != 2 {
panic!("Unexpected `ps` output"); panic!("Unexpected 'ps' output");
} }
let pid = chunk[0].parse(); let pid = chunk[0].parse();
let usage = chunk[1].parse(); let usage = chunk[1].parse();

View File

@ -40,7 +40,7 @@ impl FromStr for TemperatureType {
"kelvin" | "k" => Ok(TemperatureType::Kelvin), "kelvin" | "k" => Ok(TemperatureType::Kelvin),
"celsius" | "c" => Ok(TemperatureType::Celsius), "celsius" | "c" => Ok(TemperatureType::Celsius),
_ => Err(format!( _ => Err(format!(
"\"{s}\" is an invalid temperature type, use \"<kelvin|k|celsius|c|fahrenheit|f>\"." "'{s}' is an invalid temperature type, use one of: [kelvin, k, celsius, c, fahrenheit, f]."
)), )),
} }
} }

View File

@ -5,6 +5,7 @@
pub mod args; pub mod args;
pub mod colours; pub mod colours;
pub mod config; pub mod config;
mod error;
use std::{ use std::{
convert::TryInto, convert::TryInto,
@ -18,6 +19,7 @@ use std::{
use anyhow::{Context, Result}; use anyhow::{Context, Result};
pub use colours::ColoursConfig; pub use colours::ColoursConfig;
pub use config::ConfigV1; pub use config::ConfigV1;
pub(crate) use error::{OptionError, OptionResult};
use hashbrown::{HashMap, HashSet}; use hashbrown::{HashMap, HashSet};
use indexmap::IndexSet; use indexmap::IndexSet;
use regex::Regex; use regex::Regex;
@ -33,10 +35,7 @@ use crate::{
canvas::{components::time_chart::LegendPosition, styling::CanvasStyling, ColourScheme}, canvas::{components::time_chart::LegendPosition, styling::CanvasStyling, ColourScheme},
constants::*, constants::*,
data_collection::temperature::TemperatureType, data_collection::temperature::TemperatureType,
utils::{ utils::data_units::DataUnit,
data_units::DataUnit,
error::{self, BottomError},
},
widgets::*, widgets::*,
}; };
@ -98,11 +97,11 @@ pub fn get_config_path(override_config_path: Option<&Path>) -> Option<PathBuf> {
/// path, it will try to create a new file with the default settings, and return /// path, it will try to create a new file with the default settings, and return
/// the default config. If bottom fails to write a new config, it will silently /// the default config. If bottom fails to write a new config, it will silently
/// just return the default config. /// just return the default config.
pub fn get_or_create_config(config_path: Option<&Path>) -> error::Result<ConfigV1> { pub fn get_or_create_config(config_path: Option<&Path>) -> OptionResult<ConfigV1> {
match &config_path { match &config_path {
Some(path) => { Some(path) => {
if let Ok(config_string) = fs::read_to_string(path) { if let Ok(config_string) = fs::read_to_string(path) {
Ok(toml_edit::de::from_str(config_string.as_str())?) Ok(toml_edit::de::from_str(&config_string)?)
} else { } else {
if let Some(parent_path) = path.parent() { if let Some(parent_path) = path.parent() {
fs::create_dir_all(parent_path)?; fs::create_dir_all(parent_path)?;
@ -445,7 +444,7 @@ pub fn init_app(
pub fn get_widget_layout( pub fn get_widget_layout(
args: &BottomArgs, config: &ConfigV1, args: &BottomArgs, config: &ConfigV1,
) -> error::Result<(BottomLayout, u64, Option<BottomWidgetType>)> { ) -> OptionResult<(BottomLayout, u64, Option<BottomWidgetType>)> {
let cpu_left_legend = is_flag_enabled!(cpu_left_legend, args.cpu, config); let cpu_left_legend = is_flag_enabled!(cpu_left_legend, args.cpu, config);
let (default_widget_type, mut default_widget_count) = let (default_widget_type, mut default_widget_count) =
@ -488,8 +487,9 @@ pub fn get_widget_layout(
&mut default_widget_count, &mut default_widget_count,
cpu_left_legend, cpu_left_legend,
) )
.map_err(|err| OptionError::config(err.to_string()))
}) })
.collect::<error::Result<Vec<_>>>()?, .collect::<OptionResult<Vec<_>>>()?,
total_row_height_ratio: total_height_ratio, total_row_height_ratio: total_height_ratio,
}; };
@ -498,8 +498,8 @@ pub fn get_widget_layout(
ret_bottom_layout.get_movement_mappings(); ret_bottom_layout.get_movement_mappings();
ret_bottom_layout ret_bottom_layout
} else { } else {
return Err(BottomError::ConfigError( return Err(OptionError::config(
"please have at least one widget under the '[[row]]' section.".to_string(), "have at least one widget under the '[[row]]' section.",
)); ));
} }
}; };
@ -507,36 +507,103 @@ pub fn get_widget_layout(
Ok((bottom_layout, default_widget_id, default_widget_type)) Ok((bottom_layout, default_widget_id, default_widget_type))
} }
fn get_update_rate(args: &BottomArgs, config: &ConfigV1) -> error::Result<u64> { #[inline]
let update_rate = if let Some(update_rate) = &args.general.rate { fn try_parse_ms(s: &str) -> Result<u64, ()> {
try_parse_ms(update_rate).map_err(|_| { Ok(if let Ok(val) = humantime::parse_duration(s) {
BottomError::ArgumentError("set your update rate to be valid".to_string()) val.as_millis().try_into().map_err(|_| ())?
})? } else if let Ok(val) = s.parse::<u64>() {
} else if let Some(flags) = &config.flags { val
if let Some(rate) = &flags.rate {
match rate {
StringOrNum::String(s) => try_parse_ms(s).map_err(|_| {
BottomError::ConfigError("set your update rate to be valid".to_string())
})?,
StringOrNum::Num(n) => *n,
}
} else {
DEFAULT_REFRESH_RATE_IN_MILLISECONDS
}
} else { } else {
DEFAULT_REFRESH_RATE_IN_MILLISECONDS return Err(());
}; })
if update_rate < 250 {
return Err(BottomError::ConfigError(
"set your update rate to be at least 250 ms.".to_string(),
));
}
Ok(update_rate)
} }
fn get_temperature(args: &BottomArgs, config: &ConfigV1) -> error::Result<TemperatureType> { macro_rules! parse_arg_value {
($to_try:expr, $flag:literal) => {
$to_try.map_err(|_| OptionError::invalid_arg_value($flag))
};
}
macro_rules! parse_config_value {
($to_try:expr, $setting:literal) => {
$to_try.map_err(|_| OptionError::invalid_config_value($setting))
};
}
macro_rules! parse_ms_option {
($arg_expr:expr, $config_expr:expr, $default_value:expr, $setting:literal, $low:expr, $high:expr $(,)?) => {{
use humantime::format_duration;
if let Some(to_parse) = $arg_expr {
let value = parse_arg_value!(try_parse_ms(to_parse), $setting)?;
if let Some(limit) = $low {
if value < limit {
return Err(OptionError::arg(format!(
"'--{}' must be greater than {}",
$setting,
format_duration(Duration::from_millis(limit))
)));
}
}
if let Some(limit) = $high {
if value > limit {
return Err(OptionError::arg(format!(
"'--{}' must be less than {}",
$setting,
format_duration(Duration::from_millis(limit))
)));
}
}
Ok(value)
} else if let Some(to_parse) = $config_expr {
let value = match to_parse {
StringOrNum::String(s) => parse_config_value!(try_parse_ms(s), $setting)?,
StringOrNum::Num(n) => *n,
};
if let Some(limit) = $low {
if value < limit {
return Err(OptionError::arg(format!(
"'{}' must be greater than {}",
$setting,
format_duration(Duration::from_millis(limit))
)));
}
}
if let Some(limit) = $high {
if value > limit {
return Err(OptionError::arg(format!(
"'{}' must be less than {}",
$setting,
format_duration(Duration::from_millis(limit))
)));
}
}
Ok(value)
} else {
Ok($default_value)
}
}};
}
#[inline]
fn get_update_rate(args: &BottomArgs, config: &ConfigV1) -> OptionResult<u64> {
parse_ms_option!(
&args.general.rate,
config.flags.as_ref().and_then(|flags| flags.rate.as_ref()),
DEFAULT_REFRESH_RATE_IN_MILLISECONDS,
"rate",
Some(250),
None,
)
}
fn get_temperature(args: &BottomArgs, config: &ConfigV1) -> OptionResult<TemperatureType> {
if args.temperature.fahrenheit { if args.temperature.fahrenheit {
return Ok(TemperatureType::Fahrenheit); return Ok(TemperatureType::Fahrenheit);
} else if args.temperature.kelvin { } else if args.temperature.kelvin {
@ -545,7 +612,7 @@ fn get_temperature(args: &BottomArgs, config: &ConfigV1) -> error::Result<Temper
return Ok(TemperatureType::Celsius); return Ok(TemperatureType::Celsius);
} else if let Some(flags) = &config.flags { } else if let Some(flags) = &config.flags {
if let Some(temp_type) = &flags.temperature_type { if let Some(temp_type) = &flags.temperature_type {
return TemperatureType::from_str(temp_type).map_err(BottomError::ConfigError); return parse_config_value!(TemperatureType::from_str(temp_type), "temperature_type");
} }
} }
Ok(TemperatureType::Celsius) Ok(TemperatureType::Celsius)
@ -564,98 +631,43 @@ fn get_show_average_cpu(args: &BottomArgs, config: &ConfigV1) -> bool {
true true
} }
fn try_parse_ms(s: &str) -> error::Result<u64> { #[inline]
if let Ok(val) = humantime::parse_duration(s) {
Ok(val
.as_millis()
.try_into()
.map_err(|err| BottomError::GenericError(format!("could not parse duration, {err}")))?)
} else if let Ok(val) = s.parse::<u64>() {
Ok(val)
} else {
Err(BottomError::ConfigError(
"could not parse as a valid 64-bit unsigned integer or a human time".to_string(),
))
}
}
fn get_default_time_value( fn get_default_time_value(
args: &BottomArgs, config: &ConfigV1, retention_ms: u64, args: &BottomArgs, config: &ConfigV1, retention_ms: u64,
) -> error::Result<u64> { ) -> OptionResult<u64> {
let default_time = if let Some(default_time_value) = &args.general.default_time_value { parse_ms_option!(
try_parse_ms(default_time_value).map_err(|_| { &args.general.default_time_value,
BottomError::ArgumentError("set your default time to be valid".to_string()) config
})? .flags
} else if let Some(flags) = &config.flags { .as_ref()
if let Some(default_time_value) = &flags.default_time_value { .and_then(|flags| flags.default_time_value.as_ref()),
match default_time_value { DEFAULT_TIME_MILLISECONDS,
StringOrNum::String(s) => try_parse_ms(s).map_err(|_| { "default_time_value",
BottomError::ConfigError("set your default time to be valid".to_string()) Some(30000),
})?, Some(retention_ms),
StringOrNum::Num(n) => *n, )
}
} else {
DEFAULT_TIME_MILLISECONDS
}
} else {
DEFAULT_TIME_MILLISECONDS
};
if default_time < 30000 {
return Err(BottomError::ConfigError(
"set your default time to be at least 30s.".to_string(),
));
} else if default_time > retention_ms {
return Err(BottomError::ConfigError(format!(
"set your default time to be at most {}.",
humantime::Duration::from(Duration::from_millis(retention_ms))
)));
}
Ok(default_time)
} }
fn get_time_interval( #[inline]
args: &BottomArgs, config: &ConfigV1, retention_ms: u64, fn get_time_interval(args: &BottomArgs, config: &ConfigV1, retention_ms: u64) -> OptionResult<u64> {
) -> error::Result<u64> { parse_ms_option!(
let time_interval = if let Some(time_interval) = &args.general.time_delta { &args.general.time_delta,
try_parse_ms(time_interval).map_err(|_| { config
BottomError::ArgumentError("set your time delta to be valid".to_string()) .flags
})? .as_ref()
} else if let Some(flags) = &config.flags { .and_then(|flags| flags.time_delta.as_ref()),
if let Some(time_interval) = &flags.time_delta { TIME_CHANGE_MILLISECONDS,
match time_interval { "time_delta",
StringOrNum::String(s) => try_parse_ms(s).map_err(|_| { Some(1000),
BottomError::ArgumentError("set your time delta to be valid".to_string()) Some(retention_ms),
})?, )
StringOrNum::Num(n) => *n,
}
} else {
TIME_CHANGE_MILLISECONDS
}
} else {
TIME_CHANGE_MILLISECONDS
};
if time_interval < 1000 {
return Err(BottomError::ConfigError(
"set your time delta to be at least 1s.".to_string(),
));
} else if time_interval > retention_ms {
return Err(BottomError::ConfigError(format!(
"set your time delta to be at most {}.",
humantime::Duration::from(Duration::from_millis(retention_ms))
)));
}
Ok(time_interval)
} }
fn get_default_widget_and_count( fn get_default_widget_and_count(
args: &BottomArgs, config: &ConfigV1, args: &BottomArgs, config: &ConfigV1,
) -> error::Result<(Option<BottomWidgetType>, u64)> { ) -> OptionResult<(Option<BottomWidgetType>, u64)> {
let widget_type = if let Some(widget_type) = &args.general.default_widget_type { let widget_type = if let Some(widget_type) = &args.general.default_widget_type {
let parsed_widget = widget_type.parse::<BottomWidgetType>()?; let parsed_widget = parse_arg_value!(widget_type.parse(), "default_widget_type")?;
if let BottomWidgetType::Empty = parsed_widget { if let BottomWidgetType::Empty = parsed_widget {
None None
} else { } else {
@ -663,7 +675,7 @@ fn get_default_widget_and_count(
} }
} else if let Some(flags) = &config.flags { } else if let Some(flags) = &config.flags {
if let Some(widget_type) = &flags.default_widget_type { if let Some(widget_type) = &flags.default_widget_type {
let parsed_widget = widget_type.parse::<BottomWidgetType>()?; let parsed_widget = parse_config_value!(widget_type.parse(), "default_widget_type")?;
if let BottomWidgetType::Empty = parsed_widget { if let BottomWidgetType::Empty = parsed_widget {
None None
} else { } else {
@ -688,13 +700,13 @@ fn get_default_widget_and_count(
match (widget_type, widget_count) { match (widget_type, widget_count) {
(Some(widget_type), Some(widget_count)) => { (Some(widget_type), Some(widget_count)) => {
let widget_count = widget_count.try_into().map_err(|_| BottomError::ConfigError( let widget_count = widget_count.try_into().map_err(|_| OptionError::other(
"set your widget count to be at most 18446744073709551615.".to_string() "set your widget count to be at most 18446744073709551615.".to_string()
))?; ))?;
Ok((Some(widget_type), widget_count)) Ok((Some(widget_type), widget_count))
} }
(Some(widget_type), None) => Ok((Some(widget_type), 1)), (Some(widget_type), None) => Ok((Some(widget_type), 1)),
(None, Some(_widget_count)) => Err(BottomError::ConfigError( (None, Some(_widget_count)) => Err(OptionError::other(
"cannot set 'default_widget_count' by itself, it must be used with 'default_widget_type'.".to_string(), "cannot set 'default_widget_count' by itself, it must be used with 'default_widget_type'.".to_string(),
)), )),
(None, None) => Ok((None, 1)) (None, None) => Ok((None, 1))
@ -759,7 +771,7 @@ fn get_enable_cache_memory(args: &BottomArgs, config: &ConfigV1) -> bool {
false false
} }
fn get_ignore_list(ignore_list: &Option<IgnoreList>) -> error::Result<Option<Filter>> { fn get_ignore_list(ignore_list: &Option<IgnoreList>) -> OptionResult<Option<Filter>> {
if let Some(ignore_list) = ignore_list { if let Some(ignore_list) = ignore_list {
let list: Result<Vec<_>, _> = ignore_list let list: Result<Vec<_>, _> = ignore_list
.list .list
@ -787,7 +799,7 @@ fn get_ignore_list(ignore_list: &Option<IgnoreList>) -> error::Result<Option<Fil
}) })
.collect(); .collect();
let list = list.map_err(|err| BottomError::ConfigError(err.to_string()))?; let list = list.map_err(|err| OptionError::config(err.to_string()))?;
Ok(Some(Filter { Ok(Some(Filter {
list, list,
@ -798,7 +810,7 @@ fn get_ignore_list(ignore_list: &Option<IgnoreList>) -> error::Result<Option<Fil
} }
} }
pub fn get_color_scheme(args: &BottomArgs, config: &ConfigV1) -> error::Result<ColourScheme> { pub fn get_color_scheme(args: &BottomArgs, config: &ConfigV1) -> OptionResult<ColourScheme> {
if let Some(color) = &args.style.color { if let Some(color) = &args.style.color {
// Highest priority is always command line flags... // Highest priority is always command line flags...
return ColourScheme::from_str(color); return ColourScheme::from_str(color);
@ -851,72 +863,62 @@ fn get_network_scale_type(args: &BottomArgs, config: &ConfigV1) -> AxisScaling {
AxisScaling::Linear AxisScaling::Linear
} }
fn get_retention(args: &BottomArgs, config: &ConfigV1) -> error::Result<u64> { fn get_retention(args: &BottomArgs, config: &ConfigV1) -> OptionResult<u64> {
const DEFAULT_RETENTION_MS: u64 = 600 * 1000; // Keep 10 minutes of data. const DEFAULT_RETENTION_MS: u64 = 600 * 1000; // Keep 10 minutes of data.
if let Some(retention) = &args.general.retention { parse_ms_option!(
try_parse_ms(retention) &args.general.retention,
.map_err(|_| BottomError::ArgumentError("`retention` is an invalid value".to_string())) config
} else if let Some(flags) = &config.flags { .flags
if let Some(retention) = &flags.retention { .as_ref()
Ok(match retention { .and_then(|flags| flags.retention.as_ref()),
StringOrNum::String(s) => try_parse_ms(s).map_err(|_| { DEFAULT_RETENTION_MS,
BottomError::ConfigError("`retention` is an invalid value".to_string()) "retention",
})?, None,
StringOrNum::Num(n) => *n, None,
}) )
} else {
Ok(DEFAULT_RETENTION_MS)
}
} else {
Ok(DEFAULT_RETENTION_MS)
}
} }
fn get_network_legend_position( fn get_network_legend_position(
args: &BottomArgs, config: &ConfigV1, args: &BottomArgs, config: &ConfigV1,
) -> error::Result<Option<LegendPosition>> { ) -> OptionResult<Option<LegendPosition>> {
if let Some(s) = &args.network.network_legend { let result = if let Some(s) = &args.network.network_legend {
match s.to_ascii_lowercase().trim() { match s.to_ascii_lowercase().trim() {
"none" => Ok(None), "none" => None,
position => Ok(Some(position.parse::<LegendPosition>().map_err(|_| { position => Some(parse_config_value!(position.parse(), "network_legend")?),
BottomError::ArgumentError("`network_legend` is an invalid value".to_string())
})?)),
} }
} else if let Some(flags) = &config.flags { } else if let Some(flags) = &config.flags {
if let Some(legend) = &flags.network_legend { if let Some(legend) = &flags.network_legend {
Ok(Some(legend.parse::<LegendPosition>().map_err(|_| { Some(parse_arg_value!(legend.parse(), "network_legend")?)
BottomError::ConfigError("`network_legend` is an invalid value".to_string())
})?))
} else { } else {
Ok(Some(LegendPosition::default())) Some(LegendPosition::default())
} }
} else { } else {
Ok(Some(LegendPosition::default())) Some(LegendPosition::default())
} };
Ok(result)
} }
fn get_memory_legend_position( fn get_memory_legend_position(
args: &BottomArgs, config: &ConfigV1, args: &BottomArgs, config: &ConfigV1,
) -> error::Result<Option<LegendPosition>> { ) -> OptionResult<Option<LegendPosition>> {
if let Some(s) = &args.memory.memory_legend { let result = if let Some(s) = &args.memory.memory_legend {
match s.to_ascii_lowercase().trim() { match s.to_ascii_lowercase().trim() {
"none" => Ok(None), "none" => None,
position => Ok(Some(position.parse::<LegendPosition>().map_err(|_| { position => Some(parse_config_value!(position.parse(), "memory_legend")?),
BottomError::ArgumentError("`memory_legend` is an invalid value".to_string())
})?)),
} }
} else if let Some(flags) = &config.flags { } else if let Some(flags) = &config.flags {
if let Some(legend) = &flags.memory_legend { if let Some(legend) = &flags.memory_legend {
Ok(Some(legend.parse::<LegendPosition>().map_err(|_| { Some(parse_arg_value!(legend.parse(), "memory_legend")?)
BottomError::ConfigError("`memory_legend` is an invalid value".to_string())
})?))
} else { } else {
Ok(Some(LegendPosition::default())) Some(LegendPosition::default())
} }
} else { } else {
Ok(Some(LegendPosition::default())) Some(LegendPosition::default())
} };
Ok(result)
} }
#[cfg(test)] #[cfg(test)]

View File

@ -542,7 +542,7 @@ pub struct StyleArgs {
], ],
hide_possible_values = true, hide_possible_values = true,
help = indoc! { help = indoc! {
"Use a color scheme, use `--help` for info on the colors. [possible values: default, default-light, gruvbox, gruvbox-light, nord, nord-light]", "Use a color scheme, use '--help' for info on the colors. [possible values: default, default-light, gruvbox, gruvbox-light, nord, nord-light]",
}, },
long_help = indoc! { long_help = indoc! {
"Use a pre-defined color scheme. Currently supported values are: "Use a pre-defined color scheme. Currently supported values are:
@ -562,7 +562,7 @@ pub struct StyleArgs {
#[derive(Args, Clone, Debug)] #[derive(Args, Clone, Debug)]
#[command(next_help_heading = "Other Options", rename_all = "snake_case")] #[command(next_help_heading = "Other Options", rename_all = "snake_case")]
pub struct OtherArgs { pub struct OtherArgs {
#[arg(short = 'h', long, action = ArgAction::Help, help = "Prints help info (for more details use `--help`.")] #[arg(short = 'h', long, action = ArgAction::Help, help = "Prints help info (for more details use '--help'.")]
help: (), help: (),
#[arg(short = 'V', long, action = ArgAction::Version, help = "Prints version information.")] #[arg(short = 'V', long, action = ArgAction::Version, help = "Prints version information.")]

View File

@ -1,6 +1,6 @@
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use crate::{app::layout_manager::*, error::Result}; use crate::{app::layout_manager::*, options::OptionResult};
/// Represents a row. This has a length of some sort (optional) and a vector /// Represents a row. This has a length of some sort (optional) and a vector
/// of children. /// of children.
@ -55,7 +55,7 @@ impl Row {
&self, iter_id: &mut u64, total_height_ratio: &mut u32, default_widget_id: &mut u64, &self, iter_id: &mut u64, total_height_ratio: &mut u32, default_widget_id: &mut u64,
default_widget_type: &Option<BottomWidgetType>, default_widget_count: &mut u64, default_widget_type: &Option<BottomWidgetType>, default_widget_count: &mut u64,
cpu_left_legend: bool, cpu_left_legend: bool,
) -> Result<BottomRow> { ) -> OptionResult<BottomRow> {
// TODO: In the future we want to also add percentages. // TODO: In the future we want to also add percentages.
// But for MVP, we aren't going to bother. // But for MVP, we aren't going to bother.
let row_ratio = self.ratio.unwrap_or(1); let row_ratio = self.ratio.unwrap_or(1);
@ -243,7 +243,6 @@ mod test {
use crate::{ use crate::{
constants::{DEFAULT_LAYOUT, DEFAULT_WIDGET_ID}, constants::{DEFAULT_LAYOUT, DEFAULT_WIDGET_ID},
options::ConfigV1, options::ConfigV1,
utils::error,
}; };
const PROC_LAYOUT: &str = r#" const PROC_LAYOUT: &str = r#"
@ -284,7 +283,7 @@ mod test {
left_legend, left_legend,
) )
}) })
.collect::<error::Result<Vec<_>>>() .collect::<OptionResult<Vec<_>>>()
.unwrap(), .unwrap(),
total_row_height_ratio: total_height_ratio, total_row_height_ratio: total_height_ratio,
}; };
@ -497,7 +496,7 @@ mod test {
cpu_left_legend, cpu_left_legend,
) )
}) })
.collect::<error::Result<Vec<_>>>() .collect::<OptionResult<Vec<_>>>()
.unwrap(), .unwrap(),
total_row_height_ratio: total_height_ratio, total_row_height_ratio: total_height_ratio,
}; };
@ -530,7 +529,7 @@ mod test {
cpu_left_legend, cpu_left_legend,
) )
}) })
.collect::<error::Result<Vec<_>>>() .collect::<OptionResult<Vec<_>>>()
.unwrap(), .unwrap(),
total_row_height_ratio: total_height_ratio, total_row_height_ratio: total_height_ratio,
}; };

75
src/options/error.rs Normal file
View File

@ -0,0 +1,75 @@
use std::borrow::Cow;
/// An error around some option-setting, and the reason.
///
/// These are meant to potentially be user-facing (e.g. explain
/// why it's broken and what to fix), and as so treat it as such!
///
/// For stylistic and consistency reasons, use _single quotes_ (e.g. `'bad'`)
/// for highlighting error values. You can use (".*`.+`.*") as a regex to check
/// for this.
#[derive(Debug, PartialEq)]
pub enum OptionError {
Config(Cow<'static, str>),
Argument(Cow<'static, str>),
Other(Cow<'static, str>),
}
impl OptionError {
/// Create a new [`OptionError::Config`].
pub(crate) fn config<R: Into<Cow<'static, str>>>(reason: R) -> Self {
OptionError::Config(reason.into())
}
/// Create a new [`OptionError::Config`] for an invalid value.
pub(crate) fn invalid_config_value(value: &str) -> Self {
OptionError::Config(Cow::Owned(format!(
"'{value}' was set with an invalid value, please update it in your config file."
)))
}
/// Create a new [`OptionError::Argument`].
pub(crate) fn arg<R: Into<Cow<'static, str>>>(reason: R) -> Self {
OptionError::Argument(reason.into())
}
/// Create a new [`OptionError::Argument`] for an invalid value.
pub(crate) fn invalid_arg_value(value: &str) -> Self {
OptionError::Argument(Cow::Owned(format!(
"'--{value}' was set with an invalid value, please update your arguments."
)))
}
/// Create a new [`OptionError::Other`].
pub(crate) fn other<R: Into<Cow<'static, str>>>(reason: R) -> Self {
OptionError::Other(reason.into())
}
}
pub(crate) type OptionResult<T> = Result<T, OptionError>;
impl std::fmt::Display for OptionError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
OptionError::Config(reason) => write!(f, "Configuration file error: {reason}"),
OptionError::Argument(reason) => write!(f, "Argument error: {reason}"),
OptionError::Other(reason) => {
write!(f, "Error with the config file or the arguments: {reason}")
}
}
}
}
impl std::error::Error for OptionError {}
impl From<toml_edit::de::Error> for OptionError {
fn from(err: toml_edit::de::Error) -> Self {
OptionError::Config(err.to_string().into())
}
}
impl From<std::io::Error> for OptionError {
fn from(err: std::io::Error) -> Self {
OptionError::Other(err.to_string().into())
}
}

View File

@ -14,12 +14,6 @@ pub enum BottomError {
/// An error to represent generic errors. /// An error to represent generic errors.
#[error("Error, {0}")] #[error("Error, {0}")]
GenericError(String), GenericError(String),
/// An error to represent invalid command-line arguments.
#[error("Invalid argument, {0}")]
ArgumentError(String),
/// An error to represent errors with the config.
#[error("Configuration file error, {0}")]
ConfigError(String),
} }
impl From<std::io::Error> for BottomError { impl From<std::io::Error> for BottomError {
@ -28,20 +22,8 @@ impl From<std::io::Error> for BottomError {
} }
} }
impl From<std::num::ParseIntError> for BottomError {
fn from(err: std::num::ParseIntError) -> Self {
BottomError::ConfigError(err.to_string())
}
}
impl From<String> for BottomError { impl From<String> for BottomError {
fn from(err: String) -> Self { fn from(err: String) -> Self {
BottomError::GenericError(err) BottomError::GenericError(err)
} }
} }
impl From<toml_edit::de::Error> for BottomError {
fn from(err: toml_edit::de::Error) -> Self {
BottomError::ConfigError(err.to_string())
}
}

View File

@ -13,9 +13,7 @@ fn test_small_rate() {
.arg("249") .arg("249")
.assert() .assert()
.failure() .failure()
.stderr(predicate::str::contains( .stderr(predicate::str::contains("'--rate' must be greater"));
"set your update rate to be at least 250 ms.",
));
} }
#[test] #[test]
@ -26,7 +24,7 @@ fn test_large_default_time() {
.assert() .assert()
.failure() .failure()
.stderr(predicate::str::contains( .stderr(predicate::str::contains(
"set your default time to be valid", "'--default_time_value' was set with an invalid value",
)); ));
} }
@ -38,7 +36,7 @@ fn test_small_default_time() {
.assert() .assert()
.failure() .failure()
.stderr(predicate::str::contains( .stderr(predicate::str::contains(
"set your default time to be at least", "'--default_time_value' must be greater",
)); ));
} }
@ -49,7 +47,9 @@ fn test_large_delta_time() {
.arg("18446744073709551616") .arg("18446744073709551616")
.assert() .assert()
.failure() .failure()
.stderr(predicate::str::contains("set your time delta to be valid")); .stderr(predicate::str::contains(
"'--time_delta' was set with an invalid value",
));
} }
#[test] #[test]
@ -59,9 +59,7 @@ fn test_small_delta_time() {
.arg("900") .arg("900")
.assert() .assert()
.failure() .failure()
.stderr(predicate::str::contains( .stderr(predicate::str::contains("'--time_delta' must be greater"));
"set your time delta to be at least",
));
} }
#[test] #[test]
@ -71,7 +69,9 @@ fn test_large_rate() {
.arg("18446744073709551616") .arg("18446744073709551616")
.assert() .assert()
.failure() .failure()
.stderr(predicate::str::contains("set your update rate")); .stderr(predicate::str::contains(
"'--rate' was set with an invalid value",
));
} }
#[test] #[test]
@ -92,7 +92,9 @@ fn test_invalid_rate() {
.arg("100-1000") .arg("100-1000")
.assert() .assert()
.failure() .failure()
.stderr(predicate::str::contains("set your update rate")); .stderr(predicate::str::contains(
"'--rate' was set with an invalid value",
));
} }
#[test] #[test]