<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/111951/">http://git.reviewboard.kde.org/r/111951/</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 21st, 2013, 9:28 p.m. UTC, <b>Frank Reininghaus</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 guess what Thomas meant is not that you should use QStringList_removeDuplicates directly, but that you should follow a similar approach to resolve the following problem with your current patch:
You have two nested loops which iterate over the list "lstServiceTypes". Therefore, the patched function has O(N^2) complexity, where N is the length of the list. If N ever becomes large (I'm not sure how likely that is), then there is a very bad performance problem.
BTW, your patch still has coding style issues. Always put a space behind "if", do not put spaces inside "(...)", and use braces even for single-line if/else blocks, see
http://techbase.kde.org/Policies/Kdelibs_Coding_Style
for details. Even if the existing code does not follow the style, you should format any new or changed lines correctly.</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;">The two nice things about the stringlist function are that it doesn't require a second list (but just collapses the current) and checks against a growing (but pre-allocated) set.
I guess the problem here is that KMimeType::is() is not commutative.
In this case that does unfortunately not work.
Since performance seems not critical anyway, that doesn't matter though.</pre>
<br />
<p>- Thomas</p>
<br />
<p>On August 21st, 2013, 10:25 p.m. UTC, Mathias Tillman 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 kdelibs.</div>
<div>By Mathias Tillman.</div>
<p style="color: grey;"><i>Updated Aug. 21, 2013, 10:25 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;">This fixes a bug introduced by https://projects.kde.org/projects/kde/kdelibs/repository/revisions/871cccc8a88a600c8f850a020d44bfc5f5858caa that made it impossible to re-order file type associations both in System settings and in the open with list. Hence it contains a new way of detecting duplicate (inherited) mimetype entries, that the original was supposed to fix.</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;">I have tested so that file type associations can be ordered correctly, in addition to making sure that parent mimetypes have precedence when an app lists two or more mimetypes where one is the parent of the other.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=321706">321706</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kdecore/services/kservice.cpp <span style="color: grey">(8e81929b91803a3eed586d9fc15cdd78165b6bce)</span></li>
<li>kded/kbuildservicefactory.cpp <span style="color: grey">(7f89a991d088476d8ed94763e6fa65dcc3d0603c)</span></li>
<li>kded/kmimeassociations.h <span style="color: grey">(4a2c71312ade32a9ac779495496bf6ebb78b37a4)</span></li>
<li>kded/kmimeassociations.cpp <span style="color: grey">(b0af7bcc4a39e5cce1fa6abe86cace476313702a)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111951/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>