String interpolation

[geekQ]

= String interpolation

As already mentioned by Masao and Mateo the default active
record messages should be changed according to the pattern
"{{attribute}} should not be blank"

There is no need for a switch to turn on/off the
concatenation. The desired result can be controlled
entirely by the placeholder. Either use the placeholder in
your message or not, compare:

"{{attribute}} should not be blank"
- attribute name is included

This should not be blank"
- attribute name is not included


== Symbols and Strings

Even if you see a string literal in
_("%{attribute} is too long (maximum is %{maximum} characters)")
what it actually means is a key to translation string
catalog. So conceptually it is closer to a symbol than to a
string. So universal usage of symbols for translatable
message is perfect.

However I strongly believe that Rails.I18n should allow arbitrary
content there without any restrictions on the usage of dots,
escaping or double escaping of backslashes or similar. So
that complete sentences can be used here without fear.

The gettext like fall back functionality on the other hand
can be implemented by the gettext backend and does not need
to be considered in the Rails.I18n.

Using of the deferred (or as Masao said "lazy") translation
string selection and parameter interpolation can reduce the
frequency of lamdba usage. But we probably need it anyway,
can discuss on the other thread
http://groups.google.com/group/rails-i18n/browse_thread/thread/2466a1f15e447b6e


== Placeholder syntax

Are there particular reasons for choosing double curly
braces in Rails.I18n or for choosing %{} in gettext?

Just wanted to be sure we have the best solution.


== ActiveModel::Errors.add vs. a[]

We could probably combine the choosing of the translation
string and providing additional parameters for string
interpolation in the same method, like
http://github.com/geekq/rails/blob/ad46280cf63cecb0016e69d52733c2f5279dce00/activemodel/lib/active_model/errors.rb#L53

The Rails 3 trunk uses a[], but it does not provide enough space
for parameters.

These examples written one-two weeks ago use the underscore method,
just imagine I18n.t instead of _ and {{foo}} instead of %{foo}

I've also adjusted the validates_length_of as an example.
Please have a look at
http://github.com/geekq/rails/blob/master/activemodel/lib/active_model/validations/length.rb#L71

record.errors.add(attr, options[:too_short],
:minimum => option_value.begin)


== Reference

A complete implementation by Masao for the pre Rails 3
version can be found here and perfectly illustrates
mentioned concepts

http://github.com/mutoh/gettext_activerecord/blob/3ffab6dab1badaf1e273960af0bbbbc4688a74b0/lib/gettext_activerecord/validations.rb#L88


--
Vladimir
http://blog.geekq.net

[Sven Fuchs]

On 01.05.2009, at 19:15, geekQ wrote:
> As already mentioned by Masao and Mateo the default active
> record messages should be changed according to the pattern
> "{{attribute}} should not be blank"
>
> There is no need for a switch to turn on/off the
> concatenation. The desired result can be controlled
> entirely by the placeholder. Either use the placeholder in
> your message or not, compare:
>
> "{{attribute}} should not be blank"
> - attribute name is included
>
> This should not be blank"
> - attribute name is not included

Do I understand that right that you propose that there's only one
message and the dev controls the inclusion of the attribute by the
message? Or would we have more than one message per validation?

I'm not sure merging message and full_message into one concept is
sufficient. Wouldn't that remove flexibility from the validations API?

E.g. people might want to use both full_message (in a flash at the top
of the page) and message (beneath the individual form fields) on the
same page. Or they might want to use them separately in different
contexts.

But let's try to identify options we have to solve this.

1. "Smart mode" - the full_message method looks at the message and
interpolates the attribute name if the translation has that
placeholder, otherwise concats it
2. "Join functionality" - message and full_message are the same
concept, the user controls the behaviour through including the
attribute placeholder
3. "More contexts" - similar to 2.) but we allow to define a separate
full_message translation through a separate scope (for defining them
on the AR model class there would need to be another validation
option). full_message could use this scope as key and the message key
as a default.

Looking at the current implementation, one problem I see with 1. and
3. is that they would (unless I'm mistaken) require significant
refactoring to ActiveRecord::Errors. Currently the validation methods
(e.g. validates_presence_of) check the model and then call #add or one
of its variants. #add then calls #generate_message and adds the result
to the errors. #generate_message already calls the I18n API and
generates the translated message. So @errors obviously holds
"finalized" translations. When the user now calls full_messages
there's no way to get hands on the original translation keys and
interpolation values etc.

> == Symbols and Strings
>
> Even if you see a string literal in
> _("%{attribute} is too long (maximum is %{maximum} characters)")
> what it actually means is a key to translation string
> catalog. So conceptually it is closer to a symbol than to a
> string. So universal usage of symbols for translatable
> message is perfect.
>
> However I strongly believe that Rails.I18n should allow arbitrary
> content there without any restrictions on the usage of dots,
> escaping or double escaping of backslashes or similar. So
> that complete sentences can be used here without fear.

I'm not sure I understand the problem with the escaping/backslashes.
Can you give an example?

Regarding the dot as a separator I agree that this is indeed a
shortcoming of the current API.

To fix that I see the following options:

1. remove the dot separation syntax from the API
2. make the separator globally configurable
3. make the separator configurable per API call (as an option on
#translate)

I guess 1. is not a real option.

2. would be slightly simplify things but might result in clashes
across libraries. Rails core could even be made avoiding this syntax
and instead use the :scope => [:foo, :bar] syntax instead (would also
require some work though). But then still plugins/engines/libraries
will most probably be coded using a dot separating syntax and that
would clash with users who want to set the separator to something else.

I can not see any problems with 3. though.

As full sentences as keys only make sense for people who want to treat
the key also as a default translation we can easily add the separator
to the call in a helper:

def _(msgid)
I18n.t(msgid, :default => msgid, :separator => whatever)
end

The separator "\000" is a bit problematic here because that's the only
character that can not be included in a Symbol in Ruby. Any other non-
printable character should work fine for separating scopes ("contexts"
in gettext).

> The gettext like fall back functionality on the other hand
> can be implemented by the gettext backend and does not need
> to be considered in the Rails.I18n.

Of course, but that would change the behaviour of #translate, reduce
interoperability and make things harder to understand for the user.
I'd therefor strongly argue for adding this to a helper on top of
#translate. _() seems just fine to me.

> Using of the deferred (or as Masao said "lazy") translation
> string selection and parameter interpolation can reduce the
> frequency of lamdba usage. But we probably need it anyway,
> can discuss on the other thread
> http://groups.google.com/group/rails-i18n/browse_thread/thread/2466a1f15e447b6e
>
>
> == Placeholder syntax
>
> Are there particular reasons for choosing double curly
> braces in Rails.I18n or for choosing %{} in gettext?
>
> Just wanted to be sure we have the best solution.

IIRC the reason for why people voted for {{}} over %{} is that it's
harder to type. Today I kind of regret we've picked that one. Not sure
it's an important issue though.

> == ActiveModel::Errors.add vs. a[]
>
> We could probably combine the choosing of the translation
> string and providing additional parameters for string
> interpolation in the same method, like
> http://github.com/geekq/rails/blob/ad46280cf63cecb0016e69d52733c2f5279dce00/activemodel/lib/active_model/errors.rb#L53
>
> The Rails 3 trunk uses a[], but it does not provide enough space
> for parameters.
>
> These examples written one-two weeks ago use the underscore method,
> just imagine I18n.t instead of _ and {{foo}} instead of %{foo}
>
> I've also adjusted the validates_length_of as an example.
> Please have a look at
> http://github.com/geekq/rails/blob/master/activemodel/lib/active_model/validations/length.rb#L71
>
> record.errors.add(attr, options[:too_short],
> :minimum => option_value.begin)

I'm not quite sure I follow here. What exactly do you propose? What's
the problem solved here?

At least from reading the code I wonder why here

http://github.com/geekq/rails/blob/ad46280cf63cecb0016e69d52733c2f5279dce00/activemodel/lib/active_model/errors.rb#L55
http://github.com/geekq/rails/blob/ad46280cf63cecb0016e69d52733c2f5279dce00/activemodel/lib/active_model/errors.rb#L60

both interpolation and evaluation of lambdas happens. I'd guess that
really should happen through the API.

[Sven Fuchs]

So, what's the requirements a solution to full_messages needs to
fullfil?

Must have:

1. allows to access both message (e.g. "is invalid") and full_message
(e.g. "Title is invalid.") separately on the same attribute during the
same request.
2. allows to configure messages through AR validates_* class method
statements (and as such supports source code parsers)
3. allows to configure messages through translation keys (based on
model class/attribute names including inheritance, see [1])
4. does not require to configure anything for people who are just
working with :en (i.e. the solution supports the current behaviour)

Nice to have:

5. ideally does not use concatenation at all (might still do that
for :en)

Does everybody agree? Anything else?

[1] http://guides.rubyonrails.org/i18n.html#translations-for-active-record-models

[Masao Mutoh]

Hi,

On Sat, 2 May 2009 13:00:56 +0200
Sven Fuchs <sven...@artweb-design.de> wrote:

> Looking at the current implementation, one problem I see with 1. and
> 3. is that they would (unless I'm mistaken) require significant
> refactoring to ActiveRecord::Errors. Currently the validation methods
> (e.g. validates_presence_of) check the model and then call #add or one
> of its variants. #add then calls #generate_message and adds the result
> to the errors. #generate_message already calls the I18n API and
> generates the translated message. So @errors obviously holds
> "finalized" translations. When the user now calls full_messages
> there's no way to get hands on the original translation keys and
> interpolation values etc.

Yes. That's the problem.
Application may want to show messages in 2 languages, but can't do so now.

> > == Placeholder syntax
> >
> > Are there particular reasons for choosing double curly
> > braces in Rails.I18n or for choosing %{} in gettext?
> >
> > Just wanted to be sure we have the best solution.
>
> IIRC the reason for why people voted for {{}} over %{} is that it's
> harder to type. Today I kind of regret we've picked that one. Not sure
> it's an important issue though.

ruby-1.9.x has %{}, %<>.
To use ruby-1.9 standard syntax will be more familier with
non-i18n people and more flexible.

% irb19
irb(main):001:0> RUBY_VERSION
=> "1.9.1"
irb(main):002:0> "%{attribute} is blank." % {:attribute => "foo"}
=> "foo is blank."
irb(main):003:0> "%<value>2.5f" % {:value => 1.0/3}
=> "0.33333"
irb(main):007:0> sprintf("%{attribute} is blank.", :attribute => "foo")
=> "foo is blank."

--
Masao Mutoh <muto...@gmail.com>

[mateo murphy]

On Sat, May 2, 2009 at 7:00 AM, Sven Fuchs <sven...@artweb-design.de> wrote:

> Do I understand that right that you propose that there's only one
> message and the dev controls the inclusion of the attribute by the
> message? Or would we have more than one message per validation?
>
> I'm not sure merging message and full_message into one concept is
> sufficient. Wouldn't that remove flexibility from the validations API?

I think having more than one message may be the best way to properly
support a full range of languages. The problem is that the current
flexibility is highly restrictive in other ways.

> But let's try to identify options we have to solve this.
>
> 1. "Smart mode" - the full_message method looks at the message and
> interpolates the attribute name if the translation has that
> placeholder, otherwise concats it

The problem with this is that there may be cases where you want
neither interpolation nor concatenation, so there should be a way to
do that. e.g.

validates_acceptance_of :tos, :message => 'You must accept the
terms of service"

> 2. "Join functionality" - message and full_message are the same
> concept, the user controls the behaviour through including the
> attribute placeholder
> 3. "More contexts" - similar to 2.) but we allow to define a separate
> full_message translation through a separate scope (for defining them
> on the AR model class there would need to be another validation
> option). full_message could use this scope as key and the message key
> as a default.

How about this; we provide a full_message context, with the following behavior:

1) If message and full_messsage are both defined, each is used in it's
appropriate context
2) If just message is defined, full_message is generated by
concatenating the attribute name to the beginning
3) If just full_message is definied, it is used for both cases.

