From 57cf5003bdd22b71f0b50cab9b05c1bb9db0ab05 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 20:08:10 +0100 Subject: [PATCH 01/21] Move common functionality to utility module --- lib/domain/libimaghabit/src/habit.rs | 18 +++++------------- lib/domain/libimaghabit/src/instance.rs | 20 ++++---------------- lib/domain/libimaghabit/src/util.rs | 15 +++++++++++++-- 3 files changed, 22 insertions(+), 31 deletions(-) diff --git a/lib/domain/libimaghabit/src/habit.rs b/lib/domain/libimaghabit/src/habit.rs index 99e1bacc..ab22ed09 100644 --- a/lib/domain/libimaghabit/src/habit.rs +++ b/lib/domain/libimaghabit/src/habit.rs @@ -30,6 +30,7 @@ use error::*; use iter::HabitInstanceStoreIdIterator; use util::date_to_string; use util::IsHabitCheck; +use util::get_string_header_from_entry; use libimagentrylink::internal::InternalLinker; use libimagstore::store::Store; @@ -202,19 +203,19 @@ impl HabitTemplate for Entry { } fn habit_name(&self) -> Result { - get_string_header_from_habit(self, "habit.template.name") + get_string_header_from_entry(self, "habit.template.name") } fn habit_basedate(&self) -> Result { - get_string_header_from_habit(self, "habit.template.basedate") + get_string_header_from_entry(self, "habit.template.basedate") } fn habit_recur_spec(&self) -> Result { - get_string_header_from_habit(self, "habit.template.recurspec") + get_string_header_from_entry(self, "habit.template.recurspec") } fn habit_comment(&self) -> Result { - get_string_header_from_habit(self, "habit.template.comment") + get_string_header_from_entry(self, "habit.template.comment") } fn habit_until_date(&self) -> Result> { @@ -239,15 +240,6 @@ fn instance_id_for_name_and_datestr(habit_name: &String, habit_date: &String) -> .map_err(HE::from) } -#[inline] -fn get_string_header_from_habit(e: &Entry, path: &'static str) -> Result { - match e.get_header().read(path)? { - Some(&Value::String(ref s)) => Ok(s.clone()), - Some(_) => Err(HEK::HeaderTypeError(path, "String").into()), - None => Err(HEK::HeaderFieldMissing(path).into()), - } -} - pub mod builder { use toml::Value; use toml_query::insert::TomlValueInsertExt; diff --git a/lib/domain/libimaghabit/src/instance.rs b/lib/domain/libimaghabit/src/instance.rs index 84bd9b5c..773ccaba 100644 --- a/lib/domain/libimaghabit/src/instance.rs +++ b/lib/domain/libimaghabit/src/instance.rs @@ -22,7 +22,6 @@ use toml::Value; use toml_query::read::TomlValueReadExt; use toml_query::set::TomlValueSetExt; -use error::HabitErrorKind as HEK; use error::*; use util::*; @@ -61,11 +60,8 @@ impl HabitInstance for Entry { } fn get_date(&self) -> Result { - match self.get_header().read("habit.instance.date")? { - Some(&Value::String(ref s)) => date_from_string(s), - Some(_) => Err(HEK::HeaderTypeError("habit.instance.date", "String").into()), - None => Err(HEK::HeaderFieldMissing("habit.instance.date").into()), - } + use util::date_from_string; + get_string_header_from_entry(self, "habit.instance.date").and_then(date_from_string) } fn set_date(&mut self, n: &NaiveDate) -> Result<()> { @@ -77,11 +73,7 @@ impl HabitInstance for Entry { } fn get_comment(&self) -> Result { - match self.get_header().read("habit.instance.comment")? { - Some(&Value::String(ref s)) => Ok(s.clone()), - Some(_) => Err(HEK::HeaderTypeError("habit.instance.comment", "String").into()), - None => Err(HEK::HeaderFieldMissing("habit.instance.comment").into()), - } + get_string_header_from_entry(self, "habit.instance.comment") } fn set_comment(&mut self, c: String) -> Result<()> { @@ -93,11 +85,7 @@ impl HabitInstance for Entry { } fn get_template_name(&self) -> Result { - match self.get_header().read("habit.instance.name")? { - Some(&Value::String(ref s)) => Ok(s.clone()), - Some(_) => Err(HEK::HeaderTypeError("habit.instance.name", "String").into()), - None => Err(HEK::HeaderFieldMissing("habit.instance.name").into()), - } + get_string_header_from_entry(self, "habit.instance.name") } } diff --git a/lib/domain/libimaghabit/src/util.rs b/lib/domain/libimaghabit/src/util.rs index 4acd6282..3598bedf 100644 --- a/lib/domain/libimaghabit/src/util.rs +++ b/lib/domain/libimaghabit/src/util.rs @@ -34,8 +34,8 @@ pub fn date_to_string(ndt: &NaiveDate) -> String { ndt.format(NAIVE_DATE_STRING_FORMAT).to_string() } -pub fn date_from_string(s: &str) -> Result { - NaiveDate::parse_from_str(s, NAIVE_DATE_STRING_FORMAT).map_err(From::from) +pub fn date_from_string(s: String) -> Result { + NaiveDate::parse_from_str(&s, NAIVE_DATE_STRING_FORMAT).map_err(From::from) } /// Helper trait to check whether a object which can be a habit instance and a habit template is @@ -90,3 +90,14 @@ impl IsHabitCheck for Entry { } } +#[inline] +pub fn get_string_header_from_entry(e: &Entry, path: &'static str) -> Result { + use error::HabitErrorKind as HEK; + use toml_query::read::TomlValueReadExt; + + e.get_header() + .read(path)? + .ok_or(HEK::HeaderFieldMissing(path).into()) + .and_then(|o| o.as_str().map(String::from).ok_or(HEK::HeaderTypeError(path, "String").into())) +} + From 7d1d41884c2856348f21148461568b96e06492aa Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 20:25:44 +0100 Subject: [PATCH 02/21] Replace matching with function chaining --- lib/core/libimagrt/src/configuration.rs | 11 +-- lib/core/libimagstore/src/store.rs | 106 ++++++++++-------------- 2 files changed, 49 insertions(+), 68 deletions(-) diff --git a/lib/core/libimagrt/src/configuration.rs b/lib/core/libimagrt/src/configuration.rs index 88ed6512..deafe8cf 100644 --- a/lib/core/libimagrt/src/configuration.rs +++ b/lib/core/libimagrt/src/configuration.rs @@ -114,13 +114,10 @@ pub fn override_config(val: &mut Value, v: Vec) -> Result<()> { let iter = v.into_iter() .map(|s| { debug!("Trying to process '{}'", s); s }) - .filter_map(|s| match s.into_kv() { - Some(kv) => Some(kv.into()), - None => { - warn!("Could split at '=' - will be ignore override"); - None - } - }) + .filter_map(|s| s.into_kv().map(Into::into).or_else(|| { + warn!("Could split at '=' - will be ignore override"); + None + })) .map(|(k, v)| val .read(&k[..]) .chain_err(|| REK::ConfigTOMLParserError) diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index 8064fdf7..400e8bef 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -325,10 +325,11 @@ impl Store { debug!("Creating id: '{}'", id); { - let mut hsmap = match self.entries.write() { - Err(_) => return Err(SE::from_kind(SEK::LockPoisoned)).chain_err(|| SEK::CreateCallError), - Ok(s) => s, - }; + let mut hsmap = self + .entries + .write() + .map_err(|_| SE::from_kind(SEK::LockPoisoned)) + .chain_err(|| SEK::CreateCallError)?; if hsmap.contains_key(&id) { debug!("Cannot create, internal cache already contains: '{}'", id); @@ -480,10 +481,7 @@ impl Store { /// - Errors StoreEntry::write_entry() might return /// fn _update<'a>(&'a self, entry: &mut FileLockEntry<'a>, modify_presence: bool) -> Result<()> { - let mut hsmap = match self.entries.write() { - Err(_) => return Err(SE::from_kind(SEK::LockPoisoned)), - Ok(e) => e, - }; + let mut hsmap = self.entries.write().map_err(|_| SE::from_kind(SEK::LockPoisoned))?; let se = hsmap.get_mut(&entry.location).ok_or_else(|| { SE::from_kind(SEK::IdNotFound(entry.location.clone())) @@ -519,13 +517,9 @@ impl Store { pub fn retrieve_copy(&self, id: S) -> Result { let id = id.into_storeid()?.with_base(self.path().clone()); debug!("Retrieving copy of '{}'", id); - let entries = match self.entries.write() { - Err(_) => { - return Err(SE::from_kind(SEK::LockPoisoned)) - .chain_err(|| SEK::RetrieveCopyCallError); - }, - Ok(e) => e, - }; + let entries = self.entries.write() + .map_err(|_| SE::from_kind(SEK::LockPoisoned)) + .chain_err(|| SEK::RetrieveCopyCallError)?; // if the entry is currently modified by the user, we cannot drop it if entries.get(&id).map(|e| e.is_borrowed()).unwrap_or(false) { @@ -552,11 +546,11 @@ impl Store { debug!("Deleting id: '{}'", id); { - let mut entries = match self.entries.write() { - Err(_) => return Err(SE::from_kind(SEK::LockPoisoned)) - .chain_err(|| SEK::DeleteCallError), - Ok(e) => e, - }; + let mut entries = self + .entries + .write() + .map_err(|_| SE::from_kind(SEK::LockPoisoned)) + .chain_err(|| SEK::DeleteCallError)?; // if the entry is currently modified by the user, we cannot drop it match entries.get(&id) { @@ -588,11 +582,11 @@ impl Store { // remove the entry first, then the file entries.remove(&id); let pb = id.clone().with_base(self.path().clone()).into_pathbuf()?; - if let Err(e) = self.backend.remove_file(&pb) { - return Err(e) - .chain_err(|| SEK::FileError) - .chain_err(|| SEK::DeleteCallError); - } + let _ = self + .backend + .remove_file(&pb) + .chain_err(|| SEK::FileError) + .chain_err(|| SEK::DeleteCallError)?; } debug!("Deleted"); @@ -631,14 +625,13 @@ impl Store { let old_id_as_path = old_id.clone().with_base(self.path().clone()).into_pathbuf()?; let new_id_as_path = new_id.clone().with_base(self.path().clone()).into_pathbuf()?; - self.backend.copy(&old_id_as_path, &new_id_as_path) - .and_then(|_| { - if remove_old { - debug!("Removing old '{:?}'", old_id_as_path); - self.backend.remove_file(&old_id_as_path) - } else { - Ok(()) - } + self.backend + .copy(&old_id_as_path, &new_id_as_path) + .and_then(|_| if remove_old { + debug!("Removing old '{:?}'", old_id_as_path); + self.backend.remove_file(&old_id_as_path) + } else { + Ok(()) }) .chain_err(|| SEK::FileError) .chain_err(|| SEK::MoveCallError) @@ -684,10 +677,7 @@ impl Store { debug!("Moving '{}' to '{}'", old_id, new_id); { - let mut hsmap = match self.entries.write() { - Err(_) => return Err(SE::from_kind(SEK::LockPoisoned)), - Ok(m) => m, - }; + let mut hsmap = self.entries.write().map_err(|_| SE::from_kind(SEK::LockPoisoned))?; if hsmap.contains_key(&new_id) { return Err(SE::from_kind(SEK::EntryAlreadyExists(new_id.clone()))); @@ -711,22 +701,21 @@ impl Store { } debug!("New entry does not yet exist on filesystem. Good."); - match self.backend.rename(&old_id_pb, &new_id_pb) { - Err(e) => return Err(e).chain_err(|| SEK::EntryRenameError(old_id_pb, new_id_pb)), - Ok(_) => { - debug!("Rename worked on filesystem"); + let _ = self + .backend + .rename(&old_id_pb, &new_id_pb) + .chain_err(|| SEK::EntryRenameError(old_id_pb, new_id_pb))?; - // assert enforced through check hsmap.contains_key(&new_id) above. - // Should therefor never fail - assert!(hsmap - .remove(&old_id) - .and_then(|mut entry| { - entry.id = new_id.clone(); - hsmap.insert(new_id.clone(), entry) - }).is_none()) - } - } + debug!("Rename worked on filesystem"); + // assert enforced through check hsmap.contains_key(&new_id) above. + // Should therefor never fail + assert!(hsmap + .remove(&old_id) + .and_then(|mut entry| { + entry.id = new_id.clone(); + hsmap.insert(new_id.clone(), entry) + }).is_none()) } debug!("Moved"); @@ -1035,16 +1024,11 @@ mod glob_store_iter { while let Some(o) = self.paths.next() { debug!("GlobStoreIdIterator::next() => {:?}", o); match o.chain_err(|| SEK::StoreIdHandlingError) { - Err(e) => return Some(Err(e)), - Ok(path) => { - if path.exists() && path.is_file() { - return match StoreId::from_full_path(&self.store_path, path) { - Ok(id) => Some(Ok(id)), - Err(e) => Some(Err(e)), - } - /* } else { */ - /* continue */ - } + Err(e) => return Some(Err(e)), + Ok(path) => if path.exists() && path.is_file() { + return Some(StoreId::from_full_path(&self.store_path, path)); + /* } else { */ + /* continue */ } } } From a9135a80fb4f19a68288aaa31aaae98067a20f0b Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 20:26:01 +0100 Subject: [PATCH 03/21] Replace matching with command chaining --- lib/etc/libimaginteraction/src/ui.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/etc/libimaginteraction/src/ui.rs b/lib/etc/libimaginteraction/src/ui.rs index 9c16a2a5..501f792e 100644 --- a/lib/etc/libimaginteraction/src/ui.rs +++ b/lib/etc/libimaginteraction/src/ui.rs @@ -70,14 +70,13 @@ pub fn get_id(matches: &ArgMatches) -> Result> { pub fn get_or_select_id(matches: &ArgMatches, store_path: &PathBuf) -> Result> { use interactor::{pick_file, default_menu_cmd}; - match get_id(matches).chain_err(|| IEK::IdSelectingError) { - Ok(v) => Ok(v), - Err(_) => { + get_id(matches) + .chain_err(|| IEK::IdSelectingError) + .or_else(|_| { let path = store_path.clone(); - let p = pick_file(default_menu_cmd, path).chain_err(|| IEK::IdSelectingError)?; - let id = StoreId::new_baseless(p).chain_err(|| IEK::StoreIdParsingError)?; + let p = pick_file(default_menu_cmd, path).chain_err(|| IEK::IdSelectingError)?; + let id = StoreId::new_baseless(p).chain_err(|| IEK::StoreIdParsingError)?; Ok(vec![id]) - }, - } + }) } From c1ff2b14f0fa8da667fe777f4e71131bf3bc94aa Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 20:26:23 +0100 Subject: [PATCH 04/21] Replace matching with function chaining --- lib/domain/libimagbookmark/src/collection.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/domain/libimagbookmark/src/collection.rs b/lib/domain/libimagbookmark/src/collection.rs index 858460d0..8842b022 100644 --- a/lib/domain/libimagbookmark/src/collection.rs +++ b/lib/domain/libimagbookmark/src/collection.rs @@ -90,13 +90,8 @@ impl<'a> BookmarkCollection<'a> { .and_then(|id| store.get(id)) .chain_err(|| BEK::StoreReadError) .and_then(|fle| { - match fle { - None => Err(BE::from_kind(BEK::CollectionNotFound)), - Some(e) => Ok(BookmarkCollection { - fle: e, - store: store, - }), - } + fle.ok_or(BE::from_kind(BEK::CollectionNotFound)) + .map(|e| BookmarkCollection { fle: e, store: store }) }) } From dbd9a2faafe1e62c6af827c4557272a2e0864f00 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 20:26:42 +0100 Subject: [PATCH 05/21] Replace matching with function chaining --- lib/domain/libimaghabit/src/habit.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/domain/libimaghabit/src/habit.rs b/lib/domain/libimaghabit/src/habit.rs index ab22ed09..29794101 100644 --- a/lib/domain/libimaghabit/src/habit.rs +++ b/lib/domain/libimaghabit/src/habit.rs @@ -219,11 +219,10 @@ impl HabitTemplate for Entry { } fn habit_until_date(&self) -> Result> { - match self.get_header().read("habit.template.until")? { - Some(&Value::String(ref s)) => Ok(Some(s.clone())), - Some(_) => Err(HEK::HeaderTypeError("habit.template.until", "String").into()), - None => Ok(None), - } + self.get_header() + .read("habit.template.until")? + .map(|v| v.as_str().map(String::from)) + .ok_or(HEK::HeaderTypeError("habit.template.until", "String").into()) } fn instance_id_for(habit_name: &String, habit_date: &NaiveDate) -> Result { From fa8ac037017bffbe4a72bea498276dd65623ec1c Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 20:27:29 +0100 Subject: [PATCH 06/21] Replace matching with function chaining --- lib/core/libimagrt/src/configuration.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/core/libimagrt/src/configuration.rs b/lib/core/libimagrt/src/configuration.rs index deafe8cf..680a4d2c 100644 --- a/lib/core/libimagrt/src/configuration.rs +++ b/lib/core/libimagrt/src/configuration.rs @@ -80,21 +80,18 @@ pub fn fetch_config(searchpath: &PathBuf) -> Result { s }; - match ::toml::de::from_str::<::toml::Value>(&content[..]) { - Ok(res) => Some(res), - Err(e) => { + ::toml::de::from_str::<::toml::Value>(&content[..]) + .map(Some) + .unwrap_or_else(|e| { let line_col = e .line_col() - .map(|(line, col)| { - format!("Line {}, Column {}", line, col) - }) + .map(|(line, col)| format!("Line {}, Column {}", line, col)) .unwrap_or_else(|| String::from("Line unknown, Column unknown")); let _ = write!(stderr(), "Config file parser error at {}", line_col); trace_error(&e); None - } - } + }) }) .nth(0) .ok_or(RE::from_kind(REK::ConfigNoConfigFileFound)) From 4184a1e5d0ec396fae739e8819040ced4074a78b Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 20:44:11 +0100 Subject: [PATCH 07/21] Replace matching with function chaining --- lib/core/libimagrt/src/configuration.rs | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/lib/core/libimagrt/src/configuration.rs b/lib/core/libimagrt/src/configuration.rs index 680a4d2c..b4c931cd 100644 --- a/lib/core/libimagrt/src/configuration.rs +++ b/lib/core/libimagrt/src/configuration.rs @@ -115,20 +115,16 @@ pub fn override_config(val: &mut Value, v: Vec) -> Result<()> { warn!("Could split at '=' - will be ignore override"); None })) - .map(|(k, v)| val - .read(&k[..]) - .chain_err(|| REK::ConfigTOMLParserError) - .map(|toml| match toml { - Some(value) => match into_value(value, v) { - Some(v) => { - info!("Successfully overridden: {} = {}", k, v); - Ok(()) - }, - None => Err(RE::from_kind(REK::ConfigOverrideTypeNotMatching)), - }, - None => Err(RE::from_kind(REK::ConfigOverrideKeyNotAvailable)), - }) - ); + .map(|(k, v)| { + let value = val + .read(&k) + .chain_err(|| REK::ConfigTOMLParserError)? + .ok_or(RE::from_kind(REK::ConfigOverrideKeyNotAvailable))?; + + into_value(value, v) + .map(|v| info!("Successfully overridden: {} = {}", k, v)) + .ok_or_else(|| RE::from_kind(REK::ConfigOverrideTypeNotMatching)) + }); for elem in iter { let _ = try!(elem.chain_err(|| REK::ConfigOverrideError)); From d5ce99b17054f45792320a69276d15df1c05c282 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:08:14 +0100 Subject: [PATCH 08/21] Refactor toml destructure matching. Use accessor functionality and function chaining now. --- lib/core/libimagrt/src/logger.rs | 117 +++++++++++++++++-------------- 1 file changed, 66 insertions(+), 51 deletions(-) diff --git a/lib/core/libimagrt/src/logger.rs b/lib/core/libimagrt/src/logger.rs index 5a4d80e6..27d1a67b 100644 --- a/lib/core/libimagrt/src/logger.rs +++ b/lib/core/libimagrt/src/logger.rs @@ -238,16 +238,16 @@ fn aggregate_global_loglevel(matches: &ArgMatches, config: Option<&Value>) } if let Some(cfg) = config { - let cfg_loglevel = match cfg.read("imag.logging.level") { - Ok(Some(&Value::String(ref s))) => match_log_level_str(s), - Ok(Some(_)) => { + let cfg_loglevel = cfg + .read("imag.logging.level")? + .ok_or(RE::from_kind(EK::GlobalLogLevelConfigMissing))? + .as_str() + .ok_or_else(|| { let path = "imag.logging.level".to_owned(); let ty = "String"; - return Err(RE::from_kind(EK::ConfigTypeError(path, ty))) - }, - Ok(None) => return Err(RE::from_kind(EK::GlobalLogLevelConfigMissing)), - Err(e) => return Err(e).map_err(From::from), - }?; + RE::from_kind(EK::ConfigTypeError(path, ty)) + }) + .and_then(match_log_level_str)?; if let Some(cli_loglevel) = get_arg_loglevel(matches)? { if cli_loglevel > cfg_loglevel { @@ -285,14 +285,13 @@ fn translate_destinations(raw: &Vec) -> Result> { raw.iter() .fold(Ok(vec![]), |acc, val| { acc.and_then(|mut v| { - let dest = match *val { - Value::String(ref s) => translate_destination(s)?, - _ => { + let dest = val.as_str() + .ok_or_else(|| { let path = "imag.logging.modules..destinations".to_owned(); let ty = "Array"; - return Err(RE::from_kind(EK::ConfigTypeError(path, ty))) - }, - }; + RE::from_kind(EK::ConfigTypeError(path, ty)) + }) + .and_then(translate_destination)?; v.push(dest); Ok(v) }) @@ -304,16 +303,16 @@ fn aggregate_global_destinations(matches: &ArgMatches, config: Option<&Value>) { match config { - Some(cfg) => match cfg.read("imag.logging.destinations") { - Ok(Some(&Value::Array(ref a))) => translate_destinations(a), - Ok(Some(_)) => { + Some(cfg) => cfg + .read("imag.logging.destinations")? + .ok_or_else(|| RE::from_kind(EK::GlobalDestinationConfigMissing))? + .as_array() + .ok_or_else(|| { let path = "imag.logging.destinations".to_owned(); let ty = "Array"; - Err(RE::from_kind(EK::ConfigTypeError(path, ty))) - }, - Ok(None) => Err(RE::from_kind(EK::GlobalDestinationConfigMissing)), - Err(e) => Err(e).map_err(From::from), - }, + RE::from_kind(EK::ConfigTypeError(path, ty)) + }) + .and_then(translate_destinations), None => { if let Some(values) = matches.value_of(Runtime::arg_logdest_name()) { // parse logdest specification from commandline @@ -334,12 +333,12 @@ fn aggregate_global_destinations(matches: &ArgMatches, config: Option<&Value>) macro_rules! aggregate_global_format { ($read_str:expr, $error_kind_if_missing:expr, $config:expr) => { - match try!($config.ok_or(RE::from_kind($error_kind_if_missing))).read($read_str) { - Ok(Some(&Value::String(ref s))) => Ok(s.clone()), - Ok(Some(_)) => Err(RE::from_kind(EK::ConfigTypeError($read_str.to_owned(), "String"))), - Ok(None) => Err(RE::from_kind($error_kind_if_missing)), - Err(e) => Err(e).map_err(From::from), - } + try!($config.ok_or(RE::from_kind($error_kind_if_missing))) + .read($read_str)? + .ok_or_else(|| RE::from_kind($error_kind_if_missing))? + .as_str() + .map(String::from) + .ok_or_else(|| RE::from_kind(EK::ConfigTypeError($read_str.to_owned(), "String"))) }; } @@ -386,6 +385,18 @@ fn aggregate_global_format_error(config: Option<&Value>) fn aggregate_module_settings(_matches: &ArgMatches, config: Option<&Value>) -> Result> { + // Helper macro to return the error from Some(Err(_)) and map everything else to an + // Option<_> + macro_rules! inner_try { + ($v:expr) => { + match $v { + Some(Ok(v)) => Some(v), + Some(Err(e)) => return Err(e), + None => None, + } + } + }; + match config { Some(cfg) => match cfg.read("imag.logging.modules") { Ok(Some(&Value::Table(ref t))) => { @@ -393,35 +404,39 @@ fn aggregate_module_settings(_matches: &ArgMatches, config: Option<&Value>) let mut settings = BTreeMap::new(); for (module_name, v) in t { - let destinations = match v.read("destinations")? { - Some(&Value::Array(ref a)) => Some(translate_destinations(a)?), - None => None, - Some(_) => { - let path = "imag.logging.modules..destinations".to_owned(); - let ty = "Array"; - return Err(RE::from_kind(EK::ConfigTypeError(path, ty))) - }, + let destinations = inner_try! { + v.read("destinations")? + .map(|val| { + val.as_array() + .ok_or_else(|| { + let path = "imag.logging.modules..destinations".to_owned(); + let ty = "Array"; + RE::from_kind(EK::ConfigTypeError(path, ty)) + }) + .and_then(translate_destinations) + }) }; - let level = match v.read("level")? { - Some(&Value::String(ref s)) => Some(match_log_level_str(s)?), - None => None, - Some(_) => { - let path = "imag.logging.modules..level".to_owned(); - let ty = "String"; - return Err(RE::from_kind(EK::ConfigTypeError(path, ty))) - }, + let level = inner_try! { + v.read("level")? + .map(|val| { + val.as_str() + .ok_or_else(|| { + let path = "imag.logging.modules..level".to_owned(); + let ty = "String"; + RE::from_kind(EK::ConfigTypeError(path, ty)) + }) + .and_then(match_log_level_str) + }) }; - let enabled = match v.read("enabled")? { - Some(&Value::Boolean(b)) => b, - None => false, - Some(_) => { + let enabled = v.read("enabled")? + .map(|v| v.as_bool().unwrap_or(false)) + .ok_or_else(|| { let path = "imag.logging.modules..enabled".to_owned(); let ty = "Boolean"; - return Err(RE::from_kind(EK::ConfigTypeError(path, ty))) - }, - }; + RE::from_kind(EK::ConfigTypeError(path, ty)) + })?; let module_settings = ModuleSettings { enabled: enabled, From 4bb0d0f073a7fb30d6a18cc6a3ebb7efb8b00aa5 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:09:06 +0100 Subject: [PATCH 09/21] Refactor: Use function chaining instead of matching --- lib/domain/libimaglog/src/log.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/domain/libimaglog/src/log.rs b/lib/domain/libimaglog/src/log.rs index a172eae4..651d61a3 100644 --- a/lib/domain/libimaglog/src/log.rs +++ b/lib/domain/libimaglog/src/log.rs @@ -36,11 +36,10 @@ pub trait Log : DiaryEntry { impl Log for Entry { fn is_log(&self) -> Result { let location = "log.is_log"; - match self.get_header().read(location)? { - Some(&Value::Boolean(b)) => Ok(b), - Some(_) => Err(LE::from_kind(LEK::HeaderTypeError("boolean", location))), - None => Ok(false) - } + self.get_header() + .read(location)? + .ok_or(LE::from_kind(LEK::HeaderTypeError("boolean", location))) + .map(|v| v.as_bool().unwrap_or(false)) } fn make_log_entry(&mut self) -> Result<()> { From e7d5e9ebc28a0b8232e7f84998c1c16b64bd50f8 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:09:30 +0100 Subject: [PATCH 10/21] Refactoring: Use function chaining rather than matching --- lib/domain/libimagnotes/src/note.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/domain/libimagnotes/src/note.rs b/lib/domain/libimagnotes/src/note.rs index f7b0ed77..882fc481 100644 --- a/lib/domain/libimagnotes/src/note.rs +++ b/lib/domain/libimagnotes/src/note.rs @@ -46,13 +46,12 @@ impl Note for Entry { } fn get_name(&self) -> Result { - match self.get_header().read("note.name") { - Ok(Some(&Value::String(ref s))) => Ok(s.clone()), - Ok(_) => { - Err(NE::from_kind(NEK::HeaderTypeError)).chain_err(|| NEK::StoreReadError) - }, - Err(e) => Err(e).chain_err(|| NEK::StoreReadError) - } + self.get_header() + .read("note.name") + .chain_err(|| NEK::StoreReadError)? + .and_then(Value::as_str) + .map(String::from) + .ok_or(NE::from_kind(NEK::HeaderTypeError)) } fn set_text(&mut self, n: String) { From a386d50df3415f0d07e1c7ba7f4d51fb8eb518ad Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:09:30 +0100 Subject: [PATCH 11/21] Refactoring: Use function chaining rather than matching --- lib/domain/libimagtimetrack/src/iter/get.rs | 9 ++++----- lib/domain/libimagtimetrack/src/timetracking.rs | 12 +++++++----- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/domain/libimagtimetrack/src/iter/get.rs b/lib/domain/libimagtimetrack/src/iter/get.rs index 526c1c97..739e07a2 100644 --- a/lib/domain/libimagtimetrack/src/iter/get.rs +++ b/lib/domain/libimagtimetrack/src/iter/get.rs @@ -45,11 +45,10 @@ impl<'a> Iterator for GetTimeTrackIter<'a> { fn next(&mut self) -> Option { self.inner.next().map(|sid| { - match self.store.get(sid).chain_err(|| TTEK::StoreReadError) { - Ok(None) => Err(TTE::from_kind(TTEK::StoreReadError)), - Ok(Some(s)) => Ok(s), - Err(e) => Err(e) - } + self.store + .get(sid) + .chain_err(|| TTEK::StoreReadError)? + .ok_or(TTE::from_kind(TTEK::StoreReadError)) }) } } diff --git a/lib/domain/libimagtimetrack/src/timetracking.rs b/lib/domain/libimagtimetrack/src/timetracking.rs index 7c3bf11d..bbf8b06f 100644 --- a/lib/domain/libimagtimetrack/src/timetracking.rs +++ b/lib/domain/libimagtimetrack/src/timetracking.rs @@ -65,11 +65,13 @@ impl TimeTracking for Entry { fn get_timetrack_tag(&self) -> Result { self.get_header() .read(DATE_TIME_TAG_HEADER_PATH) - .chain_err(|| TTEK::HeaderReadError) - .and_then(|value| match value { - Some(&Value::String(ref s)) => Ok(s.clone().into()), - Some(_) => Err(TTE::from_kind(TTEK::HeaderFieldTypeError)), - _ => Err(TTE::from_kind(TTEK::HeaderReadError)) + .chain_err(|| TTEK::HeaderReadError)? + .ok_or(TTE::from_kind(TTEK::HeaderReadError)) + .and_then(|v| { + v.as_str() + .map(String::from) + .map(Into::into) + .ok_or(TTE::from_kind(TTEK::HeaderFieldTypeError)) }) } From dd66936e48684b6cad599ee0041486b439a6f898 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:09:30 +0100 Subject: [PATCH 12/21] Refactoring: Use function chaining rather than matching --- lib/domain/libimagtodo/src/task.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/domain/libimagtodo/src/task.rs b/lib/domain/libimagtodo/src/task.rs index 039bdbfb..88c2e1f0 100644 --- a/lib/domain/libimagtodo/src/task.rs +++ b/lib/domain/libimagtodo/src/task.rs @@ -25,7 +25,6 @@ use error::Result; use libimagstore::store::Entry; use uuid::Uuid; -use toml::Value; use toml_query::read::TomlValueReadExt; pub trait Task { @@ -34,14 +33,13 @@ pub trait Task { impl Task for Entry { fn get_uuid(&self) -> Result { - match self.get_header().read("todo.uuid") { - Ok(Some(&Value::String(ref uuid))) => { - Uuid::parse_str(uuid).chain_err(|| TEK::UuidParserError) - }, - Ok(Some(_)) => Err(TE::from_kind(TEK::HeaderTypeError)), - Ok(None) => Err(TE::from_kind(TEK::HeaderFieldMissing)), - Err(e) => Err(e).chain_err(|| TEK::StoreError), - } + self.get_header() + .read("todo.uuid") + .chain_err(|| TEK::StoreError)? + .ok_or(TE::from_kind(TEK::HeaderFieldMissing))? + .as_str() + .ok_or(TE::from_kind(TEK::HeaderTypeError)) + .and_then(|u| Uuid::parse_str(u).chain_err(|| TEK::UuidParserError)) } } From 40490c10d64cebc3be43a7447cd33bd2517c23b1 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:09:30 +0100 Subject: [PATCH 13/21] Refactoring: Use function chaining rather than matching --- lib/etc/libimaginteraction/src/readline.rs | 56 ++++++++++------------ 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/lib/etc/libimaginteraction/src/readline.rs b/lib/etc/libimaginteraction/src/readline.rs index 86aa51a9..321580eb 100644 --- a/lib/etc/libimaginteraction/src/readline.rs +++ b/lib/etc/libimaginteraction/src/readline.rs @@ -42,40 +42,36 @@ impl Readline { let histignspace = c.lookup("ui.cli.readline_history_ignore_space").ok_or(IEK::ConfigError)?; let prompt = c.lookup("ui.cli.readline_prompt").ok_or(IEK::ConfigError)?; - let histfile = match histfile { - Value::String(s) => PathBuf::from(s), - _ => Err(IE::from_kind(IEK::ConfigTypeError)) - .chain_err(|| IEK::ConfigError) - .chain_err(|| IEK::ReadlineError) - }?; + let histfile = histfile + .as_str() + .map(PathBuf::from) + .ok_or(IE::from_kind(IEK::ConfigTypeError)) + .chain_err(|| IEK::ConfigError) + .chain_err(|| IEK::ReadlineError)?; - let histsize = match histsize { - Value::Integer(i) => i, - _ => Err(IE::from_kind(IEK::ConfigTypeError)) - .chain_err(|| IEK::ConfigError) - .chain_err(|| IEK::ReadlineError) - }?; + let histsize = histsize + .as_int() + .ok_or(IE::from_kind(IEK::ConfigTypeError)) + .chain_err(|| IEK::ConfigError) + .chain_err(|| IEK::ReadlineError)?; - let histigndups = match histigndups { - Value::Boolean(b) => b, - _ => Err(IE::from_kind(IEK::ConfigTypeError)) - .chain_err(|| IEK::ConfigError) - .chain_err(|| IEK::ReadlineError) - }?; + let histigndups = histigndups + .as_bool() + .ok_or(IE::from_kind(IEK::ConfigTypeError)) + .chain_err(|| IEK::ConfigError) + .chain_err(|| IEK::ReadlineError)?; - let histignspace = match histignspace { - Value::Boolean(b) => b, - _ => Err(IE::from_kind(IEK::ConfigTypeError)) - .chain_err(|| IEK::ConfigError) - .chain_err(|| IEK::ReadlineError) - }?; + let histignspace = histignspace + .as_bool() + .ok_or(IE::from_kind(IEK::ConfigTypeError)) + .chain_err(|| IEK::ConfigError) + .chain_err(|| IEK::ReadlineError)?; - let prompt = match prompt { - Value::String(s) => s, - _ => Err(IE::from_kind(IEK::ConfigTypeError)) - .chain_err(|| IEK::ConfigError) - .chain_err(|| IEK::ReadlineError) - }?; + let prompt = prompt + .as_str() + .ok_or(IE::from_kind(IEK::ConfigTypeError)) + .chain_err(|| IEK::ConfigError) + .chain_err(|| IEK::ReadlineError)?; let config = Config::builder(). .max_history_size(histsize) From 3294a77346857c2e011e7fd3429fa19c8945e7a4 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:09:30 +0100 Subject: [PATCH 14/21] Refactoring: Use function chaining rather than matching --- lib/entry/libimagentryannotation/src/annotateable.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/entry/libimagentryannotation/src/annotateable.rs b/lib/entry/libimagentryannotation/src/annotateable.rs index ffd40879..aa027720 100644 --- a/lib/entry/libimagentryannotation/src/annotateable.rs +++ b/lib/entry/libimagentryannotation/src/annotateable.rs @@ -97,13 +97,9 @@ impl Annotateable for Entry { fn is_annotation(&self) -> Result { self.get_header() - .read("annotation.is_annotation") - .map_err(From::from) - .and_then(|res| match res { - Some(&Value::Boolean(b)) => Ok(b), - None => Ok(false), - _ => Err(AE::from_kind(AEK::HeaderTypeError)), - }) + .read("annotation.is_annotation")? + .map(|val| val.as_bool().unwrap_or(false)) + .ok_or(AE::from_kind(AEK::HeaderTypeError)) } } From feaa32196b83d060ba19df498dd0fc13b5d96e9b Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:09:30 +0100 Subject: [PATCH 15/21] Refactoring: Use function chaining rather than matching Also introduce error links for this. --- .../libimagentrycategory/src/category.rs | 23 +++++++------- lib/entry/libimagentrycategory/src/error.rs | 8 +++++ .../libimagentrycategory/src/register.rs | 30 +++++++++---------- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/lib/entry/libimagentrycategory/src/category.rs b/lib/entry/libimagentrycategory/src/category.rs index f2a2b2cf..5cef0d6d 100644 --- a/lib/entry/libimagentrycategory/src/category.rs +++ b/lib/entry/libimagentrycategory/src/category.rs @@ -19,7 +19,6 @@ use toml_query::insert::TomlValueInsertExt; use toml_query::read::TomlValueReadExt; -use toml_query::error::ErrorKind as TQEK; use toml::Value; use libimagstore::store::Entry; @@ -81,17 +80,17 @@ impl EntryCategory for Entry { } fn get_category(&self) -> Result> { - match self.get_header().read(&String::from("category.value")) { - Err(res) => match res.kind() { - &TQEK::IdentifierNotFoundInDocument(_) => Ok(None), - _ => Err(res), - } - .chain_err(|| CEK::HeaderReadError), - - Ok(Some(&Value::String(ref s))) => Ok(Some(s.clone().into())), - Ok(None) => Err(CE::from_kind(CEK::StoreReadError)).chain_err(|| CEK::HeaderReadError), - Ok(_) => Err(CE::from_kind(CEK::TypeError)).chain_err(|| CEK::HeaderReadError), - } + self.get_header() + .read("category.value") + .chain_err(|| CEK::HeaderReadError) + .and_then(|opt| { + opt.map(|v| { + v.as_str() + .map(String::from) + .map(Category::from) + }) + .ok_or(CE::from_kind(CEK::TypeError)) + }) } fn has_category(&self) -> Result { diff --git a/lib/entry/libimagentrycategory/src/error.rs b/lib/entry/libimagentrycategory/src/error.rs index bd3da1a0..32642043 100644 --- a/lib/entry/libimagentrycategory/src/error.rs +++ b/lib/entry/libimagentrycategory/src/error.rs @@ -22,6 +22,14 @@ error_chain! { CategoryError, CategoryErrorKind, ResultExt, Result; } + links { + StoreError(::libimagstore::error::StoreError, ::libimagstore::error::StoreErrorKind); + } + + foreign_links { + TomlQueryError(::toml_query::error::Error); + } + errors { StoreReadError { description("Store Read error") diff --git a/lib/entry/libimagentrycategory/src/register.rs b/lib/entry/libimagentrycategory/src/register.rs index d5b40584..bc1aa929 100644 --- a/lib/entry/libimagentrycategory/src/register.rs +++ b/lib/entry/libimagentrycategory/src/register.rs @@ -225,14 +225,13 @@ fn represents_category(store: &Store, sid: StoreId, name: &str) -> Result .chain_err(|| CEK::StoreReadError) .and_then(|fle| { if let Some(fle) = fle { - match fle.get_header() + fle.get_header() .read(&String::from(CATEGORY_REGISTER_NAME_FIELD_PATH)) - .chain_err(|| CEK::HeaderReadError) - { - Ok(Some(&Value::String(ref s))) => Ok(s == name), - Ok(_) => Err(CE::from_kind(CEK::TypeError)), - Err(e) => Err(e).chain_err(|| CEK::HeaderReadError), - } + .chain_err(|| CEK::HeaderReadError)? + .ok_or(CE::from_kind(CEK::TypeError))? + .as_str() + .map(|s| s == name) + .ok_or(CE::from_kind(CEK::TypeError)) } else { Ok(false) } @@ -276,14 +275,15 @@ impl<'a> Iterator for CategoryNameIter<'a> { .next() .map(|sid| { self.0 - .get(sid) - .chain_err(|| CEK::StoreReadError) - .and_then(|fle| fle.ok_or(CE::from_kind(CEK::StoreReadError))) - .and_then(|fle| match fle.get_header().read(&query) { - Ok(Some(&Value::String(ref s))) => Ok(Category::from(s.clone())), - Ok(_) => Err(CE::from_kind(CEK::TypeError)), - Err(e) => Err(e).chain_err(|| CEK::HeaderReadError), - }) + .get(sid)? + .ok_or_else(|| CE::from_kind(CEK::StoreReadError))? + .get_header() + .read(&query) + .chain_err(|| CEK::HeaderReadError)? + .and_then(Value::as_str) + .map(String::from) + .map(Category::from) + .ok_or_else(|| CE::from_kind(CEK::TypeError)) }) } } From 5db3d0c278574d0230e80ea3e55e27ddc31667ff Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:12:02 +0100 Subject: [PATCH 16/21] Refactor: Use function chaining rather than matching And use a helper function for common functionality --- .../libimagentrydatetime/src/datetime.rs | 64 ++++++------------- 1 file changed, 21 insertions(+), 43 deletions(-) diff --git a/lib/entry/libimagentrydatetime/src/datetime.rs b/lib/entry/libimagentrydatetime/src/datetime.rs index efe90280..defa7202 100644 --- a/lib/entry/libimagentrydatetime/src/datetime.rs +++ b/lib/entry/libimagentrydatetime/src/datetime.rs @@ -63,12 +63,11 @@ impl EntryDate for Entry { .read(&DATE_HEADER_LOCATION) .chain_err(|| DEK::ReadDateError) .and_then(|v| { - match v { - Some(&Value::String(ref s)) => s.parse::() - .chain_err(|| DEK::DateTimeParsingError), - Some(_) => Err(DE::from_kind(DEK::DateHeaderFieldTypeError)), - _ => Err(DE::from_kind(DEK::ReadDateError)), - } + v.ok_or(DE::from_kind(DEK::ReadDateError))? + .as_str() + .ok_or(DE::from_kind(DEK::DateHeaderFieldTypeError))? + .parse::() + .chain_err(|| DEK::DateTimeParsingError) }) } @@ -94,11 +93,10 @@ impl EntryDate for Entry { self.get_header_mut() .insert(&DATE_HEADER_LOCATION, Value::String(date)) .map(|opt| opt.map(|stri| { - match stri { - Value::String(ref s) => s.parse::() - .chain_err(|| DEK::DateTimeParsingError), - _ => Err(DE::from_kind(DEK::DateHeaderFieldTypeError)), - } + stri.as_str() + .ok_or(DE::from_kind(DEK::DateHeaderFieldTypeError))? + .parse::() + .chain_err(|| DEK::DateTimeParsingError) })) .chain_err(|| DEK::SetDateError) } @@ -129,30 +127,15 @@ impl EntryDate for Entry { .get_header() .read(&DATE_RANGE_START_HEADER_LOCATION) .chain_err(|| DEK::ReadDateTimeRangeError) - .and_then(|v| { - match v { - Some(&Value::String(ref s)) => s.parse::() - .chain_err(|| DEK::DateTimeParsingError), - Some(_) => Err(DE::from_kind(DEK::DateHeaderFieldTypeError)), - _ => Err(DE::from_kind(DEK::ReadDateError)), - } - })?; + .and_then(|v| str_to_ndt(v.ok_or(DE::from_kind(DEK::ReadDateError))?))?; let end = self .get_header() .read(&DATE_RANGE_START_HEADER_LOCATION) .chain_err(|| DEK::ReadDateTimeRangeError) - .and_then(|v| { - match v { - Some(&Value::String(ref s)) => s.parse::() - .chain_err(|| DEK::DateTimeParsingError), - Some(_) => Err(DE::from_kind(DEK::DateHeaderFieldTypeError)), - _ => Err(DE::from_kind(DEK::ReadDateError)), - } - })?; + .and_then(|v| str_to_ndt(v.ok_or(DE::from_kind(DEK::ReadDateError))?))?; - DateTimeRange::new(start, end) - .chain_err(|| DEK::DateTimeRangeError) + DateTimeRange::new(start, end).chain_err(|| DEK::DateTimeRangeError) } /// Set the date range @@ -171,25 +154,13 @@ impl EntryDate for Entry { let opt_old_start = self .get_header_mut() .insert(&DATE_RANGE_START_HEADER_LOCATION, Value::String(start)) - .map(|opt| opt.map(|stri| { - match stri { - Value::String(ref s) => s.parse::() - .chain_err(|| DEK::DateTimeParsingError), - _ => Err(DE::from_kind(DEK::DateHeaderFieldTypeError)), - } - })) + .map(|opt| opt.as_ref().map(str_to_ndt)) .chain_err(|| DEK::SetDateTimeRangeError)?; let opt_old_end = self .get_header_mut() .insert(&DATE_RANGE_END_HEADER_LOCATION, Value::String(end)) - .map(|opt| opt.map(|stri| { - match stri { - Value::String(ref s) => s.parse::() - .chain_err(|| DEK::DateTimeParsingError), - _ => Err(DE::from_kind(DEK::DateHeaderFieldTypeError)), - } - })) + .map(|opt| opt.as_ref().map(str_to_ndt)) .chain_err(|| DEK::SetDateTimeRangeError)?; match (opt_old_start, opt_old_end) { @@ -210,6 +181,13 @@ impl EntryDate for Entry { } +fn str_to_ndt(v: &Value) -> Result { + v.as_str() + .ok_or(DE::from_kind(DEK::DateHeaderFieldTypeError))? + .parse::() + .chain_err(|| DEK::DateTimeParsingError) +} + #[cfg(test)] mod tests { use std::path::PathBuf; From c92e459e3aedac2492314eaa8e06fa149d04eb10 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:09:30 +0100 Subject: [PATCH 17/21] Refactoring: Use function chaining rather than matching --- .../src/builtin/header/field_predicate.rs | 7 +------ .../src/builtin/header/version/eq.rs | 13 +++---------- .../src/builtin/header/version/gt.rs | 13 +++---------- .../src/builtin/header/version/lt.rs | 13 +++---------- 4 files changed, 10 insertions(+), 36 deletions(-) diff --git a/lib/entry/libimagentryfilter/src/builtin/header/field_predicate.rs b/lib/entry/libimagentryfilter/src/builtin/header/field_predicate.rs index a167f762..8c98aad8 100644 --- a/lib/entry/libimagentryfilter/src/builtin/header/field_predicate.rs +++ b/lib/entry/libimagentryfilter/src/builtin/header/field_predicate.rs @@ -52,12 +52,7 @@ impl Filter for FieldPredicate

