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

i18n: Set locale data per domain #5168

Closed
aduth opened this issue Feb 20, 2018 · 6 comments
Closed

i18n: Set locale data per domain #5168

aduth opened this issue Feb 20, 2018 · 6 comments
Assignees
Labels
Internationalization (i18n) Issues or PRs related to internationalization efforts [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@aduth
Copy link
Member

aduth commented Feb 20, 2018

The current implementation of the JavaScript wp.i18n.setLocaleData and PHP gutenberg_get_jed_locale_data functions assume to set a singular blob of Jed data, and do not accommodate multiple domains.

We should seek to change this in two ways:

gutenberg_get_jed_locale_data should not set the default domain, and in-fact may only return just the inner array currently represented as $domain => array(:

gutenberg/lib/i18n.php

Lines 25 to 27 in 1ca5f6c

'domain' => $domain,
'locale_data' => array(
$domain => array(

wp.i18n.setLocaleData should accept a domain argument, and merge strings for the given domain into the current (or a new) global Jed configuration:

gutenberg/i18n/index.js

Lines 15 to 17 in 1ca5f6c

export function setLocaleData( data ) {
i18n = new Jed( data );
}

@aduth aduth added [Type] Task Issues or PRs that have been broken down into an individual action to take Internationalization (i18n) Issues or PRs related to internationalization efforts labels Feb 20, 2018
@aduth aduth self-assigned this Mar 8, 2018
@tpaksu
Copy link

tpaksu commented Nov 8, 2019

IMO, a fallback to the "default" domain when the key doesn't exist in the given domain would also be a good feature for this.

@aduth
Copy link
Member Author

aduth commented Nov 8, 2019

Hi @tpaksu , can you elaborate on what you're expecting? Are you referring to something like a specialized language code falling back to the more generalized form? (For example, with a complete set of strings in fr and a subset of specialized strings in Brazilian fr-CA, fall back to fr if it can't be found in fr-CA?)

I think this should be possible already, as long as you merge the string data before calling setLocaleData.

Something like:

const fr = { '': { lang: 'fr' }, 'Why?': [ 'Pourquoi?' ], 'Title': [ 'Titre' ] };
const frCA = { '': { lang: 'fr-CA' }, 'Why?': [ 'À cause?' ] };

wp.i18n.setLocaleData( lodash.assign( {}, fr, frCA ), 'mydomain' );

wp.i18n.__( 'Why?', 'mydomain' )
// "À cause?"

wp.i18n.__( 'Title', 'mydomain' )
// "Titre"

Is that what you have in mind?

Otherwise, it would be best to create a separate issue where we can discuss an implementation which supports your needs.

@tpaksu
Copy link

tpaksu commented Nov 8, 2019

Hi @aduth,

I thought that if someone uses the domain component-wise, such as having the default domain to include the core translations and mydomain as a custom block translation, I suppose he/she will call the translation in the custom block for example like this (on fr locale specifically):

wp.i18n.__('Title', 'mydomain')

will show Title if the mydomain translations are not loaded or any other thing has happened etc, which is the less wanted case for fr locale. But, the core translation (default domain) may have that replacement translation instead. so if the translation doesn't exist in mydomain configuration, then look into the default domain and if that also doesn't exist, show the given singular key instead. This may lead some incomplete translations, wrong results etc, but will cover most of the missing key cases IMHO.

@tpaksu
Copy link

tpaksu commented Nov 8, 2019

But, of course this may better suit into the Tannin package instead the wp.i18n. Maybe

https://github.com/aduth/tannin/blob/e189cff824dff9f8868e5d3d66469bbbfba737d4/packages/tannin/index.js#L185

may contain an extra parameter for the fallback domain, if will be used, and:

https://github.com/aduth/tannin/blob/e189cff824dff9f8868e5d3d66469bbbfba737d4/packages/tannin/index.js#L203-L209

may be extended with:

entry = this.data[ fallback ][ key ];
if ( entry && entry[ index ] ) {

@aduth
Copy link
Member Author

aduth commented Nov 8, 2019

Oh, I think I understand what you're suggesting now. I'm a bit reluctant to revise the argument signature, largely because it is meant to precisely mimic the equivalent function in GNU gettext.

But I wonder if there might be some option around the onMissingKey option there in the code.

  • It could be adapted to return the value from onMissingKey, letting someone call to the default domain
  • Otherwise, it could be used to "mark" that the result is missing

Example on how that can be achieved today:

https://runkit.com/aduth/5dc5e439ab7d84001af84373

@tpaksu
Copy link

tpaksu commented Nov 8, 2019

Yes, about onMissingKey, I opened a new feature request today, #18392 as it can't be accessed from outside. If it was available, it was the best place to hook on to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internationalization (i18n) Issues or PRs related to internationalization efforts [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

No branches or pull requests

2 participants