This would provide the flexibility required as well as retaining full
compatibility with the current implementation.

> Looking at the current implementation, one problem I see with 1. and
> 3. is that they would (unless I'm mistaken) require significant
> refactoring to ActiveRecord::Errors. Currently the validation methods
> (e.g. validates_presence_of) check the model and then call #add or one
> of its variants. #add then calls #generate_message and adds the result
> to the errors. #generate_message already calls the I18n API and
> generates the translated message. So @errors obviously holds
> "finalized" translations. When the user now calls full_messages
> there's no way to get hands on the original translation keys and
> interpolation values etc.

I think this needs to be done anyway, due to the fact that it prevents
one fro displaying errors in multiple languages simultaneously. And
since all of this refactoring can be done while retaining backwards
compatibility in the case of option 3, I don't see it as an issue.

> 3. make the separator configurable per API call (as an option on
> #translate)

> [...]

> I can not see any problems with 3. though.

Agreed

>
> IIRC the reason for why people voted for {{}} over %{} is that it's
> harder to type. Today I kind of regret we've picked that one. Not sure
> it's an important issue though.

Can't this be made configurable anyway? I don't see it as an important
issue either, though

[Lawrence Pit]

That's a strong argument to go for the %{} syntax. Certainly for rails 3.

Could we support both %{} and {{}} in rails 2.4 ?


Lawrence

[Sven Fuchs]

On 02.05.2009, at 14:51, Masao Mutoh wrote:
>> IIRC the reason for why people voted for {{}} over %{} is that it's
>> harder to type. Today I kind of regret we've picked that one. Not
>> sure
>> it's an important issue though.
>
> ruby-1.9.x has %{}, %<>.
> To use ruby-1.9 standard syntax will be more familier with
> non-i18n people and more flexible.
>
> % irb19
> irb(main):001:0> RUBY_VERSION
> => "1.9.1"
> irb(main):002:0> "%{attribute} is blank." % {:attribute => "foo"}
> => "foo is blank."
> irb(main):003:0> "%<value>2.5f" % {:value => 1.0/3}
> => "0.33333"
> irb(main):007:0> sprintf("%{attribute} is blank.", :attribute =>
> "foo")
> => "foo is blank."

I didn't know about that. This certainly makes a strong argument for
adopting at least %{}.

I can't find any docs about that syntax. What happens to extra
arguments?

"%{attribute} is blank." % {:attribute => "foo", :bar => 'bar'}

Would they be silently skipped?

Of course it would be cool to adopt %<> as well. How hard is it to
come up with a compat layer for Ruby 1.8.x?

[Sven Fuchs]

On 02.05.2009, at 18:59, mateo murphy wrote:
> I think having more than one message may be the best way to properly
> support a full range of languages. The problem is that the current
> flexibility is highly restrictive in other ways.

Yeah, I'm currently not seeing any other solution that would allow to
support all the requirements.

> How about this; we provide a full_message context, with the
> following behavior:
>
> 1) If message and full_messsage are both defined, each is used in it's
> appropriate context
> 2) If just message is defined, full_message is generated by
> concatenating the attribute name to the beginning
> 3) If just full_message is definied, it is used for both cases.
>
> This would provide the flexibility required as well as retaining full
> compatibility with the current implementation.

Sounds like a plan to me. Any other opinions?

Who could have a look at an implementation?

>> 3. make the separator configurable per API call (as an option on
>> #translate)
>> [...]
>> I can not see any problems with 3. though.
> Agreed

I guess we can just add the feature. It doesn't hurt but should make a
huge difference for any gettext-style accessors.

[Masao Mutoh]

Hi,

On Sun, 3 May 2009 14:55:38 +0200
Sven Fuchs <sven...@artweb-design.de> wrote:

>
> On 02.05.2009, at 14:51, Masao Mutoh wrote:
> >> IIRC the reason for why people voted for {{}} over %{} is that it's
> >> harder to type. Today I kind of regret we've picked that one. Not
> >> sure
> >> it's an important issue though.
> >
> > ruby-1.9.x has %{}, %<>.
> > To use ruby-1.9 standard syntax will be more familier with
> > non-i18n people and more flexible.
> >
> > % irb19
> > irb(main):001:0> RUBY_VERSION
> > => "1.9.1"
> > irb(main):002:0> "%{attribute} is blank." % {:attribute => "foo"}
> > => "foo is blank."
> > irb(main):003:0> "%<value>2.5f" % {:value => 1.0/3}
> > => "0.33333"
> > irb(main):007:0> sprintf("%{attribute} is blank.", :attribute =>
> > "foo")
> > => "foo is blank."
>
> I didn't know about that. This certainly makes a strong argument for
> adopting at least %{}.
>
> I can't find any docs about that syntax. What happens to extra
> arguments?

I couldn't find it, too.
In my investigations:

* %{}: same with ruby-gettext
* %<foo>?: equivalant with %nth$?
e.g.) "%d, %d" % [1, 2] #=> "1, 2"
"%2$d, %1$d" % [1, 2] #=> "2, 1"
"%<a>d, %<b>d" % {:a => 1, :b => 2} #=> "1, 2"
"%<b>d, %<a>d" % {:a => 1, :b => 2} #=> "2, 1"

> "%{attribute} is blank." % {:attribute => "foo", :bar => 'bar'}
>
> Would they be silently skipped?

In Ruby-1.9.x. the answer is no.
When the key doesn't match, it occurs TypeError.

Ruby-GetText overrides String#% not to occur any Exceptions
because gettext uses this as the msgid(key for translation) and some
languages may not be match the place holders with msgid.

> Of course it would be cool to adopt %<> as well. How hard is it to
> come up with a compat layer for Ruby 1.8.x?

See
http://github.com/mutoh/gettext/blob/f6566738b981fe0952548c421042ad1e0cdfb31e/lib/gettext/core_ext/string.rb
--
Masao Mutoh <muto...@gmail.com>

[mateo murphy]

>> How about this; we provide a full_message context, with the
>> following behavior:
>>
>> 1) If message and full_messsage are both defined, each is used in it's
>> appropriate context
>> 2) If just message is defined, full_message is generated by
>> concatenating the attribute name to the beginning
>> 3) If just full_message is definied, it is used for both cases.
>>
>> This would provide the flexibility required as well as retaining full
>> compatibility with the current implementation.
>
> Sounds like a plan to me. Any other opinions?
>
> Who could have a look at an implementation?

I can work on it this week, I'll leave a bit of time for discussion
before I start though.

[geekQ]

= Arbitrary message strings

Sven wrote:

> 3. make the separator configurable per API call (as an option on
> #translate)

> def _(msgid)
> I18n.t(msgid, :default => msgid, :separator => whatever)
> end
>

> The separator "\000" is a bit problematic here ...

Agree, sure it can be handled this way in the gettext libraries.
And we should solve it without binary null.
Maybe with `:separator => nil` meaning "no hierarchical processing".
(not with :separator => "\0")


= placeholder syntax

Masao wrote:

> ruby-1.9.x has %{}, %<>.
> To use ruby-1.9 standard syntax will be more familier with
> non-i18n people and more flexible.

Lawrence wrote:

> That's a strong argument to go for the %{} syntax. Certainly for rails 3.

+1

Sven wrote:

> I can't find any docs about that syntax. What happens to extra
> arguments?

For internationalization purposes and string interpolation we need
to ignore extra arguments. I am surprised, that it is not the default
behaviour or Ruby. But it will probably change until the Ruby 2
release.
For now and for Ruby 1.8 compatibility we could use the Masao's
implementation.


= full_messages

> I'm not sure merging message and full_message into one concept is
> sufficient. Wouldn't that remove flexibility from the validations API?

There is no general way to infer short from full messages or vice
versa. If you want to have both, you need to provide both. In our
projects however we never had a need for both, even not for UK
projects.
The validates_* :message => ... API also provides space for only one
parameter. I would remove such a feature provided we have enough
backing from the core team. If not, I would prefer a monkey patching
to an overcomplicated API.

Mateo wrote:

> I think having more than one message may be the best way to properly
> support a full range of languages. The problem is that the current
> flexibility is highly restrictive in other ways.

That is what I was trying to say. ;-)

