themosis / plugin

Themosis framework plugin boilerplate.
54 stars 19 forks source link

[BUG] Loading textdomain fails #20

Closed Jaspervv closed 5 years ago

Jaspervv commented 5 years ago

Current code

Currently textdomains are loaded in the code here like so:

load_plugin_textdomain(
    $plugin->getHeader('text_domain'),
    false,
    $plugin->getPath(trim($plugin->getHeader('domain_path'), '\/'))
);

This doesn't work tho because the function load_plugin_textdomain makes use of the constant WP_PLUGIN_DIR and requires a relative path, not an absolute one like is given.

Fixed code

Here the last parameter uses a relative path:

load_plugin_textdomain(
    $plugin->getHeader('text_domain'),
    false,
    $plugin->getDirectory() .DS. trim($plugin->getHeader('domain_path'), '\/')
);

Dynamic function

We also had an issue that, when our plugin was installed as a must-use plugin, it wouldn't work since that requires the load_muplugin_textdomain function.
So we made our own function, based on the load_muplugin_textdomain function, that works when the plugin is installed as either a muplugin or regular plugin:

/**
 * Load text domain for the given Themosis plugin
 *
 * @param string $plugin
 * @return bool True on success, false on failure.
 */
function loadThemosisPluginTextDomain(string $plugin)
{
    $plugin = app("wp.plugin.{$plugin}");

    // get domain
    $domain = $plugin->getHeader('text_domain');
    $locale = apply_filters( 'plugin_locale', determine_locale(), $domain );

    // get path to .mo file
    $domain_path = trim($plugin->getHeader('domain_path'), '\/');
    $mofile = $domain . '-' . $locale . '.mo';
    $path = $plugin->getPath($domain_path .DS. $mofile);

    // load
    return load_textdomain($domain, $path);
}
loadThemosisPluginTextDomain('my-plugin');
jlambe commented 5 years ago

Thanks for reporting this one.

Regarding the plugin relative path, I suggest to add a method to the PluginManager class so we get simply do this:

load_plugin_textdomain(
    $plugin->getHeader('text_domain'),
    false,
    $plugin->getRelativePath())
);

And now regarding the helper function, can you describe where do you actually define it ? You seem to pass the plugin directoy as a parameter, using your function from a plugin root, the following snippet is supposed to work, right ?

<?php
// Plugin headers
// ....

loadThemosisPluginTextDomain($plugin->getDirectory());
Jaspervv commented 5 years ago

I made the function wrapper for this issue, we currently have the code without it being inside a function. (don't worry, I did test it 😋)

loadThemosisPluginTextDomain($plugin->getDirectory()); does indeed work, that would also be the best way to use it dynamically.

I made an issue instead of a pull request because I have no idea what a good place for the function would be.

jlambe commented 5 years ago

@Jaspervv I've currently developped an application textdomain function inside the Core/helpers.php called load_application_textdomain in order to set translations files on the application root. We could add your function there as well certainly so it is sure that it is available on all plugins and mu-plugins.

https://github.com/themosis/framework/blob/master/src/Core/helpers.php#L279

jlambe commented 5 years ago

@Jaspervv I'm already working on a solution for this. I'm adding a load_themosis_plugin_textdomain function to handle correct .mo file loading.