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

Do not assume that singular form in _n() is used just for single item #9711

Merged
merged 2 commits into from Sep 10, 2018

Conversation

Projects
None yet
2 participants
@dimadin
Contributor

dimadin commented Sep 8, 2018

Description

As explained in this comment (and linked pages), in languages other than English, singular form in _n() can be used when there is more than one item, not just for one item.

In two cases, we use placeholder in plural form in _n() but not in singular form, because we assume that it’ll be used for just one item.

This PR splits those strings so that we have separate strings when there is just one item and for more than one item, and always include placeholder in _n() singular forms.

How has this been tested?

By selecting different number of blocks after running npm run build. Also npm run test.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@aduth aduth added the i18n label Sep 10, 2018

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Sep 10, 2018

Member

Related: Calypso has adopted its own linting rules to capture these sorts of issues, based on a mismatch of the number of placeholders in the plural forms string:

https://github.com/Automattic/eslint-plugin-wpcalypso/blob/master/docs/rules/i18n-mismatched-placeholders.md

Might be worth exploring a port here.

Member

aduth commented Sep 10, 2018

Related: Calypso has adopted its own linting rules to capture these sorts of issues, based on a mismatch of the number of placeholders in the plural forms string:

https://github.com/Automattic/eslint-plugin-wpcalypso/blob/master/docs/rules/i18n-mismatched-placeholders.md

Might be worth exploring a port here.

@aduth

aduth approved these changes Sep 10, 2018

Thanks 👍

I was finding each line to be quite long, so opted to break them across multiple lines for easier reading. Hope you don't mind the change.

@aduth aduth added this to the 3.9 milestone Sep 10, 2018

@aduth aduth merged commit c967c21 into WordPress:master Sep 10, 2018

2 checks passed

codecov/project 49.03% (+0%) compared to 410f78a
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment