diff --git a/bin/domain/imag-contact/src/main.rs b/bin/domain/imag-contact/src/main.rs index 05599965..80cc92cc 100644 --- a/bin/domain/imag-contact/src/main.rs +++ b/bin/domain/imag-contact/src/main.rs @@ -119,14 +119,11 @@ fn list(rt: &Runtime) { .all_contacts() .map_err_trace_exit_unwrap(1) .into_get_iter(rt.store()) - .map(|fle| { - let fle = fle - .map_err_trace_exit_unwrap(1) - .ok_or_else(|| Error::from(err_msg("StoreId not found".to_owned()))) - .map_err_trace_exit_unwrap(1); - - fle.deser().map_err_trace_exit_unwrap(1) - }) + .trace_unwrap_exit(1) + .map(|fle| fle.ok_or_else(|| Error::from(err_msg("StoreId not found".to_owned())))) + .trace_unwrap_exit(1) + .map(|e| e.deser()) + .trace_unwrap_exit(1) .enumerate(); if scmd.is_present("json") { @@ -140,21 +137,19 @@ fn list(rt: &Runtime) { } } } else { - iterator - .map(|(i, deservcard)| { - let data = build_data_object_for_handlebars(i, &deservcard); + let rendered = iterator + .map(|(i, dvcard)| build_data_object_for_handlebars(i, &dvcard)) + .map(|data| list_format.render("format", &data).map_err(Error::from)) + .trace_unwrap_exit(1) + .collect::>(); + // collect, so that we can have rendered all the things and printing is faster. - list_format.render("format", &data) - .map_err(Error::from) - .map_err_trace_exit_unwrap(1) - }) + let output = rt.stdout(); + let mut output = output.lock(); - // collect, so that we can have rendered all the things and printing is faster. - .collect::>() - .into_iter() - .for_each(|s| { - writeln!(rt.stdout(), "{}", s).to_exit_code().unwrap_or_exit() - }); + rendered.into_iter().for_each(|s| { + writeln!(output, "{}", s).to_exit_code().unwrap_or_exit() + }); } } diff --git a/bin/domain/imag-diary/src/delete.rs b/bin/domain/imag-diary/src/delete.rs index 085b79a9..d8ccd47d 100644 --- a/bin/domain/imag-diary/src/delete.rs +++ b/bin/domain/imag-diary/src/delete.rs @@ -19,7 +19,7 @@ use std::process::exit; -use chrono::naive::NaiveDateTime; +use chrono::naive::NaiveDateTime as NDT; use libimagdiary::diaryid::DiaryId; use libimagrt::runtime::Runtime; @@ -44,17 +44,10 @@ pub fn delete(rt: &Runtime) { .value_of("datetime") .map(|dt| { debug!("DateTime = {:?}", dt); dt }) .and_then(DateTime::parse) - .map(|dt| dt.into()) - .ok_or_else(|| { - warn!("Not deleting entries, because missing date/time specification"); - exit(1); - }) - .and_then(|dt: NaiveDateTime| { - DiaryId::from_datetime(diaryname.clone(), dt) - .into_storeid() - .map(|id| rt.store().retrieve(id)) - .map_err_trace_exit_unwrap(1) - }) + .map(Into::into) + .ok_or_else(|| warn_exit("Not deleting entries: missing date/time specification", 1)) + .and_then(|dt: NDT| DiaryId::from_datetime(diaryname.clone(), dt).into_storeid()) + .and_then(|id| rt.store().retrieve(id)) .map_err_trace_exit_unwrap(1) .get_location() .clone(); diff --git a/bin/domain/imag-diary/src/list.rs b/bin/domain/imag-diary/src/list.rs index a1f32ff2..e5a2b3ff 100644 --- a/bin/domain/imag-diary/src/list.rs +++ b/bin/domain/imag-diary/src/list.rs @@ -51,10 +51,9 @@ pub fn list(rt: &Runtime) { [id.year() as u32, id.month(), id.day(), id.hour(), id.minute(), id.second()] }); - for id in ids.into_iter().map(|id| id.into_storeid().map_err_trace_exit_unwrap(1)) { - writeln!(rt.stdout(), "{}", id) - .to_exit_code() - .unwrap_or_exit(); - } + ids.into_iter() + .map(IntoStoreId::into_storeid) + .trace_unwrap_exit(1) + .for_each(|id| writeln!(rt.stdout(), "{}", id).to_exit_code().unwrap_or_exit()); } diff --git a/bin/domain/imag-habit/src/main.rs b/bin/domain/imag-habit/src/main.rs index 629afa10..7aa49664 100644 --- a/bin/domain/imag-habit/src/main.rs +++ b/bin/domain/imag-habit/src/main.rs @@ -69,6 +69,7 @@ use libimagstore::store::FileLockEntry; use libimagstore::store::Store; use libimagstore::storeid::StoreId; use libimaginteraction::ask::ask_bool; +use libimagutil::debug_result::DebugResult; mod ui; @@ -115,20 +116,16 @@ fn create(rt: &Runtime) { let date = scmd.value_of("create-date").unwrap(); // safe by clap let parsedate = |d, pname| match kairos_parse(d).map_err_trace_exit_unwrap(1) { - Parsed::TimeType(tt) => match tt.calculate() { - Ok(tt) => match tt.get_moment() { - Some(mom) => mom.date(), - None => { - debug!("TimeType yielded: '{:?}'", tt); - error!("Error: '{}' parameter does not yield a point in time", pname); - exit(1); - }, - }, - Err(e) => { - error!("Error: '{:?}'", e); - exit(1); - } - }, + Parsed::TimeType(tt) => tt.calculate() + .map_dbg(|y| format!("TimeType yielded: '{:?}'", y)) + .map_err_trace_exit_unwrap(1) + .get_moment() + .ok_or_else(|| { + error!("Error: '{}' parameter does not yield a point in time", pname); + exit(1) + }) + .unwrap() // safe by above + .date(), _ => { error!("Error: '{}' parameter does not yield a point in time", pname); exit(1); diff --git a/lib/core/libimagrt/src/io.rs b/lib/core/libimagrt/src/io.rs index 8107db48..a031b04e 100644 --- a/lib/core/libimagrt/src/io.rs +++ b/lib/core/libimagrt/src/io.rs @@ -68,13 +68,9 @@ impl Debug for OutputProxy { impl OutputProxy { pub fn lock(&self) -> LockedOutputProxy { match *self { - OutputProxy::Out(ref r) => { - LockedOutputProxy::Out(r.lock()) - }, - OutputProxy::Err(ref r) => { - LockedOutputProxy::Err(r.lock()) - }, - OutputProxy::Sink => LockedOutputProxy::Sink, + OutputProxy::Out(ref r) => LockedOutputProxy::Out(r.lock()), + OutputProxy::Err(ref r) => LockedOutputProxy::Err(r.lock()), + OutputProxy::Sink => LockedOutputProxy::Sink, } } } diff --git a/lib/core/libimagrt/src/logger.rs b/lib/core/libimagrt/src/logger.rs index 4fd773a6..de03b5d5 100644 --- a/lib/core/libimagrt/src/logger.rs +++ b/lib/core/libimagrt/src/logger.rs @@ -88,34 +88,12 @@ impl ImagLogger { ::libimaginteraction::format::register_all_format_helpers(&mut handlebars); { - let fmt = aggregate_global_format_trace(config)?; - handlebars.register_template_string("TRACE", fmt) - .map_err(Error::from) - .context(err_msg("Handlebars template error"))?; // name must be uppercase - } - { - let fmt = aggregate_global_format_debug(config)?; - handlebars.register_template_string("DEBUG", fmt) - .map_err(Error::from) - .context(err_msg("Handlebars template error"))?; // name must be uppercase - } - { - let fmt = aggregate_global_format_info(config)?; - handlebars.register_template_string("INFO", fmt) - .map_err(Error::from) - .context(err_msg("Handlebars template error"))?; // name must be uppercase - } - { - let fmt = aggregate_global_format_warn(config)?; - handlebars.register_template_string("WARN", fmt) - .map_err(Error::from) - .context(err_msg("Handlebars template error"))?; // name must be uppercase - } - { - let fmt = aggregate_global_format_error(config)?; - handlebars.register_template_string("ERROR", fmt) - .map_err(Error::from) - .context(err_msg("Handlebars template error"))?; // name must be uppercase + use self::log_lvl_aggregate::*; + let _ = aggregate::(&mut handlebars, config, "TRACE")?; + let _ = aggregate::(&mut handlebars, config, "DEBUG")?; + let _ = aggregate::(&mut handlebars, config, "INFO")?; + let _ = aggregate::(&mut handlebars, config, "WARN")?; + let _ = aggregate::(&mut handlebars, config, "ERROR")?; } Ok(ImagLogger { @@ -289,9 +267,8 @@ fn translate_destinations(raw: &Vec) -> Result> { acc.and_then(|mut v| { let dest = val.as_str() .ok_or_else(|| { - let path = "imag.logging.modules..destinations"; - let ty = "Array"; - Error::from(format_err!("Type error at {}, expected {}", path, ty)) + let msg = "Type error at 'imag.logging.modules..destinations', expected Array"; + Error::from(err_msg(msg)) }) .and_then(translate_destination)?; v.push(dest); @@ -312,9 +289,8 @@ fn aggregate_global_destinations(matches: &ArgMatches, config: Option<&Value>) .ok_or_else(|| err_msg("Global log destination config missing"))? .as_array() .ok_or_else(|| { - let path = "imag.logging.destinations"; - let ty = "Array"; - Error::from(format_err!("Type error at {}, expected {}", path, ty)) + let msg = "Type error at 'imag.logging.destinations', expected 'Array'"; + Error::from(err_msg(msg)) }) .and_then(translate_destinations), None => { @@ -335,54 +311,55 @@ fn aggregate_global_destinations(matches: &ArgMatches, config: Option<&Value>) } } -macro_rules! aggregate_global_format { - ($read_str:expr, $error_msg_if_missing:expr, $config:expr) => { - try!($config.ok_or_else(|| Error::from(err_msg($error_msg_if_missing)))) - .read_string($read_str) - .map_err(Error::from) - .context(EM::TomlQueryError)? - .ok_or_else(|| Error::from(err_msg($error_msg_if_missing))) - }; -} +mod log_lvl_aggregate { + use failure::Fallible as Result; + use failure::Error as E; + use failure::ResultExt; + use failure::err_msg; + use toml::Value; + use toml_query::read::TomlValueReadTypeExt; + use handlebars::Handlebars; -fn aggregate_global_format_trace(config: Option<&Value>) - -> Result -{ - aggregate_global_format!("imag.logging.format.trace", - "Config missing: Logging format: Trace", - config) -} + use libimagerror::errors::ErrorMsg as EM; -fn aggregate_global_format_debug(config: Option<&Value>) - -> Result -{ - aggregate_global_format!("imag.logging.format.debug", - "Config missing: Logging format: Debug", - config) -} + macro_rules! aggregate_global_format_with { + ($t:ident, $read_str:expr) => { + pub struct $t; + impl LogLevelAggregator for $t { + fn aggregate(config: Option<&Value>) -> Result { + config.ok_or_else(|| { + E::from(err_msg(concat!("Config missing: Logging format: ", stringify!($t)))) + })? + .read_string($read_str) + .map_err(E::from) + .context(EM::TomlQueryError)? + .ok_or_else(|| { + E::from(err_msg(concat!("Config missing: Logging format: ", stringify!($t)))) + }) + } + } + }; + } -fn aggregate_global_format_info(config: Option<&Value>) - -> Result -{ - aggregate_global_format!("imag.logging.format.info", - "Config missing: Logging format: Info", - config) -} + pub trait LogLevelAggregator { + fn aggregate(config: Option<&Value>) -> Result; + } -fn aggregate_global_format_warn(config: Option<&Value>) - -> Result -{ - aggregate_global_format!("imag.logging.format.warn", - "Config missing: Logging format: Warn", - config) -} + pub fn aggregate(hb: &mut Handlebars, config: Option<&Value>, lvlstr: &str) + -> Result<()> + { + hb.register_template_string(lvlstr, T::aggregate(config)?) + .map_err(E::from) + .context(err_msg("Handlebars template error")) + .map_err(E::from) + } + + aggregate_global_format_with!(Trace, "imag.logging.format.trace"); + aggregate_global_format_with!(Debug, "imag.logging.format.debug"); + aggregate_global_format_with!(Info, "imag.logging.format.info"); + aggregate_global_format_with!(Warn, "imag.logging.format.warn"); + aggregate_global_format_with!(Error, "imag.logging.format.error"); -fn aggregate_global_format_error(config: Option<&Value>) - -> Result -{ - aggregate_global_format!("imag.logging.format.error", - "Config missing: Logging format: Error", - config) } fn aggregate_module_settings(_matches: &ArgMatches, config: Option<&Value>) @@ -414,9 +391,8 @@ fn aggregate_module_settings(_matches: &ArgMatches, config: Option<&Value>) .map(|val| { val.as_array() .ok_or_else(|| { - let path = "imag.logging.modules..destinations"; - let ty = "Array"; - Error::from(format_err!("Type error at {}, expected {}", path, ty)) + let msg = "Type error at 'imag.logging.modules..destinations', expected 'Array'"; + Error::from(err_msg(msg)) }) .and_then(translate_destinations) }) @@ -434,9 +410,8 @@ fn aggregate_module_settings(_matches: &ArgMatches, config: Option<&Value>) .context(EM::TomlQueryError)? .map(|v| v.as_bool().unwrap_or(false)) .ok_or_else(|| { - let path = "imag.logging.modules..enabled"; - let ty = "Boolean"; - Error::from(format_err!("Type error at {}, expected {}", path, ty)) + let msg = "Type error at 'imag.logging.modules..enabled', expected 'Boolean'"; + Error::from(err_msg(msg)) })?; let module_settings = ModuleSettings { @@ -452,9 +427,8 @@ fn aggregate_module_settings(_matches: &ArgMatches, config: Option<&Value>) Ok(settings) }, Ok(Some(_)) => { - let path = "imag.logging.modules"; - let ty = "Table"; - Err(Error::from(format_err!("Type error at {}, expected {}", path, ty))) + let msg = "Type error at 'imag.logging.modules', expected 'Table'"; + Err(Error::from(err_msg(msg))) }, Ok(None) => { // No modules configured. This is okay! diff --git a/lib/core/libimagrt/src/runtime.rs b/lib/core/libimagrt/src/runtime.rs index 04ee77e2..83876ec9 100644 --- a/lib/core/libimagrt/src/runtime.rs +++ b/lib/core/libimagrt/src/runtime.rs @@ -137,13 +137,11 @@ impl<'a> Runtime<'a> { Store::new(storepath, &config) }; - store_result.map(|store| { - Runtime { - cli_matches: matches, - configuration: config, - rtp: rtp, - store: store, - } + store_result.map(|store| Runtime { + cli_matches: matches, + configuration: config, + rtp: rtp, + store: store, }) .context(err_msg("Cannot instantiate runtime")) .map_err(Error::from) @@ -390,15 +388,17 @@ impl<'a> Runtime<'a> { self.cli() .value_of("editor") .map(String::from) - .or_else(|| { + .ok_or_else(|| { self.config() - .and_then(|v| match v.read("rt.editor") { - Ok(Some(&Value::String(ref s))) => Some(s.clone()), - _ => None, // FIXME silently ignore errors in config is bad + .ok_or_else(|| Error::from(err_msg("No Configuration!"))) + .and_then(|v| match v.read("rt.editor")? { + Some(&Value::String(ref s)) => Ok(Some(s.clone())), + Some(_) => Err(Error::from(err_msg("Type error at 'rt.editor', expected 'String'"))), + None => Ok(None), }) }) - .or(env::var("EDITOR").ok()) - .ok_or_else(|| Error::from(EM::IO)) + .or(env::var("EDITOR")) + .map_err(|_| Error::from(EM::IO)) .map_dbg(|s| format!("Editing with '{}'", s)) .and_then(|s| { let mut split = s.split_whitespace(); diff --git a/lib/core/libimagstore/src/file_abstraction/fs.rs b/lib/core/libimagstore/src/file_abstraction/fs.rs index 131edcba..fe1407e3 100644 --- a/lib/core/libimagstore/src/file_abstraction/fs.rs +++ b/lib/core/libimagstore/src/file_abstraction/fs.rs @@ -105,15 +105,14 @@ impl FileAbstraction for FSFileAbstraction { } fn rename(&self, from: &PathBuf, to: &PathBuf) -> Result<()> { - match to.parent() { - Some(p) => if !p.exists() { + if let Some(p) = to.parent() { + if !p.exists() { debug!("Creating: {:?}", p); - let _ = create_dir_all(&PathBuf::from(p)).context(EM::DirNotCreated)?; - }, - None => { - debug!("Failed to find parent. This looks like it will fail now"); - //nothing - }, + let _ = create_dir_all(&p).context(EM::DirNotCreated)?; + } + } else { + debug!("Failed to find parent. This looks like it will fail now"); + //nothing } debug!("Renaming {:?} to {:?}", from, to); @@ -149,10 +148,9 @@ impl FileAbstraction for FSFileAbstraction { /// FileAbstraction::fill implementation that consumes the Drain and writes everything to the /// filesystem fn fill(&mut self, mut d: Drain) -> Result<()> { - d.iter() - .fold(Ok(()), |acc, (path, element)| { - acc.and_then(|_| self.new_instance(path).write_file_content(&element)) - }) + d.iter().fold(Ok(()), |acc, (path, element)| { + acc.and_then(|_| self.new_instance(path).write_file_content(&element)) + }) } fn pathes_recursively(&self, @@ -190,11 +188,9 @@ impl PathIterBuilder for WalkDirPathIterBuilder { fn open_file>(p: A) -> ::std::io::Result> { match OpenOptions::new().write(true).read(true).open(p) { - Err(e) => { - match e.kind() { - ::std::io::ErrorKind::NotFound => return Ok(None), - _ => return Err(e), - } + Err(e) => match e.kind() { + ::std::io::ErrorKind::NotFound => return Ok(None), + _ => return Err(e), }, Ok(file) => Ok(Some(file)) } @@ -205,9 +201,7 @@ fn create_file>(p: A) -> ::std::io::Result { trace!("'{}' is directory = {}", parent.display(), parent.is_dir()); if !parent.is_dir() { trace!("Implicitely creating directory: {:?}", parent); - if let Err(e) = create_dir_all(parent) { - return Err(e); - } + let _ = create_dir_all(parent)?; } } OpenOptions::new().write(true).read(true).create(true).open(p) diff --git a/lib/core/libimagstore/src/file_abstraction/inmemory.rs b/lib/core/libimagstore/src/file_abstraction/inmemory.rs index a499b697..d5e89ac7 100644 --- a/lib/core/libimagstore/src/file_abstraction/inmemory.rs +++ b/lib/core/libimagstore/src/file_abstraction/inmemory.rs @@ -140,7 +140,7 @@ impl FileAbstraction for InMemoryFileAbstraction { let mut mtx = self.backend().lock().expect("Locking Mutex failed"); let backend = mtx.get_mut(); - let a = backend.get(from).cloned().ok_or_else(|| EM::FileNotFound)?; + let a = backend.remove(from).ok_or_else(|| EM::FileNotFound)?; backend.insert(to.clone(), a); debug!("Renaming: {:?} -> {:?} worked", from, to); Ok(()) diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index 64ab7fc4..36904a3b 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -222,11 +222,13 @@ impl Store { debug!("Creating id: '{}'", id); - let exists = id.exists()? || self.entries - .read() - .map(|map| map.contains_key(&id)) - .map_err(|_| Error::from(EM::LockError)) - .context(format_err!("CreateCallError: {}", id))?; + let exists = + self.entries + .read() + .map(|map| map.contains_key(&id)) + .map_err(|_| Error::from(EM::LockError)) + .context(format_err!("CreateCallError: {}", id))? || + self.backend.exists(&id.clone().into_pathbuf()?)?; if exists { debug!("Entry exists: {:?}", id); @@ -304,11 +306,14 @@ impl Store { debug!("Getting id: '{}'", id); - let exists = id.exists()? || self.entries - .read() - .map(|map| map.contains_key(&id)) - .map_err(|_| Error::from(EM::LockError)) - .context(format_err!("GetCallError: {}", id))?; + let exists = + self.entries + .read() + .map(|map| map.contains_key(&id)) + .map_err(|_| Error::from(EM::LockError)) + .context(format_err!("CreateCallError: {}", id))? || + self.backend.exists(&id.clone().into_pathbuf()?)?; + if !exists { debug!("Does not exist in internal cache or filesystem: {:?}", id); @@ -676,8 +681,10 @@ impl<'a> FileLockEntry<'a, > { impl<'a> Debug for FileLockEntry<'a> { fn fmt(&self, fmt: &mut Formatter) -> RResult<(), FMTError> { - write!(fmt, "FileLockEntry(Store = {})", self.store.location.to_str() - .unwrap_or("Unknown Path")) + write!(fmt, + "FileLockEntry(Store = {store}, location = {location:?})", + store = self.store.location.to_str().unwrap_or("Unknown Path"), + location = self.entry.get_location()) } } @@ -1214,10 +1221,13 @@ mod store_tests { assert!(store.entries.read().unwrap().get(&id_mv_with_base).is_some()); } - assert!(match store.get(id.clone()) { Ok(None) => true, _ => false }, - "Moved id ({:?}) is still there", id); - assert!(match store.get(id_mv.clone()) { Ok(Some(_)) => true, _ => false }, - "New id ({:?}) is not in store...", id_mv); + let res = store.get(id.clone()); + assert!(match res { Ok(None) => true, _ => false }, + "Moved id ({:?}) is still there: {:?}", id, res); + + let res = store.get(id_mv.clone()); + assert!(match res { Ok(Some(_)) => true, _ => false }, + "New id ({:?}) is not in store: {:?}", id_mv, res); } } } diff --git a/lib/core/libimagstore/src/util.rs b/lib/core/libimagstore/src/util.rs index e19fcc19..7a73f36d 100644 --- a/lib/core/libimagstore/src/util.rs +++ b/lib/core/libimagstore/src/util.rs @@ -69,8 +69,14 @@ pub fn entry_buffer_to_header_content(buf: &str) -> Result<(Value, String)> { #[cfg(test)] mod test { + extern crate env_logger; + use super::entry_buffer_to_header_content; + fn setup_logging() { + let _ = env_logger::try_init(); + } + fn mkfile(content: &str) -> String { format!(r#"--- [imag] @@ -97,16 +103,17 @@ version = '{version}' #[test] fn test_entry_buffer_to_header_content_2() { + setup_logging(); let content = r#"Hai "#; let file = mkfile(&content); - eprintln!("FILE: <<<{}>>>", file); + debug!("FILE: <<<{}>>>", file); let res = entry_buffer_to_header_content(&file); assert!(res.is_ok()); let (_, res_content) = res.unwrap(); - eprintln!("CONTENT: <<<{}>>>", res_content); + debug!("CONTENT: <<<{}>>>", res_content); assert_eq!(res_content, content) } diff --git a/lib/domain/libimagmail/src/mail.rs b/lib/domain/libimagmail/src/mail.rs index a1b33a14..f2d759c3 100644 --- a/lib/domain/libimagmail/src/mail.rs +++ b/lib/domain/libimagmail/src/mail.rs @@ -194,4 +194,8 @@ impl<'a> Mail<'a> { self.get_field("In-Reply-To") } + pub fn fle(&self) -> &FileLockEntry<'a> { + &self.0 + } + } diff --git a/lib/domain/libimagtodo/src/taskstore.rs b/lib/domain/libimagtodo/src/taskstore.rs index a66f9516..8a6f3d83 100644 --- a/lib/domain/libimagtodo/src/taskstore.rs +++ b/lib/domain/libimagtodo/src/taskstore.rs @@ -140,7 +140,6 @@ impl<'a> TaskStore<'a> for Store { // the change. The (maybe modified) second one is // expected by taskwarrior. let val = serde_to_string(&ttask).context(err_msg("Import error"))?; - println!("{}", val); // Taskwarrior does not have the concept of deleted tasks, but only modified // ones.