>> How about this; we provide a full_message context, with the
>> following behavior:

>> 1) If message and full_messsage are both defined, each is used in it's
>> appropriate context
>> 2) If just message is defined, full_message is generated by
>> concatenating the attribute name to the beginning
>> 3) If just full_message is definied, it is used for both cases.

>> This would provide the flexibility required as well as retaining full
>> compatibility with the current implementation.

Sounds complicated to me. Then we will need to store both short
and full message for every field, right? Currently the errors
collection
is implemented as a Hash of Arrays of Strings:
http://github.com/rails/rails/blob/bcc4537f2a0d37fc02d67e9564fa5c9c5555b3d5/activemodel/lib/active_model/errors.rb#L43
Two messages per validation error idea would make it even more
complicated.

I believe, that for a ground breaking 3.0 release a simple clean
API and simple reliable implementation are more important than
backward compatibility.

If you have time to prepare an implementation and usage example
on github, then everybody can form a substantiated
view on this proposal.

>> record.errors.add(attr, options[:too_short],
>> :minimum => option_value.begin)

> I'm not quite sure I follow here. What exactly do you propose? What's
> the problem solved here?
>
> At least from reading the code I wonder why here
>

> http://github.com/geekq/rails/blob/ad46280cf63cecb0016e69d52733c2f527...
> http://github.com/geekq/rails/blob/ad46280cf63cecb0016e69d52733c2f527...

>
> both interpolation and evaluation of lambdas happens. I'd guess that
> really should happen through the API.

Sorry! Now I see, that this older (2 weeks ;-) ) implementation
is rather misleading *in the context of current discussion.*
I'll prepare a fresh proposal based on the I18n API.

[mateo murphy]

On Mon, May 4, 2009 at 10:32 AM, geekQ <vlad...@geekq.net> wrote:

> There is no general way to infer short from full messages or vice
> versa. If you want to have both, you need to provide both. In our
> projects however we never had a need for both, even not for UK
> projects.

I've never had need for more than one message either, but I can see
why it could be useful, and it seems to have been one of the goals of
the current API.

>>> This would provide the flexibility required as well as retaining full
>>> compatibility with the current implementation.
>
> Sounds complicated to me. Then we will need to store both short
> and full message for every field, right?

You'd have the option to only store the full message, and that would
be used for the short message as well. Probably like:

validates_* :full_message =>

Which essentially means that using :full_message would be the same as
:message but without concatenation. You'd only have to specify both
messages if you want to use both messages.

> Currently the errors
> collection
> is implemented as a Hash of Arrays of Strings:
> http://github.com/rails/rails/blob/bcc4537f2a0d37fc02d67e9564fa5c9c5555b3d5/activemodel/lib/active_model/errors.rb#L43
> Two messages per validation error idea would make it even more
> complicated.

Yes the implementation of how message are stored will need to be
changed, but I really don't think it'll be all that complicated.

>
> I believe, that for a ground breaking 3.0 release a simple clean
> API and simple reliable implementation are more important than
> backward compatibility.

I agree that it's not the best solution, and that for 3.0 the API
should be rethought. However, I think that coming up with a simple
patch that supports the current API but allows people to do what they
need without monkeypatching is a very good idea in the short term.

> If you have time to prepare an implementation and usage example
> on github, then everybody can form a substantiated
> view on this proposal.

I'll probably start on this tonight

[mateo murphy]

On Mon, May 4, 2009 at 11:56 AM, mateo murphy <mateo....@gmail.com> wrote:

> I believe, that for a ground breaking 3.0 release a simple clean
> API and simple reliable implementation are more important than
> backward compatibility.

I agree that it's not the best solution, and that for 3.0 the API
should be rethought. However, I think that coming up with a simple
patch that supports the current API but allows people to do what they
need without monkeypatching is a very good idea in the short term.

Ok, looking at the 3.0 validation code for the first time, I see that refactoring has already been started, and the previous API has been deprecated. Considering this, I don't think my proposal makes as much sense; if we're going to be changing the API anyway, we may as well do it properly. 

Also of note is that the previously deprecated default_error_messages is back, and generate_message is gone completely, I18n support along with it. Is anyone aware of any specific reason for this? 

[Sven Fuchs]

Hi Mateo,

On 05.05.2009, at 02:49, mateo murphy wrote:
> Ok, looking at the 3.0 validation code for the first time, I see
> that refactoring has already been started, and the previous API has
> been deprecated. Considering this, I don't think my proposal makes
> as much sense; if we're going to be changing the API anyway, we may
> as well do it properly.
>
> Also of note is that the previously deprecated
> default_error_messages is back, and generate_message is gone
> completely, I18n support along with it. Is anyone aware of any
> specific reason for this?

I don't know of any such thing and I would be surprised if anybody
removed previously added features right away without any discussion.

But I find the relevant bits of code still the same and the i18n tests
haven't changed a lot:

