<table><tr><td style="">jtamate updated this revision to Diff 29572.<br />jtamate edited the summary of this revision. <a href="https://phabricator.kde.org/transactions/detail/PHID-XACT-DREV-q7kdpab6uouboek/">(Show Details)</a><br />jtamate edited the test plan for this revision. <a href="https://phabricator.kde.org/transactions/detail/PHID-XACT-DREV-bqfznb2xp2nrw52/">(Show Details)</a><br />jtamate added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D10742">View Revision</a></tr></table><br /><div><div><p>Pass all the unittests, without modifications, finally.<br />
Use as much as possible the recently added move semantics in KFileItem.</p></div></div><br /><div><strong>CHANGES TO REVISION SUMMARY</strong><div><div style="white-space: pre-wrap; color: #74777D;"><span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">This summary will change when the doubts are resolved (if they can be resolved!).<br />
<br />
Implement the first part of a TODO: get rid of the raw KFileItem pointers in KCoreDirListerCache<br />
<br />
I've added a way to remove the item from the lister in findByUrl, to be modified and, if needed, added back.<br />
<br />
Questions to be answered:<br />
  * In findByUrl, the dirItem->rootItem should also be removed?<br />
  * fileItem is get twice in slotFileRenamed. Is this patch removing the right fileitem?</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">Do a TODO introduced in 4b498196899223692e8a7d334618b2874bb6c77e in 2014.</span><br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">  * Should the fileitem removed in slotFileRenamed be added back or is it added back in emitRefreshItem? If should be  added back</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">Don't depend on the internal memory layout of the containers</span>, <span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">to which list?</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">and get rid of NonMovableFileItem.</span><br />
<br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">I know the patched code doesn't follow the original semantics because dolphin see duplicated entries while renaming takes place</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">Any tiny performance regression is eclipsed by the change of lstItems to hashItems</span>.</div></div></div><br /><div><strong>CHANGES TO TEST PLAN</strong><div><div style="white-space: pre-wrap; color: #74777D;"><span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">findByUrl is slow, for example, renaming 50.000 small files, it has to go through a list of 50.000 items 50.000 times, so renaming that number of files takes more than an hour.<br />
This patch does not improve findByUrl performance, but allows future work on it.</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">Pass the unittests.<br />
<br />
</span></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R241 KIO</div></div></div><br /><div><strong>CHANGES SINCE LAST UPDATE</strong><div><a href="https://phabricator.kde.org/D10742?vs=27909&id=29572">https://phabricator.kde.org/D10742?vs=27909&id=29572</a></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D10742">https://phabricator.kde.org/D10742</a></div></div><br /><div><strong>AFFECTED FILES</strong><div><div>src/core/kcoredirlister.cpp<br />
src/core/kcoredirlister_p.h</div></div></div><br /><div><strong>To: </strong>jtamate, Frameworks, dfaure<br /><strong>Cc: </strong>markg, michaelh, ngraham<br /></div>