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.

Sign up for GitHub

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

Jump to bottom

Preserve count option #503

Merged
merged 2 commits into from
Jan 7, 2020
Merged

Preserve count option #503

merged 2 commits into from
Jan 7, 2020

Conversation

CrAsH1101
Copy link
Contributor

Some rails validation messages are broken after applying PR #480
More info in comment of #480

@CrAsH1101 CrAsH1101 force-pushed the preserve-count-option branch 3 times, most recently from d5c5552 to 9a5ae3e Compare December 17, 2019 14:08
radar
radar reviewed Dec 17, 2019
View reviewed changes
lib/i18n/backend/base.rb Outdated
@@ -163,7 +163,7 @@ def resolve(locale, object, subject, options = EMPTY_HASH)
# not standard with regards to the CLDR pluralization rules.
# Other backends can implement more flexible or complex pluralization rules.
def pluralize(locale, entry, count)
return entry unless entry.is_a?(Hash) && count
return entry unless entry.is_a?(Hash) && count && entry.values.select{|v| v.is_a?(Hash)}.size == 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps:

entry.values.none? { |v| v.is_a?(Hash) }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

Copy link
Contributor

Choose a reason for hiding this comment

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

To help with archeology:

This implicitly changed the behaviour of pluralization contexts with nested hashes:

en:
  stars:
    one: "%{count} star"
    other: "%{count} stars"
    special:
      one: "%{count} special star"
      other: "%{count} special stars"

Before:

> I18n.t("stars", count: 1)
"1 star"

After:

> I18n.t("stars", count: 1)
{:one=>"one", :other=>"other", :special=>{:one=>"one special star", :other=>"other special star"}}

Which was reported as #510, and was reverted in the case of Backend::Pluralization in 1b5e345, but not Backend::Base. The Backend::Base was reported as #514.

@radar
Copy link
Collaborator

radar commented Jan 7, 2020

Could you please add a test to this PR to ensure that the behaviour doesn't get re-broken later on?

@CrAsH1101
Copy link
Contributor Author

Which test file would be most suitable for this in your opinion?

@radar
Copy link
Collaborator

radar commented Jan 7, 2020

I figured fallbacks_test.rb was the best place for it, since it had lots of other tests for defaults. I've put a test there for your changes now and have verified that your patch does indeed fix the issue.

Thanks again! I'll do a release a little later on today (within an hour or two). Sorry for the trouble caused by #480 and thanks for your patience!

@radar radar merged commit b7f69f7 into ruby-i18n:master Jan 7, 2020
@caifara
Copy link

caifara commented Jan 8, 2020 •
edited
Loading

Hit this with an semi-obscure message deep in ActiveAdmin (took me a while to get here at least). A release will help more people like me. Thanks for all the hard work!

I had a translation file with an extra hash inside models.<model>. Such a construction is no longer (implicitly) allowed.

  activerecord:
    models:
      article: 
        one: artikel
        other: artikels
        choices:
          first_choice: foo
          second_choice: bar

@CrAsH1101 CrAsH1101 deleted the preserve-count-option branch January 8, 2020 13:02
jhanggi added a commit to wyeworks/nucore-open that referenced this pull request Jan 8, 2020
@jhanggi
Upgrade I18n to 1.7.1
80e5a20 
This change in I18n ruby-i18n/i18n#503 breaks our lookup for one/many when there is also a nested hash.

`Account.model_name.human` was returning the hash `{:one=>"Payment Source", :other=>"Payment Sources", :owner=>"Owner", :statuses=>{:active=>"Active", :suspended=>"Suspended"}}`.

Cleaning this up seemed simpler than trying to dig in to I18n.
@jhanggi jhanggi mentioned this pull request Jan 8, 2020
Merged
jhanggi added a commit to wyeworks/nucore-open that referenced this pull request Jan 8, 2020
@jhanggi
Upgrade I18n to 1.7.1
c57b8d6 
This change in I18n ruby-i18n/i18n#503 breaks our lookup for one/many when there is also a nested hash.

`Account.model_name.human` was returning the hash `{:one=>"Payment Source", :other=>"Payment Sources", :owner=>"Owner", :statuses=>{:active=>"Active", :suspended=>"Suspended"}}`.

Cleaning this up seemed simpler than trying to dig in to I18n.
jhanggi added a commit to wyeworks/nucore-open that referenced this pull request Jan 9, 2020
@jhanggi
Upgrade I18n to 1.7.1 (#2152)
8d6fb9d 
This change in I18n ruby-i18n/i18n#503 breaks our lookup for one/many when there is also a nested hash.

`Account.model_name.human` was returning the hash `{:one=>"Payment Source", :other=>"Payment Sources", :owner=>"Owner", :statuses=>{:active=>"Active", :suspended=>"Suspended"}}`.

Cleaning this up seemed simpler than trying to dig in to I18n.
joshea0 pushed a commit to SquaredLabs/nucore-uconn that referenced this pull request Mar 18, 2020
@jhanggi @joshea0
Upgrade I18n to 1.7.1 (wyeworks#2152)
87605ba 
This change in I18n ruby-i18n/i18n#503 breaks our lookup for one/many when there is also a nested hash.

`Account.model_name.human` was returning the hash `{:one=>"Payment Source", :other=>"Payment Sources", :owner=>"Owner", :statuses=>{:active=>"Active", :suspended=>"Suspended"}}`.

Cleaning this up seemed simpler than trying to dig in to I18n.
chrixp pushed a commit to SquaredLabs/nucore-uconn that referenced this pull request Oct 21, 2020
@jhanggi @chrixp
Upgrade I18n to 1.7.1 (wyeworks#2152)
40c2367 
This change in I18n ruby-i18n/i18n#503 breaks our lookup for one/many when there is also a nested hash.

`Account.model_name.human` was returning the hash `{:one=>"Payment Source", :other=>"Payment Sources", :owner=>"Owner", :statuses=>{:active=>"Active", :suspended=>"Suspended"}}`.

Cleaning this up seemed simpler than trying to dig in to I18n.
joshea0 pushed a commit to SquaredLabs/nucore-uconn that referenced this pull request Oct 23, 2020
@jhanggi @joshea0
Upgrade I18n to 1.7.1 (wyeworks#2152)
1027913 
This change in I18n ruby-i18n/i18n#503 breaks our lookup for one/many when there is also a nested hash.

`Account.model_name.human` was returning the hash `{:one=>"Payment Source", :other=>"Payment Sources", :owner=>"Owner", :statuses=>{:active=>"Active", :suspended=>"Suspended"}}`.

Cleaning this up seemed simpler than trying to dig in to I18n.
This was referenced Aug 19, 2022
Closed
Closed
Merged
movermeyer added a commit to movermeyer/i18n that referenced this pull request Aug 23, 2022
@movermeyer
Revert ruby-i18n#503 changes for Backend::Base
4c5687e 
See ruby-i18n#635 for discussion
@movermeyer movermeyer mentioned this pull request Aug 23, 2022
Merged
movermeyer added a commit to movermeyer/i18n that referenced this pull request Oct 7, 2022
@movermeyer
Revert ruby-i18n#503 changes for Backend::Base
cda0ea5 
See ruby-i18n#635 for discussion
radar added a commit that referenced this pull request Feb 3, 2023
@radar
Merge pull request #637 from movermeyer/movermeyer/nested_pluralizati…
42052e8 
…on_with_base_backend

Revert #503 changes for `Backend::Base`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@movermeyer movermeyer movermeyer left review comments

@radar radar radar left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@CrAsH1101 @radar @caifara @movermeyer