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

Milian Wolff noreply at phabricator.kde.org
Thu Sep 28 15:55:49 UTC 2017


mwolff added inline comments.

INLINE COMMENTS

> rjvbb wrote in abstractfilemanagerplugin.cpp:297
> (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).

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 `dirty` instead of `created`.

> rjvbb wrote in abstractfilemanagerplugin.cpp:409
> So, what branch (this shouldn't break anything in 5.2) ?

5.2 is fine with me

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, mwolff
Cc: aaronpuchert, 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/20170928/5919f4f2/attachment.html>


More information about the KDevelop-devel mailing list