{ fn filter(&self, e: &Entry) -> bool { e.get_header() .read(&self.header_field_path[..]) - .map(|val| { - match val { - None => false, - Some(v) => (*self.predicate).evaluate(v), - } - }) + .map(|val| val.map(|v| (*self.predicate).evaluate(v)).unwrap_or(false)) .unwrap_or(false) } diff --git a/lib/entry/libimagentryfilter/src/builtin/header/version/eq.rs b/lib/entry/libimagentryfilter/src/builtin/header/version/eq.rs index 5ba9f306..138d1da0 100644 --- a/lib/entry/libimagentryfilter/src/builtin/header/version/eq.rs +++ b/lib/entry/libimagentryfilter/src/builtin/header/version/eq.rs @@ -18,7 +18,6 @@ // use semver::Version; -use toml::Value; use libimagstore::store::Entry; @@ -44,15 +43,9 @@ impl Filter for VersionEq { .read("imag.version") .map(|val| { val.map_or(false, |v| { - match *v { - Value::String(ref s) => { - match Version::parse(&s[..]) { - Ok(v) => v == self.version, - _ => false - } - }, - _ => false, - } + v.as_str() + .map(|s| Version::parse(s).map(|v| v == self.version).unwrap_or(false)) + .unwrap_or(false) }) }) .unwrap_or(false) diff --git a/lib/entry/libimagentryfilter/src/builtin/header/version/gt.rs b/lib/entry/libimagentryfilter/src/builtin/header/version/gt.rs index edd0580f..edc89145 100644 --- a/lib/entry/libimagentryfilter/src/builtin/header/version/gt.rs +++ b/lib/entry/libimagentryfilter/src/builtin/header/version/gt.rs @@ -18,7 +18,6 @@ // use semver::Version; -use toml::Value; use libimagstore::store::Entry; @@ -44,15 +43,9 @@ impl Filter for VersionGt { .read("imag.version") .map(|val| { val.map_or(false, |v| { - match *v { - Value::String(ref s) => { - match Version::parse(&s[..]) { - Ok(v) => v > self.version, - _ => false - } - }, - _ => false, - } + v.as_str() + .map(|s| Version::parse(s).map(|v| v > self.version).unwrap_or(false)) + .unwrap_or(false) }) }) .unwrap_or(false) diff --git a/lib/entry/libimagentryfilter/src/builtin/header/version/lt.rs b/lib/entry/libimagentryfilter/src/builtin/header/version/lt.rs index 629be9ae..846c7f62 100644 --- a/lib/entry/libimagentryfilter/src/builtin/header/version/lt.rs +++ b/lib/entry/libimagentryfilter/src/builtin/header/version/lt.rs @@ -18,7 +18,6 @@ // use semver::Version; -use toml::Value; use libimagstore::store::Entry; @@ -44,15 +43,9 @@ impl Filter for VersionLt { .read("imag.version") .map(|val| { val.map_or(false, |v| { - match *v { - Value::String(ref s) => { - match Version::parse(&s[..]) { - Ok(v) => v < self.version, - _ => false - } - }, - _ => false, - } + v.as_str() + .map(|s| Version::parse(s).map(|v| v < self.version).unwrap_or(false)) + .unwrap_or(false) }) }) .unwrap_or(false) From 1e2ac14d3b0176d538b03c9be17b536b9bcee4c1 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:09:30 +0100 Subject: [PATCH 18/21] Refactoring: Use function chaining rather than matching --- lib/entry/libimagentrygps/src/entry.rs | 7 ++-- lib/entry/libimagentrygps/src/types.rs | 44 +++++++++++--------------- 2 files changed, 21 insertions(+), 30 deletions(-) diff --git a/lib/entry/libimagentrygps/src/entry.rs b/lib/entry/libimagentrygps/src/entry.rs index 5a7c2df2..641fefde 100644 --- a/lib/entry/libimagentrygps/src/entry.rs +++ b/lib/entry/libimagentrygps/src/entry.rs @@ -64,10 +64,9 @@ impl GPSEntry for Entry { } fn get_coordinates(&self) -> Result> { - match self.get_header().read("gps.coordinates").chain_err(|| GPSEK::HeaderWriteError) { - Ok(Some(hdr)) => Coordinates::from_value(hdr).map(Some), - Ok(None) => Ok(None), - Err(e) => Err(e), + match self.get_header().read("gps.coordinates").chain_err(|| GPSEK::HeaderWriteError)? { + Some(hdr) => Coordinates::from_value(hdr).map(Some), + None => Ok(None), } } diff --git a/lib/entry/libimagentrygps/src/types.rs b/lib/entry/libimagentrygps/src/types.rs index 2b70de19..569013e9 100644 --- a/lib/entry/libimagentrygps/src/types.rs +++ b/lib/entry/libimagentrygps/src/types.rs @@ -78,31 +78,27 @@ impl Into for GPSValue { impl FromValue for GPSValue { fn from_value(v: &Value) -> Result { + let int_to_appropriate_width = |v: &Value| { + v.as_integer() + .ok_or(GPSE::from_kind(GPSEK::HeaderTypeError)).and_then(i64_to_i8) + }; + match *v { Value::Table(ref map) => { Ok(GPSValue::new( map.get("degree") .ok_or_else(|| GPSE::from_kind(GPSEK::DegreeMissing)) - .and_then(|v| match *v { - Value::Integer(i) => i64_to_i8(i), - _ => Err(GPSE::from_kind(GPSEK::HeaderTypeError)), - })?, + .and_then(&int_to_appropriate_width)?, map .get("minutes") .ok_or_else(|| GPSE::from_kind(GPSEK::MinutesMissing)) - .and_then(|v| match *v { - Value::Integer(i) => i64_to_i8(i), - _ => Err(GPSE::from_kind(GPSEK::HeaderTypeError)), - })?, + .and_then(&int_to_appropriate_width)?, map .get("seconds") .ok_or_else(|| GPSE::from_kind(GPSEK::SecondsMissing)) - .and_then(|v| match *v { - Value::Integer(i) => i64_to_i8(i), - _ => Err(GPSE::from_kind(GPSEK::HeaderTypeError)), - })? + .and_then(&int_to_appropriate_width)? )) } _ => Err(GPSE::from_kind(GPSEK::TypeError)) @@ -154,22 +150,18 @@ impl Into for Coordinates { impl FromValue for Coordinates { fn from_value(v: &Value) -> Result { - match *v { - Value::Table(ref map) => { - Ok(Coordinates::new( - match map.get("longitude") { - Some(v) => GPSValue::from_value(v), - None => Err(GPSE::from_kind(GPSEK::LongitudeMissing)), - }?, + v.as_table() + .ok_or(GPSE::from_kind(GPSEK::TypeError)) + .and_then(|t| { + let get = |m: &BTreeMap<_, _>, what: &'static str, ek| -> Result { + m.get(what).ok_or(GPSE::from_kind(ek)).and_then(GPSValue::from_value) + }; - match map.get("latitude") { - Some(v) => GPSValue::from_value(v), - None => Err(GPSE::from_kind(GPSEK::LongitudeMissing)), - }? + Ok(Coordinates::new( + get(t, "longitude", GPSEK::LongitudeMissing)?, + get(t, "latitude", GPSEK::LatitudeMissing)? )) - } - _ => Err(GPSE::from_kind(GPSEK::TypeError)) - } + }) } } From dac817f318f1d35723d1df104ff2d12d42751cf4 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:14:02 +0100 Subject: [PATCH 19/21] Refactor: Use function chaining and new error type link --- lib/entry/libimagentrylink/src/error.rs | 1 + lib/entry/libimagentrylink/src/internal.rs | 10 +++------- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/entry/libimagentrylink/src/error.rs b/lib/entry/libimagentrylink/src/error.rs index b54c9d61..a961cb6e 100644 --- a/lib/entry/libimagentrylink/src/error.rs +++ b/lib/entry/libimagentrylink/src/error.rs @@ -30,6 +30,7 @@ error_chain! { foreign_links { TomlQueryError(::toml_query::error::Error); + UrlError(::url::ParseError); } errors { diff --git a/lib/entry/libimagentrylink/src/internal.rs b/lib/entry/libimagentrylink/src/internal.rs index a03d7ac9..47291d2f 100644 --- a/lib/entry/libimagentrylink/src/internal.rs +++ b/lib/entry/libimagentrylink/src/internal.rs @@ -641,13 +641,9 @@ pub mod store_check { let mut map = HashMap::new(); for element in iter { debug!("Checking element = {:?}", element); - let entry = match element? { - Some(e) => e, - None => { - let e = String::from("TODO: Not yet handled"); - return Err(e).map_err(From::from); - }, - }; + let entry = element?.ok_or_else(|| { + LE::from(String::from("TODO: Not yet handled")) + })?; debug!("Checking entry = {:?}", entry.get_location()); From 66b061110356441f5b7c6a1576aa93658dde3359 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:09:30 +0100 Subject: [PATCH 20/21] Refactoring: Use function chaining rather than matching --- lib/entry/libimagentryref/src/flags.rs | 12 +++----- lib/entry/libimagentryref/src/lister.rs | 12 +++----- lib/entry/libimagentryref/src/reference.rs | 36 +++++++++------------- lib/entry/libimagentryref/src/util.rs | 23 +++++++------- 4 files changed, 35 insertions(+), 48 deletions(-) diff --git a/lib/entry/libimagentryref/src/flags.rs b/lib/entry/libimagentryref/src/flags.rs index 2d87c7c0..21d822dd 100644 --- a/lib/entry/libimagentryref/src/flags.rs +++ b/lib/entry/libimagentryref/src/flags.rs @@ -21,6 +21,7 @@ use std::collections::BTreeMap; use toml::Value; +use error::RefError as RE; use error::RefErrorKind as REK; use error::Result; @@ -39,13 +40,10 @@ impl RefFlags { fn get_field(v: &Value, key: &str) -> Result { use toml_query::read::TomlValueReadExt; - v.read(key) - .map_err(From::from) - .and_then(|toml| match toml { - Some(&Value::Boolean(b)) => Ok(b), - Some(_) => Err(REK::HeaderTypeError.into()), - None => Err(REK::HeaderFieldMissingError.into()), - }) + v.read(key)? + .ok_or(RE::from_kind(REK::HeaderFieldMissingError))? + .as_bool() + .ok_or(REK::HeaderTypeError.into()) } Ok(RefFlags { diff --git a/lib/entry/libimagentryref/src/lister.rs b/lib/entry/libimagentryref/src/lister.rs index e3d9bdfa..3e06f003 100644 --- a/lib/entry/libimagentryref/src/lister.rs +++ b/lib/entry/libimagentryref/src/lister.rs @@ -142,20 +142,16 @@ fn check_changed(r: &R) -> bool { } fn check_changed_content(r: &R) -> bool { - let eq = r.get_current_hash() + r.get_current_hash() .and_then(|hash| r.get_stored_hash().map(|stored| (hash, stored))) - .map(|(hash, stored)| hash == stored); - - match eq { - Ok(eq) => eq, - Err(e) => { + .map(|(hash, stored)| hash == stored) + .unwrap_or_else(|e| { warn!("Could not check whether the ref changed on the FS"); trace_error(&e); // We continue here and tell the callee that this reference is unchanged false - }, - } + }) } fn check_changed_permiss(_: &R) -> bool { diff --git a/lib/entry/libimagentryref/src/reference.rs b/lib/entry/libimagentryref/src/reference.rs index c043b1e0..9c20011b 100644 --- a/lib/entry/libimagentryref/src/reference.rs +++ b/lib/entry/libimagentryref/src/reference.rs @@ -140,17 +140,12 @@ impl Ref for Entry { /// Get the hahs of the link target which is stored in the ref object, which is hashed with a /// custom Hasher instance. fn get_stored_hash_with_hasher(&self, h: &H) -> Result { - match self.get_header().read(&format!("ref.content_hash.{}", h.hash_name())[..])? { - // content hash stored... - Some(&Value::String(ref s)) => Ok(s.clone()), - - // content hash header field has wrong type - Some(_) => Err(RE::from_kind(REK::HeaderTypeError)), - - // content hash not stored - None => Err(RE::from_kind(REK::HeaderFieldMissingError)), - - } + self.get_header() + .read(&format!("ref.content_hash.{}", h.hash_name())[..])? + .ok_or(RE::from_kind(REK::HeaderFieldMissingError))? + .as_str() + .map(String::from) + .ok_or(RE::from_kind(REK::HeaderTypeError)) } /// Get the hash of the link target by reading the link target and hashing the contents @@ -207,11 +202,9 @@ impl Ref for Entry { .read("ref.permissions.ro") .chain_err(|| REK::HeaderFieldReadError) .and_then(|ro| { - match ro { - Some(&Value::Boolean(b)) => Ok(b), - Some(_) => Err(RE::from_kind(REK::HeaderTypeError)), - None => Err(RE::from_kind(REK::HeaderFieldMissingError)), - } + ro.ok_or(RE::from_kind(REK::HeaderFieldMissingError))? + .as_bool() + .ok_or(RE::from_kind(REK::HeaderTypeError)) }) .and_then(|ro| self.get_current_permissions().map(|perm| ro == perm.readonly())) .chain_err(|| REK::RefTargetCannotReadPermissions) @@ -251,11 +244,12 @@ impl Ref for Entry { /// Get the path of the file which is reffered to by this Ref fn fs_file(&self) -> Result { - match self.get_header().read("ref.path")? { - Some(&Value::String(ref s)) => Ok(PathBuf::from(s)), - Some(_) => Err(RE::from_kind(REK::HeaderTypeError)), - None => Err(RE::from_kind(REK::HeaderFieldMissingError)), - } + self.get_header() + .read("ref.path")? + .ok_or(RE::from_kind(REK::HeaderFieldMissingError))? + .as_str() + .map(PathBuf::from) + .ok_or(RE::from_kind(REK::HeaderTypeError)) } /// Re-find a referenced file diff --git a/lib/entry/libimagentryref/src/util.rs b/lib/entry/libimagentryref/src/util.rs index 39896d42..0055991d 100644 --- a/lib/entry/libimagentryref/src/util.rs +++ b/lib/entry/libimagentryref/src/util.rs @@ -25,7 +25,6 @@ use error::Result; use libimagstore::store::Entry; -use toml::Value; use toml_query::read::TomlValueReadExt; /// Creates a Hash from a PathBuf by making the PathBuf absolute and then running a hash @@ -34,22 +33,22 @@ pub fn hash_path(pb: &PathBuf) -> Result { use crypto::sha1::Sha1; use crypto::digest::Digest; - match pb.to_str() { - Some(s) => { + pb.to_str() + .ok_or(RE::from_kind(REK::PathUTF8Error)) + .map(|s| { let mut hasher = Sha1::new(); hasher.input_str(s); - Ok(hasher.result_str()) - }, - None => return Err(RE::from_kind(REK::PathUTF8Error)), - } + hasher.result_str() + }) } /// Read the reference from a file pub fn read_reference(refentry: &Entry) -> Result { - match refentry.get_header().read("ref.path")? { - Some(&Value::String(ref s)) => Ok(PathBuf::from(s)), - Some(_) => Err(RE::from_kind(REK::HeaderTypeError)), - None => Err(RE::from_kind(REK::HeaderFieldMissingError)), - } + refentry.get_header() + .read("ref.path")? + .ok_or(RE::from_kind(REK::HeaderFieldMissingError))? + .as_str() + .ok_or(RE::from_kind(REK::HeaderTypeError)) + .map(PathBuf::from) } From 824f88e4fd2b1e10798cfe461b6c365d74a7b9d4 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 4 Jan 2018 23:09:30 +0100 Subject: [PATCH 21/21] Refactoring: Use function chaining rather than matching --- lib/entry/libimagentrytag/src/tagable.rs | 56 +++++++++++++----------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/lib/entry/libimagentrytag/src/tagable.rs b/lib/entry/libimagentrytag/src/tagable.rs index 601bc900..94f9a2a8 100644 --- a/lib/entry/libimagentrytag/src/tagable.rs +++ b/lib/entry/libimagentrytag/src/tagable.rs @@ -49,33 +49,39 @@ pub trait Tagable { impl Tagable for Value { fn get_tags(&self) -> Result> { - let tags = self.read("tag.values").chain_err(|| TagErrorKind::HeaderReadError)?; - - match tags { - Some(&Value::Array(ref tags)) => { - if !tags.iter().all(|t| is_match!(*t, Value::String(_))) { - return Err(TagErrorKind::TagTypeError.into()); - } - if tags.iter().any(|t| match *t { - Value::String(ref s) => !is_tag_str(s).is_ok(), - _ => unreachable!()}) - { - return Err(TagErrorKind::NotATag.into()); - } - - Ok(tags.iter() - .cloned() - .map(|t| { - match t { - Value::String(s) => s, - _ => unreachable!(), + self.read("tag.values") + .chain_err(|| TagErrorKind::HeaderReadError)? + .map(|val| { + debug!("Got Value of tags..."); + val.as_array() + .map(|tags| { + debug!("Got Array of tags..."); + if !tags.iter().all(|t| is_match!(*t, Value::String(_))) { + debug!("Got Array, T != String of tags: {:?}", tags); + return Err(TagErrorKind::TagTypeError.into()); } + debug!("Got Array of tags..."); + if tags.iter().any(|t| match *t { + Value::String(ref s) => !is_tag_str(s).is_ok(), + _ => unreachable!()}) + { + debug!("At least one tag is not a valid tag string"); + return Err(TagErrorKind::NotATag.into()); + } + + Ok(tags.iter() + .cloned() + .map(|t| { + match t { + Value::String(s) => s, + _ => unreachable!(), + } + }) + .collect()) }) - .collect()) - }, - None => Ok(vec![]), - _ => Err(TagErrorKind::TagTypeError.into()), - } + .unwrap_or(Ok(vec![])) + }) + .unwrap_or(Ok(vec![])) } fn set_tags(&mut self, ts: &[Tag]) -> Result<()> {