Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: add date and time functionality #4904
base: master
Are you sure you want to change the base?
feat: add date and time functionality #4904
Changes from 66 commits
9c1be6c
856f569
a9bf653
36108dd
a2e3a4e
21563e8
e0da8a6
5e269e2
8d0f196
2c8ef66
858415c
882fb0e
5c01abe
cf9b567
5d058ad
4e3b9b1
abfd3be
bd236c1
4032ad2
2376e62
d53c100
f724551
248ed5e
58a446b
c480249
9abb7b6
e9be735
89cfd2c
b193aac
a48f4ff
7b5754e
842fb3b
3b83c6c
d1bef18
ee0578f
ab1d8cd
cafb98f
c82e856
073409d
091a969
6c3cec4
c30e4a1
ada5670
408fc65
3f9899b
ebadb4f
8e0794f
0f755ec
751b852
ec3968c
86fec58
3dabdcd
1cccd65
b1da2da
7bc1a7c
fe48007
f85c0cf
79c90c9
b32c8a6
60f5b92
27e9893
8d23eb0
b84a0e2
3031f26
16b4a72
c772bd7
cab1802
d152573
c6659e9
594aa87
5367936
18feda1
b213fc7
9e74aac
fb8f037
d353fa6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that it is currently not possible to parse dates with small or large years (like negative years or years which are not four digits long.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to https://www.tondering.dk/claus/cal/iso8601.php, ISO8601 says that five digit years are to be written
+10000
, with a leading plusThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is
date%
supposed to be ISO 8601 conform? I went through the bug tracker of https://github.com/time-rs/time and https://github.com/chronotope/chrono to look for some parsing bugs etc. that might show interesting edge cases and there are tickets that contain stringsdate%
cannot parse but their 8601 parser can parse, for example:Are these in scope?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that these are not supposed to be compliant, mainly because it seems we cannot use
.
in our notation for parsing reasons? It might be a good idea to change the notation from separating date and time with a colon to separating them with aT
instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also looked in ISO 8601:2004 (I haven't got hold of ISO 8601:2019 yet, but I might later), and there for the decimal separator they allow
.
and,
and comma is preferred over period. Colon is disallowed. So one could argue that the best format would bedate% YYYY-MM-DDTHH:mm:ss,sssssssss
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm having problems trying to create a syntax with
noWs 'T' noWs
, thequotation
gets confused with something like$date:dateT$time:time
. The other problem is that thesssssssss
should be the fractional part, not the nanoseconds part, so if someone writes13:12:11,1
the 1 will be understood as0000000001
nanoseconds.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$(date)T$(time)
seems to work.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried using your suggestion with
syntax date noWs "T" noWs time : datetime
, but I'm still running into issues. specifically, when I changed this line to(datetime| $(date)T$(time))
, I got anunexpected token 'T'; expected ')'
error for some weird reason.