ice-cube-ruby / ice_cube

Ruby Date Recurrence Library - Allows easy creation of recurrence rules and fast querying
MIT License
2.41k stars 357 forks source link

Translations for date overriding main project translations #431

Open MGPalmer opened 6 years ago

MGPalmer commented 6 years ago

Hi! I've been lagging behind the ice_cube master for a while now, and while checking out the last changes, I noticed some of our date/time display was off. I tracked it down to one commit, I think: https://github.com/seejohnrun/ice_cube/commit/c7b305c3f1abc98102dd28e473ecd6983b8d3bf8

Let me try to illustrate: This is a Rails app with the ice_cube gem loaded via a Gemfile, but with a local bundler override so it uses my local repo of ice_cube. With my very old master (at d8614f739bc6edf51fb5fb18c9fd19f710c2d9da) , the default date translation looks like this:

cormorant:~/Development/promptus/sports-mssngr [master] $ rails c
Loading development environment (Rails 4.2.10)
mssngr(dev)> I18n.t("date.formats.default", locale: :de)
=> "%d.%m.%Y"
mssngr(dev)> IceCube::Schedule.new.to_s
=> "December 18, 2017"
mssngr(dev)> I18n.t("date.formats.default", locale: :de)
=> "%d.%m.%Y"

but after switching to upstream:

cormorant:~/Development/promptus/ice_cube [master] $ git co c7b305c3f1abc98102dd28e473ecd6983b8d3bf8
Note: checking out 'c7b305c3f1abc98102dd28e473ecd6983b8d3bf8'.
HEAD is now at c7b305c... Fix core extension load order between ActiveSupport and I18n

...loading the ice cube code now clobbers our translation:

cormorant:~/Development/promptus/sports-mssngr [master] $ rails c
Loading development environment (Rails 4.2.10)
mssngr(dev)> I18n.t("date.formats.default", locale: :de)
=> "%d.%m.%Y"
mssngr(dev)> IceCube::Schedule.new.to_s
=> "December 18, 2017"
mssngr(dev)> I18n.t("date.formats.default", locale: :de)
=> "%Y-%m-%d"

(btw, "%Y-%m-%d" is also not really used in germany ;))

seejohnrun commented 6 years ago

@MGPalmer what do you think of submitting a PR to correct the date format? cc / @joelmeyerhamme

MGPalmer commented 6 years ago

@seejohnrun Sure I can do that, but that wouldn't fix the translation-clobbering issue, right?

MGPalmer commented 6 years ago

Come to think of it, why does ice_cube even need to define translation keys which match the commonly used keys from rails/ruby I18n, like "date.formats.default" etc., since there are also formatting keys scoped to "ice_cube" like "ice_cube.date.formats.default"?

MGPalmer commented 6 years ago

Hah, turns out you can just remove those keys that duplicate the formatting from I18n. There weren't even in some locales. I left in the day/month names because without them some specs fail.

avit commented 6 years ago

Those keys come from rails-i18n so we'll need them to allow IceCube to work without Rails. It's all a bit mixed up, some are under ice_cube.date.defaults and some are top-level date.defaults so I'd like to see if we can separate that and use the actual i18n default keys.

I started some cleanup but I'm still thinking about how to best handle this. Maybe we can have a rake task to copy the keys we need from the rails-i18n locale files automatically instead of trying to keep them synchronized.

markedmondson commented 5 years ago

~Spent~ Wasted a lot of time on this today before finding this, we effectively got to the same point as @avit's cleanup but the requirement of date.format.default when Rails wasn't present was painful to support.

Ended up with a Railtie which tosses the IceCube translations before Rails-i18n if it's present. Slightly imperfect but good enough for our needs.

MGPalmer commented 5 years ago

I'm not working on that project anymore, but great to see someone is picking this up :)

Faq commented 5 months ago

This still active after https://github.com/ice-cube-ruby/ice_cube/pull/546 ?