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
