Script messages not translated

Bug #425202 reported by jazzynico
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
Fix Released
Low
jazzynico

Bug Description

The scripts error messages are not translated in the dialog box, even when they are in the po files.
The problem is that an extra \n is added to the message (see src/extension/implementation/script.cpp, line 887).

jazzynico (jazzynico)
description: updated
jazzynico (jazzynico)
Changed in inkscape:
milestone: none → 0.47.1
jazzynico (jazzynico)
Changed in inkscape:
milestone: 0.47.1 → 0.48
jazzynico (jazzynico)
Changed in inkscape:
assignee: nobody → JazzyNico (jazzynico)
status: New → In Progress
description: updated
Revision history for this message
jazzynico (jazzynico) wrote :
Revision history for this message
jazzynico (jazzynico) wrote :

Fix committed in bzr revision 9371.
Please test!

Changed in inkscape:
status: In Progress → Fix Committed
Revision history for this message
jazzynico (jazzynico) wrote :

Doesn't work on Windows.
And doesn't work when the message has several individual strings (see JessyInk extension).
Only works on Linux (and probably OSX) with one string messages.

It really needs a better fix. Postponed to 0.49.

Changed in inkscape:
milestone: 0.48 → 0.49
status: Fix Committed → In Progress
Revision history for this message
ScislaC (scislac) wrote :

JazzyNico: Is this still being actively worked on (the in progress status)?

Revision history for this message
jazzynico (jazzynico) wrote :

> Is this still being actively worked on (the in progress status)?

Yes, it's still in my 0.49 todo list. It's just not high priority...

Revision history for this message
jazzynico (jazzynico) wrote :

Fix committed revision 11546.
Tested successfully on Windows XP and Ubuntu 11.04.

I've modified the strategy so that the messages are translated in the python code and not in script.cpp (using stderr to translate variable content is just not possible).
It should now work with almost all the messages. The remaining untranslated strings are mainly due to incompatibility between the their content and gettext in the python files, and will be worked on later.

Revision history for this message
jazzynico (jazzynico) wrote :

New patch.
Should work better now and not break the extension system when no translation is available...

Revision history for this message
jazzynico (jazzynico) wrote :
Revision history for this message
jazzynico (jazzynico) wrote :

New patch committed revision 11551.
Tested successfully on Windows XP, Seven, and Ubuntu 11.04.
Work in progress for OSX.

Revision history for this message
su_v (suv-lp) wrote :

> Tested successfully on Windows XP, Seven, and Ubuntu 11.04.
> Work in progress for OSX.

Based on the information in bug #1024851 (duplicate of bug #1024325), I don't think not really fixed for the other platforms either (in bug #1024851 python-based extensions failed the same way on Ubuntu 12.04 has did the command line version built on OS X).

Revision history for this message
jazzynico (jazzynico) wrote :

> I don't think not really fixed for the other platforms either

Confirmed. Forgot to write that I'm going to test on Debian testing soon.

Revision history for this message
jazzynico (jazzynico) wrote :

Apart from the operating system issues, the current implementation is still not satisfying because translating everything in inkex.py doesn't work when the string as %s, %d, etc. placeholders. I'm working on something different, but it requires changes in every python file that needs to be translated, and affects the way imported extensions (jessyink, gcodetool) should be written.
The idea is that the extensions import a common init_localization module and use it to find the translation file (the same way inkex is used when path manipulation is needed).

Revision history for this message
jazzynico (jazzynico) wrote :

New patch attached.

A new localize() function has been added to inkex.py and called from the translatable extensions.
To make a extension translatable, you now need to remove:

 import gettext
 _ = gettex.gettext

and add:

 import inkex (if not already imported)
 inkex.localize()

Texted successfully on Windows XP and Debian Wheezy (and yes, the correct locale file is used if I use a local build instead of the packaged version!).

Revision history for this message
su_v (suv-lp) wrote :

Patch '425202-PythonGettext-v2.diff' tested with command line version (r11552) on OS X 10.7.4 [1]:
Error messages returned from extensions script are not translated (tested with LANG="fr_FR.UTF-8" and running the extension 'Perspective' with two circles selected).

[1] will test with packaged version later: at the moment, the patch does not consider osx packaging yet (platform: darwin).
With the osx-packaged version, the environment variable is set in the launcher script inside the relocatable application bundle (not in 'src/main.cpp'):
    <http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/view/head:/packaging/macosx/Resources/bin/inkscape#L68>

Revision history for this message
su_v (suv-lp) wrote :

If I change the fallback to 'False' on line 64 of the patched 'inkex.py', extensions fail with the same error message as reported in bug #1024325.

