From 76a51ce425cabfedb6a02e487aef86e4dd527276 Mon Sep 17 00:00:00 2001 From: Rick Richardson Date: Wed, 30 Dec 2020 13:01:43 -0800 Subject: [PATCH 1/5] using the integer-based Std duration and f32::round() to eliminate float drift /fixes #1 --- Cargo.toml | 2 +- src/duration.rs | 23 ++++++++++++++--------- src/lib.rs | 25 ++++++++++++++++++------- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1705ea6..ebd943c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,4 +13,4 @@ keywords = ["iso8601", "duration", "parser"] travis-ci = { repository = "PoiScript/iso8601-duration" } [dependencies] -nom = "5.0.1" +nom = "^6" diff --git a/src/duration.rs b/src/duration.rs index 07befa2..769e501 100644 --- a/src/duration.rs +++ b/src/duration.rs @@ -6,12 +6,14 @@ use nom::{ bytes::complete::tag, character::complete::digit1, combinator::{all_consuming, map_res, opt}, - error::{ErrorKind, ParseError}, + error::{Error, ErrorKind, ParseError}, number::complete::float, sequence::{preceded, separated_pair, terminated, tuple}, Err, IResult, }; +const YEAR_IN_S: u64 = 31556952; // gregorian - includes leap-seconds + #[derive(Debug, PartialEq)] pub struct Duration { pub year: f32, @@ -35,17 +37,20 @@ impl Duration { } pub fn to_std(&self) -> StdDuration { - StdDuration::from_secs_f32( - self.year * 60. * 60. * 24. * 30. * 12. - + self.month * 60. * 60. * 24. * 30. - + self.day * 60. * 60. * 24. - + self.hour * 60. * 60. - + self.minute * 60. - + self.second, + let millis = (self.second.fract() * 1000.0).round() as u64; + StdDuration::from_millis( + (self.year.round() as u64 * YEAR_IN_S + + self.month.round() as u64 * 60 * 60 * 24 * 30 // there is no official answer on how long a month is, so 30 days will have to do + + self.day.round() as u64 * 60 * 60 * 24 + + self.hour.round() as u64 * 60 * 60 + + self.minute.round() as u64 * 60 + + self.second.trunc() as u64) + * 1000 + + millis, ) } - pub fn parse(input: &str) -> Result> { + pub fn parse(input: &str) -> Result>> { let (_, duration) = all_consuming(preceded( tag("P"), alt((parse_week_format, parse_basic_format)), diff --git a/src/lib.rs b/src/lib.rs index f8b75d1..00cbb55 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,8 +10,8 @@ //! //! ```rust //! use iso8601_duration::Duration; -//! use nom::{error::ErrorKind, Err}; -//! +//! use nom::{error::ErrorKind, error::Error}; +//! use std::time::Duration as StdDuration; //! assert_eq!( //! Duration::parse("P23DT23H"), //! Ok(Duration::new(0., 0., 23., 23., 0., 0.)) @@ -32,22 +32,33 @@ //! Duration::parse("P12W"), //! Ok(Duration::new(0., 0., 84., 0., 0., 0.)) //! ); -//! +//! assert_eq!( +//! Duration::parse("PT30M5S").unwrap().to_std(), +//! StdDuration::new(30 * 60 + 5, 0) +//! ); +//! assert_eq!( +//! Duration::parse("PT5H5M5S").unwrap().to_std(), +//! StdDuration::new(5 * 3600 + 5 * 60 + 5, 0) +//! ); +//! assert_eq!( +//! Duration::parse("PT5H5M5.555S").unwrap().to_std(), +//! StdDuration::new(5 * 3600 + 5 * 60 + 5, 555_000_000) +//! ); //! assert_eq!( //! Duration::parse("PT"), -//! Err(Err::Error(("", ErrorKind::Verify))) +//! Err(nom::Err::Error(Error { input: "", code: ErrorKind::Verify })) //! ); //! assert_eq!( //! Duration::parse("P12WT12H30M5S"), -//! Err(Err::Error(("T12H30M5S", ErrorKind::Eof))) +//! Err(nom::Err::Error(Error { input: "T12H30M5S", code: ErrorKind::Eof })) //! ); //! assert_eq!( //! Duration::parse("P0.5S0.5M"), -//! Err(Err::Error(("0.5S0.5M", ErrorKind::Verify))) +//! Err(nom::Err::Error(Error { input: "0.5S0.5M", code: ErrorKind::Verify })) //! ); //! assert_eq!( //! Duration::parse("P0.5A"), -//! Err(Err::Error(("0.5A", ErrorKind::Verify))) +//! Err(nom::Err::Error(Error { input: "0.5A", code: ErrorKind::Verify })) //! ); //! ``` From cc45cd21e5461d1c15c3b623143ef0bc12f8e676 Mon Sep 17 00:00:00 2001 From: Rick Richardson Date: Wed, 30 Dec 2020 13:22:19 -0800 Subject: [PATCH 2/5] no longer exposes Nom crate details, adds Custom error type and FromStr implementation --- src/duration.rs | 24 +++++++++++++++++++++++- src/lib.rs | 13 ++++++------- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/duration.rs b/src/duration.rs index 769e501..9fb0548 100644 --- a/src/duration.rs +++ b/src/duration.rs @@ -50,7 +50,7 @@ impl Duration { ) } - pub fn parse(input: &str) -> Result>> { + pub fn parse(input: &str) -> Result { let (_, duration) = all_consuming(preceded( tag("P"), alt((parse_week_format, parse_basic_format)), @@ -60,6 +60,28 @@ impl Duration { } } +impl FromStr for Duration { + type Err = DurationParseError; + fn from_str(s: &str) -> Result { + Duration::parse(s).map_err(DurationParseError::from) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DurationParseError(String); + +impl DurationParseError { + pub fn new>(s: S) -> DurationParseError { + DurationParseError(s.into()) + } +} + +impl From>> for DurationParseError { + fn from(err: Err>) -> Self { + DurationParseError(err.to_string()) + } +} + fn decimal_comma_number(input: &str) -> IResult<&str, f32> { map_res(separated_pair(digit1, tag(","), digit1), |(a, b)| { f32::from_str(&format!("{}.{}", a, b)) diff --git a/src/lib.rs b/src/lib.rs index 00cbb55..14911d8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,8 +9,7 @@ //! # Usage //! //! ```rust -//! use iso8601_duration::Duration; -//! use nom::{error::ErrorKind, error::Error}; +//! use iso8601_duration::{ Duration, DurationParseError }; //! use std::time::Duration as StdDuration; //! assert_eq!( //! Duration::parse("P23DT23H"), @@ -46,22 +45,22 @@ //! ); //! assert_eq!( //! Duration::parse("PT"), -//! Err(nom::Err::Error(Error { input: "", code: ErrorKind::Verify })) +//! Err(DurationParseError::new("Parsing Error: Error { input: \"\", code: Verify }")) //! ); //! assert_eq!( //! Duration::parse("P12WT12H30M5S"), -//! Err(nom::Err::Error(Error { input: "T12H30M5S", code: ErrorKind::Eof })) +//! Err(DurationParseError::new("Parsing Error: Error { input: \"T12H30M5S\", code: Eof }")) //! ); //! assert_eq!( //! Duration::parse("P0.5S0.5M"), -//! Err(nom::Err::Error(Error { input: "0.5S0.5M", code: ErrorKind::Verify })) +//! Err(DurationParseError::new("Parsing Error: Error { input: \"0.5S0.5M\", code: Verify }")) //! ); //! assert_eq!( //! Duration::parse("P0.5A"), -//! Err(nom::Err::Error(Error { input: "0.5A", code: ErrorKind::Verify })) +//! Err(DurationParseError::new("Parsing Error: Error { input: \"0.5A\", code: Verify }")) //! ); //! ``` mod duration; -pub use crate::duration::Duration; +pub use crate::duration::{Duration, DurationParseError}; From cc0d5877ab9f2465586a678ae69f0b50d968ade9 Mon Sep 17 00:00:00 2001 From: Rick Richardson Date: Wed, 30 Dec 2020 13:34:46 -0800 Subject: [PATCH 3/5] added Display for DurationParseError --- src/duration.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/duration.rs b/src/duration.rs index 9fb0548..ca148bd 100644 --- a/src/duration.rs +++ b/src/duration.rs @@ -1,3 +1,4 @@ +use std::fmt; use std::str::FromStr; use std::time::Duration as StdDuration; @@ -82,6 +83,12 @@ impl From>> for DurationParseError { } } +impl fmt::Display for DurationParseError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.0) + } +} + fn decimal_comma_number(input: &str) -> IResult<&str, f32> { map_res(separated_pair(digit1, tag(","), digit1), |(a, b)| { f32::from_str(&format!("{}.{}", a, b)) From 9e01f51ea253e95e0fba5e4d7ad0c537922931e7 Mon Sep 17 00:00:00 2001 From: Rick Richardson Date: Thu, 7 Jan 2021 11:42:20 -0800 Subject: [PATCH 4/5] changing to_std to multiply units to seconds before rounding, added basic tests --- src/duration.rs | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/src/duration.rs b/src/duration.rs index ca148bd..b281f59 100644 --- a/src/duration.rs +++ b/src/duration.rs @@ -13,7 +13,7 @@ use nom::{ Err, IResult, }; -const YEAR_IN_S: u64 = 31556952; // gregorian - includes leap-seconds +const YEAR_IN_S: f64 = 31556952.0; // gregorian - includes leap-seconds #[derive(Debug, PartialEq)] pub struct Duration { @@ -37,14 +37,17 @@ impl Duration { } } + // In this scheme we try to balance the flexibility of fractional units + // with the need to avoid rounding errors caused by floating point drift. + // The smaller we keep the floats, the better. pub fn to_std(&self) -> StdDuration { let millis = (self.second.fract() * 1000.0).round() as u64; StdDuration::from_millis( - (self.year.round() as u64 * YEAR_IN_S - + self.month.round() as u64 * 60 * 60 * 24 * 30 // there is no official answer on how long a month is, so 30 days will have to do - + self.day.round() as u64 * 60 * 60 * 24 - + self.hour.round() as u64 * 60 * 60 - + self.minute.round() as u64 * 60 + ((self.year as f64 * YEAR_IN_S).round() as u64 + + (self.month * 30.42 * 60.0 * 60.0 * 24.0).round() as u64 + + (self.day * 24.0 * 60.0 * 60.0).round() as u64 + + (self.hour * 60.0 * 60.0).round() as u64 + + (self.minute * 60.0).round() as u64 + self.second.trunc() as u64) * 1000 + millis, @@ -171,3 +174,29 @@ fn parse_week_format(input: &str) -> IResult<&str, Duration> { fn _parse_extended_format(_input: &str) -> IResult<&str, Duration> { unimplemented!() } + +#[cfg(test)] +mod test { + use super::*; + use std::time::Duration as StdDuration; + #[test] + fn parsing_units() { + let d1: Duration = "P10Y10M10DT10H10M10S".parse().unwrap(); + assert_eq!(d1.to_std(), StdDuration::from_secs(342753010)); + let d2: Duration = "P10Y10M10DT10H10M10.5S".parse().unwrap(); + assert_eq!(d2.to_std(), StdDuration::from_millis(342753010500)); + let d3: Duration = "P10.5Y10M10DT10H10M10S".parse().unwrap(); + assert_eq!(d3.to_std(), StdDuration::from_secs(358531486)); + let d4: Duration = "P10Y10.5M10DT10H10M10S".parse().unwrap(); + assert_eq!(d4.to_std(), StdDuration::from_secs(344067154)); + let d5: Duration = "P10Y10M10.5DT10H10M10S".parse().unwrap(); + assert_eq!(d5.to_std(), StdDuration::from_secs(342796210)); + let d6: Duration = "P10Y10M10DT10.5H10M10S".parse().unwrap(); + assert_eq!(d6.to_std(), StdDuration::from_secs(342754810)); + let d7: Duration = "PT5.5H5.5M".parse().unwrap(); + assert_eq!( + d7.to_std(), + StdDuration::from_secs((5.5 * 60. * 60.) as u64 + (5.5 * 60.) as u64) + ); + } +} From 0c174343b3d403492f56631d881d18eff1b903d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9rick=20Sauvage?= Date: Fri, 15 Oct 2021 17:25:36 +0200 Subject: [PATCH 5/5] to nom version 7 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index ebd943c..6dad588 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,4 +13,4 @@ keywords = ["iso8601", "duration", "parser"] travis-ci = { repository = "PoiScript/iso8601-duration" } [dependencies] -nom = "^6" +nom = "^7"