D23902: [KCoreDirLister] replace deprecated foreach with range-for

David Faure noreply at phabricator.kde.org
Tue Sep 17 20:34:34 BST 2019


dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kcoredirlister.cpp:833
>      qCDebug(KIO_CORE_DIRLISTER) << urlDir; // output urls, not qstrings, since they might contain a password
> -    Q_FOREACH (const QUrl &u, directoriesForCanonicalPath(urlDir)) {
> +    for (const QUrl &u : directoriesForCanonicalPath(urlDir)) {
>          updateDirectory(u);

Well, then you need a local const variable here.

> kcoredirlister.cpp:1079
>      if (isDir) {
> -        Q_FOREACH (const QUrl &dir, directoriesForCanonicalPath(url)) {
> +        for (const QUrl &dir : directoriesForCanonicalPath(url)) {
>              handleFileDirty(dir); // e.g. for permission changes

and here

> kcoredirlister.cpp:1084
>      } else {
> -        Q_FOREACH (const QUrl &dir, directoriesForCanonicalPath(url.adjusted(QUrl::RemoveFilename | QUrl::StripTrailingSlash))) {
> +        for (const QUrl &dir : directoriesForCanonicalPath(url.adjusted(QUrl::RemoveFilename | QUrl::StripTrailingSlash))) {
>              QUrl aliasUrl(dir);

and here

> kcoredirlister.cpp:1158
>      QStringList fileUrls;
> -    Q_FOREACH (const QUrl &url, directoriesForCanonicalPath(dirUrl.adjusted(QUrl::RemoveFilename | QUrl::StripTrailingSlash))) {
> +    for (const QUrl &url : directoriesForCanonicalPath(dirUrl.adjusted(QUrl::RemoveFilename | QUrl::StripTrailingSlash))) {
>          QUrl urlInfo(url);

and here

> kcoredirlister.cpp:2248
> +        auto kit = itemList->cbegin();
> +        auto kend = itemList->cend();
>          for (; kit != kend; ++kit) {

Why did you remove the const in front of "auto kend"?

[alternatively, the next line could be ported to a range for, I guess]

> ahmadsamir wrote in kcoredirlister.cpp:2222
> I thought that way it doesn't get cast (with qAsConst) twice, as lstDirs is used in another for loop a couple of lines down. But I was wrong, qAsConst doesn't copy its arguments, so qAsConst is better suited.
> 
> However this loop doesn't look like it modifies lstDirs.

Did you forget to change it to `qAsConst(listDirs)` ?

> ahmadsamir wrote in kcoredirlister.cpp:2237
> But this one might change lstDirs, as itemsDeleted is emitted, which calls deleteDir() and forgetDirs(); I am not sure though. I'll use a const var for this loop.

well spotted.

REPOSITORY
  R241 KIO

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

To: ahmadsamir, kde-frameworks-devel, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190917/08c58642/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list