WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 6 years ago

Last modified 4 years ago

#3089 closed enhancement (fixed)

Localized values for plugin metadata

Reported by: nbachiyski Owned by: ryan
Milestone: 2.7 Priority: normal
Severity: major Version:
Component: I18N Keywords: has-patch tested phpdoc dev-feedback
Focuses: Cc:

Description

As Ryan suggested the way in a mail to wp-polyglots here is an impelmentation of the freedesktop.org standard for wordpress plugins.

Unfortunately the same system for themes is not ready yet. The moment I find some time it will be shipped also.

NOTE: the functions contain phpdoc, which could be removed if not needed.

Attachments (7)

localized-keys-in-plugins.diff (6.2 KB) - added by nbachiyski 8 years ago.
localized-headers-in-exts.diff (9.3 KB) - added by nbachiyski 8 years ago.
The revised patch
3089.patch1.diff (1.6 KB) - added by darkdragon 6 years ago.
Implements gettext method for plugin metadata
plugin_new.diff (1.8 KB) - added by jhodgdon 6 years ago.
Alternate (I think better) patch that implements gettext method
3089.patch2.diff (1.1 KB) - added by darkdragon 6 years ago.
Uses translate() instead and combines patch from jhodgdon
3089.5651.patch.diff (5.4 KB) - added by darkdragon 6 years ago.
Combined #3089 completed patch, along with #5651 optimizations, with phpdoc and based off of r6666
3089.diff (5.3 KB) - added by DD32 6 years ago.
Moves Translation and Display markup to Plugins page. Note that PHPDoc has not been updated to reflect; May break compat with Updater's web api due to markup changes, May not actually translate(do not have testing plugin)

Download all attachments as: .zip

Change History (65)

comment:1 majelbstoat8 years ago

As the author of Gengo, I'm interested in this. If this was committed, how would plugin authors use it effectively? A short example plugin definition would be good.

comment:2 nbachiyski8 years ago

Here is the new patch :)
Theme headers now also support localizations.
In addition some the code is cleaner and some common functions for parsing the theme/plugin headers are introduced.

nbachiyski8 years ago

The revised patch

comment:3 nbachiyski8 years ago

@majelbstoat:

Just append [locale] after the header name and voilla, it is localized :)

Here is an example plugin header:
/*
Plugin Name: Bender, bender, where are you?
Plugin Name[bg_BG]: Бендър, бендър, къде си?
Plugin URI: http://bender.wow/
Plugin URI[bg_BG]: http://bg.bender.wow/
... and so on
*/

comment:4 nbachiyski8 years ago

Oops, I forgot the formatting ;)
Here are the sampe headers again:

/*
Plugin Name: Bender, bender, where are you?
Plugin Name[bg_BG]: Бендър, бендър, къде си?
Plugin URI: http://bender.wow/
Plugin URI[bg_BG]: http://bg.bender.wow/
... and so on
*/

comment:5 ryan8 years ago

  • Owner changed from anonymous to ryan

comment:6 SteveAgl8 years ago

Even fo I agree on the ML discussion about this method now I think its a bit a mess, till now all localization was made (except for a couple of files like license.html and the distro themes) using language files, so localizing team didn't have to touch directly code of plugins etc.

This way we will have to touch code, and distribute locale versione of plugin distrubuted with WP, for other plugins if i do the italian version i ghave to send back tto author not only the language files but the plugin code too and he have to merge the header with my new additions and redistribute the files.. that sound a bit a mess :)

Actually i don't have any idea about an alternative method. A contorted one should be having a series of constant declaration at the top of the plugin code that will be gettexted so they will be included into the .pot file anc can be easily localized. Other Idea ?

comment:7 matt7 years ago

  • Milestone changed from 2.1 to 2.2

comment:8 majelbstoat7 years ago

All these plugins will have there own plugin localisation domain defined right? So how about something like this:

/*
Plugin Name: Bender, bender, where are you?
Plugin Name[bg_BG]: Бендър, бендър, къде си?
Plugin Domain: testdomain
*/

$plugin_domain = 'testdomain'; Or a constant, or however the plugin defines it.
$plugin_meta[$plugin_domain][name] = ('Bender, bender, where are you?', $plugin_domain);
$plugin_meta[$plugin_domain][uri] =
('http://bender.wow', $plugin_domain);

Then in the parsing code, if 'Plugin Domain:' is found, try to load $plugin_meta[$parsed_domain] name and uri, or fallback to the defaults parsed from the comments.

It does require a little bit of duplication on the plugin author's part, but it's backwards compatible and will still work if it's not supplied... And this way, the name and uri can just become part of the POT file...

I'd argue that a Plugin Domain would be a useful thing for a plugin to define anyway, something that uniquely references it... (or perhaps it could be autogenerated from a sanitised form of the plugin name?)

I'm not too keen on the proposed method, because it doesn't scale too well, as well as all the points raised by SteveAgl.

Thoughts on this?

comment:9 majelbstoat7 years ago

Forgot the code tags, d'oh. In the above example, the name and uri assignment statements should be double-underscored in case it isn't clear.

comment:10 foolswisdom7 years ago

  • Keywords has-patch needs-testing added; bg|commit bg|needs-testing removed
  • Milestone changed from 2.2 to 2.3

comment:11 ryan7 years ago

  • Milestone changed from 2.3 to 2.4 (next)

comment:12 darkdragon6 years ago

Holy crap, yo.

I think #4048 would suit well, since it would allow the plugin author to distribute the plugin themselves. The only problem with this method is that the plugin will appear in whatever language is in the top comments, then when the plugin is loaded, it will change.

If plugin is disabled, whatever appears in comments will be shown in that language.
If plugin is enabled, plugin can overwrite display and display localized version of the text.

However, the activate text should still be in the admin's language, so that shouldn't be an issue.

comment:13 santosj6 years ago

  • Component changed from General to i18n

comment:14 nbachiyski6 years ago

Keeping the translations in the plugin/theme file doesn't sounds very reasonable to me now.

However, we can try and use the functions, which eliminate and structure the parsing code.

darkdragon6 years ago

Implements gettext method for plugin metadata

comment:15 darkdragon6 years ago

As discussed on wp-hackers, the patch implements gettext method, but needs testing with plugin .mo file.

Second method will be implemented later.

comment:16 darkdragon6 years ago

Patch also has work on second method, which will only be displayed by calling the function directly.

comment:17 ryan6 years ago

__($description[1], $text_domain); 

You can't pass variables to (). Only literal strings. The plugin has to call () with literal strings for those strings to show up in the po file and thus end up in the mo file. With this method, we're loading a mo file that won't have the necessary strings in it.

comment:18 ryan6 years ago

We could make this work by having plugins call __() with the exact same string used in the header. This would get the string in the catalog. Within WP, we could use a new function in place of __() that does not result in the string ending up in the catalog. This will avoid having $description[1] in the po file.

Do we need Domain Path? In most plugins this is set dynamically based on the directory the plugin is in. We can derive the path from the location of the plugin.

comment:19 jhodgdon6 years ago

I tested the patch submitted by darkdragon. It sort of worked. I fixed it up so that it worked for me.

Issues:

  • Name, description, etc. needed to be trimmed before translation
  • I got errors from the l10n.php file, because even if the plugin did not provide a text domain, and preg_replace returned false, the match array $text_domain was being set (at least on my test server, running PHP 5). So instead of checking to see if the $text_domain array has been set, I thought it was better to use the return value of preg_replace to see if translation should be done.

I'll attach the patch... this one works for me.

jhodgdon6 years ago

Alternate (I think better) patch that implements gettext method

comment:20 ryan6 years ago

The patch pollutes the WP catalog and doesn't address getting strings into the plugin catalog.

Let's see how this thread pans out:

http://comox.textdrive.com/pipermail/wp-hackers/2008-January/017409.html

darkdragon6 years ago

Uses translate() instead and combines patch from jhodgdon

comment:21 darkdragon6 years ago

I believe this will be a nice addition to 2.5, since the other discussion won't be accepted in WordPress 2.5. In which case, the patch still needs to be tested and should not pollute the WP catalog like the other patch did.

The strings do still have to match exactly, which is an okay requirement.

comment:22 darkdragon6 years ago

Patch is based off r6666.

darkdragon6 years ago

Combined #3089 completed patch, along with #5651 optimizations, with phpdoc and based off of r6666

comment:23 darkdragon6 years ago

  • Keywords phpdoc added

comment:24 darkdragon6 years ago

Added patch which combines both #3089 and #5651.

comment:25 nbachiyski6 years ago

IMHO, the get_ext_header() function from my second patch and the parsing abstraction could be useful even out of the i18n context.

comment:27 westi6 years ago

  • Milestone changed from 2.5 to 2.6

Moving to 2.6.

Feature freeze has arrived.

comment:28 nbachiyski6 years ago

Here is a script, which adds the appropriate headers to the extension POT file:

http://svn.automattic.com/wordpress-i18n/tools/trunk/pot-ext-meta.php

I will work towards integration with the plugins infrastructure.

comment:29 santosj6 years ago

  • Milestone 2.9 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Told this won't be part of the core. Will be implemented as an external tool.

comment:30 nbachiyski6 years ago

  • Milestone set to 2.7
  • Resolution wontfix deleted
  • Status changed from closed to reopened

Jacob, we still need to run the metadata through translate for this to work.

comment:31 jacobsantos6 years ago

Yeah, no I don't really see that. If that was true, then you really just wasted a lot of my time and it would mean that the patch I created was in fact worth committing, for like WordPress 2.5.0.

If the external online tool is going to be translating the plugin metadata, then that tells me that the external tool is going to write the translation to the plugin metadata before the plugin is downloaded.

So the tool is going to extract the metadata and have it translatable somehow and then allow translators to write in in the translation in the tool. When the user chooses a different language for downloading, the plugin metadata will be pulled out and replaced with the language translation.

The only way this would make sense, is if you are adding the metadata to the .pot file and handling it that way. If that is the case, then the patch I created did in fact solve this problem and wasn't committed for at least 5 months.

It would also help if this online tool existed. I think this should be closed in the mean time until the online tool exists and a patch needs to be created for it.

It would also help if the core team could decide how they want to solve this problem, once and for all. It is difficult, because the specifications keep changing.

comment:32 santosj6 years ago

The patch shouldn't be stale, but should try to patch anyway.

comment:33 santosj6 years ago

  • Keywords tested added; needs-testing removed

comment:34 nbachiyski6 years ago

The only way this would make sense, is if you are adding the metadata to the .pot file and handling it that way.

That's exactly what I do. Plugin devs can generate their pot file from the admin interface in extend/plugins or they can use the script linked above.

Sorry for the confusion, it is entirely my fault that I didn't urge somebody of the core team to commit it weeks ago.

comment:35 ryan6 years ago

Sooo, what would I be committing? 3089.5651.patch.diff?

comment:37 ryan6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [8367]) Plugin metadata localization and get_plugin_dta() optimization from santosj. fixes #5651 #3089

comment:38 jacobsantos6 years ago

Is it possible to get this in 2.6.1 also?

comment:39 follow-up: jacobsantos6 years ago

I just remembered something. Isn't it a bad idea to allow the plugin name to be translated because it might interfere with the plugin updater.

comment:40 in reply to: ↑ 39 DD326 years ago

Replying to jacobsantos:

I just remembered something. Isn't it a bad idea to allow the plugin name to be translated because it might interfere with the plugin updater.

It might be worth allowing a bypass for the translations for the updater, AFAIK the web service utilises a combination of Plugin URI, Plugin Name, Filename, and potentially the Author details too(I think).

That would also mean with the current implementation that the files would need to be read twice as the cached results would be useless.

My only suggestion would be to move the Translations to the plugins.php file, I'm attempting a patch of that right now, But its a bit messy since theres Display Logic(Links) in the get_plugin_data() function.

DD326 years ago

Moves Translation and Display markup to Plugins page. Note that PHPDoc has not been updated to reflect; May break compat with Updater's web api due to markup changes, May not actually translate(do not have testing plugin)

comment:41 DD326 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

attachment 3089.diff added.

My suggestions, See note.

Re-opening due to changes affecting updater.

comment:42 jacobsantos6 years ago

I was just thinking of adding another parameter to the function, which if set to true will translate and if set to false (the default) will not try to translate.

Depends now that there is a plugin cache. It might be easier to have two caches one for the translated version and another for the untranslated version.

comment:43 ryan6 years ago

Can we just add another element to the plugin data for the raw/canonical name?

comment:44 DD326 years ago

I was just thinking of adding another parameter to the function, which if set to true will translate and if set to false (the default) will not try to translate.

The reason i avoided that was to remove the extra processing for when its not needed.

