Form label should use I18n

I think this boils down to String.humanize (and thus Inflector.humanize) should first check I18n to see if the argument can be located there, and then proceed with it's normal functionality.

You'd probably want something like: :'active_record.human_attribute_names.product.name'

Problem is that String.humanize doesn't know which model is used. It just knows 'name'.

So the only place to do this is in the form.label method, because it's the only place where both the model and the attribute is known.

Yes, after looking more into it, I came to this conclusion as well. So we can only hope that the form.label will perform a check in i18n similar to the error_messages helper.

Fixed it a bit nicer in the i18n project:

http://i18n.lighthouseapp.com/pr...

Additional to Iains linked patch, I would also suggest running titleize on the string returnd by I18n.

The reasoning behind that being that in your translation files you probably have your attribute names all lowercase as that's how you'd want see them in your AR error messages. For a label though, a titlecased version makes more sense.

Attached is my patch suggestion (incl. test) agains 2-2-stable.

Regarding titleizing the translation: I can see that the user might want to titleize the attribute name for certain locales. But I don't think we can safely assume this is true for all locales.

So, do we need a way to control this? Putting an option for this into the translation data would seem extremely ugly to me. Any suggestions?

If we know for sure that other languages have other "titlecasing" rules, I would suggest putting the corresponding logic right into the titleize method and storing the rules in the locale's .yml.

Sven, you are of course right though. Putting such an option or rule set into the translation data is not exactly beautiful or intuitive but on the other hand, there's already e.g. support.array.skip_last_comma in there which serves a similar purpose and frankly I can't think of a better place to store that stuff.

Although I would find a distinction between localisation (date formats, titlecasing rules, currency precision stuff, etc) and translation (strings!) files helpful, this ticket is certainly not the right place to discuss something like this..

After reading up on the issue of capitalization a little (e.g. Wikipedia: "The full rules of capitalization for English are complicated."), I think we should go ahead implementing the very crude version of just uppercasing the first letter of human_attribute_name.

Unless someone can come up with a central repository of capitalization rules for different locales, I just don't see it being worth the effort to collect that kind of information. Whenever someone isn't quite happy with just having the first character capitalised, one can always overwrite the string in the locale .ymls, no?

What do you think, Sven?

human_attribute_name when used without I18n calls humanize on your attribute name, which uppercases the first letter. So you should have capitalized names on your I18n files as well.

In other words, I think that no further processing should be done after calling human_attribute_name, because it should return a string which already has the first letter uppercased.

If someone needs lowercased words in AR error messages, we should provide a patch for it, but not adopt the wrong behavior here.

Agrees with José. Titleize is a good option when no translation has been specified, but when you specify another string in your locale I want Rails to respect my version and not to change it afterwards.

Expample: in Dutch, the letter "ij" is uppercased to IJ (e.g. IJmuiden). If it would always titleize, I could never have a form without spelling errors, which would be very bad.

I agree with you two about not blatantly overriding/manipulating a localised string already provided by the user.

However, I think one must distinguish between three basic use cases of human_attribute_name:

1. At the beginning of a sentence, the way they are currently used in validation error messages in ActiveRecord
2. As some sort of title or label, e.g. in forms, as proposed here
3. As a normal part of any sentence

Maybe I am overcomplicating here but an attribute name as simple as first_name would be expected to be represented in three different ways according to the use cases above:

1. First name
2. First Name
3. first name

So in my mind just assuming that activerecord.attributes.model.attribute will be set to whatever is right is a bit of a cop-out as "whatever is right" depends on the context it is used in. I think that ideally it should just be set to "first name" with utility methods such as titleize being locale-aware and taking care of the necessary transformations.

Obviously though, this is beyond the scope of this ticket. So I would propose going ahead with Iain's patch for the time being. Everyone then has to take care of fancy titlecasing themselves but it is certainly better than simply humanising column names as we do now.

Niels,

In the first and second cases I use: "First name". I guess then it changes from developer to developer. :)

I'm attaching Iain's patch as form_label_simple.diff, if everyone agree, this should be the one applied.

Discussing this on the rails-i18n mailinglist: http://groups.google.com/group/rails-i18n/browse_thread/thread/2d4c...

José Valim: By the way, when nested forms is used, label method don't get clean "object" to klassify from it but object would be something like "post[comments_attributes][0]" in some blog app where post model has_many :comments with accepts_nested_attributes_for :comments.

+1, this is very important to internacionalizated apps.

I have a IMHO better idea for this.

Most of the time I want the form label to display a slightly different string than just the field name. For this purpose I have created an extra scope inside the YML activerecord structure, named "labels":

activerecord:
... models:

customer: "Kunden"

attributes:

name: "Name"
address: "Adresse"
zip: "PLZ"

labels:

name: "Name (Vorname und Name)"
address: "Adresse (Strasse und Hausnummer)"
zip: "PLZ (immer 5-stellig)"

This makes it much more simple to control how the content of label should be presented.

I have made a simple rewrite of the to_label_tag method, to include this:

module ActionView
module Helpers

class InstanceTag
  def to_label_tag(text = nil, options = {})
    options = options.stringify_keys
    tag_value = options.delete("value")
    name_and_id = options.dup
    name_and_id["id"] = name_and_id["for"]
    add_default_name_and_id_for_value(tag_value, name_and_id)
    options.delete("index")
    options["for"] ||= name_and_id["id"]

    content = text.blank? ? nil : text.to_s
    content ||= I18n.t("activerecord.labels.#{object_name}.#{method_name}", :raise => true) rescue nil
    content ||= method_name.humanize
    label_tag(name_and_id["id"], content, options)
  end
end

end

end

I hope you'll find this interesting

Bloody hell, something happened to the formatting. :-) Here's the meat - hopefully more readable:

YML:

activerecord:
  ...
  models:

customer: "Kunden"

  attributes:

name: "Name"
address: "Adresse"
zip: "PLZ"

  labels:

name: "Name (Vorname und Name)"
address: "Adresse (Strasse und Hausnummer)"
zip: &quot;PLZ (immer 5-stellig)&quot;</code>

The code:

module ActionView
  module Helpers

class InstanceTag
  def to_label_tag(text = nil, options = {})
    options = options.stringify_keys
    tag_value = options.delete("value")
    name_and_id = options.dup
    name_and_id["id"] = name_and_id["for"]
    add_default_name_and_id_for_value(tag_value, name_and_id)
    options.delete("index")
    options["for"] ||= name_and_id["id"]

    content = text.blank? ? nil : text.to_s
    content ||= I18n.t("activerecord.labels.#{object_name}.#{method_name}", :raise => true) rescue nil
    content ||= method_name.humanize

    label_tag(name_and_id["id"], content, options)
  end
end

  end
end

Sorry for that.

• Carsten

Yes, Carsten, some time ago we had this discussion on I18n mailing list and we agreed in something quite similar! We agreed in:

• First, look in views.labels.model.attribute
• Then use Model.human_attribute_name
• And finally use attribute.humanize

Could you provide a patch with tests according to the contributors guide? https://rails.lighthouseapp.com/projects/8994/sending-patches

Thanks!

Ahh thanks - I wasn't aware of that.

views.labels.model.attribute sounds perfect indeed (along with the other defaults as well of course)

I will submit a patch ASAP

• Carsten

José,

I have been coding and testing on this patch now, but one thing puzzles me: Hasn't Model.human_attribute_name been deprecated and removed from Rails 2.3.x?

Carsten

No, it has not. At least, it should not.

Here's the patch with tests and modified documentation.

Please note, that this is my first contribution to the core framework. I've followed the guidelines to the best of my knowledge, but if I missed something, please let me know. :-)

Carsten

Added a little more examples to the label helper doc.

Carsten

@Carsten, congrats, the patch and tests look great!

I will ask just a small change though, instead of raising an error and rescuing on I18n call (which is slow), please do this instead:

content = if text.blank?
  i18n_label = I18n.t("views.labels.#{object_name}.#{method_name}", :default => "")
  i18n_label if i18n_label.present?
else
  text.to_s
end

content ||= object.class.human_attribute_name(method_name) if object
content ||= method_name.humanize

Yes of course - bad coding habit of mine :-)

Here you go.

Carsten

Thanks Carsten, finally, the patch you gave me contains three commits. Could you please squash them and provide a patch with just one commit?

Hey guys,

this looks good to me, except that:

• I don't get why the "".present? thing is better than using :raise => true / rescue nil because this is exactly what :raise is for. But maybe that's a matter of taste.
• The patch includes the translations "Total cost" and "Write entire text here" into actionpack/lib/action_view/locale/en.yml Instead it should set these translations to the backend in the test setup or test.
• (Why is it that there's an extra local variable content created instead of just using the existing text argument?)

José, thanks for the ping!

@Sven:

1) José argued that "".present? should execute faster than raising and catching an exception. I personally have no benchmarks proving either case. And I certainly have no special preference for one over the other. :-)

2) Whoops! Sorry I thought that the en.yml was a test-specific fixture-file. I'll fix it asap

3) Well it just follows the original code-pattern which read:

content = (text.blank? ? nil : text.to_s) || method_name.humanize
label_tag(name_and_id["id"], content, options)

I'll fix this no. 2 (and any of the others if you want it) on saturday and send a new patch.

@José: I am - ahem - quite new to Git. Is there a simple way to mash these commits down to one in the patch, or should I checkout the Rails code and start over?

Happy Christmas all of you! See you in a couple of days. :-)

• Carsten

Carsten, there are probably several ways to do this in Git, but they way I usually do is to execute "git reset HEAD^1" until all the commits were reverted (or "git reset HEAD^3"). The nice thing is that this still leaves the code in your working branch, so you are able to work on it and then provide a brand new patch. :)

Mery christmas eveybody!

I've modified the patch so that:

• Everything is now in one single commit (thanks José)
• The changes to the en.yml locale file has been removed. Instead I create a separate locale as part of the form_helper_test setup

@Sven: If you would rather prefer me using the :raise => true and catch pattern instead of .present?, then please let me knowæ. Otherwise the patch should be complete.

Carsten

Thanks Carsten, I'm cool with present?, too :)

So, +1

+1, been waiting on this for ages!

Thanks Paul :-)

José? You're cool?

Oh, yeah! Thanks Carsten!

(from [bef968d37942bfb2b7a59fca0e4451e096197c0a]) I18n label helper [#745 Form label should use I18n status:resolved]

Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/bef968d37942bfb2b7a59fca0e4451...

(from [f5714abc3d6dcda3851610419c00554ada8386ed]) I18n label helper [#745 Form label should use I18n status:resolved]

Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/f5714abc3d6dcda3851610419c0055...

big applause :) thanks josé!

Thanks Sven :-) Got a nice warm feeling inside - since this is my first patch.

Makes me want to do some more with this nice reception. When I look at https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/milesto... are these all the tickets that needs to be resolved, before 2.3.6 is released?

Carsten

Carsten, those tickets in the milestone are not official, since I think anyone can assign any milestone. In other words, feel free to tackle any issue you find on lighthouse, not specifically those. :)

And you are more than welcome to participate in the next bugmash: http://railsbridge.org/news_items/9

I'm on holiday that weekend, but I'll just mash some bugs on my own accord :-)