improved localization

Bug #679715 reported by Yuv
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Hugin
Confirmed
Wishlist
Yuv
Revision history for this message
Yuv (yuv) wrote :

The file lang.patch was added: None

Revision history for this message
Yuv (yuv) wrote :

The file lang_fix.patch was added: current status, improved and working

Revision history for this message
Yuv (yuv) wrote :

uploaded current status. still need to follow up on Thomas Modes' feedback:

Punkt 1: Du verwendest feste Pfadangaben. Somit funktioniert es in Windows nur im Build-Verzeichnis, aber nicht installiert. Diese Pfadangaben müssen zur Laufzeit ausgewertet werden. Es müsste ähnlich wie in Celeste_standalone funktionieren (Celeste/main.cpp, um Zeile 470).

Punkt 2: Ich würde die Zeilen gettextdomain/textdomain in eine extra Funktion (sowas wie InitTranslation) auslagern und nur einmal nach der Initialisierung der wxLocale aufrufen. Aber ich weiß aber nicht, wie das mit wxWidgets-Internationalisierung zusammenarbeitet.

Punkt 3: Der Leerzeichen nach dem Doppelpunkt ("Loading image: "). Dies werden die meisten Übersetzer nicht richtig mitkriegen und in der Übersetzung weglassen. Dann wird der Dateiname im GUI direkt an den Doppelpunkt gehängt. Besser wäre etwas wie "Loading image: %s" und dann mit sprintf o. ä. zu formatieren. Wir haben ein paar solche Strings im aktuellen Code. Ich bin aber noch nicht dazugekommen, das zu ändern. Außerdem würde das wieder neue Strings für die Übersetzung generieren.

tmodes (tmodes)
Changed in hugin:
importance: Undecided → Low
status: New → Triaged
rew (r-e-wolff)
Changed in hugin:
assignee: nobody → Yuv (yuv)
tags: added: localization
Yuv (yuv)
Changed in hugin:
status: Triaged → Confirmed
importance: Low → Wishlist
Revision history for this message
tmodes (tmodes) wrote :

Patch was commited to own branch by Yuv. But given feedback in comment #3 was not considered, especially point 1 and 2 needs to be done.

Some more points to consider:
Problem is gettext for windows. In the last versions they dropped the support for MSVC. So it is difficult to compile the necessary prerequisite. So the first issue is to compile the gettext package on windows with MSVC. Then the paths/filenames needs to be checked.

tags: added: i18n
Revision history for this message
Yuv (yuv) wrote :

Hi Thomas,

feedback was "considered" to the extent that I could consider / make sense of it with my limited skillset.

point 1: I don't find any path -- relative or absolute -- in src/celeste/Main.cpp around line 470. Nor do I see where in my code I refer to paths? please advise.

point 2: The way I understood your feedback was to implement this in a single location of the code rather than in every single file that has to be translated, as with my first attempt. This is now implemented once in /src/hugin_base/hugin_utils/utils.h while before it was implemented in src/hugin_base/huginapp/ImageCache.cpp and not available from other parts of the code. I don't understand the feedback regarding xwLocale - this is outside of wX. Please explain what you mean.

I was not aware of gettext dropping support for MSVC and am sorry for MSVC users that gettext is causing problems for them. Is there a better alternative to gettext that is easier on the multi-platform requirements of Hugin?

Last but not least, I branched this out in its own branch because it is a superior way to get people (inlcuding myself) who care about localization to contribute / try to improve. It keeps it on the radar screen. I started this patch as an itch, more than a year ago, to help a translator who asked about translating those untranslated strings; and I realize that I have not been very good at keeping track of what I wanted to do. This is because personally I like my software to be in English and so localization is not on my radar screen where it should be. The hg branch is cheap, but if it disturbs you feel free to purge it.

Revision history for this message
Yuv (yuv) wrote :
Revision history for this message
Yuv (yuv) wrote :
Revision history for this message
tmodes (tmodes) wrote :

Hi Yuv,

Point 1: You are using INSTALL_LOCALE_DIR. This is a fixed absolute path, which does not work on Windows. A solution can be found now in lines 288ff in src/celeste/Main.cpp. It moved some lines since the last comment. So looking at the code, I see that this does also not work with the bundled Mac version.

Point 2: e.g. in ImagesCaches.cpp you call twice

bindtextdomain( "hugin", INSTALL_LOCALE_DIR );
textdomain( "hugin" );

This code is linux only and should be moved into an own function providing the platform specific part, see point 1

Concerning wxWidgets. wxWidgets provides own routines for translations. So you need to check if wxWidgets own translation work together with the new introduced gettext lib translation in part of Hugin libaries or if there are cross over effects.

I have no idea for an other lib.

Concerning the links: They don't help further. Link 1 would introduce a dll -> dll hell. All other libraries in Hugin on Windows are static. Also the mentioned way does not provide the necessary files for linking the dll with Hugin.
Link 2 is using a compiliation model which is incompatible with Hugin.

Revision history for this message
Yuv (yuv) wrote : Re: [Bug 679715] Re: improved localization

Hi Thomas,

On January 8, 2011 09:33:15 am tmodes wrote:
> Point 1: You are using INSTALL_LOCALE_DIR. This is a fixed absolute
> path, which does not work on Windows. A solution can be found now in
> lines 288ff in src/celeste/Main.cpp. It moved some lines since the last
> comment. So looking at the code, I see that this does also not work with
> the bundled Mac version.

Thank you for pointing me in the right direction. I think I fixed this with
my last commit.

> Point 2: e.g. in ImagesCaches.cpp you call twice
>
> bindtextdomain( "hugin", INSTALL_LOCALE_DIR );
> textdomain( "hugin" );
>
> This code is linux only and should be moved into an own function
> providing the platform specific part, see point 1

I moved it into its own function, although I am not sure that how I call that
function from within ImageCache.cpp (line 510) is the right way of doing it.

I also realize that the changes to nona, which I did as well in the i18n
branch, are not effective because I do not call that function. I first
thought that everything was called by the includes that I have moved into
src/hugin_base/hugin_utils/utils.cpp - I forgot about the two lines you
mention above.

Where should I put the call to hugin_utils::TranslateText() ?

Thank you for your feedback, it help me learn and understand coding.

> you need to check if wxWidgets own translation work together with the
> new introduced gettext lib translation in part of Hugin libaries or if
> there are cross over effects.

it seem to be working. the only problem is an aesthetic one: wX translations
are now in _() while non-wX translations are in _X()

> Concerning the links: They don't help further. Link 1 would introduce a dll
> -> dll hell. All other libraries in Hugin on Windows are static. Also the
> mentioned way does not provide the necessary files for linking the dll
> with Hugin. Link 2 is using a compiliation model which is incompatible
> with Hugin.

Sorry for that. Do you have gettext working at all on your Windows machine
(i.e. does this "only" make it for a more difficult SDK creation) or is this
completely broken on Windows at this time?

Revision history for this message
tmodes (tmodes) wrote :

> I think I fixed this with
> my last commit.

It's only the first step. First you forgot to modify the paths (copy and paste error). On Windows the translations are in \share\locale and not in \share\hugin\data, on Linux in INSTALL_LOCALE_DIR and not in INSTALL_DATA_DIR. Second the name TranslateText is not approbiate. Better would be InitTranslation or something similiar. Also add a short documentation what the function do. Also for Windows and MacOS some includes are missing.
In ImageCache.cpp you forgot to replace a second call of bindtextdomain with the new function.

For nona you need to put InitTranslation in nonas main() procedure.

The gettext tools are working on windows (there are binary releases, but they are outdated). The gettext lib is completly broken on Windows with MSVC.

Revision history for this message
Yuv (yuv) wrote :
Download full text (3.3 KiB)

On January 9, 2011 04:05:32 am tmodes wrote:
> It's only the first step.

One step at a time. Thank you for helping me improve.

