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
Merged

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 graffic commented Mar 15, 2012

+1

@tigrish
Copy link
Collaborator

@tigrish tigrish commented Jul 5, 2012

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

@nikosd
Copy link
Contributor Author

@nikosd 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]

This comment has been minimized.

@radar

radar Nov 16, 2016
Collaborator

args.empty? would look better here imo.

@radar
Copy link
Collaborator

@radar 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 tlatsas force-pushed the e-travel:gettext_plural_keys_access branch from 98e8d79 to 0cc2cad Dec 24, 2016
@tlatsas
Copy link
Contributor

@tlatsas 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 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 tlatsas commented Dec 31, 2016

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

@radar
Copy link
Collaborator

@radar radar commented Jan 23, 2017

@tlatsas Yes please.

@atzorvas
Copy link
Contributor

@atzorvas atzorvas commented May 11, 2017

@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 radar commented May 14, 2017

LGTM. Thanks for your work on this!

@radar radar merged commit b04a982 into ruby-i18n:master May 14, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@atzorvas
Copy link
Contributor

@atzorvas atzorvas commented May 17, 2017

@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
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants