-
-
Notifications
You must be signed in to change notification settings - Fork 410
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
Decompose I18n::Backend::Base#localize #347
Conversation
Thanks for the contribution! Could you please demonstrate a use case of this feature? |
I believe it's a matter of extension format types. |
something like module TranslatedZoneName
private
def translate_format(locale, format, object)
super.gsub(
'%O',
(object.respond_to?(:time_zone) ? I18n.t(:"timezones.#{object.time_zone.name}",
locale: locale, default: object.time_zone.name) : '')
)
end
end
I18n::Backend::Base.prepend TranslatedZoneName (does not tested. just as an expample) |
Ok, thanks. Seems like a good addition to me. Can you please write a test for this? |
Looks like this would be a good workaround for #210. |
I believe that there is no need to test something. I didn't change behaviour, split method though. And looks like it is not good practice to test private methods, it's better to test interface and i suppose it's already done. Maybe I don't understand somethig. Could you please explain than? |
end | ||
end | ||
|
||
format = translate_format(locale, format, object) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be good to have the same method signature as parent has:
locale, object, format = :default, options = {}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean that it would be better to have access to options
inside translate_format for future customisation? format
doen't need default value it will always set from parent method
Hi @radar, so how do you think? could we merge or you still think I should write some tests on it? |
@corlinus I think it's fine to merge without them. I can't work out a way to write tests myself. |
Thanks for your contribution! |
It allows to customise directives without overwriting whole localize method