diff --git a/src/flags/blocks.rs b/src/flags/blocks.rs index 4473a09..44b2e1b 100644 --- a/src/flags/blocks.rs +++ b/src/flags/blocks.rs @@ -71,24 +71,24 @@ impl Blocks { /// This errors if any of the parameter arguments causes [Block]'s implementation of /// [TryFrom::try_from] to return an [Err]. fn from_arg_matches(matches: &ArgMatches) -> Option> { - if matches.occurrences_of("blocks") > 0 { - if let Some(values) = matches.values_of("blocks") { - let mut blocks: Vec = vec![]; - for value in values { - match Block::try_from(value) { - Ok(block) => blocks.push(block), - Err(message) => { - return Some(Err(Error::with_description( - &message, - ErrorKind::ValueValidation, - ))) - } + if matches.occurrences_of("blocks") == 0 { + return None; + } + + if let Some(values) = matches.values_of("blocks") { + let mut blocks: Vec = Vec::with_capacity(values.len()); + for value in values { + match Block::try_from(value) { + Ok(block) => blocks.push(block), + Err(message) => { + return Some(Err(Error::with_description( + &message, + ErrorKind::ValueValidation, + ))) } } - Some(Ok(Self(blocks))) - } else { - None } + Some(Ok(Self(blocks))) } else { None } diff --git a/src/flags/color.rs b/src/flags/color.rs index e39a184..a111cc8 100644 --- a/src/flags/color.rs +++ b/src/flags/color.rs @@ -4,7 +4,6 @@ use super::Configurable; use crate::config_file::Config; -use crate::print_error; use clap::ArgMatches; use serde::de::{self, Deserializer, Visitor}; @@ -45,18 +44,15 @@ pub enum ThemeOption { impl ThemeOption { fn from_config(config: &Config) -> ThemeOption { - if let Some(classic) = config.classic { - if classic { - return ThemeOption::NoColor; - } + if config.classic == Some(true) { + ThemeOption::NoColor + } else { + config + .color + .as_ref() + .and_then(|c| c.theme.clone()) + .unwrap_or_default() } - if let Some(c) = &config.color { - if let Some(t) = &c.theme { - return t.clone(); - } - } - - ThemeOption::default() } } @@ -105,18 +101,13 @@ pub enum ColorOption { } impl ColorOption { - /// Get a Color value from a [String]. - fn from_str(value: &str) -> Option { + fn from_arg_str(value: &str) -> Self { match value { - "always" => Some(Self::Always), - "auto" => Some(Self::Auto), - "never" => Some(Self::Never), + "always" => Self::Always, + "auto" => Self::Auto, + "never" => Self::Never, _ => { - print_error!( - "Config color.when could only be one of auto, always and never, got {}.", - &value - ); - None + unreachable!("Invalid value should be handled by `clap`"); } } } @@ -132,11 +123,7 @@ impl Configurable for ColorOption { if matches.is_present("classic") { Some(Self::Never) } else if matches.occurrences_of("color") > 0 { - if let Some(color) = matches.values_of("color")?.last() { - Self::from_str(color) - } else { - panic!("Bad color args. This should not be reachable!"); - } + matches.values_of("color")?.last().map(Self::from_arg_str) } else { None } @@ -148,14 +135,10 @@ impl Configurable for ColorOption { /// Otherwise if the `Config::color::when` has value and is one of "always", "auto" or "never" /// this returns its corresponding variant in a [Some]. Otherwise this returns [None]. fn from_config(config: &Config) -> Option { - if let Some(true) = config.classic { - return Some(Self::Never); - } - - if let Some(c) = &config.color { - c.when + if config.classic == Some(true) { + Some(Self::Never) } else { - None + config.color.as_ref().and_then(|c| c.when) } } diff --git a/src/flags/date.rs b/src/flags/date.rs index 8c674ad..29d70a6 100644 --- a/src/flags/date.rs +++ b/src/flags/date.rs @@ -30,7 +30,8 @@ impl DateFlag { } /// Get a value from a str. - fn from_str(value: &str) -> Option { + fn from_str>(value: S) -> Option { + let value = value.as_ref(); match value { "date" => Some(Self::Date), "relative" => Some(Self::Relative), @@ -53,14 +54,7 @@ impl Configurable for DateFlag { if matches.is_present("classic") { Some(Self::Date) } else if matches.occurrences_of("date") > 0 { - match matches.values_of("date")?.last() { - Some("date") => Some(Self::Date), - Some("relative") => Some(Self::Relative), - Some(format) if format.starts_with('+') => { - Some(Self::Formatted(format[1..].to_owned())) - } - _ => panic!("This should not be reachable!"), - } + matches.values_of("date")?.last().and_then(Self::from_str) } else { None } @@ -73,14 +67,10 @@ impl Configurable for DateFlag { /// this returns its corresponding variant in a [Some]. /// Otherwise this returns [None]. fn from_config(config: &Config) -> Option { - if let Some(true) = &config.classic { - return Some(Self::Date); - } - - if let Some(date) = &config.date { - Self::from_str(date) + if config.classic == Some(true) { + Some(Self::Date) } else { - None + config.date.as_ref().and_then(Self::from_str) } } diff --git a/src/flags/dereference.rs b/src/flags/dereference.rs index 96b0726..7e6de97 100644 --- a/src/flags/dereference.rs +++ b/src/flags/dereference.rs @@ -29,7 +29,7 @@ impl Configurable for Dereference { /// If the `Config::dereference` has value, this returns its value /// as the value of the `Dereference`, in a [Some], Otherwise this returns [None]. fn from_config(config: &Config) -> Option { - config.dereference.as_ref().map(|deref| Self(*deref)) + config.dereference.map(Self) } } diff --git a/src/flags/hyperlink.rs b/src/flags/hyperlink.rs index d7b1fbc..5afecfd 100644 --- a/src/flags/hyperlink.rs +++ b/src/flags/hyperlink.rs @@ -17,6 +17,17 @@ pub enum HyperlinkOption { Never, } +impl HyperlinkOption { + fn from_arg_str(value: &str) -> Self { + match value { + "always" => Self::Always, + "auto" => Self::Auto, + "never" => Self::Never, + _ => unreachable!("Invalid value should be handled by `clap`"), + } + } +} + impl Configurable for HyperlinkOption { /// Get a potential `HyperlinkOption` variant from [ArgMatches]. /// @@ -27,12 +38,10 @@ impl Configurable for HyperlinkOption { if matches.is_present("classic") { Some(Self::Never) } else if matches.occurrences_of("hyperlink") > 0 { - match matches.values_of("hyperlink")?.last() { - Some("always") => Some(Self::Always), - Some("auto") => Some(Self::Auto), - Some("never") => Some(Self::Never), - _ => panic!("This should not be reachable!"), - } + matches + .values_of("hyperlink")? + .last() + .map(Self::from_arg_str) } else { None } @@ -45,11 +54,11 @@ impl Configurable for HyperlinkOption { /// this returns its corresponding variant in a [Some]. /// Otherwise this returns [None]. fn from_config(config: &Config) -> Option { - if let Some(true) = &config.classic { - return Some(Self::Never); + if config.classic == Some(true) { + Some(Self::Never) + } else { + config.hyperlink } - - config.hyperlink } } diff --git a/src/flags/icons.rs b/src/flags/icons.rs index c8d41bf..6e84e08 100644 --- a/src/flags/icons.rs +++ b/src/flags/icons.rs @@ -45,6 +45,19 @@ pub enum IconOption { Never, } +impl IconOption { + fn from_arg_str(value: &str) -> Self { + match value { + "always" => Self::Always, + "auto" => Self::Auto, + "never" => Self::Never, + _ => { + unreachable!("Invalid value should be handled by `clap`"); + } + } + } +} + impl Configurable for IconOption { /// Get a potential `IconOption` variant from [ArgMatches]. /// @@ -55,12 +68,7 @@ impl Configurable for IconOption { if matches.is_present("classic") { Some(Self::Never) } else if matches.occurrences_of("icon") > 0 { - match matches.values_of("icon")?.last() { - Some("always") => Some(Self::Always), - Some("auto") => Some(Self::Auto), - Some("never") => Some(Self::Never), - _ => panic!("This should not be reachable!"), - } + matches.values_of("icon")?.last().map(Self::from_arg_str) } else { None } @@ -73,14 +81,10 @@ impl Configurable for IconOption { /// this returns its corresponding variant in a [Some]. /// Otherwise this returns [None]. fn from_config(config: &Config) -> Option { - if let Some(true) = &config.classic { - return Some(Self::Never); - } - - if let Some(icon) = &config.icons { - icon.when + if config.classic == Some(true) { + Some(Self::Never) } else { - None + config.icons.as_ref().and_then(|icon| icon.when) } } } @@ -100,6 +104,18 @@ pub enum IconTheme { Fancy, } +impl IconTheme { + fn from_arg_str(value: &str) -> Self { + match value { + "fancy" => Self::Fancy, + "unicode" => Self::Unicode, + _ => { + unreachable!("Invalid value should be handled by `clap`"); + } + } + } +} + impl Configurable for IconTheme { /// Get a potential `IconTheme` variant from [ArgMatches]. /// @@ -107,11 +123,10 @@ impl Configurable for IconTheme { /// [Some]. Otherwise this returns [None]. fn from_arg_matches(matches: &ArgMatches) -> Option { if matches.occurrences_of("icon-theme") > 0 { - match matches.values_of("icon-theme")?.last() { - Some("fancy") => Some(Self::Fancy), - Some("unicode") => Some(Self::Unicode), - _ => panic!("This should not be reachable!"), - } + matches + .values_of("icon-theme")? + .last() + .map(Self::from_arg_str) } else { None } @@ -123,12 +138,7 @@ impl Configurable for IconTheme { /// this returns its corresponding variant in a [Some]. /// Otherwise this returns [None]. fn from_config(config: &Config) -> Option { - if let Some(icon) = &config.icons { - if let Some(theme) = icon.theme { - return Some(theme); - } - } - None + config.icons.as_ref().and_then(|icon| icon.theme) } } diff --git a/src/flags/permission.rs b/src/flags/permission.rs index 551e37e..e7ae0a0 100644 --- a/src/flags/permission.rs +++ b/src/flags/permission.rs @@ -19,15 +19,12 @@ pub enum PermissionFlag { } impl PermissionFlag { - fn from_str(value: &str) -> Option { + fn from_arg_str(value: &str) -> Self { match value { - "rwx" => Some(Self::Rwx), - "octal" => Some(Self::Octal), + "rwx" => Self::Rwx, + "octal" => Self::Octal, _ => { - panic!( - "Permissions can only be one of rwx or octal, but got {}.", - value - ); + unreachable!("Invalid value should be handled by `serde` or `clap`"); } } } @@ -42,13 +39,15 @@ impl Configurable for PermissionFlag { /// Sets permissions to rwx if classic flag is enabled. fn from_arg_matches(matches: &ArgMatches) -> Option { if matches.is_present("classic") { - return Some(Self::Rwx); + Some(Self::Rwx) } else if matches.occurrences_of("permission") > 0 { - if let Some(permissions) = matches.values_of("permission")?.last() { - return Self::from_str(permissions); - } + matches + .values_of("permission")? + .last() + .map(Self::from_arg_str) + } else { + None } - None } /// Get a potential `PermissionFlag` variant from a [Config]. @@ -58,7 +57,7 @@ impl Configurable for PermissionFlag { /// Otherwise this returns [None]. /// Sets permissions to rwx if classic flag is enabled. fn from_config(config: &Config) -> Option { - if let Some(true) = config.classic { + if config.classic == Some(true) { Some(Self::Rwx) } else { config.permission diff --git a/src/flags/size.rs b/src/flags/size.rs index c10820a..52a126e 100644 --- a/src/flags/size.rs +++ b/src/flags/size.rs @@ -21,16 +21,13 @@ pub enum SizeFlag { } impl SizeFlag { - fn from_str(value: &str) -> Option { + fn from_arg_str(value: &str) -> Self { match value { - "default" => Some(Self::Default), - "short" => Some(Self::Short), - "bytes" => Some(Self::Bytes), + "default" => Self::Default, + "short" => Self::Short, + "bytes" => Self::Bytes, _ => { - panic!( - "Size can only be one of default, short or bytes, but got {}.", - value - ); + unreachable!("Invalid value should be handled by `clap`"); } } } @@ -44,13 +41,12 @@ impl Configurable for SizeFlag { /// [None]. fn from_arg_matches(matches: &ArgMatches) -> Option { if matches.is_present("classic") { - return Some(Self::Bytes); + Some(Self::Bytes) } else if matches.occurrences_of("size") > 0 { - if let Some(size) = matches.values_of("size")?.last() { - return Self::from_str(size); - } + matches.values_of("size")?.last().map(Self::from_arg_str) + } else { + None } - None } /// Get a potential `SizeFlag` variant from a [Config]. @@ -59,7 +55,7 @@ impl Configurable for SizeFlag { /// this returns the corresponding `SizeFlag` variant in a [Some]. /// Otherwise this returns [None]. fn from_config(config: &Config) -> Option { - if let Some(true) = config.classic { + if config.classic == Some(true) { Some(Self::Bytes) } else { config.size diff --git a/src/flags/sorting.rs b/src/flags/sorting.rs index e16c411..14416b1 100644 --- a/src/flags/sorting.rs +++ b/src/flags/sorting.rs @@ -51,10 +51,8 @@ impl Configurable for SortColumn { /// If either the "timesort" or "sizesort" arguments are passed, this returns the corresponding /// `SortColumn` variant in a [Some]. Otherwise this returns [None]. fn from_arg_matches(matches: &ArgMatches) -> Option { - let sort = match matches.values_of("sort") { - Some(s) => s.last(), - None => None, - }; + let sort = matches.values_of("sort").and_then(|s| s.last()); + if matches.is_present("timesort") || sort == Some("time") { Some(Self::Time) } else if matches.is_present("sizesort") || sort == Some("size") { @@ -76,11 +74,7 @@ impl Configurable for SortColumn { /// this returns the corresponding variant in a [Some]. /// Otherwise this returns [None]. fn from_config(config: &Config) -> Option { - if let Some(sort) = &config.sorting { - sort.column - } else { - None - } + config.sorting.as_ref().and_then(|s| s.column) } } @@ -118,19 +112,11 @@ impl Configurable for SortOrder { /// Otherwise [None] is returned. /// A `true` maps to [SortOrder::Reverse] while `false` maps to [SortOrder::Default]. fn from_config(config: &Config) -> Option { - if let Some(sort) = &config.sorting { - if let Some(reverse) = sort.reverse { - if reverse { - Some(Self::Reverse) - } else { - Some(Self::Default) - } - } else { - None - } - } else { - None - } + config.sorting.as_ref().and_then(|s| match s.reverse { + Some(true) => Some(Self::Reverse), + Some(false) => Some(Self::Default), + None => None, + }) } } @@ -151,15 +137,14 @@ pub enum DirGrouping { } impl DirGrouping { - fn from_str(value: &str) -> Option { + fn from_arg_str(value: &str) -> Self { match value { - "first" => Some(Self::First), - "last" => Some(Self::Last), - "none" => Some(Self::None), - _ => panic!( - "Group Dir can only be one of first, last or none, but got {}.", - value - ), + "first" => Self::First, + "last" => Self::Last, + "none" => Self::None, + _ => { + unreachable!("Invalid value should be handled by `clap`"); + } } } } @@ -179,10 +164,12 @@ impl Configurable for DirGrouping { } if matches.occurrences_of("group-dirs") > 0 { - if let Some(group_dirs) = matches.values_of("group-dirs")?.last() { - return Self::from_str(group_dirs); - } + return matches + .values_of("group-dirs")? + .last() + .map(Self::from_arg_str); } + None } @@ -194,13 +181,11 @@ impl Configurable for DirGrouping { /// is one of "first", "last" or "none", this returns its corresponding variant in a [Some]. /// Otherwise this returns [None]. fn from_config(config: &Config) -> Option { - if let Some(true) = config.classic { - return Some(Self::None); + if config.classic == Some(true) { + Some(Self::None) + } else { + config.sorting.as_ref().and_then(|s| s.dir_grouping) } - if let Some(sort) = &config.sorting { - return sort.dir_grouping; - } - None } } @@ -488,11 +473,9 @@ mod test_dir_grouping { use crate::flags::Configurable; #[test] - #[should_panic( - expected = "Group Dir can only be one of first, last or none, but got bad value." - )] + #[should_panic] fn test_from_str_bad_value() { - assert_eq!(None, DirGrouping::from_str("bad value")); + DirGrouping::from_arg_str("bad value"); } #[test]