<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/100008/">http://git.reviewboard.kde.org/r/100008/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 29th, 2010, 12:18 p.m., <b>Rick W. Chen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="/r/100008/diff/1/?file=463#file463line176" style="color: black; font-weight: bold; text-decoration: underline;">src/browsers/collectionbrowser/CollectionWidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">CollectionWidget::init()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">176</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">combo</span><span class="o">-></span><span class="n">addItem</span><span class="p">(</span> <span class="n">icon</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Added Today"</span><span class="p">),</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"added<span class="hl">"</span></span><span class="p"><span class="hl">)</span></span><span class="hl"> </span><span class="o"><span class="hl">+</span></span><span class="hl"> </span><span class="n"><span class="hl">QString</span></span><span class="p"><span class="hl">(</span></span><span class="s"><span class="hl">"</span>:<1d"</span><span class="p">)</span> <span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">176</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">combo</span><span class="o">-></span><span class="n">addItem</span><span class="p">(</span> <span class="n">icon</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Added Today"</span><span class="p">),</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"added:<1d"</span> <span class="p">)</span> <span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">177</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">combo</span><span class="o">-></span><span class="n">addItem</span><span class="p">(</span> <span class="n">icon</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Added This Week"</span><span class="p">),</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"added<span class="hl">"</span></span><span class="p"><span class="hl">)</span></span><span class="hl"> </span><span class="o"><span class="hl">+</span></span><span class="hl"> </span><span class="n"><span class="hl">QString</span></span><span class="p"><span class="hl">(</span></span><span class="s"><span class="hl">"</span>:<1w"</span><span class="p">)</span> <span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">177</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">combo</span><span class="o">-></span><span class="n">addItem</span><span class="p">(</span> <span class="n">icon</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Added This Week"</span><span class="p">),</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"added:<1w"</span><span class="p">)</span> <span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">178</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">combo</span><span class="o">-></span><span class="n">addItem</span><span class="p">(</span> <span class="n">icon</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Added This Month"</span><span class="p">),</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"added<span class="hl">"</span></span><span class="p"><span class="hl">)</span></span><span class="hl"> </span><span class="o"><span class="hl">+</span></span><span class="hl"> </span><span class="n"><span class="hl">QString</span></span><span class="p"><span class="hl">(</span></span><span class="s"><span class="hl">"</span>:<1m"</span><span class="p">)</span> <span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">178</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">combo</span><span class="o">-></span><span class="n">addItem</span><span class="p">(</span> <span class="n">icon</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Added This Month"</span><span class="p">),</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"added:<1m"</span><span class="p">)</span> <span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The "added" i18n string is taken from CollectionTreeItemModelBase::addFilters(). They need to be the same so that the filtering will work. So it might break if the translation uses different "added" for the two places.
Similarly, the 'd', 'w', 'm' etc. are also hardcoded in CollectionTreeItemModelBase::semanticDateTimeParser(). So the same thing applies here.
Ideally we should make all of them translatable with context while ensuring they are consistent.</pre>
</blockquote>
<p>On September 30th, 2010, 12:12 p.m., <b>Mark Kretschmann</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;">Could you please elaborate? What concrete solution do you suggest, in code?</pre>
</blockquote>
<p>On September 30th, 2010, 4:26 p.m., <b>Ian Monroe</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;">Well for this we just need to keep the "1d" "1w" stuff out of i18n. I'm not really sure why the old version doesn't compile so dunno a solution.
Maybe we should use i18n args? doesn't something like
i18n("added %1", ":<1d") work? I forget the exact syntax. This has the advantage of allow languages to move the verb relative to the less then sign.
would that break filtering Rick?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">What's happening is this:
In the filtering code we compare "added" and i18n("added") for the token. In the
above code we have a combo box storing 3 presets. When the user selects "Added
Today", the string "added:<1d" will be inserted in the line edit and filtering
is triggered.
If we change the i18n("added") stored in the combobox, the translator will see
separate strings to translate. If "added" is translated differently for the two
cases then filtering will not work.
Right now the translators might ignore this completely anyway since
i18n("added") is just confusing. So adding some context should help, e.g.
i18nc("Token for filtering tracks added since a certain time", "added"). If we
do that, the same context will need to be added to the same token in the
filtering code to make them the same localization string. We should do the same
for all other tokens as well.
For the filter string ("<1d" etc.), since they are not made translatable these
should be kept out of i18n, as Ian suggested.
With something like i18n("added:%1", "<1d"), the issue remains where there will
be separate strings to translate. It will break if the translations are
different.
I don't think this is a problem (concatenating strings as it is now), since
these are only done once and only a handful of strings. Furthermore we have the
EditFilterDialog which presumably the users will first find out about this
feature. To help with translations I suggest adding contexts to all those
tokens. A minor speedup here would be to use QLatin1String instead of QString
for the ":<1d" etc.</pre>
<br />
<p>- Rick W.</p>
<br />
<p>On September 30th, 2010, 4:12 p.m., Mark Kretschmann wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for amarok.</div>
<div>By Mark Kretschmann.</div>
<p style="color: grey;"><i>Updated 2010-09-30 16:12:51</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;"> Use QT_USE_FAST_CONCATENATION and QT_USE_FAST_OPERATOR_PLUS for performance.
See http://doc.trolltech.com/4.6/qstring.html#more-efficient-string-construction.
I had to clean up lots of incorrect string concatenations. Please check for
correctness.
</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;">This still fails to compile because of one error in libLastFM. Who is up for fixing the lib? :)
</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/CMakeLists.txt <span style="color: grey">(abdcee8)</span></li>
<li>src/aboutdialog/OcsPersonItem.cpp <span style="color: grey">(6685aa1)</span></li>
<li>src/browsers/collectionbrowser/CollectionWidget.cpp <span style="color: grey">(196837c)</span></li>
<li>src/browsers/playlistbrowser/DynamicModel.cpp <span style="color: grey">(af7742d)</span></li>
<li>src/context/applets/similarartists/SimilarArtistsApplet.cpp <span style="color: grey">(e1e5d59)</span></li>
<li>src/context/applets/upcomingevents/UpcomingEventsApplet.cpp <span style="color: grey">(7233785)</span></li>
<li>src/core-impl/collections/audiocd/AudioCdCollection.cpp <span style="color: grey">(a10976b)</span></li>
<li>src/core-impl/collections/daap/DaapCollection.cpp <span style="color: grey">(06e97b2)</span></li>
<li>src/core-impl/collections/sqlcollection/SqlCollectionLocation.cpp <span style="color: grey">(e9411e6)</span></li>
<li>src/core-impl/collections/sqlcollection/mysqlecollection/MySqlEmbeddedStorage.cpp <span style="color: grey">(b3b40bc)</span></li>
<li>src/core/support/Debug.h <span style="color: grey">(62766c4)</span></li>
<li>src/dynamic/Bias.cpp <span style="color: grey">(dec2db2)</span></li>
<li>src/dynamic/BiasSolver.cpp <span style="color: grey">(0be91da)</span></li>
<li>src/dynamic/biases/EchoNest.cpp <span style="color: grey">(2d0038a)</span></li>
<li>src/scriptengine/AmarokCollectionScript.cpp <span style="color: grey">(da87d33)</span></li>
<li>src/services/lastfm/CMakeLists.txt <span style="color: grey">(7f17ff7)</span></li>
<li>src/services/lastfm/LastFmService.cpp <span style="color: grey">(3b291fe)</span></li>
<li>src/services/lastfm/LastFmServiceSettings.cpp <span style="color: grey">(6517fea)</span></li>
<li>src/services/lastfm/biases/LastFmBias.cpp <span style="color: grey">(6f3286c)</span></li>
<li>src/services/lastfm/biases/WeeklyTopBias.cpp <span style="color: grey">(98f9257)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100008/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>