Rethink domain handling about i18n-command HOT 11 CLOSED

I guess the utility of "adding" text domains really comes down to how people use these commands. From what I've seen:

• Some people don't use a text domain during development, but run addtextdomain during a build process.
• Some use it on occasion or before commit to correct missing or misspelled text domains.
• Then there's the scenario with bundled libraries that need to have their text domains replaced initially and each time a new version is pulled in.

The name of the task isn't totally accurate since it adds, fixes, and replaces text domains, but adding can be an important function depending on how the command is integrated into a workflow.

The node and grunt packages are primarily used as build tools, so they can be configured to update specified domains or all domains, which is what that set_domains_to_update() method is for.

I imagine the WP-CLI command could default to updating all text domains in a project, but to be used in conjunction with scenarios 2 and 3 above, the ability to specify which ones are replaced seems like it would be necessary.

from i18n-command.

I'm pretty sure the current WP tools don't check the text domain at all when extracting strings. They all get included in the generated POT file.

With a plugin that looks like this:

<?php
/**
 * Plugin Name: Test Plugin
 */

__( 'No Text Domain' )
__( 'Valid Text Domain', 'test-plugin' );
__( 'Library Text Domain', 'library' );

node-wp-i18n, which is based on a forked version of the WP tools, generates this POT file:

# Copyright (C) 2017 
# This file is distributed under the same license as the Test Plugin package.
msgid ""
msgstr ""
"Project-Id-Version: Test Plugin\n"
"Report-Msgid-Bugs-To: https://wordpress.org/support/plugin/test-plugin\n"
"POT-Creation-Date: 2017-12-15 16:34:09+00:00\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=utf-8\n"
"Content-Transfer-Encoding: 8bit\n"
"PO-Revision-Date: 2017-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <[email protected]>\n"
"X-Generator: node-wp-i18n 1.0.4\n"

#: test-plugin.php:6
msgid "No Text Domain"
msgstr ""

#: test-plugin.php:7
msgid "Valid Text Domain"
msgstr ""

#: test-plugin.php:8
msgid "Library Text Domain"
msgstr ""

#. Plugin Name of the plugin/theme
msgid "Test Plugin"
msgstr ""

And the WP-CLI command generates this POT file:

# Copyright (C) 2017 Test Plugin
# This file is distributed under the same license as the Test Plugin package.
msgid ""
msgstr ""
"Project-Id-Version: Test Plugin\n"
"Report-Msgid-Bugs-To: https://wordpress.org/support/plugin/test-plugin\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <[email protected]>\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=UTF-8\n"
"Content-Transfer-Encoding: 8bit\n"
"POT-Creation-Date: 2017-12-15T16:34:44+00:00\n"
"PO-Revision-Date: 2017-12-15T16:34:44+00:00\n"
"X-Domain: test-plugin\n"

#: test-plugin.php:7
msgid "Valid Text Domain"
msgstr ""

#. Plugin Name of the plugin/theme
msgid "Test Plugin"
msgstr ""

I'm pretty sure some folks that have reported issues over the years would find the behavior in WP-CLI right now to be preferable. The node/Grunt tools rely on developers to exclude files or directories with strings they don't want extracted.

How are you expecting the third-party strings to be translated, with or without the --ignore-domain flag?

from i18n-command.

Thanks for your feedback!

Basically I thought it could be like this:

No flag = same behavior as now
With flag = output like node-wp-i18n

We could add a —domain parameter too. Then you could create a separate pot file (and therefore translation) for each domain. If no domain is passed, it uses the slug (i.e. what the script does now). Unless you set —ignore-domain of course.

from i18n-command.

I can see a --domain argument being helpful for generating multiple POT files.

If there are multiple text domains in a project, the makepot task is usually paired with an addtextdomain task in node-wp-i18n, which inserts or updates the text domain for any gettext calls. It seems like a similar command would be needed here to make the --ignore-domain flag useful.

from i18n-command.

@bradyvercher Could you give me an example why addtextdomain would be needed to make that flag useful?

At the moment I can think of the following scenarios:

1. One text domain, one POT file
   This is already covered.
2. Multiple text domains, ignore all but the determined domain (same as the slug)
   This is already covered.
3. Multiple text domains, one POT file for each
   This is where --domain would be useful.
4. Multiple text domains, one POT file containing all of them
   This is where --ignore-domain would be useful.

Now, how does using addtextdomain change things? Wouldn't it just turn scenario 4 into scenario 1? Perhaps I can't see the forest for the trees :-)

Besides that, having addtextdomain could definitely be useful for devs. The current implementation in the i18n-tools doesn't look too complicated. Guess we could rename the repo to wp-cli-i18n or something.

from i18n-command.

It's entirely possible I'm missing something myself, but here's what I'm thinking would happen in scenario 4:

All the strings will be extracted into a single POT file. However, when the translations are eventually loaded in WordPress via load_textdomain(), they'll all be associated with the project's text domain. So when a gettext call is run in one of the libraries that uses a different text domain, WordPress won't translate the string since the text domain doesn't match the one loaded for the project.

Using something like addtextdomain would turn scenario 4 into scenario 1, but isn't that necessary for WordPress to translate the strings properly?

There really isn't too much code to that addtextdomain task, so it shouldn't take much to port it over. I don't think that implementation is what you want though. If I recall correctly, it's not aware of the text domain position in gettext calls and will append the text domain as a new parameter if a different text domain is being used.

The command in node-wp-i18n allows for adding or replacing text domains instead. Feel free to grab anything from there if you want.

from i18n-command.

Using something like addtextdomain would turn scenario 4 into scenario 1, but isn't that necessary for WordPress to translate the strings properly?

Yeah that's true. You're right, scenario 4 isn't a realistic one.

Now that I think more about it, I wonder how often addtextdomain is really needed and if this command is the right place for it.

Replacing textdomains with a new one would definitely be a must-have feature, but it would need to work without explicitly calling set_domains_to_update() as a user. Otherwise one can just use search/replace in their editor. WordPress_Functions_Scanner is aware of the text domain position, so it probably could be done.

For now, I'll add a --domain flag shortly.

from i18n-command.

I feel like it makes sense to create a separate repository to explore a wp addtextdomain command. Even though there are many solutions for that already, including yours and the one WPCS/PHPCS/phpcbf provides.

from i18n-command.

That works for me, I just thought I'd offer feedback in case it was helpful. I didn't realize WPCS did this either, so I'll have to look into that.

from i18n-command.

YES PLEASE
... give the option to only include gettext calls with the plugin / theme domain!!

Use case: WooCommerce
WooCommerce has several template files like for the cart page. If you want to change the layout of the page, you simply copy the template to your theme and then edit it.
But just because the layout has changed does not mean the text shown is also different.
And WooCommerce is already translated in many languages, so you could use it in other languages easily.

Currently I'm using grunt-wp-i18n for generating the .pot file and do not have this option. So the wc strings are in my pot-file but did not get used.

And since the discussion for adding this was not successful I would cool to have it through WP-CLI

from i18n-command.

I feel like it makes sense to create a separate repository to explore a wp addtextdomain command.

This command here has since been renamed to wp i18n. A wp i18n addtextdomain command can be explored in a separate PR.

give the option to only include gettext calls with the plugin / theme domain!!

@michakrapp That's already the case. The command by default uses the plugin/theme slug as the text domain. The text domain can be overridden with the --domain argument though.

However, I would not recommend using the woocommerce text domain in your template files. I share the same concerns as here: cedaro/grunt-wp-i18n#64 (comment).

I'm closing this issue for now as I think there's nothing that needs to be changed right now. Feel free leave a comment if you think otherwise.

from i18n-command.