Proposal: Small (but important!) API change/addition in SimpleBackend#localize

[Clemens Kofler]

Hi there,

I've only been following your project for a week or so now but it got
me really excited because it fixed some of my everyday problems of
using Rails in a German speaking country.

If you're watching Rails on GitHub you probably also saw my pretty
massive patch that was merged into Edge yesterday:
http://github.com/rails/rails/commit/fea7771d2294bd810d04c03c125ceac5d6bb9806.
It basically makes *all* NumberHelper methods are localized. Before
the patch this was only the case for number_to_currency.

Anyway, yesterday I wanted to start on implementing l10n support in
the Date and Time classes. The groundwork was laid (i.e. the locales
are there) but it seems that you either accidentally or by intention
left out the implementation for localizing the date and time formats.

The problem with the current implementation of localize is that it
makes it really hard to localize formats while keeping the old
functionality in Rails. To say it more clearly: It's hard to make the
tests pass. This is because the localize method treats formats that it
can't find as if the were strftime compatible strings. You even have a
comment there that reads: # TODO raise exception unless format found?

Instead of copying the stuff here, check out this gist if you want to
know the way I'd love to implement it: http://gist.github.com/3351

To make this work it'd be great to have a small change in the way
SimpleBackend#localize currently works. I would like it to have a
different behavior for the format parameter - it should act
differently for symbols and strings. This would still be true to the
rest of the API since the default method has a similar approach.
Basically something like the second file in the gist: http://gist.github.com/3351

First of all, this cleans up the API and makes it more evident that
the method is really doing two totally different things for different
types of parameters.
Second, it's IMO more logical. A symbol denotes a format (or, in Ruby
speak, a key in a hash) whereas a string is interpreted as a strftime
string.
Lastly and most importantly, it makes it easier for other people to
use your library because it raises an exception when an undefined
format is passed so that the application/framework that uses the
library can decide on how it handles this. You could even go a step
further and add an else statement to the case-when that raises
something like a I18n::InvalidFormat exception.

The concrete advantage for Rails (and for others that use your
library) is evident if you look at the other files in the gist. With
the current implementation I need to do a lookup first and only call
localize if I can pass it an existing format because due to the
current implementation in Rails I can't use your strftime fallback.
Plus, strftime(:non_existent) wouldn't produce sensible output anyway.

Another option would probably be to support a :default option for
localize as translate does. But to be honest, I don't think this would
be a really good solution because as I said earlier, a library should
IMO delegate as much error handling as possible to the client that
uses it (in this case Rails).

I'm putting this up for discussion but I think it would be a really
big step in the right direction. If you decide that the points I make
are valid, please make sure that you integrate them ASAP. I'd really
like to avoid rolling out Rails' next feature release with some half-
assed and unfinished i10n. ;-)

Looking forward to hearing your comments.
- Clemens

[Sven Fuchs]

Hi Clemens,

thanks for the email :)

For clarity I'd like to add what we've discussed on IRC:

Right now we have the :default option in #translate behaving so that

- when :default => :default is a Symbol, :default will be translated
and the translation used as a default (i.e. returned)
- when :default => 'default' is a String, the string will be used as a
default directly.

I think we should have the :format option behaving in the same way.
Although that's different from the implementation you whipped up on
gist :)

IMO the :format option should behave like this:

- when :format => :key is a Symbol, :key will be translated *and* used
as a format string
- when :format => '%w' is a String, '%w' will be directly used as a
format string

This would still raise the MissingTranslationData exception if the
translation of the Symbol fails, so you can rescue that in the Rails
code.

Do you think that would work?

--
sven fuchs sven...@artweb-design.de
artweb design http://www.artweb-design.de
grünberger 65 + 49 (0) 30 - 47 98 69 96 (phone)
d-10245 berlin + 49 (0) 171 - 35 20 38 4 (mobile)

[Clemens Kofler]

Of course you're right about the wicked implementation in the gist. I
just changed it so that it makes sense - sorry about that.

One more thing that I nearly forgot: It'd be great if you could also
include the option to pass a proc as a format like Rails does it at
the moment. I think passing around closures is a pretty common pattern
in Rails and Ruby in general so IMO that'd make total sense.

> >http://github.com/rails/rails/commit/fea7771d2294bd810d04c03c125ceac5...

> sven fuchs                      svenfu...@artweb-design.de

[Sven Fuchs]

Hi Clemens,

looks good.

How about this: http://gist.github.com/3436

That way, :default and :format would behave exactly the same. One
could use lamdas for :default (makes sense, imo) and an Array
for :format (same way as with :default)

I18n.t lambda{|key| 'translation' } # => 'translation'
I18n.l time.now, :format => [:not_here, :short] # looks up :not_here
as a time format and if not found uses the :short time format

Not sure about the naming of the arguments in #resolve (what's a
better idea?) and, obviously, the :raise option would not work as
expected, yet. We'd need to figure out how to use it consistently for
the various cases.

Mind to play with some unit tests etc.?

--
sven fuchs sven...@artweb-design.de