#3601 closed defect (bug) (maybelater)
Wrong detection of localizable method in case of class
Reported by: | Mte90 | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Component: | Translate Site & Plugins | Keywords: | 2nd-opinion |
Cc: |
Description
From https://github.com/GlotPress/GlotPress-WP/issues/895
Code example: https://plugins.trac.wordpress.org/browser/vikbooking/trunk/admin/views/dashboard/tmpl/default.php#L82
Just for reference: JHTML::_('behavior.keepalive');
The method is from a class so is not from gettext, we need a way to say to GlotPress to ignore this cases.
Attachments (2)
Change History (14)
#2
@
6 years ago
- Keywords 2nd-opinion close added
As our parsers are regex based (?) I don't think it's really possible for us to add exceptions for 'same method name, different object' right now.
Unless the instances of *::$translation_functions()
is high amongst plugins I don't think the benefit of excluding such strings outweigh the time involved with adding and managing exceptions to our parser rules right now unfortunately.
Having an extra string or two end up in translations isn't a huge issue, and no matter what translators translate it to, it won't have any effect anyway. (edit: well in this case it's more like 90+..)
#3
@
6 years ago
Maybe we can evaluate that if a class has the same methods of gettext is parsed anyway because maybe is a wrapper to avoid to put the textdomain.
Or in case that the textdomain is missing also if it is a class method is not parsed like in this case.
#4
@
6 years ago
- Keywords close removed
- Resolution set to maybelater
- Status changed from new to closed
The code for the parser is in the WordPress core project. The relevant part of the extractor is here: https://core.trac.wordpress.org/browser/trunk/tools/i18n/extract.php?rev=36781#L145
If someone has patch for this please go ahead and create a new ticket on core.trac.wordpress.org for the patch. Thanks!
We could also think about switching to https://github.com/wp-cli/i18n-command for plugins and themes, assuming that it doesn't have this issue. /cc @swissspidy
#5
@
6 years ago
Hello everybody,
This topic was opened after our company reported the issue to @Mte90 for the translations used by our plugins.
We've followed what @ocean90 advised, and I built a possible patch for the StringExtractor
class that resolves this issue. I would like you to review our proposal, hoping this will be helpful.
I'm attaching a ZIP package containing the modified file extract.php (all the modifications can be found by searching for @mod
) and a file with the correct results of the test made for our plugin VikBooking (see first message).
My modification allows to ignore the translating functions that belong to classes which are out of context. For example:
<?php // the function _ is ignored as it's part of the Html class Html::_('checkbox', 1, false);
Additionally, in case certain functions belonging to specific classes should not be ignored, maybe because they are part of the gettext framework, it is possible to pass them within the class constructor as a second optional argument. For example:
<?php $extractor = new StringExtractor($rules, ['Gettext', 'Translator']); ... // this code will be ignored Route::_($url); // these lines of code will be added as new translation entries Gettext::_('This string can be translated'); Translator::_('This string can be translated too');
That's all. Hopefully someone will review our proposal.
Please let us know your thoughts.
#6
@
6 years ago
If someone has patch for this please go ahead and create a new ticket on core.trac.wordpress.org for the patch. Thanks!
@MattDotNet Would you mind making a proper patch with your modifications? See https://make.wordpress.org/core/handbook/tutorials/working-with-patches/. Happy to assist with that, too.
Also, I'm curious to know which of your plugins you have experienced this with and whether https://github.com/wp-cli/i18n-command works for you just as well.
#7
@
6 years ago
Would you mind making a proper patch with your modifications?
@swissspidy unfortunately I'm not an expert with the creation of patches via SVN. To get started, I downloaded from http://develop.svn.wordpress.org/trunk/ the whole project through SmartSVN (for Mac), then I edited the file /tools/i18n/extract.php and I created the patch file ticket3601.diff.
What am I supposed to do with this file?
I followed the instructions listed at https://make.wordpress.org/core/handbook/tutorials/working-with-patches/ but we cannot use that software.
Any advice would be appreciated.
Also, I'm curious to know which of your plugins you have experienced this with and whether https://github.com/wp-cli/i18n-command works for you just as well.
The plugin that is facing this issue is VikBooking: https://wordpress.org/plugins/vikbooking/.
Unfortunately we could not test it with wp-cli commands.
Thank you.
#8
@
6 years ago
What am I supposed to do with this file?
You can either create a new ticket on core.trac.wordpress.org for updating the i18n tools, or just upload it here and I can do that for you.
Feel free to ping me on Slack if you have any questions.
By the way, if you're more comfortable in using Git, using that instead of SVN works just as fine. See https://make.wordpress.org/core/handbook/contribute/git/ for some examples.
The plugin that is facing this issue is VikBooking
Thanks, I'll test it against the WP-CLI command to see if it extracts all strings correctly. If so, we can focus on getting that onto wordpress.org instead of trying to fix the i18n-tools.
#9
@
6 years ago
You can either create a new ticket on core.trac.wordpress.org for updating the i18n tools, or just upload it here and I can do that for you.
@swissspidy Given the situation, I would rather have you do it for me (thanks). Next time I will definitely try using Git.
As you can see, I've just uploaded a ZIP file containing the modified file extract.php and the patch file related to this ticket (3601).
Thanks again.
#10
@
6 years ago
For future reference, I just ran the VikBooking plugin through the WP-CLI wp i18n make-pot
command.
- The POT file from WordPress.org contains 2955 strings
- The one from WP-CLI contains only 1652 strings, all of which are also part of the former
- The strings mentioned in this ticket aren't being extracted
- Also, The one from WP-CLI skips any strings without the
vikbooking
text domain (e.g.mod_vikbooking_horizontalsearch
)
#11
@
6 years ago
Thanks for your tests.
It's sounds like the WP-CLI command works fine. This would solve the issue too, and if you will switch to this method there is no need I guess to apply the patch we made for the extractor class.
Also, we are going to replace the text domain of the widget (mod_vikbooking_horizontalsearch
) with the text domain of the plugin (vikbooking
) as they share the same PO/MO file.
Do you know when and whether something will be changed? Because for the moment it is not possible to reach the 100% of the translation. Also, I guess once this method will be switched, the current translation strings will be updated for the plugin.
@ocean90 Do you if it's possible to have parsers ignore a line or skip a section? Is this even something for us to fix?