> First you forgot to modify the paths (copy and
> paste error). On Windows the translations are in \share\locale and not in
> \share\hugin\data, on Linux in INSTALL_LOCALE_DIR and not in
> INSTALL_DATA_DIR.

Yes, the Linux path is an oversight. For the other systems I did not know
where those paths are.

Thank you for giving me the right value for Windows.

I tried to search for the right path on OSX but I am not sure.

Trying to infer from http://wiki.panotools.org/Hugin_translation_guide I would
guess Contents/Resources but then I look at mac/localise-help.sh and get
confused. So I left a TODO note in the code.

> Second the name TranslateText is not approbiate. Better
> would be InitTranslation or something similiar.

OK, done.

> Also add a short
> documentation what the function do.

Done. Extra bonus: I documented the function just above it that was left
undocumented too ;-) I hope I did it right, if not feel free to correct me or
improve the comments directly.

> Also for Windows and MacOS some includes are missing.

Sorry, I can't help with that. I don't have Windows nor MacOS so I don't know
which are the missing includes. If Windows or MacOS builders can tell me
which includes are missing, I will gladly add them. Even better: if you know
what is missing, please add it. I left a TODO in the code.

> In ImageCache.cpp you forgot to replace a second
> call of bindtextdomain with the new function.

Thanks for catching that. Fixed.

> For nona you need to put InitTranslation in nonas main() procedure.

I found it in src/tools/nona.cpp -- I hope my finding is right. And I also
found plenty of strings that would need translation there too - I have not
been over the src/tools folder yet (and will have to go through almost all
folders and check for untranslated strings once localization is working
properly).

Should I also add InitTranslation in src/hugin1/nona_gui ? It seems to me a
similar case to the image cache, with a wxApp that includes code from the non-
wX side of the codebase, and that code contains strings?

I was hoping I could call InitTranslation somewhere "higher up" in the
hierarchy so as not to need to call it in every place individually, just add
the include :(

> The gettext tools are working on windows (there are binary releases, but
> they are outdated). The gettext lib is completly broken on Windows with
> MSVC.

And this code would make Hugin dependent on the gettext lib, I am afraid?

I found build instructions for another software that uses gettext at
http://wl.widelands.org/wiki/BuildingWidelands/

Scroll down to "Building under MSVC"

it says to download and install
http://sourceforge.net/projects/gnuwin32/files/gettext/

can this download be added/"installed" inside the Hugin SDK? Is it possible /
advisable to mix code built with MinGW and code built with MSVC?

There do not seem to be many viable alternatives to gettext

http://stackoverflow.com/questions/2185568/alternatives-to-gettext

maybe http://cppcms.sourceforge.net/boost_locale/html/index.h...

Read more...

Revision history for this message
tmodes (tmodes) wrote :

All command line tools are not localized. Either we need to localize *all* comand line tools or none. But there is no easy way to integrate this in a "higher" level. And this will create a *lot* of new strings.

The version of gettext at gnuwin32 is outdated. They are at 0.14, the offical release is 0.18.1. But currently I have no time to test, I must fix the open issues you left with the merge.

There are more issues with your translations of nona:
1.) There could be an issue with 2-byte strings (e.g. japanese translation), I fear they don't work with your the current implementation.
2.) One sentence should only be in one translated string, don't split one sentence into different strings. This makes it for the translator impossible to get the sense and translate it (e.g. in an other language there could be a other word order).
example:
    << " " << _X("The following output formats (n option of panotools p script line)") << std::endl
    << " " << _X("are supported:") << std::endl
3.) Don't use name/argv[0] in sentences
e.g. std::cout << name << "-" << _X("GPU does not support this projection. Switch to CPU calculation.") << std::endl;
First issue: on windows argv[0] can contain the full path. This make it hard to read
Second: The translator does not know that the application adds "nona-" to translated text. In an other language than english the sentence could also start with an other word than "GPU".

Revision history for this message
Yuv (yuv) wrote :

On January 10, 2011 12:38:18 pm tmodes wrote:
> All command line tools are not localized. Either we need to localize
> *all* comand line tools or none.

agree. I only started with one CLI-tool and with one case of non-wx
transaltion in a GUI as a proof of concept. Once things work well in those,
the next step will be to localize *all* other CLI tools and non-wx
translations in GUI tools.

> this will create a *lot* of new strings.

I am aware of this. I do not expect things to happen overnight, neither on my
end, nor on the translators end. But the long term goal is to localize
everything.

> The version of gettext at gnuwin32 is outdated. They are at 0.14, the
> offical release is 0.18.1. But currently I have no time to test, I must
> fix the open issues you left with the merge.

That's OK. Don't feel under pressure. The merge of the new feature is more
important than the localization.

> There are more issues with your translations of nona:
> 1.) There could be an issue with 2-byte strings (e.g. japanese
> translation), I fear they don't work with your the current implementation.

what makes you fear so? can you point me in a direction where I can read
about the issue and try to solve it? I have no experience with translationn
to 2-byte strings.

> 2.) One sentence should only be in one translated string, don't split one
> sentence into different strings.

OK, will fix. No time now, but it is the next thing I intend to do on the
i18n branch.

> 3.) Don't use name/argv[0] in sentences

This was already in use in nona before I continued using it. I thought it was
a good thing and applied it consistently. From your feedback I understand it
is not a good things and I will revert it consistently. It will need to be
fixed in the default branch as well.

Thanks again for your feedback and your patience.

Revision history for this message
Bruno Postle (brunopostle) wrote :

Hi, I haven't tested this, but just wanted to add that this note about gettext domains/catalogues.

With linux packaging it makes sense to split the tools that use wx from the tools that don't e.g. on fedora the 'hugin-base' package can be installed independently of the 'hugin' package. So it would make sense that if all the tools are to be translatable then there should be two domains (i.e. two .pot files).

i.e. these are all the tools that use wx and that would be in the 'hugin' domain:

 /usr/bin/PTBatcher
 /usr/bin/PTBatcherGUI
 /usr/bin/hugin
 /usr/bin/hugin_stitch_project
 /usr/bin/icpfind
 /usr/bin/nona_gui

...these are the tools that don't use wx and which should be in a separate 'hugin-base' domain:

 /usr/bin/align_image_stack
 /usr/bin/autooptimiser
 /usr/bin/calibrate_lens
 /usr/bin/celeste_standalone
 /usr/bin/checkpto
 /usr/bin/cpclean
 /usr/bin/cpfind
 /usr/bin/deghosting_mask
 /usr/bin/fulla
 /usr/bin/hugin_hdrmerge
 /usr/bin/keypoints
 /usr/bin/matchpoint
 /usr/bin/nona
 /usr/bin/pano_modify
 /usr/bin/pano_trafo
 /usr/bin/pto2mk
 /usr/bin/pto_merge
 /usr/bin/tca_correct
 /usr/bin/vig_optimize

This has an advantage in that translators can choose to just work on the GUI tools, or they can choose to work on both catalogues.

Revision history for this message
tmodes (tmodes) wrote :

>> There are more issues with your translations of nona:
>> 1.) There could be an issue with 2-byte strings (e.g. japanese
>> translation), I fear they don't work with your the current implementation.

> what makes you fear so? can you point me in a direction where I can read
> about the issue and try to solve it? I have no experience with translationn
> to 2-byte strings.

Some time ago there was several bugs into hugin whichs resulted in a crash of hugin if running with japanese or chinese locale. When I remember correctly it was fixed by using there were needed some changes to wide strings.

>> 3.) Don't use name/argv[0] in sentences

>This was already in use in nona before I continued using it. I thought it was
>a good thing and applied it consistently. From your feedback I understand it
>is not a good things and I will revert it consistently. It will need to be
>fixed in the default branch as well.

It is not consitently in default branch. But this is not critical. The problem arises only with the translation of strings. So no need to fix it in default branch (this will only create conficts when merging branches.)

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.