Skip to content

Support weekdays in parse_datetime #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ readme = "README.md"
[dependencies]
regex = "1.9"
chrono = { version="0.4", default-features=false, features=["std", "alloc", "clock"] }
nom = "7.1.3"
78 changes: 77 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ use std::fmt::{self, Display};
// Expose parse_datetime
mod parse_relative_time;

use chrono::{DateTime, FixedOffset, Local, LocalResult, NaiveDateTime, TimeZone};
mod parse_weekday;

use chrono::{
DateTime, Datelike, Duration, FixedOffset, Local, LocalResult, NaiveDateTime, TimeZone,
Timelike,
};

use parse_relative_time::parse_relative_time;

Expand Down Expand Up @@ -168,6 +173,27 @@ pub fn parse_datetime_at_date<S: AsRef<str> + Clone>(
}
}

// parse weekday
if let Some(weekday) = parse_weekday::parse_weekday(s.as_ref()) {
let mut beginning_of_day = date
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see what the trouble was now. I forgot we needed 00:00 on the time. I'm not sure what the best way to do that is. This looks ok, but it would be great if chrono had some nice method for this, but I can't find any 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dit not find anything either :(

.with_hour(0)
.unwrap()
.with_minute(0)
.unwrap()
.with_second(0)
.unwrap()
.with_nanosecond(0)
.unwrap();

while beginning_of_day.weekday() != weekday {
beginning_of_day += Duration::days(1);
}

let dt = DateTime::<FixedOffset>::from(beginning_of_day);

return Ok(dt);
}

// Parse epoch seconds
if s.as_ref().bytes().next() == Some(b'@') {
if let Ok(parsed) = NaiveDateTime::parse_from_str(&s.as_ref()[1..], "%s") {
Expand Down Expand Up @@ -353,6 +379,56 @@ mod tests {
}
}

#[cfg(test)]
mod weekday {
use chrono::{DateTime, Local, TimeZone};

use crate::parse_datetime_at_date;

fn get_formatted_date(date: DateTime<Local>, weekday: &str) -> String {
let result = parse_datetime_at_date(date, weekday).unwrap();

return result.format("%F %T %f").to_string();
}
#[test]
fn test_weekday() {
// add some constant hours and minutes and seconds to check its reset
let date = Local.with_ymd_and_hms(2023, 02, 28, 10, 12, 3).unwrap();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oww, I really want parse_datetime_at_date to not take Local anymore. But we'll get to that in another PR. Looks great for now!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how to best not use local but agreed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, parse_datetime_at_date should just be generic over the timezone. And then it can be called with Utc, Local or FixedOffset. Like I said though, definitely out of scope for this PR.


// 2023-2-28 is tuesday
assert_eq!(
get_formatted_date(date, "tuesday"),
"2023-02-28 00:00:00 000000000"
);

// 2023-3-01 is wednesday
assert_eq!(
get_formatted_date(date, "wed"),
"2023-03-01 00:00:00 000000000"
);

assert_eq!(
get_formatted_date(date, "thu"),
"2023-03-02 00:00:00 000000000"
);

assert_eq!(
get_formatted_date(date, "fri"),
"2023-03-03 00:00:00 000000000"
);

assert_eq!(
get_formatted_date(date, "sat"),
"2023-03-04 00:00:00 000000000"
);

assert_eq!(
get_formatted_date(date, "sun"),
"2023-03-05 00:00:00 000000000"
);
}
}

#[cfg(test)]
mod timestamp {
use crate::parse_datetime;
Expand Down
2 changes: 2 additions & 0 deletions src/parse_relative_time.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.
use crate::ParseDateTimeError;
use chrono::{Duration, Local, NaiveDate, Utc};
use regex::Regex;
Expand Down
99 changes: 99 additions & 0 deletions src/parse_weekday.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.
use chrono::Weekday;
use nom::branch::alt;
use nom::bytes::complete::tag;
use nom::combinator::value;
use nom::{self, IResult};

// Helper macro to simplify tag matching
macro_rules! tag_match {
($day:expr, $($pattern:expr),+) => {
value($day, alt(($(tag($pattern)),+)))
};
}

pub(crate) fn parse_weekday(s: &str) -> Option<Weekday> {
let s = s.trim().to_lowercase();
let s = s.as_str();

let parse_result: IResult<&str, Weekday> = nom::combinator::all_consuming(alt((
tag_match!(Weekday::Mon, "monday", "mon"),
tag_match!(Weekday::Tue, "tuesday", "tues", "tue"),
tag_match!(Weekday::Wed, "wednesday", "wednes", "wed"),
tag_match!(Weekday::Thu, "thursday", "thurs", "thur", "thu"),
tag_match!(Weekday::Fri, "friday", "fri"),
tag_match!(Weekday::Sat, "saturday", "sat"),
tag_match!(Weekday::Sun, "sunday", "sun"),
)))(s);

match parse_result {
Ok((_, weekday)) => Some(weekday),
Err(_) => None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't you return a Result ?
here, it would be
Err(_) => Err("Failed to parse weekday"),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had this discussion with @tertsdiepraam, but I think the reason is this is used in parse_datetime and we only care about whether it was parsed or not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reasoning was that we only care about the parsing the whole datetime. If a weekday fails, another type of item might still succeedsl.

Copy link
Contributor Author

@philolo1 philolo1 Sep 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sylvestre should i change it to Result?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tertsdiepraam @sylvestre its been a while, i am fine with whatever but it would be good to get this pr also merged, before we forget what it was about :) .

}
}

#[cfg(test)]
mod tests {

use chrono::Weekday::*;

use crate::parse_weekday::parse_weekday;

#[test]
fn test_valid_weekdays() {
let days = [
("mon", Mon),
("monday", Mon),
("tue", Tue),
("tues", Tue),
("tuesday", Tue),
("wed", Wed),
("wednes", Wed),
("wednesday", Wed),
("thu", Thu),
("thursday", Thu),
("fri", Fri),
("friday", Fri),
("sat", Sat),
("saturday", Sat),
("sun", Sun),
("sunday", Sun),
];

for (name, weekday) in days {
assert_eq!(parse_weekday(name), Some(weekday));
assert_eq!(parse_weekday(&format!(" {}", name)), Some(weekday));
assert_eq!(parse_weekday(&format!(" {} ", name)), Some(weekday));
assert_eq!(parse_weekday(&format!("{} ", name)), Some(weekday));

let (left, right) = name.split_at(1);
let (test_str1, test_str2) = (
format!("{}{}", left.to_uppercase(), right.to_lowercase()),
format!("{}{}", left.to_lowercase(), right.to_uppercase()),
);

assert_eq!(parse_weekday(&test_str1), Some(weekday));
assert_eq!(parse_weekday(&test_str2), Some(weekday));
}
}

#[test]
fn test_invalid_weekdays() {
let days = [
"mond",
"tuesda",
"we",
"th",
"fr",
"sa",
"su",
"garbageday",
"tomorrow",
"yesterday",
];
for day in days {
assert!(parse_weekday(day).is_none());
}
}
}