-
Notifications
You must be signed in to change notification settings - Fork 6
Timezones; support for in x units; breaking changes.
#4
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
base: master
Are you sure you want to change the base?
Conversation
…dded 'select instant' config option to fix selectors like 'in a day', 'in an hour', etc.
|
@JellyWX , first of all, thank you for taking to trouble to submit this pull request. You should know up front that it will take me a little while to give this proper attention.
Before I get to any reviewing, which in any case won't happen today, a few points. Is the new functionality in here something that you yourself need? If this isn't answering an immediate need or fixing an existing bug, I'm a little reluctant to merge it. Every feature added imposes a cost on future maintenance, and as I said above I'm a little lazy. I notice you didn't add any tests aside from a what I assume is a proof of concept executable. The tests are easy to write now that there's a pattern to follow. Anyway, again, thank you. I'll try to keep this on my queue to review soon. |
|
No problem, totally understand. And yes, I'm going to try and adapt this to replace dateparser's functionality in one of my projects. I've already found a few strange edge cases for the parser that for some reason dateparser will process and people are actively using. So I'll continue to expand that on my own fork. I understand not wanting to add additional functionality. I can write up some tests anyway, but potentially it'd be good to just keep myself a separate fork, since there's a lot of stuff that I don't need and don't want to maintain like the SMALL_GRAMMAR and period selection, and possibly lots of weird parser cases I'll need to add |
|
Hi! I am very much in need of "in X units" functionality, do you have any plans to merge this? It's been a few years now. |
I will look into this this coming weekend. |
|
Similar to your original comment, I'm now busy with work and rarely get to write Rust.
I hope this MR might still be fine, but in the case that it isn't I probably won't have time to fix it- sorry!
…-------- Opprinnelig melding --------
Den 18.08.2025 12:33, David skrev:
dfhoughton left a comment [(dfhoughton/two-timer#4)](#4 (comment))
> Hi!
>
> I am very much in need of "in X units" functionality, do you have any plans to merge this? It's been a few years now.
I will look into this this coming weekend.
—
Reply to this email directly, [view it on GitHub](#4 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/ABKR5QR7DYQC5B6CGVFW2733OG2YNAVCNFSM6AAAAACD2CE5K6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCOJWGI3TMMJRGA).
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
This is still a draft- I haven't tested this fully yet.
1: timezone support
Just a case of going through and replacing
NaiveDateTimewithDateTime<T>. In the current iteration, Config must be initialized with a DateTime to fill out the type parameter. I'm trying to figure out if this can be avoided, to retain the old Config interface...update on this, seems it can't be easily avoided since Chrono doesn't expose a
nowmethod onTimeZone. So this is an unfortunate breaking change2: support 'in x units'
Parser can now handle 'in 10 seconds', 'in 12 days', etc.
Again, this is still pretty draft. I need to go through and check it hasn't disrupted any other parsing, and that it will recognise other common phrases like 'in a day', or 'in a week'.
3: allow 'on', 'a', 'an' prefixes
Parser can handle 'on Friday', 'in an hour', 'a minute before 8pm', etc.
4: add config option to aid with 'in a day'/'a day from now' parser patterns
Potentially there was just a bug here and so this config option should be taken out, however 'a day from now' would return the entire period encapsulating that day, rather than the instant that is one day from now. The config option 'select_instant' fixes this
Other stuff changed: