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 ModelName default pluralization #252

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

svoboda-jan
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.1%) to 80.746% when pulling 1aeaf0c on svoboda-jan:patch-3 into d61193f on TrestleAdmin:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-4.1%) to 80.746% when pulling 1aeaf0c on svoboda-jan:patch-3 into d61193f on TrestleAdmin:master.

@spohlenz
Copy link
Member

What issue are you seeing here?

The form of pluralize that is being used here forces the inflector to use the current locale's pluralization rules (otherwise it defaults to English -- see https://api.rubyonrails.org/classes/String.html#method-i-pluralize).

These rules will usually come from the `rails-i18n' gem.

@svoboda-jan
Copy link
Contributor Author

Yes, the reason was to use english locale. At the time default_plural is called, we already know, that translation is'n present and we are pluralizing the class/model name (e.g. BlogPost), this change makes the assumtion, that model names (in code) are more likely in english rather than in the current locale.

@spohlenz
Copy link
Member

To be honest, I'm not 100% convinced which route is the best way to go here.

At the time default_plural is called, we already know, that translation is'n present and we are pluralizing the class/model name (e.g. BlogPost)

Just for clarification, we know the plural translation isn't provided. However a singular translation could have been provided, so we are not necessarily just pluralizing the internal model name.

By way of example, consider a model named Dog and the German locale (for which the translation is Hund [singular] and Hunde [plural]).

If the developer provides both singular and plural options, everything is straight-forward and works as expected:

de:
  activerecord:
    models:
      dog:
        one: Hund
        other: Hunde

The difference comes when only the singular form is provided:

de:
  activerecord:
    models:
      dog: Hund

In the current version, the ModelName#plural method returns "Hund". In the PR version, this would return "Hunds". Neither is correct although using the English plural form "Hunds" here is arguably less correct.

I think I'd be open to a configuration option default_pluralization_locale defaulting to -> { I18n.locale }. That way it could be overridden on a per-app basis if required. What do you think?

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.

3 participants