feat(i18n): Add Spanish localisation framework and translations#6973
feat(i18n): Add Spanish localisation framework and translations#6973jonbragado wants to merge 2 commits into
Conversation
- Add i18n.py module with gettext integration, translateLabel(), stripAccents() - Add complete Spanish .po/.mo translation files (4878 entries) - Add language.py startup script with GAFFER_LANG environment variable - Wrap UI strings with _() in 398 Python files across all UI packages - Add _TranslatedCellColumn for Light Editor cell value translation - Add _WORD_MAP and _PHRASE_MAP for dynamic label translation - Support for CamelCase expansion and metadata label translation - Menu path disambiguation for translated menu items (menus.py) Packages modified: GafferUI, GafferSceneUI, GafferImageUI, GafferDispatchUI, GafferVDBUI, GafferOSLUI, GafferArnoldUI, GafferCyclesUI, GafferRenderManUI English UI impact: None when GAFFER_LANG is unset or 'en'. All changes are transparent - identical behavior for English users.
johnhaddon
left a comment
johnhaddon
left a comment
There was a problem hiding this comment.
Thanks @jonbragado! This has clearly involved a lot of effort, and as the Gaffer user base continues to grow it is potentially a very valuable contribution.
I have to confess to being one of those lazy folks who hasn't spoken a foreign language since leaving school, and one of those developers with no experience in internationalisation. So please bear with me while I get up to speed with it all.
Rather than review all the changes at this stage (there are a lot!), I've tried to focus on the core decisions and architecture. My goal would be to have this system be as minimally intrusive as possible, and to fit in with Gaffer's existing conventions as closely as possible, so the majority of my comments are on those topics. It may be that I am completely off base, in which case please do educate me. But I'm hopeful that with some tweaks we can keep all the great work you've done while also minimising the footprint of the changes and reducing the future maintenance burden.
Cheers...
John
| # --------------------------------------------------------------------------- | ||
| # i18n preferences config file | ||
| # --------------------------------------------------------------------------- | ||
| # Stored at ~/gaffer/i18n.json so it can be read *before* the full | ||
| # Preferences node is available. The file is a small JSON dict: | ||
| # { "language": "es", "translateNodeNames": true, "translateTooltips": true } | ||
|
|
||
| _I18N_CONF = pathlib.Path( "~/gaffer/i18n.json" ).expanduser() | ||
|
|
||
| def _readConf() : | ||
| """Return the persisted i18n preferences dict, or empty dict.""" | ||
| try : | ||
| with open( _I18N_CONF, "r", encoding = "utf-8" ) as f : | ||
| return json.load( f ) | ||
| except Exception : | ||
| return {} | ||
|
|
||
| def saveConf( language, translateNodeNames, translateTooltips ) : | ||
| """Persist the i18n preferences to ~/gaffer/i18n.json.""" | ||
| _I18N_CONF.parent.mkdir( parents = True, exist_ok = True ) | ||
| with open( _I18N_CONF, "w", encoding = "utf-8" ) as f : | ||
| json.dump( | ||
| { | ||
| "language" : language, | ||
| "translateNodeNames" : translateNodeNames, | ||
| "translateTooltips" : translateTooltips, | ||
| }, | ||
| f, indent = 2 | ||
| ) | ||
|
|
There was a problem hiding this comment.
Can you explain the need for a separate JSON configuration format. I'm pretty keen that Gaffer configuration continues to use only one mechanism - the existing `.py files in GAFFER_STARTUP_PATHS.
| # --------------------------------------------------------------------------- | ||
| # Determine effective language | ||
| # --------------------------------------------------------------------------- | ||
| # Priority: stored preference > GAFFER_LANG env var > "en" |
There was a problem hiding this comment.
Should we be consulting the system locale as well as (or instead of) these? If the system is set to Spanish, then should Gaffer not operate in Spanish by default?
|
|
||
| # Common multi-word phrases that require Spanish noun-adjective order. | ||
| # Checked BEFORE word-by-word fallback in translateLabel(). | ||
| _PHRASE_MAP = { |
There was a problem hiding this comment.
The framework is language-agnostic: adding another language only requires a new .po file
It seems like this might not apply here, and this bit is actually hardcoded to Spanish? Maintaining a language-agnostic approach throughout seems very worthwhile.
|
|
||
| "description", | ||
| """ | ||
| _(""" |
There was a problem hiding this comment.
It seems unfortunate that every single metadata registration needs to be wrapped like this. As well as being a little ugly, it feels like the sort of thing that will be easily forgotten, and I wonder what the overhead of translating during startup is.
I wonder if it might be cleaner to store non-translated metadata, and then for the consumer of the metadata to perform the translation on the fly. Perhaps that might also open the door to being able to change language without restarting Gaffer?
There was a problem hiding this comment.
From a code review perspective, it would have been really useful to have changes like this in a separate commit. With everything in one commit, it's hard to discern the important structural changes from the noise of the _( changes.
| @@ -119,13 +121,13 @@ def addItems( spreadsheet ) : | |||
| ) | |||
|
|
|||
| if alreadyConnected and other : | |||
| menuDefinition.append( "/__ConnectedDivider__", { "divider" : True, "label" : "Connected" } ) | |||
| menuDefinition.append( "/__ConnectedDivider__", { "divider" : True, "label" : _("Connected") } ) | |||
There was a problem hiding this comment.
Similar to my comment on the metadata translation, I wonder how doable it would be to have the Menu class be the one that does the translation lookup for label etc. And I wonder if Window could do the lookup for title, and Label could do the lookup for it's text and so on. If that worked, then it would massively reduce the number of things wrapped in _(), and also allow folks to translate extensions that hadn't been built with internationisation in mind.
| single-line .po msgid entries.""" | ||
| return " ".join( text.split() ) | ||
|
|
||
| def _( text ) : |
There was a problem hiding this comment.
No C++ modifications — purely Python UI layer
A large part of Gaffer is written in C++, so I think it's critical that any internationalisation system takes that into account. How hard would it be to refactor this stuff so that it's accessible from C++ too?
murraystevenson
left a comment
murraystevenson
left a comment
There was a problem hiding this comment.
Thanks @jonbragado, I think this effort will be a greatly appreciated improvement as Gaffer's international user-base grows. I've noted a couple of things inline while spot-checking these changes but to begin with my main questions are more conceptual (and maybe a little naiive as this is my first exposure to localisation) :
The use of the literal English text as keys in the .po files seems convenient to limit the amount of change in this already large PR, but I worry that it exposes us to easily breaking translations if we ever modify that text in the future. I suppose we could introduce some tests to ensure that .po files shipped with Gaffer are up to date, but we could still easily break external translations. Would using keys unrelated to the English text work better here? Are there more downsides to that approach than the current one?
The framework is language-agnostic: adding another language only requires a new .po file
Can you explain the general process for creating a new .po file as well as updating existing ones whenever we add more UI text that requires translation? How do you see the ongoing maintenance of the Spanish translation progressing as we continue to develop Gaffer?
| _channelLabelMap = { | ||
| "r" : "R", "g" : "V", "b" : "A", "a" : "\u03b1", | ||
| "h" : "T", "s" : "S", "v" : "V", | ||
| "t" : "T", "m" : "M", "i" : "I", | ||
| } | ||
| self.__channelLabels[component] = GafferUI.Label( | ||
| component.capitalize(), | ||
| _channelLabelMap.get( component, component.capitalize() ), |
There was a problem hiding this comment.
The framework is language-agnostic: adding another language only requires a new .po file
This seems to sidestep the .po file and instead hardcodes the translation of channel labels?
| updatedTabs[_(name)] = tab | ||
|
|
||
| if existingTabs.keys() != updatedTabs.keys() : | ||
| with Gaffer.Signals.BlockedConnection( self.__currentTabChangedConnection ) : | ||
| del self.__tabbedContainer[:] | ||
| for name, tab in updatedTabs.items() : | ||
| self.__tabbedContainer.append( tab, label = name ) | ||
| for translatedName, tab in updatedTabs.items() : | ||
| self.__tabbedContainer.append( tab, label = translatedName ) |
There was a problem hiding this comment.
Do we need to use the translated name as the key here, or could we just translate the label?
| if _GAFFER_LANG != "en" : | ||
| return 195 |
There was a problem hiding this comment.
Adjustments like this may be better specified as part of the language config?
| else : | ||
| s = "Pinned to %d node%s." % ( n, "" if n == 1 else "s" ) | ||
| s = _("Pinned to %d node(s).") % n |
There was a problem hiding this comment.
Are there established approaches to handling optional plural suffixes in translation, or is it better to simplify like this?
There was a problem hiding this comment.
If I understand correctly, the MO file (machine object) derives from the PO file. For security reasons, wouldn't it be better to generate the binary MO as part of the build process with msgfmt, especially when the PO source is fairly small?
(I can't speak for Team Gaffer, but just my two cents.)
|
Thanks @johnhaddon, @murraystevenson, and @brendanholt for the thoughtful review! I really appreciate you taking the time to dig into the architecture rather than just the surface changes. You're absolutely right that the consumer-side translation approach is superior. Let me address the key points and outline the refactoring plan: Configuration (JSON → startup paths)@johnhaddon — Agreed. I'll drop Translation at the consumer levelThis is the biggest architectural change and I think it's the right call. Instead of wrapping 405 files with
This will:
The only places Language-agnostic approach@johnhaddon @murraystevenson — Fair point about Keys in .po = English text@murraystevenson — This is actually the standard gettext convention (GNU gettext, Django, Flask, WordPress, etc.). The English text is the key. If upstream modifies a string, Maintenance workflow for translationsWhen new UI text is added to Gaffer:
I'm committed to maintaining the Spanish translation as Gaffer evolves. .mo not in repo@brendanholt — Agreed. I'll remove the Plurals@murraystevenson — gettext has native plural support via C++ accessibility@johnhaddon — For now, the consumer-side approach handles everything through Python (since Menu, Window, Label, PlugLayout are all Python). The graph canvas translation works via metadata overrides from Python. A future C++ gettext integration (linking What's preservedThe Spanish translation catalogue ( PlanI'll create a fresh branch with clean commits:
I'll close this PR and open a new one once the refactor is ready. Thanks again for the guidance — this will be a much cleaner result. |
4 participants
Pull Request: Add Spanish localisation framework (i18n)
Repository: GafferHQ/gaffer
Base branch: 1.6_maintenance
Head branch: jonbragado/gaffer:feature/i18n-spanish
Title: feat(i18n): Add Spanish localisation framework and translations
Description
This PR introduces a complete internationalisation (i18n) framework for Gaffer's Python UI, along with a full Spanish translation covering ~4880 strings.
What's included
GafferUI/i18n.py— Core module providing_()(gettext wrapper),translateLabel()(word-by-word fallback for dynamic strings),getNodeLabel()(node name translation), andstripAccents()(IECoreGL rendering workaround).startup/gui/language.py— Startup script that reads~/gaffer/i18n.jsonto configure the active language and translation preferences.GafferUI/locale/es/LC_MESSAGES/gaffer.po— Complete Spanish translation catalogue (4880 entries): menu items, node descriptions, plug labels, tooltips, and warning messages..pyfiles — All user-facing strings in Python UI modules wrapped with_()for gettext extraction.GraphEditor.pyenhancements — Translates node names and nodule labels in the Graph Editor canvas, using instance metadata to override shader-UI registrations.Architecture
Limitations — IECoreGL Font rendering
The OpenGL text renderer (
IECoreGL::Font) only supports ASCII characters (code points 0–127) in its glyph atlas. Characters from the Latin-1 Supplement range (ñ, á, é, ü, etc.) produce garbled output.Workaround: All text destined for
IECoreGL::Font::renderSprites()passes throughstripAccents(), which removes diacritical marks and replaces ñ→n. This produces readable but linguistically imperfect text in the graph canvas.Upstream fix required: This limitation is tracked as an issue in the Cortex repository:
→ ImageEngine/cortex#1538
Once Cortex expands the glyph atlas to 16×16 (Latin-1 coverage), the
stripAccents()workaround can be removed and full accented text will render correctly.What this PR does NOT change
~/gaffer/i18n.jsonspecifies another language.pofileTesting
"language": "en") produces identical behaviour to upstreamScreenshots
(Add screenshots of translated UI here when submitting)
Related