<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/105633/">http://git.reviewboard.kde.org/r/105633/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Long-standing open reviews are bad, they lead to duplicate work :(</pre>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/105633/diff/2/?file=74418#file74418line64" style="color: black; font-weight: bold; text-decoration: underline;">allyourbase.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">QPixmap KWalletFolderItem::getFolderIcon(KIconLoader::Group group){</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">64</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">KWalletFolderItem</span><span class="o">::</span><span class="n">refresh</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">64</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">KWalletFolderItem</span><span class="o">::</span><span class="n">refresh</span><span class="p">(</span><span class="kt"><span class="hl">int</span></span><span class="hl"> </span><span class="n"><span class="hl">count</span></span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well, it doesn't really refresh anymore, if you use a cache :-)
The whole point of this method is to get the new count from the daemon.
The problem was that it was called far too often -- I just fixed that
[and you had too, below... I hadn't seen this pending review before investigating the bug... sigh]</pre>
</div>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/105633/diff/2/?file=74419#file74419line622" style="color: black; font-weight: bold; text-decoration: underline;">kwalleteditor.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">void KWalletEditor::updateEntries(const QString& folder) {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">622</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// set up a quick lookup table</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">All this was useful when updateEntries was called too often. Now that it's only called on actual changes, does it matter as much?</pre>
</div>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/105633/diff/2/?file=74419#file74419line784" style="color: black; font-weight: bold; text-decoration: underline;">kwalleteditor.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">void KWalletEditor::listItemRenamed(QTreeWidgetItem* item) {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">773</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">_w</span> <span class="o">||</span> <span class="n">t</span><span class="p">.</span><span class="n">isEmpty</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">784</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">_w</span> <span class="o">||</span> <span class="n">t</span><span class="p">.</span><span class="n">isEmpty</span><span class="p">()</span><span class="hl"> </span><span class="o"><span class="hl">||</span></span><span class="hl"> </span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">t</span></span><span class="hl"> </span><span class="o"><span class="hl">==</span></span><span class="hl"> </span><span class="n"><span class="hl">i</span></span><span class="o"><span class="hl">-></span></span><span class="n"><span class="hl">oldName</span></span><span class="p"><span class="hl">())</span>)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is the actual bugfix -- this method was wrongly assuming the item actually got renamed.
But then, if it wasn't renamed, there's no reason to call setText() back either.
I committed my fix for this (to 4.9 and master, see bug 279161 for ref). I think this change request can be discarded, apart maybe from the QSet which could help lower CPU usage when adding something to the wallet while kwalletmanager is running, or when renaming an entry in kwalletmanager.</pre>
</div>
<br />
<p>- David</p>
<br />
<p>On July 21st, 2012, 1:20 p.m., Martin Koller wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/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 KDE Utils, Michael Leupold and Raphael Kubo da Costa.</div>
<div>By Martin Koller.</div>
<p style="color: grey;"><i>Updated July 21, 2012, 1:20 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;">I have > 340 entries in "Form Data" in the wallet. When opening the wallet editor, it takes about 9 Seconds until the tree is displayed!
I investigated the problem and found out that this is due to 2 reasons:
1.) The list of entries is checked against existing entries in the tree, which is done by linear search in the given entries list and in the tree
which results in a quadratic complexity.
2.) unneeded duplicate dbus calls
I solved the first by using QSet for fast lookup and I reduced the second problem by avoiding a duplicate, unneeded query over dbus</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=279161">279161</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=284671">284671</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>allyourbase.h <span style="color: grey">(d3452cd)</span></li>
<li>allyourbase.cpp <span style="color: grey">(f57eb68)</span></li>
<li>kwalleteditor.cpp <span style="color: grey">(777ce8d)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105633/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>