D17619: Change the path for every item of the subdirectories in a directory rename

[David Faure]

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<QUrl> deletedUrls;
>      for (int i = 0; i < m_dirLister.spyItemsDeleted.count(); ++i) {
>          deletedUrls += m_dirLister.spyItemsDeleted[i][0].value<KFileItemList>().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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20181222/6d74a985/attachment.html>