-
Notifications
You must be signed in to change notification settings - Fork 182
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
Fix an encoding error with the hour cycle #841
Conversation
Codecov Report
@@ Coverage Diff @@
## main #841 +/- ##
==========================================
- Coverage 74.80% 74.78% -0.02%
==========================================
Files 198 198
Lines 12762 12762
==========================================
- Hits 9546 9544 -2
- Misses 3216 3218 +2
Continue to review full report at Codecov.
|
Pull Request Test Coverage Report for Build c1704d9bfea998cf8db3371c90745bfb0c2aa3fa-PR-841
💛 - Coveralls |
Hour::H11 => 'K', | ||
Hour::H12 => 'h', | ||
Hour::H23 => 'H', | ||
Hour::H24 => 'k', | ||
Hour::H23 => 'k', | ||
Hour::H24 => 'H', |
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.
Reviewing: According to UTS 35:
https://unicode.org/reports/tr35/tr35-dates.html#Date_Field_Symbol_Table
h = Hour [1-12] = h12
H = Hour [0-23] = h23
K = Hour [0-11] = h11
k = Hour [1-24] = h24
j = locale preference
J = locale preference, omit day period
So, wasn't it correct before this change?
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.
Yeah, I screwed this up in this PR. I got confused that h12 and h23 are what are stored in skeletons, and then misread the symbol information. Thanks for double checking this. I'll try and sort it out in my other PR.
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.
The h
H
being h12
and h23
, and then K
being h11
and k
being h24
is inconsistent in what symbols match that my brain refuses to parse things correctly.
I confused the 'H' and 'k' symbols when doing the skeleton work. This is causing issues with implementing #671. There's also an error that can be seen in the French locale where the wrong hour cycle is used.