-
Notifications
You must be signed in to change notification settings - Fork 7
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
12hour
format is wrong for 12:00 hour
#79
Comments
Ah yeah, I never really used that part a lot and forgot to test the logic. The code right now is:
which is obviously wrong now that I look at it again. An |
That would be great! Right now I have string concatenation with a manual check for the hour to add AM/PM :) |
Does
look more right? I'll have to add test cases for this obviously, but I might as well ask while you're here : v |
Yes that seems right? That is equivalent to what I think of as the “standard” conversation (mod being maybe neater, who knows!)
let hour = if hour = 0 then 12 else if hour > 12 then hour - 12 else hour
On Wed, Apr 10, 2024, at 7:17 PM, Darren Li wrote:
Does
` let hour = if hour = 0 || hour = 12 then 12 else hour mod 12 in
`
… look more right?
I'll have to add test cases for this obviously, but I might as well ask while you're here : v
—
Reply to this email directly, view it on GitHub <#79 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAELBJMLSU3TLOTKWG5PFXTY4XB6ZAVCNFSM6AAAAABGAZBU4SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBYGU4DGMZYGY>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Added the
Feel free to try pinning to main branch and see if it resolves your issues. Otherwise you can also wait till I've added the appropriate tests and published on opam. |
Added an additional |
Submitted PR at opam repository: ocaml/opam-repository#25754 Will close this issue when PR is merged. |
PR merged, closing issue. |
For the 12:00 - 13:00 hour (in 24hr time), if you use
~format:"{12hour:X}"
, it renders as0
, which is wrong: it should be12
. I'm assuming this is due to subtracting 12 from the hour, which is correct for 13 and up, but not for 12 (or, obviously, below).The text was updated successfully, but these errors were encountered: