Opened 10 years ago

Closed 7 years ago

#4050 closed Bug (fixed)

Qt Client: translation does not loads under Linux

Reported by: taem Owned by: mike.dld
Priority: Normal Milestone: 2.90
Component: Qt Client Version: 2.52
Severity: Normal Keywords: qt patch
Cc: nikoli@…

Description

Hi,

In Qt Client translation does not loads under Linux. Please find attached proposed fix.

Thanks.

Attachments (11)

qt-tr-fix.patch (824 bytes) - added by taem 10 years ago.
trans.patch (11.9 KB) - added by samkpo 9 years ago.
deb38.png (31.4 KB) - added by samkpo 9 years ago.
deb41.png (105.7 KB) - added by samkpo 9 years ago.
translations_qmake_icons_fix.patch (15.2 KB) - added by krab 9 years ago.
Patch for translations under linux, qmake and icons.
many_new_sizes_icons_for_qt.zip (89.3 KB) - added by krab 9 years ago.
Additional sizes for transmission-qt
linux-translations-dir-fix2.patch (10.7 KB) - added by krab 8 years ago.
linux-translations-dir-fix3.patch (10.7 KB) - added by krab 8 years ago.
linux-translations-dir-fix4_and_QT5-support.patch (31.3 KB) - added by krab 8 years ago.
Qmake, Linux Translattions, Qt5 support
translations-qrc.patch (1.4 KB) - added by equeim 7 years ago.
Patch for translations in binary. Will work on all platforms.
4050-translations-dir.patch (2.5 KB) - added by mike.dld 7 years ago.

Download all attachments as: .zip

Change History (44)

Changed 10 years ago by taem

comment:1 Changed 10 years ago by jordan

  • Milestone changed from None Set to 2.22
  • Status changed from new to assigned

comment:2 Changed 10 years ago by jordan

Thanks taem!

comment:3 Changed 10 years ago by jordan

  • Resolution set to fixed
  • Status changed from assigned to closed

Committed in r12008

comment:4 Changed 10 years ago by jordan

r12031 /branches/2.2x/qt/app.cc: (2.2x) backport r12008 for #4050 "transmission-qt translations don't load under Linux" -- fixed via patch from taem

comment:5 Changed 9 years ago by lin-unix

  • Milestone changed from 2.22 to None Set
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version changed from 2.21 to 2.52

This patch needs to be reverted. Translations can't be loaded with it in place. Why is the code looking for Transmission-Qt's translations in the folder where Qt stores its own translations? Transmission-Qt's translations are stored elsewhere.

To my knowledge, distro's can install translations wherever they want but there are some common places where they can be stored. Quassel searches for various data directories (0), searches for the "translation" folder (1), then uses whatever is found (2).

(0) Line 384 (1) Line 457 (2) Line 502 http://bugs.quassel-irc.org/projects/quassel-irc/repository/revisions/master/entry/src/common/quassel.cpp

comment:6 Changed 9 years ago by lin-unix

Ironically Transmission 2.60 was released recently with a new Qt translation but with this bug that translation is useless. I know the Quassel way to fix the issue can take a little time to implement but what is the status with the patch being reverted at least?

comment:7 Changed 9 years ago by jordan

  • Milestone changed from None Set to 2.70

I've reverted r12008 in r13382.

Thanks for the Quasselclient link, I'll crib from that :)

Re-milestoning for 2.70

Changed 9 years ago by samkpo

Changed 9 years ago by samkpo

comment:8 Changed 9 years ago by samkpo

I've made a patch to make translations work, and it works, but i also made some changes in the qt/qtr.pro file, i had to make those changes (see them in the patch). As the qt/qtr.pro file was modified then compile and install the app is a little different. Before the patch, to install the app, we had to type:

qmake qtr.pro
make
make INSTALL_ROOT=prefix install

Now, it's necessary to type:

qmake PREFIX=prefix qtr.pro
make
make install

What happens is that, if we don't type the "PREFIX=prefix", then the app will suppose that the user won't install it, but just execute and hack it's code. And, if then the user executes make install it'll be installed in /bin and /share, but won't work well.

Anyway, i also modified the README file with this modification.

Changed 9 years ago by samkpo

comment:9 Changed 9 years ago by jordan

  • Milestone changed from 2.70 to Sometime

QTranslator.load() returns a boolean saying whether or not the operation succeeded, so I've gotta say I like Quassel's approach of trying different directories automatically, compared to only using one path and using #ifdefs to decide which that one path is.

sampko, could you revise your patch to use Quassel's approach?

Changed 9 years ago by krab

Patch for translations under linux, qmake and icons.

Changed 9 years ago by krab

Additional sizes for transmission-qt

comment:10 Changed 9 years ago by krab

I am rewrite qmake for transmission-qt :)

What new:
Qmake install target fixes.
QM generation in qmake target.
Support shadow build for Qt Creator IDE.
Support translations install under linux.
Additional icons for better view in various linux DE.

Translations for linux work via preprocessor "TRANSLATIONS_DIR".
I think is good way. Quassel way to many ugly.
First it is bad for translators who want to check the translation in practice.
It is not clear what should be the main directory. And why is not supported /usr/local.

All info about how-to use qmake in qt/README.txt

qmake INSTALL_PREFIX=/usr
make -j3
qmake install

it work also without INSTALL_ROOT, because INSTALL_PREFIX option care about prefix.
INSTALL_ROOT can use maintainers.

comment:11 Changed 9 years ago by krab

58:        dh_auto_install -Dqt -- INSTALL_ROOT=$(CURDIR)/debian/tmp


66:        make install INSTALL_ROOT=%{buildroot} -C qt

Readme fix :)

comment:12 Changed 9 years ago by krab

damn readme.txt fix 3:

---
 	53	   override_dh_auto_build: 
 	54	        dh_auto_build 
 	55	        dh_auto_build -Dqt -- INSTALL_PREFIX=/usr 

+++

override_dh_auto_configure:
	dh_auto_configure -- \
		--enable-cli --enable-daemon
	# qt client has a separate build-system
	dh_auto_configure -Dqt -- INSTALL_PREFIX=/usr

">_<

comment:13 Changed 9 years ago by krab

damn readme.txt fix 2:

45:          5. In the qt/ directory, as root, type "make install"


50:          5. In the qt/ directory, "make install INSTALL_ROOT={your build system variable without usr folder" 

comment:14 Changed 8 years ago by jordan

I like these improvements but it's too many unrelated things for one ticket. Please separate the icon changes and the .pro rewrite into two separate tickets, then rebase this ticket's patch on top of those.

Also, app.cc needs to know about the SVG.

Thanks!

comment:15 Changed 8 years ago by jordan

  • Status changed from reopened to new

comment:16 Changed 8 years ago by krab

Okay. But only a little later or tomorrow.

Also, app.cc needs to know about the SVG.

NO! This SVG file is bad. In the file already specified width and height. Qt such files can not scale correctly. Rather it their "scales" but smearing relative to the width and height.

comment:17 follow-ups: Changed 8 years ago by rb07

A couple of notes on the qtr.pro changes:

  • The Windows path separator substitution is not necessary (Windows understand both types of slashes), and it will break cross-compilation.
  • The translation target addition is not good enough, it does half the job. It does install the translated files (.qm), but we also need to automatically prepare the source files (.ts); in other words, running lrelease is only good if lupdate was executed before (and the translator worked on those). The problem exists now on the repository, all .ts files are lagging w.r.t. the code.
  • With Qt 5 there are more changes to the .pro file, at least one conditional (and to many of the code files).

comment:18 Changed 8 years ago by krab

but we also need to automatically prepare the source files (.ts); in other words, running lrelease is only good if lupdate was executed before (and the translator worked on those).

This is a common task such as writing a cpp, h, and testing code. Nothing complicated. We need to do a good installation for maintainers.

With Qt 5 there are more changes to the .pro file, at least one conditional (and to many of the code files).

Qt5 need to maintain much more code changes than just edit pro file.

comment:19 in reply to: ↑ 17 Changed 8 years ago by krab

Replying to rb07:

but we also need to automatically prepare the source files (.ts); in other words, running lrelease is only good if lupdate was executed before (and the translator worked on those).

You confuse translators and programmers. Translators do not build program in qt creator or in the terminal with commands qmake, make, make install

I do not know how to implement the system in transmission (using transifix.net or sending email to author), I myself would like to help in translation into Russian.

In a good way, if need to check localization it is better to use Qt Linguist. And already in the terminal copy qm file to /usr/share/transmission-qt/translations.

comment:20 Changed 8 years ago by rb07

I don't confuse anything, you just don't understand what I wrote.

I've been doing translations, and managing some which are sent to me, I know very well the tools available.

There is already a Russian translation, if you want to update it you can do as I do, re-open the ticket where it was introduced and upload an updated .ts file.

comment:21 Changed 8 years ago by krab

Ok, you need a function to prepare ts file after code changes?

I can do it in qmake, when you need use qmake for update, or i can write lupdate.sh and lupdate.bat...

comment:22 Changed 8 years ago by rb07

I'm not asking you to do anything.

My comment above is about the usefulness of the addition you show to create compiled translations: its not useful if the source of the translations are not updated, but it is useful for the release build.

We already know how things are done, we don't need a script for a one line operation. What is needed is a mechanism to update the repository, and its not part of the qtr.pro file so its really off-topic for this ticket.

Changed 8 years ago by krab

comment:23 in reply to: ↑ 17 ; follow-up: Changed 8 years ago by krab

Replying to rb07:

A couple of notes on the qtr.pro changes:

  • The Windows path separator substitution is not necessary (Windows understand both types of slashes), and it will break cross-compilation.

I do not understand. What do you mean? I did not touch windows-specific code.

I do not use cross-compilation. And not sure that transmission without problems builds on windows.

comment:24 in reply to: ↑ 23 Changed 8 years ago by rb07

Replying to krab:

I do not understand. What do you mean? I did not touch windows-specific code.

What do you call these?

  win32: LRELEASE = $$replace(LRELEASE, \\\\, \\\\)

If you don't use those, and don't know what they do, then don't add them.

I do not use cross-compilation. And not sure that transmission without problems builds on windows.

Yes it builds.

qtr.pro as it is currently does work in Windows, and also cross-compiling from Linux to Windows.

My comment was clear: your changes (specifically all those 'replace') will break cross-compilation.

comment:25 Changed 8 years ago by krab

Without "this" under windows qmake will swearing much, like:

WARNING: (eval):1: Unescaped backslashes are deprecated.
WARNING: (eval):1: Unescaped backslashes are deprecated.
WARNING: (eval):1: Unescaped backslashes are deprecated.
WARNING: (eval):1: Unescaped backslashes are deprecated.

qtr.pro as it is currently does work in Windows, and also cross-compiling from Linux to Windows.

Can you fix my patch for your cross-compilled windows port?

Last edited 8 years ago by krab (previous) (diff)

Changed 8 years ago by krab

comment:26 Changed 8 years ago by krab

In Qt Creator (also shadow builds) now load translations from DESTDIR (from folder where transmission-qt bin located).

Changed 8 years ago by krab

Qmake, Linux Translattions, Qt5 support

comment:27 Changed 8 years ago by krab

Please check Qt5 code fixes. Especially this moment:

+    beginResetModel();
     myIdToRow.clear( );
     myIdToTorrent.clear( );
     foreach( Torrent * tor, myTorrents ) delete tor;
     myTorrents.clear( );
-    reset( );
+    endResetModel();

and this

+  beginResetModel();
   clearSubtree (QModelIndex());
-
-  reset ();
+  endResetModel();

Screenshot: http://wstaw.org/m/2013/05/04/plasma-desktopPS1936.png Ubuntu 13.04, Qt 5.0.1 :)

comment:28 Changed 8 years ago by krab

Problem with exit :( http://wstaw.org/m/2013/05/04/plasma-desktopxt1936.png problem in qtdbus5?

comment:29 Changed 8 years ago by Nikoli

  • Cc nikoli@… added

comment:30 Changed 8 years ago by krab

Ping, pong.

Changed 7 years ago by equeim

Patch for translations in binary. Will work on all platforms.

comment:31 Changed 7 years ago by mike.dld

  • Owner changed from jordan to mike.dld
  • Status changed from new to assigned

comment:32 Changed 7 years ago by mike.dld

None of currently attached patches implement fallback mechanism mentioned in comment:5 and comment:9. Also, they all tend to change more than expected for this ticket fix.

I'm attaching new patch to fix translations loading on *NIX systems. TRANSLATIONS_DIR macro is being defined during Qt client compilation and used in app.cc as one of possible translation file locations. Quassel has a lot more complicated directory search logic which I believe Transmission doesn't need (at least right now), so my code is as dumb as it could get. I also didn't add the macro definition to qmake project as CMake is now the preferred way of building Qt client, and because qmake is just driving me nuts.

This patch also adds Qt client translation files installation which I missed in #5828.

Feel free to test and comment.

Last edited 7 years ago by mike.dld (previous) (diff)

Changed 7 years ago by mike.dld

comment:33 Changed 7 years ago by mike.dld

  • Milestone changed from Sometime to 2.90
  • Resolution set to fixed
  • Status changed from assigned to closed

Silence gives consent, comitted as r14396. Feel free to reopen if something's not working right for you.

Note: See TracTickets for help on using tickets.