Skip to content

Access to Gettext.plural_keys - #122

Merged
radar merged 4 commits into
ruby-i18n:masterfrom
e-travel:gettext_plural_keys_access
May 14, 2017
Merged

radar merged 4 commits into
ruby-i18n:masterfrom
e-travel:gettext_plural_keys_access

Conversation

@nikosd

@nikosd nikosd commented Nov 10, 2011

Copy link
Copy Markdown
Contributor

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

graffic commented Mar 15, 2012

Copy link
Copy Markdown

+1

@tigrish

tigrish commented Jul 5, 2012

Copy link
Copy Markdown
Collaborator

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

@nikosd

nikosd commented Nov 20, 2012

Copy link
Copy Markdown
Contributor Author

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?

Comment thread lib/i18n/gettext.rb Outdated
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
Copy Markdown
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

radar commented Nov 16, 2016

Copy link
Copy Markdown
Collaborator

@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 gettext_plural_keys_access branch from 98e8d79 to 0cc2cad Compare December 24, 2016 13:22
@tlatsas

tlatsas commented Dec 24, 2016

Copy link
Copy Markdown
Contributor

@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

radar commented Dec 29, 2016

Copy link
Copy Markdown
Collaborator

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

@tlatsas

tlatsas commented Dec 31, 2016 •

Copy link
Copy Markdown
Contributor

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

@radar

radar commented Jan 23, 2017

Copy link
Copy Markdown
Collaborator

@tlatsas Yes please.

@atzorvas

Copy link
Copy Markdown

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

Thanks 👍

@radar

radar commented May 14, 2017

Copy link
Copy Markdown
Collaborator

LGTM. Thanks for your work on this!

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

Copy link
Copy Markdown

@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.

6 participants