[Bf-codereview] Blender i18n enhancements (issue 7102054)

ideasman42 at gmail.com ideasman42 at gmail.com
Mon Jan 14 18:35:28 CET 2013


While I like that scripts could make translations Im hesitant to accept
this functionality as is, and think some areas should be done different.

- Im not convinced its a good idea to storing python data and using it
for translation lookups. It could be worth going to the extra bit of
effort to have scritps dump their own 'mo' files that blender can use.

- Not really liking storing the translation context in the text body
"SomeContext::Some Text"

- As stated in the review, Id like to be a general translation system
any script can register with, not addon specific.


https://codereview.appspot.com/7102054/diff/1/release/scripts/modules/addon_utils.py
File release/scripts/modules/addon_utils.py (right):

https://codereview.appspot.com/7102054/diff/1/release/scripts/modules/addon_utils.py#newcode297
release/scripts/modules/addon_utils.py:297: # Try to register i18n
messages (translation for the addon) if any.
as stated in IRC, Id prefer not make addons translation aware, if the
dev wants to use translations they can do it explicitly.

https://codereview.appspot.com/7102054/diff/1/release/scripts/startup/bl_ui/properties_material.py
File release/scripts/startup/bl_ui/properties_material.py (right):

https://codereview.appspot.com/7102054/diff/1/release/scripts/startup/bl_ui/properties_material.py#newcode308
release/scripts/startup/bl_ui/properties_material.py:308: col.prop(mat,
"use_specular_ramp", text="Color::Ramp")
Im not a fan of this method of passing on the encoding context.

Context should be stored in the property, in the case of labels that
dont reference a property could take an optional context argument.

# This prop can have its context defined in RNA
col.prop(mat, "use_specular_ramp", text="Ramp")

# This label could have a context passed
col.label(text="Ramp", context='COLOR')

https://codereview.appspot.com/7102054/diff/1/source/blender/makesdna/DNA_userdef_types.h
File source/blender/makesdna/DNA_userdef_types.h (right):

https://codereview.appspot.com/7102054/diff/1/source/blender/makesdna/DNA_userdef_types.h#newcode610
source/blender/makesdna/DNA_userdef_types.h:610:
USER_DOTRANSLATE_ADDON	= (1 << 8),
*picky*, could be call this SCRIPT or PYTHON? -- translations shouldnt
be addon spesific.

https://codereview.appspot.com/7102054/diff/1/source/blender/python/intern/bpy_app_i18n.c
File source/blender/python/intern/bpy_app_i18n.c (right):

https://codereview.appspot.com/7102054/diff/1/source/blender/python/intern/bpy_app_i18n.c#newcode75
source/blender/python/intern/bpy_app_i18n.c:75: const char
*BPY_app_i18n_addons_pgettext(const char *msgctxt, const char *msgid)
This runs on string drawing, and its locking the gil and doing dict
lookups, unicode conversions etc.

Did you consider having a utility function to compile the dict into a
.mo file?
http://www.gnu.org/software/gettext/manual/html_node/MO-Files.html

Or, the PyDict could be convertex into a ghash or so. something for
faster lookups that avoids conversion overhead.

This can be left as a TODO I guess.

https://codereview.appspot.com/7102054/diff/1/source/blender/python/intern/bpy_app_i18n.c#newcode99
source/blender/python/intern/bpy_app_i18n.c:99: PyObject *addon_dict;
prefer not be addon spesific, could name uuid_dict;  /* convention to
use module names */

https://codereview.appspot.com/7102054/diff/1/source/blender/python/intern/bpy_app_i18n.c#newcode104
source/blender/python/intern/bpy_app_i18n.c:104: /* For each addon dict,
we'll search for full locale, then language+country, then
language+variant,
could make this into a utility function of BLF ?

https://codereview.appspot.com/7102054/diff/1/source/blender/python/intern/bpy_app_i18n.c#newcode202
source/blender/python/intern/bpy_app_i18n.c:202: static const char
*kwlist[] = {"addon_name", "i18n_dict", NULL};
prefer call it module_name.

https://codereview.appspot.com/7102054/


More information about the Bf-codereview mailing list