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

feat(core): Add "relative dates" #185

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dtomvan
Copy link

@dtomvan dtomvan commented Jun 22, 2024

I was looking at your project and really liked it, especially for some quick
date calculations, such as #2023-05-03# - #2024-06-22# in months;weeks;days
which turned out to be so simple to do compared to spinning up either a
spreadsheet, node.js, python etc. What I was missing though was, as you might've
guessed, a feature to specify relative dates such as today:-1... This way the
above calculation could've been just:
#2023-05-03# - #today# in months;weeks;days! Hence this PR.

As for the code, it is a tad copy-pasty, but the MVP is there. I constrained it
to "just" weeks and days, as to keep it a bit more sane. I hope you like the
idea!

I was looking at your project and really liked it, especially for some quick
date calculations, such as `#2023-05-03# - #2024-06-22# in months;weeks;days`
which turned out to be so simple to do compared to spinning up either a
spreadsheet, node.js, python etc. What I was missing though was, as you might've
guessed, a feature to specify relative dates such as `today:-1`... This way the
above calculation could've been just:
 `#2023-05-03# - #today# in months;weeks;days`! Hence this PR.

As for the code, it is a tad copy-pasty, but the MVP is there. I constrained it
to "just" weeks and days, as to keep it a bit more sane. I hope you like the
idea!
@dtomvan
Copy link
Author

dtomvan commented Jun 22, 2024

Whoops, I see the tests aren't passing. Is it because I technically did a breaking change in the datepatterns.txt? Even though I just added optional parts to existing patterns...

@tiffany352
Copy link
Owner

Sorry for the late reply. I like the idea, I want to get this merged. This changes look good, but there are a few things that need to be addressed:

  • The now() calls will crash in wasm builds unfortunately, so it's important to use the Context::now field instead. That will require some plumbing to make it available here.
  • There should be some new unit tests added, like in tests/query.rs where you can run a rink query all the way from input text to output text.

I took a look at the failing datepattern tests, it seems to have been a bug in how optional patterns are matched. Not sure how to fix that, but the workaround was pretty simple. Changed:

[today[:[adddays][-subdays]]] hour12:min[:sec] meridiem[ offset]
[today[:[adddays][-subdays]]] hour24:min[:sec][ offset]

to this:

today[:[adddays][-subdays]] hour12:min[:sec] meridiem[ offset]
today[:[adddays][-subdays]] hour24:min[:sec][ offset]
hour12:min[:sec] meridiem[ offset]
hour24:min[:sec][ offset]

I can take a look into the other issues once I have more time. Thanks for working on this!

@dtomvan
Copy link
Author

dtomvan commented Jun 28, 2024

That's alright, I'm not too active on Github anyways... I don't really know your codebase well, so I hope I'm not impolite if I let you figure out how to fix those issues for now. If you do need it for some reason I'm happy to help, but I've got some other priorities right now. Nevertheless I like your program very much as it's very useful. I've got it installed as a PWA on my android phone and I use it for quick calculations on my computer as well. Thank you very much for creating this useful calculator! It seems that this is the only simple, self-contained, unit-aware one out there.

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.

None yet

2 participants