-
Notifications
You must be signed in to change notification settings - Fork 52
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
Rethink domain handling #7
Comments
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 |
Thanks for your feedback! Basically I thought it could be like this: No flag = same behavior as now We could add a |
I can see a If there are multiple text domains in a project, the |
@bradyvercher Could you give me an example why At the moment I can think of the following scenarios:
Now, how does using Besides that, having |
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 Using something like There really isn't too much code to that The command in node-wp-i18n allows for adding or replacing text domains instead. Feel free to grab anything from there if you want. |
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 Replacing textdomains with a new one would definitely be a must-have feature, but it would need to work without explicitly calling For now, I'll add a |
I guess the utility of "adding" text domains really comes down to how people use these commands. From what I've seen:
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 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. |
I feel like it makes sense to create a separate repository to explore a |
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. |
YES PLEASE Use case: WooCommerce 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 |
This command here has since been renamed to
@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 However, I would not recommend using the 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. |
Right now, the script only parses gettext calls where the text domain matches the plugin slug by setting
$this->translations->setDomain( $this->slug )
.This matches WordPress.org behaviour, see https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/.
However, sometimes developers include third-party libraries in their plugins with different text domains and still expect those to be extracted.
Since I'd like to keep the current behaviour as the default, maybe we can add an
--ignore-domain
option to the script?The code could look like this (simplified):
The text was updated successfully, but these errors were encountered: