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

Add support for meridian indicators on Date objects #640

Merged

Conversation

movermeyer
Copy link
Contributor

What are you trying to accomplish?

Fixes #639

What approach did you choose and why?

Assume that the hour of a Date is always 0. Reading the source for Date, it seems that hour is set to 0 and cannot be changed, regardless of how you create the Date.

My higher-level attempts also failed to set it to anything (adding confidence that I didn't misread the source. For example:

(byebug) DateTime.jd(2451944.45).to_datetime.iso8601
"2001-02-03T10:48:00+00:00" # Note: halfway through the day
(byebug) Date.jd(2451944.45).to_datetime.iso8601
"2001-02-03T00:00:00+00:00" # but not when using `Date`

localize is only meant for Date, DateTime, and Time objects anyway, so I'm not concerned. Besides, if one were to pass in a different object into localize that didn't respond to hour, then defaulting to 0 would be consistent with the values used in other date libraries. 🤷

What should reviewers focus on?

Is there a better way to do this?

Another way to do this would be to bypass the private method safety for Date objects by using send:

- when '%p' then I18n.t!(:"time.#{object.hour < 12 ? :am : :pm}", :locale => locale, :format => format).upcase if object.respond_to? :hour
+ when '%p' then I18n.t!(:"time.#{object.send(:hour) < 12 ? :am : :pm}", :locale => locale, :format => format).upcase if object.respond_to?(:hour) || object.is_a?(::Date)

Technically this would run the risk of Ruby changing the private implementation of Date#hour, but that feels like it could be an acceptable risk to me?

I mostly chose the 0 defaulting way since it felt cleaner and didn't rely on send, but I have no strong preferences either way. I'm happy to change it if people feel strongly otherwise.

The impact of these changes

Formats with Meridian indicators will once again work with Date objects.

@radar radar merged commit 1d886ea into ruby-i18n:master Nov 13, 2022
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.

[BUG] %P gets stripped from localize formats for Date objects
2 participants