<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/125682/">https://git.reviewboard.kde.org/r/125682/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Oktober 18th, 2015, 6:50 vorm. UTC, <b>Thomas Eschenbacher</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This should work, but IMHO it does not go far enough. I can see many more calls to QCoreApplication::translate(...) with user defined content in this file (kaboutdata.cpp), which is also affected, as for example credits, authors, copyright statements and so on...</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Furthermore feeding in already-translated strings at application start will break the feature of language switching at run time and it might confuse the developers who have to pass some strings with I18N_NOOP(), some with i18n(). I think that "translation on demand" is the right way, we just need to find a way to make it work.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As I wrote in the corresponding bug report: using gettext() should work for these use cases, but unfortunately only without context. What about using the strings like "NAMES OF TRANSLATORS" as message string, without translation context, and then using some low level API like dcgettext? I tried that here and it worked fine (using the application name as "domain" paramter of dcgettext). But that was only a quick experiment, maybe someone knows a more elegant way to solve this...</p></pre>
</blockquote>
<p>On Oktober 18th, 2015, 8:48 vorm. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What other user defined content do you see in there? everything else i see is plain calls with hardcoded strings.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KF5 based apps do not support runtime switching of language at all.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What strings do you have to pass with I18N_NOOP?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You can't use gettext, that's adding a new dependency, and it does not solve the problem, the QCoreApplication::translate should basically not be part of kcoreaddons, then the problem is gone, you have to feed your translators for them to show, and kxmlgui feeds them for you, otherwise you do it manually.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Sorry, you are right, the strings provided from outside are only <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">stored</em> "as is", the rest is hardcoded.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So if runtime language switching is not supported anymore, we don't need to care about this topic.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">And as most strings are only stored by KAboutData, one has two possibilities to handle translation:
1) set strings wrapped with I18N_NOOP and translate on use, or
2) set strings wrapped with i18n and later use them "as is"</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">And again you are right, using gettext introduces a new dependency, as well as using ki18n, that's the reason why I don't like this too.</p></pre>
<br />
<p>- Thomas</p>
<br />
<p>On Oktober 18th, 2015, 2:13 vorm. UTC, Michael Pyne wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks, Localization and Translation (l10n) and Albert Astals Cid.</div>
<div>By Michael Pyne.</div>
<p style="color: grey;"><i>Updated Okt. 18, 2015, 2:13 vorm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=345320">345320</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kxmlgui
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KAboutData has a placeholder for information regarding who translated the running KDE-based application (KAboutData::translators()). However it relies on the application developer to call KAboutData::setTranslator() since KCoreAddons (a Tier 1 framework) cannot use KI18n directly. Instead the Qt translation infrastructure is used where translations are unavoidable, but that is not compatible with KF5's i18n. This (IIUC) gives different behavior than KDE4, where KAboutData could (and did!) directly pull the translated list of translators, so application authors didn't have to do anything special for their about dialog to have the list of translators.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For the majority of our applications we can make the ::setTranslator() call on the application developer's behalf, as recommended by Albert on kdeframeworks-devel, and by doing so from KMainWindow (a relatively central location for KF5-using applications) we can use KI18n and get the accurately-translated message. Feeding the already-translated information into KAboutData should work to form the list of translators for later use by KAboutApplicationDialog, or other accessors of that information.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch implements all that. I avoided using a global startup method as suggested by Albert (since the Qt docs suggest that this startup would happen <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">before</em> the GUI starts up, and I want to make sure KI18n is available); KMainWindow seems the next best option, and even non-KXmlGuiWindow users might use this. There are probably other good alternatives too (maybe even the platform plugin?).</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Compiled, apps all still work.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I find it hard to test whether the translators actually show up or not though, as I do not use translated KF5 apps.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/kmainwindow.cpp <span style="color: grey">(7c86841)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/125682/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>