Script messages not translated

[jazzynico]

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)]

Fix committed in bzr revision 9371.
Please test!

[jazzynico (jazzynico)]

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.

[ScislaC (scislac)]

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

[jazzynico (jazzynico)]

> 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...

[jazzynico (jazzynico)]

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.

[jazzynico (jazzynico)]

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

[jazzynico (jazzynico)]

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

[su_v (suv-lp)]

> 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).

[jazzynico (jazzynico)]

> 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.

[jazzynico (jazzynico)]

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).

[jazzynico (jazzynico)]

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!).

[su_v (suv-lp)]

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>

[su_v (suv-lp)]

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.

[su_v (suv-lp)]

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

[su_v (suv-lp)]

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).

[jazzynico (jazzynico)]

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

Fixed...

[jazzynico (jazzynico)]

> 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 ?

[jazzynico (jazzynico)]

> 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.

[su_v (suv-lp)]

> 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.

[su_v (suv-lp)]

> 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.

[jazzynico (jazzynico)]

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).

[jazzynico (jazzynico)]

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?

[su_v (suv-lp)]

> 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), …

[su_v (suv-lp)]

>> 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…

[jazzynico (jazzynico)]

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.

[jazzynico (jazzynico)]

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...

[jazzynico (jazzynico)]

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

[jazzynico (jazzynico)]

This one should be ok.

[su_v (suv-lp)]

> 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

[su_v (suv-lp)]

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.

[jazzynico (jazzynico)]

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

[su_v (suv-lp)]

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

[jazzynico (jazzynico)]

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.

[su_v (suv-lp)]

@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>

[su_v (suv-lp)]

Proposed diff, tested with r12031 on Ubuntu 12.10

[jazzynico (jazzynico)]

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

[su_v (suv-lp)]

Fix committed in r12032.