-
Notifications
You must be signed in to change notification settings - Fork 460
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
Rewrite Handlers, Repeaters and improve Tagging #278
Conversation
@leejarvis have you taken a look on this? What do you think? |
I think this looks good so far. There are a few methods in |
I'll first finish this so that all tests pass and only then clean it up. Also for some places I'm not sure about best implementation so it would be nice if you can give any tips what could be made better. |
Formats: * Arrow: 3 quarters ago, before 2 quarters * Anchor: last/this/next quarter * Narrow: Q1, 2nd quarter, fourth quarter
Finally, I've something to show :)
So basically, I started this refactoring like a year ago or so. It was meant to be just few fixes, but that wasn't really possible as had to change a lot of things and now, it looks like it's quite a big rewrite. Still not yet fully finished and there's probably some bugs and things left to do but I wanted to show this what I've. All tests in
test_parsing
does pass, but some needed to be updated because previously Chronic handled incorrectly some cases. Other tests still need to be rewritten for new implementation.So anyway, what does this rewrite gives to us? This implementation will correctly parse all previously supported date/time formats with timezone support, which will resolve dozens of issues. Also DST changes won't ruin dates on other days (loads of issues like #179). And will give good foundation to implement a lot more date/time formats very simply. But most importantly Chronic will finally parse dates sanely. I'll try to explain, what I mean by sane parsing.
Take a look at this image and example code
Next really good part is, awesome tagging support (
definition.rb
), basically, it's a RegExp for date parsing, it should be really easy to add new date/time formats and override/monkey-patch if defaults doesn't suit. For examplethis definition will match only if all criteria will pass, that is, first token, must have
) or
MonthName
tag (eg. jan, january, etc), then optionally eitherSeparatorSpace
(SeparatorDash
(-
), thenScalarDay
and next it should not beSeparatorSpace
followed byUnit
(eg. 'year'), if it matches thenhandle_mn_sd
will be called. In simpler words:optional
means match ANY from array and:none
means don't match this array.Okay, I think I've described main changes and I suggest to just look at code and try it yourself, some old Chronic issues should be already resolved here, but mostly this is just main work done to be much easier fix everything later and create the most robust parsing library :) I really like some parts of this implementation, but now since I had to add a lot of features can see some things are getting really complex and ugly. But still IMO this is way better than was before and most importantly can easily add all date/time formats you've ever wanted. This is just start and like I said it's not ready, so shouldn't be merged yet.