http://github.com/rails/rails/blob/master/activerecord/lib/active_record/validations.rb
http://github.com/rails/rails/commits/master/activerecord/test/cases/validations_i18n_test.rb

Maybe you've been looking at the 3-0-unstable branch?

According to http://weblog.rubyonrails.org/2009/4/13/one-giant-leap-forward
currently master is used for 3.0 dev while 2.3.x dev takes place in
2-3-stable.

Am I missing something?

[mateo murphy]

On Tue, May 5, 2009 at 3:23 AM, Sven Fuchs <sven...@artweb-design.de> wrote:

> Maybe you've been looking at the 3-0-unstable branch?
>
> According to http://weblog.rubyonrails.org/2009/4/13/one-giant-leap-forward
>  currently master is used for 3.0 dev while 2.3.x dev takes place in
> 2-3-stable.
>
> Am I missing something?

I was looking at master, but in active model, specifically this file:
http://github.com/rails/rails/blob/bcc4537f2a0d37fc02d67e9564fa5c9c5555b3d5/activemodel/lib/active_model/errors.rb

maybe I'm incorrect in assuming this is going to be replacing the
validation currently in active record?

[Vladimir Dobriakov]

On Tue, May 5, 2009 at 4:53 PM, mateo murphy <mateo....@gmail.com> wrote:
> I was looking at master, but in active model, specifically this file:
> http://github.com/rails/rails/blob/bcc4537f2a0d37fc02d67e9564fa5c9c5555b3d5/activemodel/lib/active_model/errors.rb
>
> maybe I'm incorrect in assuming this is going to be replacing the
> validation currently in active record?

That is my understanding. How can we get an official statement from
the core team about that? That is also important to know to be able to
provide the patches.

I've emailed wycats about that, but still did not get any answer.

[Sven Fuchs]

I'm pretty sure the AR validation code is the current stuff.

AM has seen a couple of attemps but IMO it's scheduled as a GSoC
project for Joshua http://weblog.rubyonrails.org/2009/4/22/2009-rails-google-summer-of-code-projects
We've never touched AM so far because it never reached any stable
baseline.

[Sven Fuchs]

I've just talked to Pratik and he agreed that 2-3-stable is the best
place to target right now as we want to have this in 2.3.x anyway.
There's no problem with porting this to AM/2.3.x and AR/AM master/3.0
then.

Pratik said that the currently most current version of AM is in the
active_model branch, not the current master branch. (He said he
updated the active_model branch quite recently.)

Looking briefly over active_model this code looks quite familar: http://github.com/rails/rails/blob/5f3f100ce2d689480da85abc88e5e940cf90189e/activemodel/lib/active_model/errors.rb

But AM is definitely the most moving part in the game. So let's start
with AR in 2-3-stable and work from there.

wdyt?

[mateo murphy]

On Tue, May 5, 2009 at 5:53 PM, Sven Fuchs <sven...@artweb-design.de> wrote:

I've just talked to Pratik and he agreed that 2-3-stable is the best
place to target right now as we want to have this in 2.3.x anyway.
There's no problem with porting this to AM/2.3.x and AR/AM master/3.0
then.

Pratik said that the currently most current version of AM is in the
active_model branch, not the current master branch. (He said he
updated the active_model branch quite recently.)

Right; upon closer inspection, it seems that the active model errors code in master is actually quite old, which is probably why it doesn't contain the I18n code.

 

Looking briefly over active_model this code looks quite familar: http://github.com/rails/rails/blob/5f3f100ce2d689480da85abc88e5e940cf90189e/activemodel/lib/active_model/errors.rb

But AM is definitely the most moving part in the game. So let's start
with AR in 2-3-stable and work from there.

wdyt?

Works for me. What would be the best way to proceed regarding patches, opening a ticket on lighthouse?

[Sven Fuchs]

On 06.05.2009, at 04:02, mateo murphy wrote:
> Works for me. What would be the best way to proceed regarding
> patches, opening a ticket on lighthouse?

In the end we'd probably make a patch from that and open a ticket, yes.

Do we expect some discusssion and maybe a few iterations on this? In
that case maybe forking Rails and working there might be more
convenient. I'd probably prefer that.

I'm not sure I can find your Github account. If you already have a
Rails fork and can't use that for some reason please let me know. I'll
update my fork and add you then.

[Lawrence Pit]

Hi Sven,

It would be very helpful if we could also access the symbol of the
validates_* message instead of the (translated) messages.

This way:

1. The actual translation of the message can be deferred to the view
level (which is a more proper place imho, for various reasons including
not having to possibly have markup (in my models, uck!) in translation
strings when using gettext as the preferred translation API)

2. Makes testing of models easier and clean again (you test against
symbols in the errors array instead of possibly translated messages)

Inspiration for this is based on a preso Tim Lucas gave at a
ruby-on-rails meetup in Sydney this week. See also:

http://gist.github.com/107555

and my take on it using gettext API:

http://gist.github.com/108497

Cheers,
Lawrence

[Vladimir Dobriakov]

