D7995: KDevelop: address dirwatching inefficiency (WIP/PoC)

René J.V. Bertin noreply at phabricator.kde.org
Wed Sep 27 16:11:01 UTC 2017


rjvbb added inline comments.

INLINE COMMENTS

> mwolff wrote in abstractfilemanagerplugin.cpp:297
> 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.
> 
> And yes, in my opinion this cannot target 5.2, considering @kfunk and @brauch want to release 5.2 soon. Landing such a big change shortly before release is never a good idea.
> 
> 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.

(Also see my other non-inline reaction)

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*.

We no longer get `created` 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.

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 `folderAdded` 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?
This is assuming that `p->foldersForPath(indexedPath) == q->createFolderItem( p, path, parentItem )`.

The 2nd hunk containing `ProjectFileItem* file = q->createFileItem( p, path, parentItem );` ought never trigger because we only get called with folder paths (`info.isDir()` is always true).

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D7995

To: rjvbb, #kdevelop, mwolff
Cc: arrowdodger, kfunk, dfaure, mwolff, brauch, kdevelop-devel, geetamc, Pilzschaf, akshaydeo, surgenight
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170927/c524e9ee/attachment-0001.html>


More information about the KDevelop-devel mailing list