<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/112292/">http://git.reviewboard.kde.org/r/112292/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 27th, 2013, 1:47 p.m. 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;">Question is, why should it go into this function and not say into others that also accept a language string? Maybe this should go there somewhere as a static helper function? I mean yes, we are using this function from somewhere whose input is uiLanguages but maybe the fix is calling that "new" helper function from there? Or adding this code that checks the input format to all of the KCatalog functions that accept a language as string? Or? Chusslove, you there? What's your opinion?</pre>
</blockquote>
<p>On August 27th, 2013, 3:23 p.m. UTC, <b>Chusslove Illich</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;">I agree with Albert. I don't think this heuristic (which is what it is)
belongs to ki18n. Rather, it is upon the caller code to send in the language
in expected format. And the expected language format for ki18n is in fact
whatever directory name that appears as
${DATADIR}/locale/<language>/LC_MESSAGES/.
If I think of e.g. of the code in KLanguageButton, it itself should go
through guessing which languages are available and how to present them to
the user (far from trivial), and then to set up all the expected translation
systems to use this language. The expected systems are now Qt and ki18n,
but in principle could be more (maybe other platforms). So I don't think
this problem should be approached by making
LocalizedString::isApplicationTranslatedInto and QLocale::uiLanguages
(approximately) compatible.
</pre>
</blockquote>
<p>On August 28th, 2013, 11:18 a.m. UTC, <b>Chusslove Illich</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;">Having in effect said that language codes for ki18n can be anything (simply
used to construct directory path to test), why not have a function in ki18n
like KLocalizedString::availableApplicationTranslations? It would go through
all set data dirs and collect language codes from existing
${DATADIR}/locale/<language>/LC_MESSAGES/fooapp.mo paths, and return this
list. This functionality would clearly belong to ki18n, and the code such as
in KLanguageButton would then have an easier task, of only having to pair
the language codes from different translation systems for display to user as
a single known (and translated) language name entry.
</pre>
</blockquote>
<p>On August 28th, 2013, 11:23 a.m. 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;">FWIW That's what i kind of suggested in the other review Vishesh did.</pre>
</blockquote>
<p>On August 31st, 2013, 11:49 a.m. UTC, <b>Chusslove Illich</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;">Posted the review for KLocalizedString::availableApplicationTranslations:
https://git.reviewboard.kde.org/r/112401</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;">Discarding this as it is no longer required.</pre>
<br />
<p>- Vishesh</p>
<br />
<p>On August 27th, 2013, 1:14 p.m. UTC, Vishesh Handa wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks, Albert Astals Cid, Aleix Pol Gonzalez, and Chusslove Illich.</div>
<div>By Vishesh Handa.</div>
<p style="color: grey;"><i>Updated Aug. 27, 2013, 1:14 p.m.</i></p>
<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;"> Make KLocalizedString::isApplicationTranslatedInto & QLocale::uiLanguages compatible
QLocale::uiLanguages returns "lang-script-country" whereas KCatalog
expects the format to be "lang_countr@script". We need to convert the
format before checking if the corresponding catalog exists.
</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;">Limited testing done. My QLocale::uiLanguages() returns "en-US" which is correctly converted to "en_US". I haven't tested it with anything else. Any tips on how I should go about testing this?</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>staging/ki18n/src/klocalizedstring.cpp <span style="color: grey">(eab9216)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112292/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>