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

Fix translate pluralization with nil in pluralization data #401

Closed

Conversation

fatkodima
Copy link
Contributor

Fixes #303.
Looks like it is more logically correct to raise an error rather than return translation missing:... in this case. wdyt?

@dillonwelch
Copy link
Contributor

@fatkodima based on what I've seen on usage of i18n the general theme seems to be return translation missing and then let the user turn on/off the configuration setting that allows that to raise an error or not.

That being said, if you fix the merge conflict here, we can flag the maintainers and see what they think.

@fatkodima
Copy link
Contributor Author

@oniofchaos done.

@dillonwelch
Copy link
Contributor

@radar another one for you 🎁 💝 see my comment above.

* master: (131 commits)
  Use Ruby 2.7.1
  Trigger actions on pull requests to master
  Bump Rake to v13
  Bump rake to v13
  Use flat_map instead of map.flatten in defaults
  Reduce memory usage in compute
  Reduce memory usage in Tag::Parents module
  Map in-place subtags
  Use flat_map instead of map.flatten in normalize_key
  Double-splat the arguments for I18n.translate
  Revert "Fix Ruby 2.7 keyword arguments related deprecation warning in Proc call"
  Misc readme updates
  Added double splat to resolve deprecation warning
  Switch to new GitHub issue template
  Update issue templates
  Bump to 1.8.2
  Fix regression introduced by b7f69f7
  Add pry to Gemfile
  Expand post-install message to clarify for new apps
  Revert "Revert "Chain fallback backends""
  ...
@radar
Copy link
Collaborator

radar commented Jun 4, 2020

This PR is failing the build. Please update it to pass.

@fatkodima
Copy link
Contributor Author

@radar This change conflicts with change from #495. This line fails

assert_equal "cat", I18n.t("cat", count: 1, locale: "en-US")

So in that PR tests were added to check that nils are ignored, but this PR behavior is to raise when nils. So how to proceed?

@radar
Copy link
Collaborator

radar commented Jun 5, 2020

You're right. I think it is better to ignore.

@radar radar closed this Jun 5, 2020
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.

count option use causes inconsistence
3 participants