From 841c82b950bc3234a86e8f847d4d70f881335732 Mon Sep 17 00:00:00 2001 From: Raphael Nestler Date: Wed, 19 Oct 2016 22:24:33 +0200 Subject: [PATCH 1/7] Use Result for from_storeid --- libimagdiary/src/diaryid.rs | 39 +++++++++++++++++++++++++------------ libimagdiary/src/error.rs | 3 ++- libimagdiary/src/iter.rs | 6 +++--- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/libimagdiary/src/diaryid.rs b/libimagdiary/src/diaryid.rs index ad91ccf3..2a4ef8fe 100644 --- a/libimagdiary/src/diaryid.rs +++ b/libimagdiary/src/diaryid.rs @@ -30,6 +30,9 @@ use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; use libimagstore::store::Result as StoreResult; +use error::DiaryError as DE; +use error::DiaryErrorKind as DEK; + use module_path::ModuleEntryPath; #[derive(Debug, Clone)] @@ -175,7 +178,7 @@ impl Into for DiaryId { pub trait FromStoreId : Sized { - fn from_storeid(&StoreId) -> Option; + fn from_storeid(&StoreId) -> Result; } @@ -190,7 +193,7 @@ fn component_to_str<'a>(com: Component<'a>) -> Option<&'a str> { impl FromStoreId for DiaryId { - fn from_storeid(s: &StoreId) -> Option { + fn from_storeid(s: &StoreId) -> Result { use std::str::FromStr; let mut cmps = s.components().rev(); @@ -212,25 +215,37 @@ impl FromStoreId for DiaryId { }) { Some(s) => s, - None => return None, + None => return Err(DE::new(DEK::ParseError, None)), }; - let day :Option = cmps.next().and_then(component_to_str).and_then(|s| FromStr::from_str(s).ok()); - let month :Option = cmps.next().and_then(component_to_str).and_then(|s| FromStr::from_str(s).ok()); - let year :Option = cmps.next().and_then(component_to_str).and_then(|s| FromStr::from_str(s).ok()); - let name = cmps.next().and_then(component_to_str).map(String::from); + let day: Result = cmps.next().and_then(component_to_str) + .ok_or(DE::new(DEK::ParseError, None)) + .and_then(|s| s.parse::() + .map_err(|e| DE::new(DEK::ParseError, Some(Box::new(e))))); + + let month: Result = cmps.next().and_then(component_to_str) + .ok_or(DE::new(DEK::ParseError, None)) + .and_then(|s| s.parse::() + .map_err(|e| DE::new(DEK::ParseError, Some(Box::new(e))))); + + let year: Result = cmps.next().and_then(component_to_str) + .ok_or(DE::new(DEK::ParseError, None)) + .and_then(|s| s.parse::() + .map_err(|e| DE::new(DEK::ParseError, Some(Box::new(e))))); + + let name = cmps.next().and_then(component_to_str).map(String::from); debug!("Day = {:?}", day); debug!("Month = {:?}", month); debug!("Year = {:?}", year); debug!("Name = {:?}", name); - let day = match day { None => return None, Some(day) => day }; - let month = match month { None => return None, Some(month) => month }; - let year = match year { None => return None, Some(year) => year }; - let name = match name { None => return None, Some(name) => name }; + let day = try!(day); + let month = try!(month); + let year = try!(year); + let name = try!(name.ok_or(DE::new(DEK::ParseError, None))); - Some(DiaryId::new(name, year, month, day, hour, minute)) + Ok(DiaryId::new(name, year, month, day, hour, minute)) } } diff --git a/libimagdiary/src/error.rs b/libimagdiary/src/error.rs index b6dc2bfd..d70f8967 100644 --- a/libimagdiary/src/error.rs +++ b/libimagdiary/src/error.rs @@ -27,7 +27,8 @@ generate_error_module!( PathConversionError => "Error while converting paths internally", EntryNotInDiary => "Entry not in Diary", IOError => "IO Error", - ViewError => "Error viewing diary entry" + ViewError => "Error viewing diary entry", + ParseError => "Error while parsing" ); ); diff --git a/libimagdiary/src/iter.rs b/libimagdiary/src/iter.rs index 73f180d2..6a50819d 100644 --- a/libimagdiary/src/iter.rs +++ b/libimagdiary/src/iter.rs @@ -99,9 +99,9 @@ impl<'a> Iterator for DiaryEntryIterator<'a> { if next.is_in_diary(self.name) { debug!("Seems to be in diary: {:?}", next); let id = match DiaryId::from_storeid(&next) { - Some(i) => i, - None => { - debug!("Couldn't parse {:?} into DiaryId", next); + Ok(i) => i, + Err(e) => { + debug!("Couldn't parse {:?} into DiaryId: {:?}", next, e); continue; } }; From 42b0dd608368c1cc8d52898c830ebcd4eb11434b Mon Sep 17 00:00:00 2001 From: Raphael Nestler Date: Wed, 19 Oct 2016 23:31:12 +0200 Subject: [PATCH 2/7] Return Result from component_to_str --- libimagdiary/src/diaryid.rs | 54 ++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/libimagdiary/src/diaryid.rs b/libimagdiary/src/diaryid.rs index 2a4ef8fe..1896b282 100644 --- a/libimagdiary/src/diaryid.rs +++ b/libimagdiary/src/diaryid.rs @@ -184,11 +184,12 @@ pub trait FromStoreId : Sized { use std::path::Component; -fn component_to_str<'a>(com: Component<'a>) -> Option<&'a str> { +fn component_to_str<'a>(com: Component<'a>) -> Result<&'a str, DE> { match com { Component::Normal(s) => Some(s), - _ => None + _ => None, }.and_then(|s| s.to_str()) + .ok_or(DE::new(DEK::ParseError, None)) } impl FromStoreId for DiaryId { @@ -197,43 +198,46 @@ impl FromStoreId for DiaryId { use std::str::FromStr; let mut cmps = s.components().rev(); - let (hour, minute) = match cmps.next().and_then(component_to_str) - .and_then(|time| { - let mut time = time.split(":"); - let hour = time.next().and_then(|s| FromStr::from_str(s).ok()); - let minute = time.next() - .and_then(|s| s.split("~").next()) - .and_then(|s| FromStr::from_str(s).ok()); + let (hour, minute) = try!(cmps.next() + .ok_or(DE::new(DEK::ParseError, None)) + .and_then(component_to_str) + .and_then(|time| { + let mut time = time.split(":"); + let hour = time.next().and_then(|s| FromStr::from_str(s).ok()); + let minute = time.next() + .and_then(|s| s.split("~").next()) + .and_then(|s| FromStr::from_str(s).ok()); - debug!("Hour = {:?}", hour); - debug!("Minute = {:?}", minute); + debug!("Hour = {:?}", hour); + debug!("Minute = {:?}", minute); - match (hour, minute) { - (Some(h), Some(m)) => Some((h, m)), - _ => None, - } - }) - { - Some(s) => s, - None => return Err(DE::new(DEK::ParseError, None)), - }; + match (hour, minute) { + (Some(h), Some(m)) => Ok((h, m)), + _ => return Err(DE::new(DEK::ParseError, None)), + } + })); - let day: Result = cmps.next().and_then(component_to_str) + let day: Result = cmps.next() .ok_or(DE::new(DEK::ParseError, None)) + .and_then(component_to_str) .and_then(|s| s.parse::() .map_err(|e| DE::new(DEK::ParseError, Some(Box::new(e))))); - let month: Result = cmps.next().and_then(component_to_str) + let month: Result = cmps.next() .ok_or(DE::new(DEK::ParseError, None)) + .and_then(component_to_str) .and_then(|s| s.parse::() .map_err(|e| DE::new(DEK::ParseError, Some(Box::new(e))))); - let year: Result = cmps.next().and_then(component_to_str) + let year: Result = cmps.next() .ok_or(DE::new(DEK::ParseError, None)) + .and_then(component_to_str) .and_then(|s| s.parse::() .map_err(|e| DE::new(DEK::ParseError, Some(Box::new(e))))); - let name = cmps.next().and_then(component_to_str).map(String::from); + let name = cmps.next() + .ok_or(DE::new(DEK::ParseError, None)) + .and_then(component_to_str).map(String::from); debug!("Day = {:?}", day); debug!("Month = {:?}", month); @@ -243,7 +247,7 @@ impl FromStoreId for DiaryId { let day = try!(day); let month = try!(month); let year = try!(year); - let name = try!(name.ok_or(DE::new(DEK::ParseError, None))); + let name = try!(name); Ok(DiaryId::new(name, year, month, day, hour, minute)) } From 24c7bbc835f9993f75e085c2c50fbb88d3c8775c Mon Sep 17 00:00:00 2001 From: Raphael Nestler Date: Wed, 19 Oct 2016 23:52:23 +0200 Subject: [PATCH 3/7] Refactor fetching of next component into function --- libimagdiary/src/diaryid.rs | 55 ++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/libimagdiary/src/diaryid.rs b/libimagdiary/src/diaryid.rs index 1896b282..d040c725 100644 --- a/libimagdiary/src/diaryid.rs +++ b/libimagdiary/src/diaryid.rs @@ -197,47 +197,46 @@ impl FromStoreId for DiaryId { fn from_storeid(s: &StoreId) -> Result { use std::str::FromStr; + use std::path::Components; + use std::iter::Rev; + + fn next_component<'a>(components: &'a mut Rev) -> Result<&'a str, DE> { + components.next() + .ok_or(DE::new(DEK::ParseError, None)) + .and_then(component_to_str) + } + let mut cmps = s.components().rev(); - let (hour, minute) = try!(cmps.next() - .ok_or(DE::new(DEK::ParseError, None)) - .and_then(component_to_str) - .and_then(|time| { - let mut time = time.split(":"); - let hour = time.next().and_then(|s| FromStr::from_str(s).ok()); - let minute = time.next() - .and_then(|s| s.split("~").next()) - .and_then(|s| FromStr::from_str(s).ok()); - debug!("Hour = {:?}", hour); - debug!("Minute = {:?}", minute); + let (hour, minute) = try!(next_component(&mut cmps).and_then(|time| { + let mut time = time.split(":"); + let hour = time.next().and_then(|s| FromStr::from_str(s).ok()); + let minute = time.next() + .and_then(|s| s.split("~").next()) + .and_then(|s| FromStr::from_str(s).ok()); - match (hour, minute) { - (Some(h), Some(m)) => Ok((h, m)), - _ => return Err(DE::new(DEK::ParseError, None)), - } - })); + debug!("Hour = {:?}", hour); + debug!("Minute = {:?}", minute); - let day: Result = cmps.next() - .ok_or(DE::new(DEK::ParseError, None)) - .and_then(component_to_str) + match (hour, minute) { + (Some(h), Some(m)) => Ok((h, m)), + _ => return Err(DE::new(DEK::ParseError, None)), + } + })); + + let day: Result = next_component(&mut cmps) .and_then(|s| s.parse::() .map_err(|e| DE::new(DEK::ParseError, Some(Box::new(e))))); - let month: Result = cmps.next() - .ok_or(DE::new(DEK::ParseError, None)) - .and_then(component_to_str) + let month: Result = next_component(&mut cmps) .and_then(|s| s.parse::() .map_err(|e| DE::new(DEK::ParseError, Some(Box::new(e))))); - let year: Result = cmps.next() - .ok_or(DE::new(DEK::ParseError, None)) - .and_then(component_to_str) + let year: Result = next_component(&mut cmps) .and_then(|s| s.parse::() .map_err(|e| DE::new(DEK::ParseError, Some(Box::new(e))))); - let name = cmps.next() - .ok_or(DE::new(DEK::ParseError, None)) - .and_then(component_to_str).map(String::from); + let name = next_component(&mut cmps).map(String::from); debug!("Day = {:?}", day); debug!("Month = {:?}", month); From c8710a4d137926e81a381a38da5f15703174667e Mon Sep 17 00:00:00 2001 From: Raphael Nestler Date: Sat, 19 Nov 2016 23:05:26 +0100 Subject: [PATCH 4/7] Use map_err_into --- libimagdiary/src/diaryid.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libimagdiary/src/diaryid.rs b/libimagdiary/src/diaryid.rs index d040c725..45344778 100644 --- a/libimagdiary/src/diaryid.rs +++ b/libimagdiary/src/diaryid.rs @@ -32,6 +32,7 @@ use libimagstore::store::Result as StoreResult; use error::DiaryError as DE; use error::DiaryErrorKind as DEK; +use error::MapErrInto; use module_path::ModuleEntryPath; @@ -226,15 +227,15 @@ impl FromStoreId for DiaryId { let day: Result = next_component(&mut cmps) .and_then(|s| s.parse::() - .map_err(|e| DE::new(DEK::ParseError, Some(Box::new(e))))); + .map_err_into(DEK::ParseError)); let month: Result = next_component(&mut cmps) .and_then(|s| s.parse::() - .map_err(|e| DE::new(DEK::ParseError, Some(Box::new(e))))); + .map_err_into(DEK::ParseError)); let year: Result = next_component(&mut cmps) .and_then(|s| s.parse::() - .map_err(|e| DE::new(DEK::ParseError, Some(Box::new(e))))); + .map_err_into(DEK::ParseError)); let name = next_component(&mut cmps).map(String::from); From 2c16171d65a45cc3e79246bc2f04533fb8e6ea7c Mon Sep 17 00:00:00 2001 From: Raphael Nestler Date: Sat, 19 Nov 2016 23:06:02 +0100 Subject: [PATCH 5/7] Use trace_error --- libimagdiary/src/iter.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libimagdiary/src/iter.rs b/libimagdiary/src/iter.rs index 6a50819d..de849872 100644 --- a/libimagdiary/src/iter.rs +++ b/libimagdiary/src/iter.rs @@ -30,6 +30,7 @@ use entry::Entry as DiaryEntry; use error::DiaryError as DE; use error::DiaryErrorKind as DEK; use result::Result; +use libimagerror::trace::trace_error; /// A iterator for iterating over diary entries pub struct DiaryEntryIterator<'a> { @@ -101,6 +102,7 @@ impl<'a> Iterator for DiaryEntryIterator<'a> { let id = match DiaryId::from_storeid(&next) { Ok(i) => i, Err(e) => { + trace_error(&e); debug!("Couldn't parse {:?} into DiaryId: {:?}", next, e); continue; } From a676f4c148b1f8554fbd759020aaa6fb86317e48 Mon Sep 17 00:00:00 2001 From: Raphael Nestler Date: Sat, 19 Nov 2016 23:18:09 +0100 Subject: [PATCH 6/7] Use into_error() --- libimagdiary/src/diaryid.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libimagdiary/src/diaryid.rs b/libimagdiary/src/diaryid.rs index 45344778..05f95bd6 100644 --- a/libimagdiary/src/diaryid.rs +++ b/libimagdiary/src/diaryid.rs @@ -33,6 +33,7 @@ use libimagstore::store::Result as StoreResult; use error::DiaryError as DE; use error::DiaryErrorKind as DEK; use error::MapErrInto; +use libimagerror::into::IntoError; use module_path::ModuleEntryPath; @@ -190,7 +191,7 @@ fn component_to_str<'a>(com: Component<'a>) -> Result<&'a str, DE> { Component::Normal(s) => Some(s), _ => None, }.and_then(|s| s.to_str()) - .ok_or(DE::new(DEK::ParseError, None)) + .ok_or(DEK::ParseError.into_error()) } impl FromStoreId for DiaryId { @@ -203,7 +204,7 @@ impl FromStoreId for DiaryId { fn next_component<'a>(components: &'a mut Rev) -> Result<&'a str, DE> { components.next() - .ok_or(DE::new(DEK::ParseError, None)) + .ok_or(DEK::ParseError.into_error()) .and_then(component_to_str) } From c4b1f7ac8d16e6b2572f6c546f9220a9ba79fa25 Mon Sep 17 00:00:00 2001 From: Raphael Nestler Date: Sun, 20 Nov 2016 16:45:50 +0100 Subject: [PATCH 7/7] Rename ParseError to IdParseError --- libimagdiary/src/diaryid.rs | 12 ++++++------ libimagdiary/src/error.rs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/libimagdiary/src/diaryid.rs b/libimagdiary/src/diaryid.rs index 05f95bd6..906097d8 100644 --- a/libimagdiary/src/diaryid.rs +++ b/libimagdiary/src/diaryid.rs @@ -191,7 +191,7 @@ fn component_to_str<'a>(com: Component<'a>) -> Result<&'a str, DE> { Component::Normal(s) => Some(s), _ => None, }.and_then(|s| s.to_str()) - .ok_or(DEK::ParseError.into_error()) + .ok_or(DEK::IdParseError.into_error()) } impl FromStoreId for DiaryId { @@ -204,7 +204,7 @@ impl FromStoreId for DiaryId { fn next_component<'a>(components: &'a mut Rev) -> Result<&'a str, DE> { components.next() - .ok_or(DEK::ParseError.into_error()) + .ok_or(DEK::IdParseError.into_error()) .and_then(component_to_str) } @@ -222,21 +222,21 @@ impl FromStoreId for DiaryId { match (hour, minute) { (Some(h), Some(m)) => Ok((h, m)), - _ => return Err(DE::new(DEK::ParseError, None)), + _ => return Err(DE::new(DEK::IdParseError, None)), } })); let day: Result = next_component(&mut cmps) .and_then(|s| s.parse::() - .map_err_into(DEK::ParseError)); + .map_err_into(DEK::IdParseError)); let month: Result = next_component(&mut cmps) .and_then(|s| s.parse::() - .map_err_into(DEK::ParseError)); + .map_err_into(DEK::IdParseError)); let year: Result = next_component(&mut cmps) .and_then(|s| s.parse::() - .map_err_into(DEK::ParseError)); + .map_err_into(DEK::IdParseError)); let name = next_component(&mut cmps).map(String::from); diff --git a/libimagdiary/src/error.rs b/libimagdiary/src/error.rs index d70f8967..b5406b12 100644 --- a/libimagdiary/src/error.rs +++ b/libimagdiary/src/error.rs @@ -28,7 +28,7 @@ generate_error_module!( EntryNotInDiary => "Entry not in Diary", IOError => "IO Error", ViewError => "Error viewing diary entry", - ParseError => "Error while parsing" + IdParseError => "Error while parsing ID" ); );