On Fri, May 8, 2009 at 3:30 AM, Lawrence Pit <lawren...@gmail.com> wrote:
> 1. The actual translation of the message can be deferred to the view
> level (which is a more proper place imho, for various reasons including
> not having to possibly have markup (in my models, uck!) in translation
> strings when using gettext as the preferred translation API)
>
> 2. Makes testing of models easier and clean again (you test against
> symbols in the errors array instead of possibly translated messages)
>
>
> Inspiration for this is based on a preso Tim Lucas gave at a
> ruby-on-rails meetup in Sydney this week. See also:

> http://gist.github.com/108497

+1

I found both the philosophical aspect - separation of concerns,
removing UI (translated messages) from the model and the technical
advantages (easier testing) very appealing.

Vladimir

[mateo murphy]

I'm not sure I can find your Github account. If you already have a
Rails fork and can't use that for some reason please let me know. I'll
update my fork and add you then.

I've got a rails fork at http://github.com/mateomurphy/rails/tree/master, however I've yet to make modifications to it, as I'm still trying some things out.

Two conclusions so far:

1) The string concatenation issue can easily be saved by modifying full_messages:

--- a/activerecord/lib/active_record/validations.rb

+++ b/activerecord/lib/active_record/validations.rb

@@ -206,7 +206,7 @@ module ActiveRecord

             full_messages << message

           else

             attr_name = @base.class.human_attribute_name(attr)

-            full_messages << attr_name + I18n.t('activerecord.errors.format.separator', :default => ' ') + message

+            full_messages << I18n.t('activerecord.errors.format.full_message', :default => '{{attribute}} {{message}}', :attribute => attr_name, :message => message)

           end

         end

       end

That way you can set :activerecord.errors.format.full_message in non english locales to simply "{{message}}" bypassing concatenation, and it also provides more flexibility in english than what is provided by simply modifiying the seperator, which makes it a win-win for everybody

2) Deferring the translation of messages will require storing the options passed to errors.add, as these are used by generate_message. This means that errors need to be stored either as a hash, or as an object; I think the latter makes more sense. Thoughts?

[mateo murphy]

On Fri, May 8, 2009 at 7:03 PM, mateo murphy <mateo....@gmail.com> wrote:

1) The string concatenation issue can easily be saved by modifying full_messages:

Sorry, that's supposed to say "solved", not "saved" 

[Lawrence Pit]

> 2) Deferring the translation of messages will require storing the
> options passed to errors.add, as these are used by generate_message.
> This means that errors need to be stored either as a hash, or as an
> object; I think the latter makes more sense. Thoughts?

I came to the same conclusion later yesterday.. an ActiveRecord::Error
object makes more sense yes. I was thinking of this:

if f.error(:name) === :too_short

= n_("Need an apple", "Need %{num} apples",
f.error(:name).options[:count])

= _("Name is too short, must be at least %{num} characters") % {
:num => f.error(:name)[:count] }

so the Error object implements == and === which would match against the
message symbol or the translated message string or an AR::Error object,
and [] is a shortcut to .options[].

If the to_s method is also implemented which basically calls
generate_message then things should work in a backwards compatible manner.

Cheers,
Lawrence

[Masao Mutoh]

Hi,

Yes. gettext_activerecord hacks to pass a options as a hash to errors.add.
#I prefer hash because it may be able to increase the parameter easily.

Anyway,
'activerecord.errors.format.full_message' seems not good.
For example, I may want to replace it unless {{attribute}}
with validates_length_of,but want to include {{attribute}} into
validates_presence_of.
So this should be changed in each messages.

class Person < ActiveRecord::Base
validates_acceptance_of :foo, :message => "Don't accept it!" # Don't need attribute.
validates_length_of :name, :maximum=>30, :message=> "{{attribute}} is less than {{count}}"
end

And if "message" becomes hash/object, the default value of
'{{attribute}} {{message}}' doesn't work well, because count/value can't be
handled.

+ full_messages <<
I18n.t('activerecord.errors.format.full_message',
:default => '{{attribute}} {{message}}',
:attribute => attr_name,

:message => message,
:count => count, # Can't handle this.
:value => value) # Can't handle this.

So, it should be like as:

# obj is an sample with options
full_messages <<
I18n.t(obj["message"], # message or key such as "activerecords....invalid",
:attribute => attr_name,
:count => obj["count"], # Apply to message
:value => obj["value"]) # Can't handle this.

Or more general way like as:

full_messages << I18n.t(obj["message"], obj)

This way make "additional validation helper" can handle their own values.

The issue is how to set the default value.
# In gettext, it is fallback to the message in the default locale(en).

--
Masao Mutoh <muto...@gmail.com>

[mateo murphy]

On Fri, May 8, 2009 at 10:24 PM, Lawrence Pit <lawren...@gmail.com> wrote:

I came to the same conclusion later yesterday.. an ActiveRecord::Error
object makes more sense yes. I was thinking of this:


 if f.error(:name) === :too_short

   = n_("Need an apple", "Need %{num} apples",
f.error(:name).options[:count])

   = _("Name is too short, must be at least %{num} characters") % {
:num => f.error(:name)[:count] }