Since the current solution (with fallback=True) fails gracefully (i.e. doesn't translate nor crash), it's kind of ok - but then the old version could be used as well without changing every python script bundled with inkscape, and adding new requirements for extension developers.

Revision history for this message
su_v (suv-lp) wrote :

With the patch applied, the extensions used for the 'Help' menu to open the URL in the web browser fail:

Exception in thread Thread-1:
Traceback (most recent call last):
  File "/Volumes/cyan/mp-test/with-a-long-long-long-directory-name/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/threading.py", line 551, in __bootstrap_inner
    self.run()
  File "launch_webbrowser.py", line 16, in run
    webbrowser.open(_(self.options.url))
NameError: global name '_' is not defined

Revision history for this message
su_v (suv-lp) wrote :

Additional information re OS X:
If I copy the code block for Windows in 'inkex.py', change the condition for the platform to match 'darwin', and then launch the command line version with explicitly setting 'INKSCAPE_LOCALEDIR' to the '${prefix}/share/locale' of the current local build/installation, translation of messages returned from python scripts does work.

However, AFAIU it should not be required for a normal "linux-style" configured and installed version of Inkscape to depend on this variable which originally was introduced for packaging only (where the location of the locale files might not be in the same as the compiled-in absolute paths based on ${prefix} used with './configure', and change if the packaged version is self-contained and relocatable).

Revision history for this message
jazzynico (jazzynico) wrote :

> With the patch applied, the extensions used for the 'Help' menu to open the URL in the web browser fail

Fixed...

Revision history for this message
jazzynico (jazzynico) wrote :

> AFAIU it should not be required for a normal "linux-style"

Confirmed. But since you need to set envs in a launcher in order to launch Inkscape correctly, I guess the behavior is quite different.
Would it work is you set INKSCAPE_LOCALEDIR in your launcher script, with ${prefix} automatically calculated depending on the launch directory ?

Revision history for this message
jazzynico (jazzynico) wrote :

> adding new requirements for extension developers.

Just for your information, old style extensions don't break the extension system. They just don't translate. But since lots of them don't translate with 0.48.3.1, it's not a big annoyance.
We just need to be sure what we release is good enough so that we won't need to change the whole extension directory again the next Inkscape version.

Revision history for this message
su_v (suv-lp) wrote :

> But since you need to set envs in a launcher in order to
> launch Inkscape correctly, I guess the behavior is quite different.

No, you misunderstood. I _am_ using the "standard" command line version of Inkscape: it is configured, compiled and installed exactly like on linux (eg with ./configure --prefix="`pwd`/inst" && make && make install), and the translations for the error messages ot the python scripts still fail. As mentioned, I have not yet tested the osx-packaged version yet, but - based on my understanding and the simulated test, it will work with the packaged version if 'inkex.py' has a test for platform 'darwin'.

My issue is with normal builds on OS X, where none of the proposed patches works to translate those error messages.

Revision history for this message
su_v (suv-lp) wrote :

> Would it work is you set INKSCAPE_LOCALEDIR in your launcher script,
> with ${prefix} automatically calculated depending on the launch directory ?

The variable is already in use and set (correctly) in the launcher script (see my link in comment #15), because the same variable is also required for the main 'inkscape-bin' binary to locate the locale files inside the self-contained relocatable application bundle.

Revision history for this message
jazzynico (jazzynico) wrote :

Just tested on Debian Wheezy, with a local build (nothing in /usr/bin or /usr/local/bin), and extensions translation doesn't work. The .mo file needs to be in the PATH (manually copying the file to /usr/share/locale... works).

Revision history for this message
jazzynico (jazzynico) wrote :

IMHO it's worth releasing the patch in 0.49, even if it works for the packaged version only. There's an impact on the scripts because we move the translation initialization from the scripts themselves to the inkex module, but the positive point is that no changes (except in inkex) will be required if we modify the way we initialize python translation later.

Any objection?

Revision history for this message
su_v (suv-lp) wrote :

> even if it works for the packaged version only.

I see no point in doing so tbh: packaging for osx is likely to change fundamentally when moving to using the native backend of GTK+ - and there is no existing solution for getting python-based extensions to work at all with the newer proposed packaging based on gtk-mac-bundler (WIP).

Unless that major change to all python scripts and inkex.py is useful and required for other platforms (i.e. all linux distros), and works reliably without actually falling back to assumed default locations for po/mo files (which might be installed or not), …

Revision history for this message
su_v (suv-lp) wrote :

>> even if it works for the packaged version only.
>
> I see no point in doing so tbh: (…)

Sorry if my earlier comment caused more confusion than clarification - maybe you only referred to Windows packages (which IIRC is the only case supported by the latest available patch - as mentioned in earlier comments it neither worked with default builds on OS X (installed into custom prefix), nor with OS X packages based on current packaging scripts). Fixing this for Windows only is ok of course…

Revision history for this message
jazzynico (jazzynico) wrote :

The patch fixes the issue on Windows (packaged AND local builds) and GNU/Linux (at least on Debian Wheezy, packaged version only, and Ubuntu - not tested if it works with local builds). I'm going to test again on Ubuntu in case there's something different in the way the translation base is handled.

Revision history for this message
jazzynico (jazzynico) wrote :

Third patch. This one also works on Ubuntu, and should work on OSX as well.
A different env (PACKAGE_LOCALE_DIR) is already set in main.cpp, and I just had to use it in inkex...

Revision history for this message
jazzynico (jazzynico) wrote :

Removing patch v3 (still doesn't work, invalid test...)

Revision history for this message
jazzynico (jazzynico) wrote :

This one should be ok.

Revision history for this message
su_v (suv-lp) wrote :

> Third patch. This one also works on Ubuntu, and should work on OSX as well.

- works with linux-style installation on OS X (into custom prefix)
- fails with OS X package [1]

I'm fine with committing '425202-PythonGettext-v3.diff' as is - osx packaging is in a kind of unmaintained state anyway right now.

---
[1] tested with new OS X package built based on r11743 plus three patches from private branch to make current packaging scripts work on OS X 10.7 Lion (with MacPorts installed into custom prefix):
<http://bazaar.launchpad.net/~suv-lp/+junk/inkscape-osxapp-stacked/revision/11559>
<http://bazaar.launchpad.net/~suv-lp/+junk/inkscape-osxapp-stacked/revision/11562>
<http://bazaar.launchpad.net/~suv-lp/+junk/inkscape-osxapp-stacked/revision/11566>

Problem: '$PACKAGE_LOCALE_DIR' overrides (or ignores) '$INKSCAPE_LOCALEDIR' (which would need to be used if present in the environment - which is the case with the packaged OS X version, because it is set in the shell launcher script and also used by the main inkscape binary to find the locale files inside the relocatable package (Inkscape.app)):

INKSCAPE_LOCALEDIR
 /Volumes/cyan/src/inkscape/inkscape-repo/mp-pythonlocale/packaging/macosx/Inkscape.app/Contents/Resources/locale
PACKAGE_LOCALE_DIR
 /Volumes/cyan/src/inkscape/inkscape-repo/mp-pythonlocale/build-osxapp/../inst-osxapp/share/locale

Revision history for this message
su_v (suv-lp) wrote : Re: [Bug 425202] Re: Script messages not translated

On 07/10/2012 18:16, ~suv wrote:
> INKSCAPE_LOCALEDIR
> /Volumes/cyan/src/inkscape/inkscape-repo/mp-pythonlocale/packaging/macosx/Inkscape.app/Contents/Resources/locale
> PACKAGE_LOCALE_DIR
> /Volumes/cyan/src/inkscape/inkscape-repo/mp-pythonlocale/build-osxapp/../inst-osxapp/share/locale

Attaching debug extension used to verify the content of these variables
as seen in the environment of the spawned python process.

Revision history for this message
jazzynico (jazzynico) wrote :

Patch v3 committed revision 11749.
Thanks for your help and tests, ~suv!

Changed in inkscape:
status: In Progress → Fix Committed
Revision history for this message
su_v (suv-lp) wrote :

Patch to make it work for OS X packages too (based on current X11-based packaging). The python code very likely could be optimized - what I tried to implement:

if platform is 'darwin' then
  if $INKSCAPE_LOCALEDIR is set
    use it
  else
    if $PACKAGE_LOCALE_DIR is set
      use it
    else
      use fallback

Revision history for this message
jazzynico (jazzynico) wrote :

Patch from comment #35 committed revision 11767.

Great, now it works for everybody! Thank you for the OS X patch!

> The python code very likely could be optimized

Maybe we could find something more pythonic. But at least it works as expected and the code is not ugly, thus I suggest we optimize it after 0.49 is out.

Revision history for this message
su_v (suv-lp) wrote :

@JazzyNico - could you please review the changes in r11749 to 'share/extensions/uniconv-ext.py'?

On Ubuntu 12.10, opened a simple CDR file works with Inkscape 0.48.3.1, but fails with a (local) trunk build. AFAICT r11749 breaks usage of UniConvertor 1.1.4 (e.g. on Ubuntu 12.10 [1]), because 'cmd' is reset to 'None' even after having successfully found 'uniconvertor' in the tests right above.

Note: AFAICT UniConvertor 1.1.4 doesn't have 'uniconv_run()', and fails if called via the python command on line 61:
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/9706>

-----
[1] <http://packages.ubuntu.com/quantal/python-uniconvertor>

Revision history for this message
su_v (suv-lp) wrote :

Proposed diff, tested with r12031 on Ubuntu 12.10

Revision history for this message
jazzynico (jazzynico) wrote :

@~suv - I did something very ugly here, indeed...
Thanks for the patch, feel free to commit!

Revision history for this message
su_v (suv-lp) wrote :

Fix committed in r12032.

jazzynico (jazzynico)
Changed in inkscape:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.