This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103555/ |
On January 2nd, 2013, 3:18 p.m., David Faure wrote:
kio/kio/kdirlister.cpp (Diff revision 1) void KDirListerCache::slotFileDirty( const QString& path )1080 if (!itemsInUse.contains(url.path()) && !itemsCached.contains(url.path())) {I don't understand the if(). Why should the behavior differ depending on whether we have this url currently shown (itemsInUse) or in the cache (itemsCached), compared to when we don't know about this URL? The if() is wrongly written anyway: if it should be there, then it should be url.url(), not url.path(), since itemsInUse and itemsCached use URLs as keys. I should change that to QUrl in KF5. And in case you remove the if, then my old comment meant: in that case, we can simplify the code to remove the if (isDir), and simply do Q_FOREACH(const QString& dir, directoriesForCanonicalPath(url.directory())) { handleDirDirty(dir); } in all cases (file or directory). Because as you say, it's the parent that needs to be re-listed when the permissions of anything changes. Also, this changes misses a corresponding unittest in kdirlistertest.cpp.
Ahh... I see what you mean. I should have realized what I was doing is utterly unnecessary. I will make the change as you suggested and add a test case for this. In fact, I will add the unittest first to test for the failure.
- Dawit
On December 29th, 2012, 8:31 p.m., Dawit Alemayehu wrote:
Review request for kdelibs and David Faure.
By Dawit Alemayehu.
Updated Dec. 29, 2012, 8:31 p.m. Description
Testing
Bugs:
173733
Diffs
|