<table><tr><td style="">mwolff added inline comments.
</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/D7995" rel="noreferrer">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7995#inline-33265" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">rjvbb</span> wrote in <span style="color: #4b4d51; font-weight: bold;">abstractfilemanagerplugin.cpp:297</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">(Also see my other non-inline reaction)</p>

<p style="padding: 0; margin: 8px;">I'm really doubtful that this is even a good idea. I think it handled the notification of creation of new entries *in the parent directory*.</p>

<p style="padding: 0; margin: 8px;">We no longer get <tt style="background: #ebebeb; font-size: 13px;">created</tt> signals but only signals that tell us that something has changed in a given directory - a folder that is already represented in the project view.</p>

<p style="padding: 0; margin: 8px;">I don't know if/why/how there can be multiple FolderItems corresponding to the dirty path's parent, nor why you'd want to emit the <tt style="background: #ebebeb; font-size: 13px;">folderAdded</tt> signal (it wasn't just added, it was already known), and then read the folder again. Maybe we need to pretend that it was just added so that clients get the signal that they too need to reload the folder?<br />
This is assuming that <tt style="background: #ebebeb; font-size: 13px;">p->foldersForPath(indexedPath) == q->createFolderItem( p, path, parentItem )</tt>.</p>

<p style="padding: 0; margin: 8px;">The 2nd hunk containing <tt style="background: #ebebeb; font-size: 13px;">ProjectFileItem* file = q->createFileItem( p, path, parentItem );</tt> ought never trigger because we only get called with folder paths (<tt style="background: #ebebeb; font-size: 13px;">info.isDir()</tt> is always true).</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">sigh, lost in translation once more... With "keep it as-is" I meant keep your version that removed this altogether. As I said, I misunderstood this code initially - you are right in that this loop should be removed now that you listen to <tt style="background: #ebebeb; font-size: 13px;">dirty</tt> instead of <tt style="background: #ebebeb; font-size: 13px;">created</tt>.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7995#inline-33375" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">rjvbb</span> wrote in <span style="color: #4b4d51; font-weight: bold;">abstractfilemanagerplugin.cpp:409</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">So, what branch (this shouldn't break anything in 5.2) ?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">5.2 is fine with me</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R32 KDevelop</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D7995" rel="noreferrer">https://phabricator.kde.org/D7995</a></div></div><br /><div><strong>To: </strong>rjvbb, KDevelop, mwolff<br /><strong>Cc: </strong>aaronpuchert, arrowdodger, kfunk, dfaure, mwolff, brauch, kdevelop-devel, geetamc, Pilzschaf, akshaydeo, surgenight<br /></div>