Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented support for usage of alternative conventions for namespaced models in error messages. #1650

Closed
wants to merge 1 commit into from
Closed

Implemented support for usage of alternative conventions for namespaced models in error messages. #1650

wants to merge 1 commit into from

Conversation

TomTriple
Copy link

Implemented support for usage of alternative conventions for namespaced models in error messages. This solves issue #1402 completely.

This means you can write your model errors like this:

en:
activemodel:
errors:
models:
blog:
post:
attributes:
title:
blank: "can't be blank - dot notation"
header:
blank: "can't be blank - dot notation"
blog/post:
attributes:
title:
blank: "can't be blank - slash notation"
editor:
blank: "can't be blank - slash notation"

…ed models in error messages. This solves issue #1402 completely.
@josevalim
Copy link
Contributor

I am going to reject this one. This will make errors lookup even slower. :(

@josevalim
Copy link
Contributor

Tks for the pull request though. IMHO, we need to choose between the two syntaxes.

@TomTriple
Copy link
Author

Ok, I fully understand your point of view. If we drop one syntax there will be an API break as described on http://groups.google.com/group/rubyonrails-core/browse_thread/thread/eca57305b1a3ea0d. Although I´ve not benchmarked this(!), I don´t think two more hash lookups impact performance seriously negative, don´t they?!

@TomTriple
Copy link
Author

More precisly: Only not supporting both syntaxes in "Namespaced::Model.model_name.human" and "Namespaced::Model.human_attribute_name" will break API. Supporting it in "Namespaced::Model.new.errors.add(...)" too would be due to consistency reasons.

@josevalim
Copy link
Contributor

Yes, we should never have allowed the nested namespaced setting. I will find a way to deprecate it still in 3-1-stable before final release.

@TomTriple
Copy link
Author

Let me know what you do with that issue and if I can support you.

@josevalim
Copy link
Contributor

I am trying to deprecate it in 3-1-stable, but it is very hard to deprecate I18n stuff. Any ideas?

@paneq
Copy link
Contributor

paneq commented Jun 11, 2011

How about a simple setting ? We wouldn't support 2 syntax in the same time but separately. Performance would not degrade. Everyone happy ?

@josevalim
Copy link
Contributor

I have removed the alternate namespace syntax as it cannot work on all cases.

@josevalim josevalim closed this Jun 11, 2011
@josevalim
Copy link
Contributor

@paneq I don't see a strong reason to have a configuration option and support two different syntaxes as one of the options cannot work on all cases and does not provide significant benefits over the other.

@paneq
Copy link
Contributor

paneq commented Jun 11, 2011

@josevalim - You mean that alternative syntax cannot work in cases where class is nested inside another class ? Can we agree that this alternative syntax will be always supported in 3.0.* in those 2 of 3 cases (model_name, human_attribute_name) ?

@josevalim
Copy link
Contributor

Yes and yes.

@paneq
Copy link
Contributor

paneq commented Jun 11, 2011

Thank you for your support and time. I need to start thinking about rearranging my translations files now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants