D17619: Change the path for every item of the subdirectories in a directory rename
This revision was automatically updated to reflect the committed changes. Closed by commit R241:211c3a680ea7: Change the path for every item of the subdirectories in a directory rename (authored by jtamate). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D17619?vs=48048&id=48109#toc REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17619?vs=48048&id=48109 REVISION DETAIL https://phabricator.kde.org/D17619 AFFECTED FILES autotests/kdirlistertest.cpp autotests/kdirlistertest.h src/core/kcoredirlister.cpp To: jtamate, dfaure, #frameworks Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns
D17619: Change the path for every item of the subdirectories in a directory rename
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17619 To: jtamate, dfaure, #frameworks Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns
D17619: Change the path for every item of the subdirectories in a directory rename
jtamate updated this revision to Diff 48048. jtamate added a comment. With this version, the test with the patch applied is everlasting in: while [ true ]; do bin/kdirlistertest testRenameDirectory; if [ $? -ne 0 ]; then break; fi; done and is still crashing without the patch. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17619?vs=48004&id=48048 REVISION DETAIL https://phabricator.kde.org/D17619 AFFECTED FILES autotests/kdirlistertest.cpp autotests/kdirlistertest.h src/core/kcoredirlister.cpp To: jtamate, dfaure, #frameworks Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns
D17619: Change the path for every item of the subdirectories in a directory rename
jtamate updated this revision to Diff 48004. jtamate marked 4 inline comments as done. jtamate added a comment. Changes requested done and reduced code duplication. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17619?vs=47995&id=48004 REVISION DETAIL https://phabricator.kde.org/D17619 AFFECTED FILES autotests/kdirlistertest.cpp autotests/kdirlistertest.h src/core/kcoredirlister.cpp To: jtamate, dfaure, #frameworks Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns
D17619: Change the path for every item of the subdirectories in a directory rename
jtamate added inline comments. INLINE COMMENTS > dfaure wrote in kdirlistertest.cpp:1326 > What's drive_c? I started this patch following the structure of .wine/drive_c ... REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17619 To: jtamate, dfaure, #frameworks Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns
D17619: Change the path for every item of the subdirectories in a directory rename
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> kdirlistertest.cpp:1326
> +QCOMPARE(m_dirLister.url().toLocalFile(), newDir);
> +QVERIFY(ok);
> +currDir = newDir;
What's drive_c?
> kdirlistertest.cpp:1334
> {
> // ensure m_dirLister holds the items.
> m_dirLister.openUrl(QUrl::fromLocalFile(path()), KDirLister::NoFlags);
This could be done before the previous line, and used there to remove some
duplication?
> kdirlistertest.cpp:1343
> QList deletedUrls;
> for (int i = 0; i < m_dirLister.spyItemsDeleted.count(); ++i) {
> deletedUrls +=
> m_dirLister.spyItemsDeleted[i][0].value().urlList();
Waiting for 0 signals doesn't really make sense, does it? I bet it works the
same without the TRY_
> kdirlistertest.cpp:1346
> }
> //qDebug() << deletedUrls;
> QUrl currentDirUrl =
> QUrl::fromLocalFile(path()).adjusted(QUrl::StripTrailingSlash);
You could do the more compact and more informative
QVERIFY2(job->exec(), qPrintable(job->errorString());
on line 1342, obviously, not here.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D17619
To: jtamate, dfaure, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns
D17619: Change the path for every item of the subdirectories in a directory rename
jtamate updated this revision to Diff 47995. jtamate added a comment. Added the unit test. It crashes for me every time without the patch and none with the patch. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17619?vs=47731&id=47995 REVISION DETAIL https://phabricator.kde.org/D17619 AFFECTED FILES autotests/kdirlistertest.cpp autotests/kdirlistertest.h src/core/kcoredirlister.cpp To: jtamate, dfaure, #frameworks Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns
D17619: Change the path for every item of the subdirectories in a directory rename
jtamate added a comment. I'll work on the unit test this weekend. I don't currently have enough free time on weekdays. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17619 To: jtamate, dfaure, #frameworks Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns
D17619: Change the path for every item of the subdirectories in a directory rename
jtamate retitled this revision from "fix for bug 401552" to "Change the path for every item of the subdirectories in a directory rename". REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17619 To: jtamate, dfaure, #frameworks Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns
