ruby-i18n / i18n

Internationalization (i18n) library for Ruby
MIT License
977 stars 408 forks source link

Add support for meridian indicators on `Date` objects #640

Closed movermeyer closed 1 year ago

movermeyer commented 1 year ago

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.