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

Make gettext functions filterable #12517

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions packages/i18n/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 3.2.0 (Unpublished)

### Enhancement

- Added various filters to the gettext functions, just like in their PHP counterparts.

## 3.1.0 (2018-11-15)

### Enhancements
Expand Down
1 change: 1 addition & 0 deletions packages/i18n/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
},
"dependencies": {
"@babel/runtime": "^7.0.0",
"@wordpress/hooks": "file:../hooks",
"gettext-parser": "^1.3.1",
"lodash": "^4.17.10",
"memize": "^1.0.5",
Expand Down
57 changes: 52 additions & 5 deletions packages/i18n/src/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* WordPress dependencies
*/
import { applyFilters } from '@wordpress/hooks';

/**
* External dependencies
*/
Expand Down Expand Up @@ -90,7 +95,16 @@ function dcnpgettext( domain = 'default', context, single, plural, number ) {
* @return {string} Translated text.
*/
export function __( text, domain ) {
return dcnpgettext( domain, undefined, text );
const translation = dcnpgettext( domain, undefined, text );

/**
* Filters text with its translation.
*
* @param {string} translation Translated text.
* @param {string} text Text to translate.
* @param {string} domain Text domain. Unique identifier for retrieving translated strings.
*/
return applyFilters( 'i18n.gettext', translation, text, domain );
Copy link
Member

Choose a reason for hiding this comment

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

@aduth - we probably should also optimize applyFilters to return early when there are no hooks to process. See:

https://github.com/WordPress/gutenberg/blob/master/packages/hooks/src/createRunHook.js#L22-L33

It makes me think that having to run filters on every re-render might be bad for the performance of the application.

Copy link
Member

Choose a reason for hiding this comment

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

Okey, I see #12517 (comment) which aligns with my comment.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it's not too horrible though. Outside of some conditions and a one-time setup, it amounts to incrementing the runs each time it's called.

}

/**
Expand All @@ -105,7 +119,17 @@ export function __( text, domain ) {
* @return {string} Translated context string without pipe.
*/
export function _x( text, context, domain ) {
return dcnpgettext( domain, context, text );
const translation = dcnpgettext( domain, context, text );

/**
* Filters text with its translation based on context information.
*
* @param {string} translation Translated text.
* @param {string} text Text to translate.
* @param {string} context Context information for the translators.
* @param {string} domain Text domain. Unique identifier for retrieving translated strings.
*/
return applyFilters( 'i18n.gettext_with_context', translation, text, context, domain );
}

/**
Expand All @@ -123,7 +147,18 @@ export function _x( text, context, domain ) {
* @return {string} The translated singular or plural form.
*/
export function _n( single, plural, number, domain ) {
return dcnpgettext( domain, undefined, single, plural, number );
const translation = dcnpgettext( domain, undefined, single, plural, number );

/**
* Filters the singular or plural form of a string.
*
* @param {string} translation Translated text.
* @param {string} single The text to be used if the number is singular.
* @param {string} plural The text to be used if the number is plural.
* @param {string} number The number to compare against to use either the singular or plural form.
* @param {string} domain Text domain. Unique identifier for retrieving translated strings.
*/
return applyFilters( 'i18n.ngettext', translation, single, plural, number, domain );
}

/**
Expand All @@ -142,15 +177,27 @@ export function _n( single, plural, number, domain ) {
* @return {string} The translated singular or plural form.
*/
export function _nx( single, plural, number, context, domain ) {
return dcnpgettext( domain, context, single, plural, number );
const translation = dcnpgettext( domain, context, single, plural, number );

/**
* Filters the singular or plural form of a string with gettext context.
*
* @param {string} translation Translated text.
* @param {string} single The text to be used if the number is singular.
* @param {string} plural The text to be used if the number is plural.
* @param {string} number The number to compare against to use either the singular or plural form.
* @param {string} context Context information for the translators.
* @param {string} domain Text domain. Unique identifier for retrieving translated strings.
*/
return applyFilters( 'i18n.ngettext_with_context', translation, single, plural, number, context, domain );
}

/**
* Returns a formatted string. If an error occurs in applying the format, the
* original format string is returned.
*
* @param {string} format The format of the string to generate.
* @param {string[]} ...args Arguments to apply to the format.
* @param {string[]} args Arguments to apply to the format.
*
* @see http://www.diveintojavascript.com/projects/javascript-sprintf
*
Expand Down