The plugin updater runs every 12 hours, which involves parsing all the plugins, It doesnt need the translated or marked up details.
The plugins page is visited a bit less often, And is the only real place that it needs translated details to show up (There are plugins which show the plugins list, that'd need to be updated).

The cache in use is only for that pageload, it doesnt affect future page loads, that cached data should not be kept past the end of the pageload. So it seems a waste of time to cache them seperately, Esp when the likely chances are that it'll not need to be translated at all.

A parameter would allow more backwards compatibility, However moving Display-related logic into the Main file and leaving the data sourcing to the raw functions makes more sense to me. Its just a suggestion.

Can we just add another element to the plugin data for the raw/canonical name?

Theres no reason why it cant also include the untranslated details, I just felt the way i suggested it was a bit 'cleaner' in terms of code, But i'm happy as long as it works :)

comment:45 jacobsantos6 years ago

You patch looks good to me!

comment:46 ryan6 years ago

Either way is fine by me. I was just wondering if plugins would want the translated plugin data. Having them push the raw data through translate() (something we usually don't want them to do) seems a little sketchy. But, no biggie.

comment:47 jacobsantos6 years ago

I'll have to look over the inline documentation. I think it is okay as it is, since nothing really changed except that the translation moved to the plugins page. Depending on the wording (which might need to be changed), it might be okay.

comment:48 ryan6 years ago

(In [8368]) Move plugin data translation and display markup to plugins page. Props DD32. see #3089

comment:49 jacobsantos6 years ago

Inline documentation is okay. There doesn't need to be any changes.

comment:50 santosj6 years ago

Are there any other issues that need to be resolved with this ticket?

comment:51 santosj6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Seems fixed to me.

comment:52 codestyling6 years ago

  • Resolution fixed deleted
  • Severity changed from normal to major
  • Status changed from closed to reopened

I don't think so, this must not be the final solution:

foreach( (array)$all_plugins as $plugin_file => $plugin_data) { 
	 
	//Translate fields 
	if( !empty($plugin_data['TextDomain']) ) { 
		if( !empty( $plugin_data['DomainPath'] ) ) 
			load_plugin_textdomain($plugin_data['TextDomain'], dirname($plugin_file). $plugin_data['DomainPath']); 
		else 
			load_plugin_textdomain($plugin_data['TextDomain'], dirname($plugin_file)); 
 
		foreach ( array('Name', 'PluginURI', 'Description', 'Author', 'AuthorURI', 'Version') as $field ) 
			$plugin_data[ $field ] = translate($plugin_data[ $field ], $plugin_data['TextDomain']); 
	} 

If you have an environment containing a lot of plugins and each having big *.mo files (WP de_DE > 250kb / some mailing lists > 100kb and so on) or a huge count of, they would sum up in memory and may exceed the max_memory_usage value prior to produce the plugin page itself !

Because the gettext handling is not able to unload that files yet, the loop introduced keeps all of them in memory with complete content during looping all! found plugins.
I think, this is very critical at production systems having a huge count of plugins installed and are not longer able to use the plugin page without crashing the script.

In my opinion there is at least a second solution needed to unload the translation files after successful translated parts to avoid memory override and thatswhy i reopen this as potential dangerous and massive time consuming enhancement.

comment:53 follow-up: DD326 years ago

AFAIK, most plugins with translations load them on the init hook anyway, so they should be loaded allready, However that code doesnt check to see ifs its been loaded yet, so you could end up with the translations being loaded twice..

Due to the way the l10n code is written, AFAIK there isnt a way to unload any given translation file, since each translation file loaded is merged with eachother in memory.

comment:54 in reply to: ↑ 53 codestyling6 years ago

Replying to DD32:

AFAIK, most plugins with translations load them on the init hook anyway, so they should be loaded allready, However that code doesnt check to see ifs its been loaded yet, so you could end up with the translations being loaded twice..

Due to the way the l10n code is written, AFAIK there isnt a way to unload any given translation file, since each translation file loaded is merged with eachother in memory.

The loading of active plugins may be this way. Several good pluginwriter try to load them as lazy as possible or if their admin page(s) really get's displayed.
But this is not the point. Let's say you have 10 activated plugins (may be ok for memory) but 150 deactivated ones each having a translation file. The start of loading now 160 *.mo files will crash the script memory usage definitely.

And merging occures only, if more than one plugin gots the exact same textdomain name because for each textdomain name a hash entry with key = textdomain name and value = gettext object will be stored internally.

Suggestion: It will first need a test, if the textdomain name exists. If so, never unload it but use it without loading again the textdomain *.mo file.
If the textdomain name is not available at the hash, mark this temporary, load it, translate it and cleanup an free that entry now.
Doing so, you only keep those already have been loaded before and get a safe unload for temporary translation needs because you add temporary only 1 new *.mo per loop scope start and gets it unloaded at loop scope end.
After loop finalization the translation object cache (hash map) is at same state as before.

comment:55 DD326 years ago

  • Keywords dev-feedback added

And merging occures only, if more than one plugin gots the exact same textdomain name because for each textdomain name a hash entry with key = textdomain name and value = gettext object will be stored internally.

Indeed, Sorry, Looks like i was thinking the $domain was the $locale :)

I'll pass this over to any internationalisation folks now to fight it out with :)

comment:56 jacobsantos6 years ago

Is it possible to have the optimizations another ticket to better follow the arguments and progress. This ticket is basically fixed and does what was meant. Optimization is outside the scope of this ticket, I believe.

comment:57 jacobsantos6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Optimizations sent to new ticket #7820.

comment:58 hakre4 years ago

Related: #13699

Note: See TracTickets for help on using tickets.