Skip to content
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

Add support to parse relative modifiers from date strings #57

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dcechano
Copy link

My apologies for the long delay. This PR adds date modification parsing such as 2022-05-15 +1 month. This PR added the core functionality to parse_relative_time and integrated the functionality to lib.rs, specifically pub fn parse_datetime.

This did require moving things around. Specifically pub fn parse_datetime is now solely responsible for parsing date strings instead of passing off parsing to pub fn parse_datetime_at_date which is what it did before. The reason for this change is that parse_datetime_at_date was awkward.

Currently pub fn parse_datetime takes a date string and passes of the input to pub fn parse_datetime_at_date with an additional Local::now() argument and does the actual parsing there. This is awkward because according to its current docs parse_datetime_at_date takes strings such as +3 days and performs the modification on the given input DateTime. Allegedly doing no date string parsing. But it did do parsing. In fact, it did mostly parsing and only really performed its documentation described behavior after trying parsing first. Almost like it was an after thought and not its primary responsibility. 90% of the behavior was ignoring its DateTime<Local> input because most of the time it was there just in case it was needed. This made integrating my new modification parsing awkward. I hope that all made sense.

TLDR I moved the parsing out of parse_datetime_at_date and into parse_datetime to make its function consistent with its documentation. Now parse_datetime_at_date still parse strings such as +3 days and only that.

The rest is what should be as expected. This PR is to eventually address 3505 and is a more robust PR to my first attempt here.

Thank you for considering my PR. This was a big one so I fully anticipate a bit of feedback and requests for tweaks. Anyway, Happy New Year!

@sylvestre
Copy link
Contributor

sorry but it needs rebasing :(

@xeruf
Copy link

xeruf commented Aug 15, 2024

@dcechano picking this back up? Or found an alternative?

@dcechano
Copy link
Author

@dcechano picking this back up? Or found an alternative?

Unfortunately, I'm wayy smooth brained to figure out how to resolve the merge conflicts. If you want to take a stab at it you are more than welcome to give it a go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants