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

Access to Gettext.plural_keys #122

Merged
merged 4 commits into from May 14, 2017

Conversation

nikosd
Copy link
Contributor

@nikosd nikosd commented Nov 10, 2011

Allowing Gettext.plural_keys access gives to someone the opportunity to actually do custom mapping of pluralization forms to gettext msgids which currently is broken for all non english-like locales.

…an add more mappings than the :en default.
@graffic
Copy link

graffic commented Mar 15, 2012

+1

@tigrish
Copy link
Collaborator

tigrish commented Jul 5, 2012

This looks great, but would love to see a test for it!

@nikosd
Copy link
Contributor Author

nikosd commented Nov 20, 2012

This branch is way too old and I have already merged it in my master. Shall I close this and open a new one or just apply the patch on the old branch?

def plural_keys(locale)
@@plural_keys[locale] || @@plural_keys[:en]
def plural_keys(*args)
args.length == 0 ? @@plural_keys : @@plural_keys[args.first] || @@plural_keys[:en]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

args.empty? would look better here imo.

@radar
Copy link
Collaborator

radar commented Nov 16, 2016

@nikosd Could you please provide a test for this as @tigrish mentioned?

@radar radar added the waiting label Nov 16, 2016
@radar radar added this to the 0.9.0 milestone Nov 20, 2016
@tlatsas
Copy link
Contributor

tlatsas commented Dec 24, 2016

@radar added a test, however all tests with Gemfile.rails* fail with:

Your Gemfile.lock is corrupt. The following gem is missing from the DEPENDENCIES
section: 'i18n'

@radar
Copy link
Collaborator

radar commented Dec 29, 2016

Thank you @tlatsas. I think the issue you're seeing will have been fixed by #355.

@tlatsas
Copy link
Contributor

tlatsas commented Dec 31, 2016

Should I merge master on this branch or rebase this on top of master?

@radar
Copy link
Collaborator

radar commented Jan 23, 2017

@tlatsas Yes please.

@atzorvas
Copy link
Contributor

@radar merged master and suite is ok, are we ok in order for you to proceed with the merge to upstream?

Thanks 👍

@radar
Copy link
Collaborator

radar commented May 14, 2017

LGTM. Thanks for your work on this!

@radar radar merged commit b04a982 into ruby-i18n:master May 14, 2017
@atzorvas
Copy link
Contributor

@radar maybe you should also remove the "waiting" label, cheers

@radar radar removed the waiting label May 30, 2017
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

6 participants