From 9b67307512dfec9b9ec1f2858651715923695665 Mon Sep 17 00:00:00 2001 From: yuankunzhang Date: Thu, 29 May 2025 23:35:28 +0800 Subject: [PATCH 1/2] fix: relative date time handling --- src/items/mod.rs | 228 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 178 insertions(+), 50 deletions(-) diff --git a/src/items/mod.rs b/src/items/mod.rs index d06257d..39fd695 100644 --- a/src/items/mod.rs +++ b/src/items/mod.rs @@ -345,11 +345,31 @@ fn last_day_of_month(year: i32, month: u32) -> u32 { .day() } -fn at_date_inner(date: Vec, mut d: DateTime) -> Option> { - d = d.with_hour(0).unwrap(); - d = d.with_minute(0).unwrap(); - d = d.with_second(0).unwrap(); - d = d.with_nanosecond(0).unwrap(); +fn at_date_inner(date: Vec, at: DateTime) -> Option> { + let mut d = at + .with_hour(0) + .unwrap() + .with_minute(0) + .unwrap() + .with_second(0) + .unwrap() + .with_nanosecond(0) + .unwrap(); + + // This flag is used by relative items to determine which date/time to use. + // If any date/time item is set, it will use that; otherwise, it will use + // the `at` value. + let date_time_set = date.iter().any(|item| { + matches!( + item, + Item::Timestamp(_) + | Item::Date(_) + | Item::DateTime(_) + | Item::Year(_) + | Item::Time(_) + | Item::Weekday(_) + ) + }); for item in date { match item { @@ -416,54 +436,84 @@ fn at_date_inner(date: Vec, mut d: DateTime) -> Option { - let mut beginning_of_day = d - .with_hour(0) - .unwrap() - .with_minute(0) - .unwrap() - .with_second(0) - .unwrap() - .with_nanosecond(0) - .unwrap(); + Item::Weekday(weekday::Weekday { offset: x, day }) => { + let mut x = x; let day = day.into(); - while beginning_of_day.weekday() != day { - beginning_of_day += chrono::Duration::days(1); + // If the current day is not the target day, we need to adjust + // the x value to ensure we find the correct day. + // + // Consider this: + // Assuming today is Monday, next Friday is actually THIS Friday; + // but next Monday is indeed NEXT Monday. + if d.weekday() != day && x > 0 { + x -= 1; } - d = beginning_of_day - } - Item::Relative(relative::Relative::Years(x)) => { - d = d.with_year(d.year() + x)?; - } - Item::Relative(relative::Relative::Months(x)) => { - // *NOTE* This is done in this way to conform to - // GNU behavior. - let days = last_day_of_month(d.year(), d.month()); - if x >= 0 { - d += d - .date_naive() - .checked_add_days(chrono::Days::new((days * x as u32) as u64))? - .signed_duration_since(d.date_naive()); + // Calculate the delta to the target day. + // + // Assuming today is Thursday, here are some examples: + // + // Example 1: last Thursday (x = -1, day = Thursday) + // delta = (3 - 3) % 7 + (-1) * 7 = -7 + // + // Example 2: last Monday (x = -1, day = Monday) + // delta = (0 - 3) % 7 + (-1) * 7 = -3 + // + // Example 3: next Monday (x = 1, day = Monday) + // delta = (0 - 3) % 7 + (0) * 7 = 4 + // (Note that we have adjusted the x value above) + // + // Example 4: next Thursday (x = 1, day = Thursday) + // delta = (3 - 3) % 7 + (1) * 7 = 7 + let delta = (day.num_days_from_monday() as i32 + - d.weekday().num_days_from_monday() as i32) + .rem_euclid(7) + + x * 7; + + d = if delta < 0 { + d.checked_sub_days(chrono::Days::new((-delta) as u64))? } else { - d += d - .date_naive() - .checked_sub_days(chrono::Days::new((days * -x as u32) as u64))? - .signed_duration_since(d.date_naive()); + d.checked_add_days(chrono::Days::new(delta as u64))? } } - Item::Relative(relative::Relative::Days(x)) => d += chrono::Duration::days(x.into()), - Item::Relative(relative::Relative::Hours(x)) => d += chrono::Duration::hours(x.into()), - Item::Relative(relative::Relative::Minutes(x)) => { - d += chrono::Duration::minutes(x.into()); - } - // Seconds are special because they can be given as a float - Item::Relative(relative::Relative::Seconds(x)) => { - d += chrono::Duration::seconds(x as i64); + Item::Relative(rel) => { + // If date and/or time is set, use the set value; otherwise, use + // the reference value. + if !date_time_set { + d = at; + } + + match rel { + relative::Relative::Years(x) => { + d = d.with_year(d.year() + x)?; + } + relative::Relative::Months(x) => { + // *NOTE* This is done in this way to conform to + // GNU behavior. + let days = last_day_of_month(d.year(), d.month()); + if x >= 0 { + d += d + .date_naive() + .checked_add_days(chrono::Days::new((days * x as u32) as u64))? + .signed_duration_since(d.date_naive()); + } else { + d += d + .date_naive() + .checked_sub_days(chrono::Days::new((days * -x as u32) as u64))? + .signed_duration_since(d.date_naive()); + } + } + relative::Relative::Days(x) => d += chrono::Duration::days(x.into()), + relative::Relative::Hours(x) => d += chrono::Duration::hours(x.into()), + relative::Relative::Minutes(x) => { + d += chrono::Duration::minutes(x.into()); + } + // Seconds are special because they can be given as a float + relative::Relative::Seconds(x) => { + d += chrono::Duration::seconds(x as i64); + } + } } Item::TimeZone(offset) => { d = with_timezone_restore(offset, d)?; @@ -476,9 +526,9 @@ fn at_date_inner(date: Vec, mut d: DateTime) -> Option, - d: DateTime, + at: DateTime, ) -> Result, ParseDateTimeError> { - at_date_inner(date, d).ok_or(ParseDateTimeError::InvalidInput) + at_date_inner(date, at).ok_or(ParseDateTimeError::InvalidInput) } pub(crate) fn at_local(date: Vec) -> Result, ParseDateTimeError> { @@ -488,10 +538,12 @@ pub(crate) fn at_local(date: Vec) -> Result, ParseDa #[cfg(test)] mod tests { use super::{at_date, date::Date, parse, time::Time, Item}; - use chrono::{DateTime, FixedOffset}; + use chrono::{ + DateTime, FixedOffset, NaiveDate, NaiveDateTime, NaiveTime, TimeZone, Timelike, Utc, + }; fn at_utc(date: Vec) -> DateTime { - at_date(date, chrono::Utc::now().fixed_offset()).unwrap() + at_date(date, Utc::now().fixed_offset()).unwrap() } fn test_eq_fmt(fmt: &str, input: &str) -> String { @@ -610,4 +662,80 @@ mod tests { assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("unexpected input")); } + + #[test] + fn relative_weekday() { + // Jan 1 2025 is a Wed + let now = Utc + .from_utc_datetime(&NaiveDateTime::new( + NaiveDate::from_ymd_opt(2025, 1, 1).unwrap(), + NaiveTime::from_hms_opt(0, 0, 0).unwrap(), + )) + .fixed_offset(); + + assert_eq!( + at_date(parse(&mut "last wed").unwrap(), now).unwrap(), + now - chrono::Duration::days(7) + ); + assert_eq!(at_date(parse(&mut "this wed").unwrap(), now).unwrap(), now); + assert_eq!( + at_date(parse(&mut "next wed").unwrap(), now).unwrap(), + now + chrono::Duration::days(7) + ); + assert_eq!( + at_date(parse(&mut "last thu").unwrap(), now).unwrap(), + now - chrono::Duration::days(6) + ); + assert_eq!( + at_date(parse(&mut "this thu").unwrap(), now).unwrap(), + now + chrono::Duration::days(1) + ); + assert_eq!( + at_date(parse(&mut "next thu").unwrap(), now).unwrap(), + now + chrono::Duration::days(1) + ); + assert_eq!( + at_date(parse(&mut "1 wed").unwrap(), now).unwrap(), + now + chrono::Duration::days(7) + ); + assert_eq!( + at_date(parse(&mut "1 thu").unwrap(), now).unwrap(), + now + chrono::Duration::days(1) + ); + assert_eq!( + at_date(parse(&mut "2 wed").unwrap(), now).unwrap(), + now + chrono::Duration::days(14) + ); + assert_eq!( + at_date(parse(&mut "2 thu").unwrap(), now).unwrap(), + now + chrono::Duration::days(8) + ); + } + + #[test] + fn relative_date_time() { + let now = Utc::now().fixed_offset(); + + let result = at_date(parse(&mut "2 days ago").unwrap(), now).unwrap(); + assert_eq!(result, now - chrono::Duration::days(2)); + assert_eq!(result.hour(), now.hour()); + assert_eq!(result.minute(), now.minute()); + assert_eq!(result.second(), now.second()); + + let result = at_date(parse(&mut "2025-01-01 2 days ago").unwrap(), now).unwrap(); + assert_eq!(result.hour(), 0); + assert_eq!(result.minute(), 0); + assert_eq!(result.second(), 0); + + let result = at_date(parse(&mut "3 weeks").unwrap(), now).unwrap(); + assert_eq!(result, now + chrono::Duration::days(21)); + assert_eq!(result.hour(), now.hour()); + assert_eq!(result.minute(), now.minute()); + assert_eq!(result.second(), now.second()); + + let result = at_date(parse(&mut "2025-01-01 3 weeks").unwrap(), now).unwrap(); + assert_eq!(result.hour(), 0); + assert_eq!(result.minute(), 0); + assert_eq!(result.second(), 0); + } } From 6e64f21967b604294e37010ec9293c39df4028a5 Mon Sep 17 00:00:00 2001 From: yuankunzhang Date: Sun, 15 Jun 2025 17:12:42 +0800 Subject: [PATCH 2/2] refactor: move fundamental combinators from mod.rs to the primitive module --- src/items/combined.rs | 2 +- src/items/date.rs | 2 +- src/items/mod.rs | 131 +++-------------------------------------- src/items/ordinal.rs | 3 +- src/items/primitive.rs | 128 ++++++++++++++++++++++++++++++++++++++++ src/items/relative.rs | 5 +- src/items/time.rs | 5 +- src/items/weekday.rs | 2 +- 8 files changed, 150 insertions(+), 128 deletions(-) create mode 100644 src/items/primitive.rs diff --git a/src/items/combined.rs b/src/items/combined.rs index c1df933..d452196 100644 --- a/src/items/combined.rs +++ b/src/items/combined.rs @@ -22,7 +22,7 @@ use crate::items::space; use super::{ date::{self, Date}, - s, + primitive::s, time::{self, Time}, }; diff --git a/src/items/date.rs b/src/items/date.rs index 86cc992..b99dc12 100644 --- a/src/items/date.rs +++ b/src/items/date.rs @@ -35,7 +35,7 @@ use winnow::{ ModalResult, Parser, }; -use super::{dec_uint, s}; +use super::primitive::{dec_uint, s}; use crate::ParseDateTimeError; #[derive(PartialEq, Eq, Clone, Debug, Default)] diff --git a/src/items/mod.rs b/src/items/mod.rs index 39fd695..28e4aa7 100644 --- a/src/items/mod.rs +++ b/src/items/mod.rs @@ -30,6 +30,7 @@ mod combined; mod date; mod ordinal; +mod primitive; mod relative; mod time; mod weekday; @@ -37,16 +38,18 @@ mod weekday; mod epoch { use winnow::{combinator::preceded, ModalResult, Parser}; - use super::{dec_int, s}; + use super::primitive::{dec_int, s}; + pub fn parse(input: &mut &str) -> ModalResult { s(preceded("@", dec_int)).parse_next(input) } } mod timezone { - use super::time; use winnow::ModalResult; + use super::time; + pub(crate) fn parse(input: &mut &str) -> ModalResult { time::timezone(input) } @@ -55,12 +58,11 @@ mod timezone { use chrono::NaiveDate; use chrono::{DateTime, Datelike, FixedOffset, TimeZone, Timelike}; +use primitive::space; use winnow::{ - ascii::{digit1, multispace0}, - combinator::{alt, delimited, not, opt, peek, preceded, repeat, separated, trace}, - error::{AddContext, ContextError, ErrMode, ParserError, StrContext, StrContextValue}, - stream::{AsChar, Stream}, - token::{none_of, one_of, take_while}, + combinator::{alt, trace}, + error::{AddContext, ContextError, ErrMode, StrContext, StrContextValue}, + stream::Stream, ModalResult, Parser, }; @@ -78,121 +80,6 @@ pub enum Item { TimeZone(time::Offset), } -/// Allow spaces and comments before a parser -/// -/// Every token parser should be wrapped in this to allow spaces and comments. -/// It is only preceding, because that allows us to check mandatory whitespace -/// after running the parser. -fn s<'a, O, E>(p: impl Parser<&'a str, O, E>) -> impl Parser<&'a str, O, E> -where - E: ParserError<&'a str>, -{ - preceded(space, p) -} - -/// Parse the space in-between tokens -/// -/// You probably want to use the [`s`] combinator instead. -fn space<'a, E>(input: &mut &'a str) -> winnow::Result<(), E> -where - E: ParserError<&'a str>, -{ - separated(0.., multispace0, alt((comment, ignored_hyphen_or_plus))).parse_next(input) -} - -/// A hyphen or plus is ignored when it is not followed by a digit -/// -/// This includes being followed by a comment! Compare these inputs: -/// ```txt -/// - 12 weeks -/// - (comment) 12 weeks -/// ``` -/// The last comment should be ignored. -/// -/// The plus is undocumented, but it seems to be ignored. -fn ignored_hyphen_or_plus<'a, E>(input: &mut &'a str) -> winnow::Result<(), E> -where - E: ParserError<&'a str>, -{ - ( - alt(('-', '+')), - multispace0, - peek(not(take_while(1, AsChar::is_dec_digit))), - ) - .void() - .parse_next(input) -} - -/// Parse a comment -/// -/// A comment is given between parentheses, which must be balanced. Any other -/// tokens can be within the comment. -fn comment<'a, E>(input: &mut &'a str) -> winnow::Result<(), E> -where - E: ParserError<&'a str>, -{ - delimited( - '(', - repeat(0.., alt((none_of(['(', ')']).void(), comment))), - ')', - ) - .parse_next(input) -} - -/// Parse a signed decimal integer. -/// -/// Rationale for not using `winnow::ascii::dec_int`: When upgrading winnow from -/// 0.5 to 0.7, we discovered that `winnow::ascii::dec_int` now accepts only the -/// following two forms: -/// -/// - 0 -/// - [+-]?[1-9][0-9]* -/// -/// Inputs like [+-]?0[0-9]* (e.g., `+012`) are therefore rejected. We provide a -/// custom implementation to support such zero-prefixed integers. -fn dec_int<'a, E>(input: &mut &'a str) -> winnow::Result -where - E: ParserError<&'a str>, -{ - (opt(one_of(['+', '-'])), digit1) - .void() - .take() - .verify_map(|s: &str| s.parse().ok()) - .parse_next(input) -} - -/// Parse an unsigned decimal integer. -/// -/// See the rationale for `dec_int` for why we don't use -/// `winnow::ascii::dec_uint`. -fn dec_uint<'a, E>(input: &mut &'a str) -> winnow::Result -where - E: ParserError<&'a str>, -{ - digit1 - .void() - .take() - .verify_map(|s: &str| s.parse().ok()) - .parse_next(input) -} - -/// Parse a float number. -/// -/// Rationale for not using `winnow::ascii::float`: the `float` parser provided -/// by winnow accepts E-notation numbers (e.g., `1.23e4`), whereas GNU date -/// rejects such numbers. To remain compatible with GNU date, we provide a -/// custom implementation that only accepts inputs like [+-]?[0-9]+(\.[0-9]+)?. -fn float<'a, E>(input: &mut &'a str) -> winnow::Result -where - E: ParserError<&'a str>, -{ - (opt(one_of(['+', '-'])), digit1, opt(preceded('.', digit1))) - .void() - .take() - .verify_map(|s: &str| s.parse().ok()) - .parse_next(input) -} - // Parse an item pub fn parse_one(input: &mut &str) -> ModalResult { trace( diff --git a/src/items/ordinal.rs b/src/items/ordinal.rs index 73cd36b..7de4f6f 100644 --- a/src/items/ordinal.rs +++ b/src/items/ordinal.rs @@ -1,13 +1,14 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -use super::{dec_uint, s}; use winnow::{ ascii::alpha1, combinator::{alt, opt}, ModalResult, Parser, }; +use super::primitive::{dec_uint, s}; + pub fn ordinal(input: &mut &str) -> ModalResult { alt((text_ordinal, number_ordinal)).parse_next(input) } diff --git a/src/items/primitive.rs b/src/items/primitive.rs new file mode 100644 index 0000000..c1f5180 --- /dev/null +++ b/src/items/primitive.rs @@ -0,0 +1,128 @@ +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +//! Primitive combinators. + +use winnow::{ + ascii::{digit1, multispace0}, + combinator::{alt, delimited, not, opt, peek, preceded, repeat, separated}, + error::ParserError, + stream::AsChar, + token::{none_of, one_of, take_while}, + Parser, +}; + +/// Allow spaces and comments before a parser +/// +/// Every token parser should be wrapped in this to allow spaces and comments. +/// It is only preceding, because that allows us to check mandatory whitespace +/// after running the parser. +pub(super) fn s<'a, O, E>(p: impl Parser<&'a str, O, E>) -> impl Parser<&'a str, O, E> +where + E: ParserError<&'a str>, +{ + preceded(space, p) +} + +/// Parse the space in-between tokens +/// +/// You probably want to use the [`s`] combinator instead. +pub(super) fn space<'a, E>(input: &mut &'a str) -> winnow::Result<(), E> +where + E: ParserError<&'a str>, +{ + separated(0.., multispace0, alt((comment, ignored_hyphen_or_plus))).parse_next(input) +} + +/// A hyphen or plus is ignored when it is not followed by a digit +/// +/// This includes being followed by a comment! Compare these inputs: +/// ```txt +/// - 12 weeks +/// - (comment) 12 weeks +/// ``` +/// The last comment should be ignored. +/// +/// The plus is undocumented, but it seems to be ignored. +fn ignored_hyphen_or_plus<'a, E>(input: &mut &'a str) -> winnow::Result<(), E> +where + E: ParserError<&'a str>, +{ + ( + alt(('-', '+')), + multispace0, + peek(not(take_while(1, AsChar::is_dec_digit))), + ) + .void() + .parse_next(input) +} + +/// Parse a comment +/// +/// A comment is given between parentheses, which must be balanced. Any other +/// tokens can be within the comment. +fn comment<'a, E>(input: &mut &'a str) -> winnow::Result<(), E> +where + E: ParserError<&'a str>, +{ + delimited( + '(', + repeat(0.., alt((none_of(['(', ')']).void(), comment))), + ')', + ) + .parse_next(input) +} + +/// Parse a signed decimal integer. +/// +/// Rationale for not using `winnow::ascii::dec_int`: When upgrading winnow from +/// 0.5 to 0.7, we discovered that `winnow::ascii::dec_int` now accepts only the +/// following two forms: +/// +/// - 0 +/// - [+-]?[1-9][0-9]* +/// +/// Inputs like [+-]?0[0-9]* (e.g., `+012`) are therefore rejected. We provide a +/// custom implementation to support such zero-prefixed integers. +pub(super) fn dec_int<'a, E>(input: &mut &'a str) -> winnow::Result +where + E: ParserError<&'a str>, +{ + (opt(one_of(['+', '-'])), digit1) + .void() + .take() + .verify_map(|s: &str| s.parse().ok()) + .parse_next(input) +} + +/// Parse an unsigned decimal integer. +/// +/// See the rationale for `dec_int` for why we don't use +/// `winnow::ascii::dec_uint`. +pub(super) fn dec_uint<'a, E>(input: &mut &'a str) -> winnow::Result +where + E: ParserError<&'a str>, +{ + digit1 + .void() + .take() + .verify_map(|s: &str| s.parse().ok()) + .parse_next(input) +} + +/// Parse a float number. +/// +/// Rationale for not using `winnow::ascii::float`: the `float` parser provided +/// by winnow accepts E-notation numbers (e.g., `1.23e4`), whereas GNU date +/// rejects such numbers. To remain compatible with GNU date, we provide a +/// custom implementation that only accepts inputs like [+-]?[0-9]+(\.[0-9]+)?. +pub(super) fn float<'a, E>(input: &mut &'a str) -> winnow::Result +where + E: ParserError<&'a str>, +{ + (opt(one_of(['+', '-'])), digit1, opt(preceded('.', digit1))) + .void() + .take() + .verify_map(|s: &str| s.parse().ok()) + .parse_next(input) +} diff --git a/src/items/relative.rs b/src/items/relative.rs index 701a197..6830082 100644 --- a/src/items/relative.rs +++ b/src/items/relative.rs @@ -37,7 +37,10 @@ use winnow::{ ModalResult, Parser, }; -use super::{float, ordinal::ordinal, s}; +use super::{ + ordinal::ordinal, + primitive::{float, s}, +}; #[derive(Clone, Copy, Debug, PartialEq)] pub enum Relative { diff --git a/src/items/time.rs b/src/items/time.rs index 7fa8d3f..ad23a2c 100644 --- a/src/items/time.rs +++ b/src/items/time.rs @@ -52,7 +52,10 @@ use winnow::{ use crate::ParseDateTimeError; -use super::{dec_uint, relative, s}; +use super::{ + primitive::{dec_uint, s}, + relative, +}; #[derive(PartialEq, Debug, Clone, Default)] pub struct Offset { diff --git a/src/items/weekday.rs b/src/items/weekday.rs index 15fff59..138e14c 100644 --- a/src/items/weekday.rs +++ b/src/items/weekday.rs @@ -23,7 +23,7 @@ use winnow::{ascii::alpha1, combinator::opt, seq, ModalResult, Parser}; -use super::{ordinal::ordinal, s}; +use super::{ordinal::ordinal, primitive::s}; #[derive(PartialEq, Eq, Debug)] pub(crate) enum Day {