so the Error object implements == and === which would match against the
message symbol or the translated message string or an AR::Error object,
and [] is a shortcut to .options[].

Yeah, that makes sense, although ideally I think that it should match against something other than the message; either a symbol that represents the actual validation rule, or maybe a subclass or AR::Error. 

 

If the to_s method is also implemented which basically calls
generate_message then things should work in a backwards compatible manner.

That's how the code I've started writing handles it, but a fair number of tests will need to be rewritten to see if it all works properly.

[Lawrence Pit]

so the Error object implements == and === which would match against the
message symbol or the translated message string or an AR::Error object,
and [] is a shortcut to .options[].

Yeah, that makes sense, although ideally I think that it should match against something other than the message; either a symbol that represents the actual validation rule, or maybe a subclass or AR::Error.

Why the symbol that matches the validation rule?

I would first try a match against the symbol that represents the message (:invalid, :too_short, etc.). If it's not a symbol try matching a subclass or AR::Error, if it's not that either try matching against to_s. This way, I expect, no tests need to be rewritten, because it should be backwards compatible. Esp. if AR::Error also implements a method_missing which does to_s.send(method, arguments). "Old" statements like:


  f.errors.on(:name).should == "is invalid"


should then still work. Which would be good because loads of plugins and existing code rely on this.



Cheers,
Lawrence

[mateo murphy]

On Fri, May 8, 2009 at 10:24 PM, Lawrence Pit <lawren...@gmail.com> wrote:

I came to the same conclusion later yesterday.. an ActiveRecord::Error
object makes more sense yes. I was thinking of this:


 if f.error(:name) === :too_short

This, unfortunately, doesn't seem to be possible without patching Symbol, as

:too_short === f.error(:name)

will always return false.

It would have to be 

    if f.error(:name).to_sym === :too_short

Or something like it.

 

so the Error object implements == and === which would match against the
message symbol or the translated message string or an AR::Error object,
and [] is a shortcut to .options[].

Matching against an AR::Error subclass is trivial, but I'm worried that it would adding custom errors a lot more involved than it is currently; having to write a subclass for each custom error so that you can match it later will be annoying

In any case, I'm going to clean up my code and put it online tomorrow with what's currently working, and we can take it from there.

[Lawrence Pit]

ruby-1.9.x has %{}, %<>.
To use ruby-1.9 standard syntax will be more familier with
non-i18n people  and more flexible.
    

That's a strong argument to go for the %{} syntax. Certainly for rails 3.
    

+1
  

To continue on this road, I propose the following :

1. Masao Mutoh's excellent extension of the string class is merged into core_ext/string of ActiveSupport:

http://github.com/mutoh/gettext/blob/ee09a33de94a2699fb5d8b6894cb16a383a2963a/lib/gettext/core_ext/string.rb

2. The interpolate method of i18n is changed very slightly:

http://github.com/lawrencepit/i18n/commit/91103e4386d18ab27a6634e191ace210f020c0a6

This way both the {{key}} and %{key} syntaxes are supported at the same time.

There would be a change in behaviour in that I18n will not raise a MissingInterpolationArgument anymore. This follows the behavior of the string interpolation method of ruby 1.9.



PS. I assume we cannot remove support for the {{key}} syntax entirely in rails 3 ?


Cheers,
Lawrence

[Sven Fuchs]

Ooops. I've already pulled Lawrence's change without noticing that it includes Masao's String patch.

Although I agree with the general direction I think we should not patch String from the I18n gem. If we want Rails to patch it we (imo) must get it into ActiveSupport. I think the clear path making it easier to upgrade to Ruby 1.9 AND making our stuff more powerful/flexible is a very strong argument :)

Lawrence, would you care enough to propose this on Rails core? 

[Lawrence Pit]

Created a rails ticket with the patch:

https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/2870-ruby-19-style-string-interpolation-support-for-lower-ruby-versions#ticket-2870-1



LP

[Sven Fuchs]

I +1'ed and asked Pratik to apply it.

[Sven Fuchs]

It's already applied, but only to Rails 3, not to Rails 2.3. Are we cool with only having that in Rails 3?

Also, I've worked on the interpolation String extension patch a bit and made it raise a KeyError when an interpolation value is missing: http://github.com/svenfuchs/i18n/blob/a27b74df9d2e4357140d77506e1d9c43f7cff63a/lib/i18n/string.rb#L62

That's the same behaviour as Ruby 1.9 shows:

irb(main):001:0> "%{foo}" % {}

KeyError: key not found

Unfortunately it does not add the actual key to the exception or the message, so I've changed the Simple backend to rescue and re-raise this exception: http://github.com/svenfuchs/i18n/blob/a27b74df9d2e4357140d77506e1d9c43f7cff63a/lib/i18n/backend/simple.rb#L178

So, with this change we'd now have:

1 - support for {{foo}}, %{foo} and %<foo>.d style interpolation placeholders

2 - exactly the same behaviour for both Ruby 1.8.x and Ruby 1.9

I wonder what to do with the now equivalent {{foo}} and %{foo} placeholder styles. Would we want to get rid of the old style? How/when?