leobaiano / baianada

This project aims to provide the basic framework for creating a WordPress plugin.
GNU General Public License v3.0
24 stars 2 forks source link

Don't use variables for the $textdomain parameter in localization functions #1

Open nikolov-tmw opened 8 years ago

nikolov-tmw commented 8 years ago

Using variables in localization functions(and possibly in register_plugin_textdomain()) is not advisable.

Here's a reference as to why it's not a good idea: https://markjaquith.wordpress.com/2011/10/06/translating-wordpress-plugins-and-themes-dont-get-clever/

To summarise it, GNU gettext doesn't parse PHP and instead just scans all files for localization functions(__(), _e(), etc.) and therefore has no way of knowing what $domain stands for( as seen here - https://github.com/leobaiano/baianada/blob/master/helper/post-type.php#L49 ). Therefore you should replace all uses of $domain with 'lb-baianada'.

Following the same logic, don't pass variables as the first parameter either(in the line above you used $this->name). Here's another article written by Otto on how to properly use translation functions - http://ottopress.com/2012/internationalization-youre-probably-doing-it-wrong/

leobaiano commented 8 years ago

Thank you for the information, I will check the impact of these changes because, as you can see, leave the domain as a string can derail the operation. For example, LB_Post_Type class is used to abstract the creation of post types, and the aim of the project is server as the basis for creating plugins, if I put the domain and the name as string is meaningless as who is using will to manually modify these values instead of simply passing them on instantiates the object.

nikolov-tmw commented 8 years ago

I understand where you're coming from, but having those as variables basically prevents you from generating the .po(or was it .mo) files necessary to make translation possible.

Simple solution is to do a global search/replace as you start working on the project(this is how http://underscores.me/ does it).

leobaiano commented 8 years ago

Could you help me understand a little better the problem? When I read the links you spent as reference understand that the problem is that he could not create translations simply, however, as you can see in my last commit, just create the .mo and .po for the en_US language, with poedit , and it worked perfectly.

nikolov-tmw commented 8 years ago

Actually your commit proves that at least using variables as the first parameter doesn't work. Take a look at the .po file and you'll see that you're missing an entry for line 49 of helper/post-type.php, even though you're using a translation function. This is because you're using a variable($this->name), which at runtime could be anything and therefore is impossible to translate.

I still can't find any concrete evidence that using variables as the textdomain parameter will not work, but I wouldn't do it anyway(it's very likely that the upcoming feature where you'll be able to manage the translations of your themes and plugins that are on WordPress.org won't support that). The logic is the same though - an automated script that goes through your code and looks for translation functions has no way of knowing what $this->domain means as it can really change to anything during runtime.

leobaiano commented 8 years ago

You're right about the use of variable as the first parameter, I make a change to solve this problem, thank you. Regarding the second I understand your point and I agree with you, I will see how I can change following the example of the underscore.

Thank you for the tips!