Improve language handling when creating JSON files

[swissspidy]

Turns out \Gettext\Translations::setLanguage() throws an exception when trying to pass a language that's not in the list provided by https://packagist.org/packages/gettext/languages (CLDR).

That means it fails when passing de_DE, because it expects de-DE.

This PR does two things:

• Replaces underscore with hyphens
• Catches exceptions and falls back to "en" in that case.

[alialaa]

Hello, I still get this error even after replacing underscores with dashes. When I run the make-json command, I get absolutely nothing. No errors, no success messages and no files created. I went to users/myuser/.wp-cli/packages/vendor/wp-cli/i18n-command to debug and I var_dumped the error in this handler. And it says: string(33) "The language "de-DE" is not valid" apparently this error is thrown no matter what string is passed there. So even $mapping[ $source ]->setLanguage( 'en' ); in the error handler will throw an error. I tried hardcoding different strings and none worked.

[swissspidy]

@alialaa Are you using the latest version of the command that includes the fixes from this PR?

[alialaa]

I guess so I ran php -d memory_limit=512M "$(which wp)" package install git@github.com:wp-cli/i18n-command.git and I am trying the code on this repository.

[alialaa]

Also in users/myuser/.wp-cli/packages/vendor/wp-cli/i18n-command/src/MakeJsonCommand I have this line $mapping[ $source ]->setLanguage( str_replace( '_', '-', $translations->getLanguage() ) );. I guess that means I am having the latest version?

[swissspidy]

Ah yes, that looks correct.

Looks like the issue is that the language list (https://github.com/mlocati/cldr-to-gettext-plural-rules/blob/master/src/cldr-data/main/en-US/languages.json) includes de but not de-DE. However, it does include en-US and en.

The upstream library we use leverages the language information solely for the purpose of adding plural forms: https://github.com/oscarotero/Gettext/blob/545f716f08338030d2d0d35a1835d04a9d4f18ed/src/Translations.php#L352-L361

However, we already do that ourselves anyway:

i18n-command/src/MakeJsonCommand.php

Lines 168 to 173 in af14c16

	$plural_forms = $translations->getPluralForms();
	if ( $plural_forms ) {
	list( $count, $rule ) = $plural_forms;
	$mapping[ $source ]->setPluralForms( $count, $rule );
	}

So I guess we can just silently ignore that exception...

[alialaa]

The thing is if I hard code 'en' like so in the try catch block, I also get The languane 'en' is not valid:

try {
   var_dump(Language::getAll());
   $mapping[ $source ]->setLanguage( 'en' );
} catch ( \InvalidArgumentException $e ) {
   // The locale had an invalid format.
   var_dump($e->getMessage());
   $mapping[ $source ]->setLanguage( 'en' );
}

Also not sure if this is related but Language::getAll() that I am dumping returns an empty array. Languages is from use Gettext\Languages\Language;

[swissspidy]

Weird, I'm pretty sure that worked in tests. But not really a problem. With my suggestion (created #142 for it) we don't need that list anyway.

[alialaa]

So I can just remove that whole try, catch block if I need to test it now, correct?

[swissspidy]

You could certainly do that for testing, yes, but that's not really the solution. We definitely should set the language for the translations.

The correct solution would probably be more like this:

try {
	$mapping[ $source ]->setLanguage( $translations->getLanguage() );
} catch ( \InvalidArgumentException $e ) {
	// Do nothing.
}

Or perhaps

$mapping[ $source ]->setHeader( \Gettext\Translations::HEADER_LANGUAGE, $translations->getLanguage() );

[alialaa]

Thanks, I will wait for your fix. I just tried this and it worked but I have some notes that maybe could be helpful to you:

1. The js strings are removed from the PO files after generating the JSON. I know this is intended but what if I wanted to update the PO file later, how will I get the old values that are now gone?
2. Also a weird thing happened with me. Not sure if it's intentional but I had multiple json files generated. 1 for each JS source file in the PO file.
3. Also the output file should be called {pluginName}-{locale}-{handler}.json (where handler is the first arg in wp_set_script_translations) in order for the translations to work. So maybe you can use that instead of the hash?

Thanks for your help!

[swissspidy]

@alialaa

1. Use the --no-purge flag to skip this, as per the documentation.
2. That is intentional. Please read the documentation and this longer explanation.
3. No. Or to be more precise: It depends. {pluginName}-{locale}-{handle}.json and {pluginName}-{locale}-{hash}.json both work. The problem is, WP-CLI does not know your script handle. Neither does WordPress.org. That's why we need to use the hash variant. However, for WP-CLI we might need to figure out how you could inform WP-CLI of the script handles in order to use the other variant. See Improve path handling when generating JSON files #127 for this.

[alialaa]

Thanks a lot :)

[michaelzangl]

I enocuntered the same issue. This is what I do to fix the wp-cli.phar

• Remove Symfony\Component\Finder\
• Replace $mapping[ $source ]->setLanguage( $translations->getLanguage() ); by $mapping[ $source ]->setHeader( \\Gettext\\Translations::HEADER_LANGUAGE, $translations->getLanguage() );

$phar = new Phar('.....wp-cli.phar'); $phar['vendor/wp-cli/i18n-command/src/MakeJsonCommand.php'] = str_replace( ['Symfony\\Component\\Finder\\', '$mapping[ $source ]->setLanguage( $translations->getLanguage() );'], ['', '$mapping[ $source ]->setHeader( \\Gettext\\Translations::HEADER_LANGUAGE, $translations->getLanguage() );'], $phar['vendor/wp-cli/i18n-command/src/MakeJsonCommand.php']->getContent());

[cbratschi]

Which WP-CLI version will include this fix?

[swissspidy]

@cbratschi The next one? :-)

Once #150 is merged, you can install the latest version of this package over what's included in WP-CLI by running this:

wp package install git@github.com:wp-cli/i18n-command.git

This way you don't have to wait for the next WP-CLI release.

[cbratschi]

@swissspidy Thanks a lot, it works now fine.