This project is archived and is in readonly mode.

#748 ✓wontfix
Clemens Kofler

Date and Time classes i18n/l10n implementation

Reported by Clemens Kofler | August 2nd, 2008 @ 05:56 PM | in 2.x

The current Edge lacks proper support for i18n/l10n in the Date/Time/DateTime/TimeWithZone classes. The definitions in the locale were there but they weren't used yet.

This patch adds the missing i18n/l10n support.

Here's a list of the changes I made:

  • Removed all formats that aren't used internally by Rails from Time::DATE_FORMATS and Date::DATE_FORMATS and put them in the locale instead.
  • Renamed DATE_FORMATS to DEFAULT_FORMATS because it seems more fitting.
  • Restructured locale to match the hierarchical style I introduced for the localized NumberHelper (see this ticket.
  • Fixed formats in locale since they weren't using Rails' default formats. Also fixed the tests that failed because of this change.
  • Slightly changed the implementation of how Procs can be used as formats to make them i18n/l10n compatible. Instead of lambda { |time| time.strftime("%B #{time.day.ordinalize}, %Y %H:%M") } one should now use lambda { |time| "%B #{time.day.ordinalize}, %Y %H:%M" }. The old style should work 99% of the time - the only exception should be if %% is used in the formatting string since it would be evaluated twice.
  • Added implementation tests.
  • Updated documentation to explain how to add formats.
  • Added deprecation warnings as singleton methods for DATE_FORMATS constants.

What probably needs to be done is update the tests for custom date formats to use the locale instead of the DATE_FORMATS constant. But I don't really like touching existing (stable) tests so I didn't include the change in the patch.

Feedback, as usual, is very much appreciated.

Comments and changes to this ticket

  • Geoff Buesing

    Geoff Buesing August 3rd, 2008 @ 02:37 PM

    Building localization into Date/Time classes is a cool idea, but it seems to be a step ahead of the existing i18n/l10n strategy, which requires that you pass objects into locale-aware helpers, like so:

    i18n.l time, :format => :long
    

    Or am I missing something?

  • Clemens Kofler

    Clemens Kofler August 3rd, 2008 @ 06:40 PM

    While I agree that this is a possibility, I think it would be inconsistent with the i18n/l10n stuff that has made it into core so far.

    Pretty much everything till now has been a drop-in replacement: If I use number_to_currency, I can now remove any hacks that I used before (no more number_to_currency_with_euro and alias_method_chaining it). Same goes for ActiveRecord error messages, for example.

    Now if we don't localize date and time formatting, people have to use localization the way you wrote it: i18n.l(time, :format => :long). The Rails standard way to format dates, however, is time.to_s(:long). IMO it's quite obvious that this is inconsistent.

    While I'm perfectly aware that this is a bigger change than, say, the number helper methods, I think it's really important to not stop the localization efforts halfway through. I know that localized pluralization is still missing (and probably will stay that way for some time since it's a very complex topic), personally I don't see any reason to not include something that is quite simple and is backwards compatible (apart from the slight change in behavior for procs that I mentioned).

    We can discuss this further on the mailing list or in IRC if you think this is necessary.

  • Sven Fuchs

    Sven Fuchs August 3rd, 2008 @ 06:54 PM

    Although I agree think that this should eventually go into core, I don't think that this is a good situation. We should follow the strategy "implement as plugins - review, discuss, extract - suggest core patches" for another dev cycle for everything that not definitely needs to go into core right now (like necessary api changes, bug fixes). This strategy prooved extremely valuable in the past and we should stick to it for now even if patches will become more "obvious" over time.

    I've talked this over with Clemens on IRC and he agreed to contribute his code as a plugin, which I think is the way to go.

  • Clemens Kofler
  • Clemens Kofler

    Clemens Kofler August 3rd, 2008 @ 09:38 PM

    And here it is: http://github.com/clemens/locali.... ;-)

    So Geoff, if you really don't want to include this into core, this ticket can be closed.

  • Geoff Buesing

    Geoff Buesing August 3rd, 2008 @ 10:50 PM

    Let's leave this open -- I like the idea of #to_s being locale-aware, but I agree with Sven's approach of testing this out in a plugin first.

  • Geoff Buesing

    Geoff Buesing August 3rd, 2008 @ 10:54 PM

    • State changed from “new” to “open”
  • Joshua Peek

    Joshua Peek November 22nd, 2008 @ 07:51 PM

    This ticket is over 3 months old. Any updates?

  • Clemens Kofler

    Clemens Kofler November 23rd, 2008 @ 11:49 PM

    The plugin is still available and I've received some feedback of people who liked it. If you want it included in the core and like me to come up with a fresh patch to the current edge, I'm more than happy to do it - just let me know!

  • Yaroslav Markin

    Yaroslav Markin November 24th, 2008 @ 07:30 AM

    Plugin is really nice but I'd say stuff like this needs to be implemented in I18n itself, not on top of Rails.

  • Geoff Buesing

    Geoff Buesing February 8th, 2009 @ 05:47 PM

    @Sven, what are your current thoughts on this -- should this be considered for 3.0, or is this best left as a plugin?

  • Clemens Kofler

    Clemens Kofler February 17th, 2009 @ 04:14 PM

    Geoff, I talked to Sven today about this. We'll have a closer look at this and then let you know what we think about it.

  • Sven Fuchs

    Sven Fuchs February 20th, 2009 @ 10:50 AM

    Geoff, Clemens and I have talked this over and we think the proposed patch does a bit too much. Also, we agree with Yaroslav that some of this should be solved in I18n instead of Rails.

    We'll look into this next week and come up with separate patches for I18n and ActiveSupport.

    So, I guess you can close this ticket and we'll open a new, more focussed one then.

  • Geoff Buesing

    Geoff Buesing February 20th, 2009 @ 04:50 PM

    • State changed from “open” to “wontfix”

    Thanks for looking into this.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Pages