WordPress.org

Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#11012 closed enhancement (fixed)

Filter load_textdomain to control mofiles loaded

Reported by: johanee Owned by: westi
Milestone: 2.9 Priority: normal
Severity: normal Version: 2.9
Component: I18N Keywords: has-patch
Focuses: Cc:

Description

There are no (good, complete) hooks to control loading of textdomains.

Core wordpress uses the various load_*_textdomain functions which allows us too hook in through get_locale, but a number of plugins and themes calls load_textdomain() directly which bypasses that.

In my case I want it to cache the l10n object (http://wordpress.org/extend/plugins/cache-translation-object/ -- which currently abuses the locale hook) but it should also be useful for multi-language plugins. I've talked with michelwppi and it should solve #9686 for him.

Attachments (3)

l10n-filter-textdomain.patch (400 bytes) - added by johanee 11 years ago.
Filter mofile in load_textdomain
load_textdomain-hook.diff (4.6 KB) - added by nbachiyski 11 years ago.
load_textdomain-hook.2.diff (4.7 KB) - added by nbachiyski 11 years ago.

Download all attachments as: .zip

Change History (12)

@johanee
11 years ago

Filter mofile in load_textdomain

#1 @johanee
11 years ago

Ping!

Please comment if this is an acceptable approach or if I should try something else.

I'm also considering functionality to handle split mo files (admin / non-admin) which this hook should make possible but which would be a very, very ugly hack without it.

#2 @westi
11 years ago

  • Owner changed from nbachiyski to westi
  • Status changed from new to reviewing

#3 @nbachiyski
11 years ago

I like the idea about the filter. I went ahead and added another filter, which will allow plugins to override the whole function. This way somebody can use a different technology from gettext.

Also, I couldn't hold myself and the patch includes some distracting style changes.

#4 follow-up: @westi
11 years ago

I don't think this is a good idea.

        $plugin_override = apply_filters( 'load_textdomain', $domain, $mofile );
        
        if ( 'already-loaded' == $plugin_override ) {
                return true;
        }        

Now we can't have a plugin with a textdomain of "already-loaded" which although a bizarre name would be a bizarre bug!

How about this?

        $plugin_override = apply_filters( 'override_load_textdomain', false, $domain, $mofile );
        
        if ( true === $plugin_override ) {
                return true;
        }        

#5 in reply to: ↑ 4 @nbachiyski
11 years ago

Replying to westi:

Now we can't have a plugin with a textdomain of "already-loaded" which although a bizarre name would be a bizarre bug!

We can. The value of $plugin_override isn't used for anything, except to determine whether to go on.

How about this?

$plugin_override = apply_filters( 'override_load_textdomain', false, $domain, $mofile );

I didn't want to name the filter override_load_textdomain, because sometimes we want to use it to just execute our own code there and it doesn't have to do anything with overriding. For example, if I want to re-generate MO files each time a domain is loaded or if I want to log each load_textdomain call, it is bizarre to use a hook, which has override in its name.

Of course, we can have both. The filter you proposed and an action called load_textdomain. I don't have a preference.

#6 @ryan
11 years ago

The verdict?

#7 @johanee
11 years ago

FWIW any of the suggested approaches would solve my situation and would, I think, be a good addition.

It would appear to be mostly a question of naming and how/if to split the two desired features: to filter what mo-files load_textdomain() loads, and to override (or add to) the function.

Personally I would probably combine them in one filter of the mo-file that overrides the function on empty return similarly to other filters, but I'll gladly leave that decision to you professionals!

#8 @nbachiyski
11 years ago

The new patch uses westi-style override hook, has an action and the mofile filter.

#9 @westi
11 years ago

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

(In [12251]) Allow plugins to override the behaviour of load_textdomain() in a variety of flexible ways. Fixes #11012 props johanee and nbachiyski.

Note: See TracTickets for help on using tickets.