<table><tr><td style="">rjvbb 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-33252" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mwolff</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;">actually, ignore the part about a separate patch, now that I re-read the old code I finally understand what it's doing - it adds a new folder or file item when none exists. So this is an atomic change after all, keep it as-is.</p>

<p style="padding: 0; margin: 8px;">And yes, in my opinion this cannot target 5.2, considering <a href="https://phabricator.kde.org/p/kfunk/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;" rel="noreferrer">@kfunk</a> and <a href="https://phabricator.kde.org/p/brauch/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;" rel="noreferrer">@brauch</a> want to release 5.2 soon. Landing such a big change shortly before release is never a good idea.</p>

<p style="padding: 0; margin: 8px;">I honestly cannot understand your reasoning though. If you want to help out with KDevelop development, you have to target master. It's as simple as that.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><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></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>arrowdodger, kfunk, dfaure, mwolff, brauch, kdevelop-devel, geetamc, Pilzschaf, akshaydeo, surgenight<br /></div>