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

Fix 12hr time formatting #4988

Merged
merged 1 commit into from Sep 21, 2017
Merged

Fix 12hr time formatting #4988

merged 1 commit into from Sep 21, 2017

Conversation

molovo
Copy link
Contributor

@molovo molovo commented Sep 16, 2017

Currently, the time formatting directives %I and %l return 0 (zero) for midnight/midday, yet in most locales 12 would be preferred. This PR adds a simple ternary check to return 12 when time.hour % 12 resolves as zero.

@asterite
Copy link
Member

Thank you! Would be nice to see how other languages behave here.

Also, without specs this is impossible to merge.

@oprypin
Copy link
Member

oprypin commented Sep 16, 2017

Python 3.6.2
>>> import datetime
>>> datetime.time(0, 0, 0).strftime("%l %p")
'12 AM'
>>> datetime.time(12, 0, 0).strftime("%l %p")
'12 PM'

@Sija
Copy link
Contributor

Sija commented Sep 16, 2017

Ruby 2.4.1

DateTime.new(2015, 1, 1, 0, 0, 0).strftime("%l %p")
# => "12 AM"
DateTime.new(2015, 1, 1, 12, 0, 0).strftime("%l %p")
# => "12 PM"

Currently, the time formatting directives `%I` and `%l` return `0` (zero) for midnight/midday, yet in most locales `12` would be preferred. This PR adds a simple ternary check to return `12` when `time.hour % 12` resolves as zero.
@molovo
Copy link
Contributor Author

molovo commented Sep 16, 2017

@asterite 👍 I've amended my original commit to add specs

@molovo
Copy link
Contributor Author

molovo commented Sep 17, 2017

Hmm, is that failure on x86_64 because of something I've done? It passed on i386

@oprypin
Copy link
Member

oprypin commented Sep 17, 2017

VM ran out of memory

@Sija
Copy link
Contributor

Sija commented Sep 21, 2017

🕚 to 🚢

@RX14 RX14 added this to the Next milestone Sep 21, 2017
@RX14 RX14 merged commit 788e069 into crystal-lang:master Sep 21, 2017
@asterite
Copy link
Member

@RX14 Thank you! Your help is really appreciated 🙇

@Sija And you too! You have an 🦅 pair of 👀 for everything! 🌈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants