D24372: Compile without deprecated foreach

2019-10-12 Thread Ahmad Samir
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:7c0402ed9fd7: Compile without deprecated foreach 
(authored by ahmadsamir).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24372?vs=67804=67808

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

AFFECTED FILES
  src/core/batchrenamejob.cpp
  src/core/connection.cpp
  src/core/copyjob.cpp
  src/core/desktopexecparser.cpp
  src/core/job.cpp
  src/core/kprotocolinfofactory.cpp
  src/core/kprotocolmanager.cpp
  src/core/ksslcertificatemanager.cpp
  src/core/ktcpsocket.cpp
  src/core/scheduler.cpp
  src/core/tcpslavebase.cpp
  src/urifilters/ikws/ikwsopts.cpp
  src/urifilters/ikws/searchproviderdlg.cpp
  src/urifilters/shorturi/kshorturifilter.cpp
  src/widgets/accessmanager.cpp
  src/widgets/clipboardupdater.cpp
  src/widgets/delegateanimationhandler.cpp
  src/widgets/dropjob.cpp
  src/widgets/kdesktopfileactions.cpp
  src/widgets/kdirmodel.cpp
  src/widgets/kfileitemactions.cpp
  src/widgets/kfileitemdelegate.cpp
  src/widgets/kpropertiesdialog.cpp
  src/widgets/krun_win.cpp
  src/widgets/ksslcertificatebox.cpp
  src/widgets/ksslinfodialog.cpp
  src/widgets/kurifiltersearchprovideractions.cpp
  src/widgets/previewjob.cpp
  src/widgets/sslui.cpp

To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24372: Compile without deprecated foreach

2019-10-12 Thread David Faure
dfaure accepted this revision.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D24372 (branched from master)

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

To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24372: Compile without deprecated foreach

2019-10-12 Thread Ahmad Samir
ahmadsamir added a comment.


  I had to fix one conflict when rebasing, and basically D24419 
 ate the foreach 
src/core/ksslcertificatemanager.cpp@line 128 (by returning the result of 
calling contains() on the container, neater :)).

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D24372 (branched from master)

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

To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24372: Compile without deprecated foreach

2019-10-12 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 67804.
ahmadsamir added a comment.


  Rebase

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24372?vs=67510=67804

BRANCH
  arcpatch-D24372 (branched from master)

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

AFFECTED FILES
  src/core/batchrenamejob.cpp
  src/core/connection.cpp
  src/core/copyjob.cpp
  src/core/desktopexecparser.cpp
  src/core/job.cpp
  src/core/kprotocolinfofactory.cpp
  src/core/kprotocolmanager.cpp
  src/core/ksslcertificatemanager.cpp
  src/core/ktcpsocket.cpp
  src/core/scheduler.cpp
  src/core/tcpslavebase.cpp
  src/urifilters/ikws/ikwsopts.cpp
  src/urifilters/ikws/searchproviderdlg.cpp
  src/urifilters/shorturi/kshorturifilter.cpp
  src/widgets/accessmanager.cpp
  src/widgets/clipboardupdater.cpp
  src/widgets/delegateanimationhandler.cpp
  src/widgets/dropjob.cpp
  src/widgets/kdesktopfileactions.cpp
  src/widgets/kdirmodel.cpp
  src/widgets/kfileitemactions.cpp
  src/widgets/kfileitemdelegate.cpp
  src/widgets/kpropertiesdialog.cpp
  src/widgets/krun_win.cpp
  src/widgets/ksslcertificatebox.cpp
  src/widgets/ksslinfodialog.cpp
  src/widgets/kurifiltersearchprovideractions.cpp
  src/widgets/previewjob.cpp
  src/widgets/sslui.cpp

To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24372: Compile without deprecated foreach

2019-10-12 Thread David Faure
dfaure accepted this revision.
dfaure added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> ahmadsamir wrote in job.cpp:181
> IIUC, subjobs() returns a const QList &, do we still need a local const var?
> https://api.kde.org/frameworks/kcoreaddons/html/classKCompositeJob.html#aaec8d9b05c7c4194c5ba121d43f2997e

Well spotted. Unusual

> ahmadsamir wrote in dropjob.cpp:270
> m_urls is declared const in DropJobPrivate: 
> https://cgit.kde.org/kio.git/tree/src/widgets/dropjob.cpp#n142

Oh, interesting :-)

REPOSITORY
  R241 KIO

BRANCH
  ahmad/foreach-urifilters2 (branched from master)

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

To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24372: Compile without deprecated foreach

2019-10-08 Thread Ahmad Samir
ahmadsamir added inline comments.

INLINE COMMENTS

> dfaure wrote in job.cpp:181
> You need a local const var to hold the result of the subjobs() method call.
> 
> (repeats)

IIUC, subjobs() returns a const QList &, do we still need a local const var?
https://api.kde.org/frameworks/kcoreaddons/html/classKCompositeJob.html#aaec8d9b05c7c4194c5ba121d43f2997e

> dfaure wrote in ktcpsocket.cpp:729
> or just iterate over `ciphers`, which is already const

Yep.

> dfaure wrote in scheduler.cpp:214
> qAsConst not needed, this method is const

(... and m_runningJobs is a member var).

> dfaure wrote in scheduler.cpp:377
> Did you try enabling this to make sure your ported code compiles?

Yes, I did. (I, like everyone else, hate to be embarrassed, so I always make 
sure it builds and passes unittests whenever I change anything except maybe 
comments :)).

> dfaure wrote in dropjob.cpp:270
> qAsConst

m_urls is declared const in DropJobPrivate: 
https://cgit.kde.org/kio.git/tree/src/widgets/dropjob.cpp#n142

> dfaure wrote in kfileitemdelegate.cpp:233
> not needed, method is const and informationList is a member

"member" is what made that concept finally click in my head; (I kept thinking 
calling begin() on a qt container won't call the const overload, but it will if 
the container is a member and the this pointer is a pointer to const).

REPOSITORY
  R241 KIO

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

To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24372: Compile without deprecated foreach

2019-10-08 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 67510.
ahmadsamir marked 7 inline comments as done.
ahmadsamir added a comment.


  - User more descriptive var names other than list2
  - qAsConst isn't needed if the method is const and the container is a member 
var

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24372?vs=67217=67510

BRANCH
  ahmad/foreach-urifilters2 (branched from master)

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

AFFECTED FILES
  src/core/batchrenamejob.cpp
  src/core/connection.cpp
  src/core/copyjob.cpp
  src/core/desktopexecparser.cpp
  src/core/job.cpp
  src/core/kprotocolinfofactory.cpp
  src/core/kprotocolmanager.cpp
  src/core/ksslcertificatemanager.cpp
  src/core/ktcpsocket.cpp
  src/core/scheduler.cpp
  src/core/tcpslavebase.cpp
  src/urifilters/ikws/ikwsopts.cpp
  src/urifilters/ikws/searchproviderdlg.cpp
  src/urifilters/shorturi/kshorturifilter.cpp
  src/widgets/accessmanager.cpp
  src/widgets/clipboardupdater.cpp
  src/widgets/delegateanimationhandler.cpp
  src/widgets/dropjob.cpp
  src/widgets/kdesktopfileactions.cpp
  src/widgets/kdirmodel.cpp
  src/widgets/kfileitemactions.cpp
  src/widgets/kfileitemdelegate.cpp
  src/widgets/kpropertiesdialog.cpp
  src/widgets/krun_win.cpp
  src/widgets/ksslcertificatebox.cpp
  src/widgets/ksslinfodialog.cpp
  src/widgets/kurifiltersearchprovideractions.cpp
  src/widgets/previewjob.cpp
  src/widgets/sslui.cpp

To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24372: Compile without deprecated foreach

2019-10-07 Thread David Faure
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> job.cpp:181
>  // kill all subjobs, without triggering their result slot
> -Q_FOREACH (KJob *it, subjobs()) {
> -it->kill(KJob::Quietly);
> +for (KJob *job : subjobs()) {
> +job->kill(KJob::Quietly);

You need a local const var to hold the result of the subjobs() method call.

(repeats)

> ksslcertificatemanager.cpp:380
>  
> -foreach (const QSslCertificate , 
> QSslCertificate::fromPath(userCertDir + QLatin1Char('*'),
> - QSsl::Pem, QRegExp::Wildcard)) {
> +const QList list2 = 
> QSslCertificate::fromPath(userCertDir + QLatin1Char('*'), QSsl::Pem,
> +   
> QRegExp::Wildcard);

`userList` would be a better variable name

> ktcpsocket.cpp:729
>  cl.reserve(d->ciphers.size());
> -foreach (const KSslCipher , d->ciphers) {
> +for (const KSslCipher  : qAsConst(d->ciphers)) {
>  cl.append(d->ccc.converted(c));

or just iterate over `ciphers`, which is already const

> scheduler.cpp:214
>  ret.reserve(m_runningJobs.size());
> -Q_FOREACH (SimpleJob *job, m_runningJobs) {
> +for (SimpleJob *job : qAsConst(m_runningJobs)) {
>  Slave *slave = jobSlave(job);

qAsConst not needed, this method is const

> scheduler.cpp:299
>  PerSlaveQueue  = it.value();
> -Q_FOREACH (SimpleJob *job, jobs.waitingList) {
> +for (SimpleJob *job : qAsConst(jobs.waitingList)) {
>  // ### for compatibility with the old scheduler we don't touch the 
> running job, if any.

I'm afraid that kill() ends up removing the job from waitingList. I would 
iterate over a copy to be safe.

> scheduler.cpp:377
>  Q_UNUSED(runningJobsCount);
>  #ifdef SCHEDULER_DEBUG
>  int realRunningJobsCount = 0;

Did you try enabling this to make sure your ported code compiles?

> dropjob.cpp:270
>  bool containsTrashRoot = false;
> -foreach (const QUrl , m_urls) {
> +for (const QUrl  : m_urls) {
>  const bool local = url.isLocalFile();

qAsConst

> dropjob.cpp:329
>  fileItems.reserve(m_urls.size());
> -foreach (const QUrl , m_urls) {
> +for (const QUrl  : m_urls) {
>  fileItems.append(KFileItem(url));

qAsConst

> dropjob.cpp:361
>  bool equalDestination = true;
> -foreach (const QUrl , m_urls) {
> +for (const QUrl  : m_urls) {
>  if (!m_destUrl.matches(src.adjusted(QUrl::RemoveFilename), 
> QUrl::StripTrailingSlash)) {

qAsConst

> kdirmodel.cpp:151
>  urls.reserve(urls.size() + m_childNodes.size());
> -Q_FOREACH (KDirModelNode *node, m_childNodes) {
> +for (KDirModelNode *node : qAsConst(m_childNodes)) {
>  const KFileItem  = node->item();

qAsConst not needed, method is const

> kfileitemdelegate.cpp:233
>  
> -foreach (KFileItemDelegate::Information info, informationList) {
> +for (KFileItemDelegate::Information info : qAsConst(informationList)) {
>  if (info == KFileItemDelegate::NoInformation) {

not needed, method is const and informationList is a member

REPOSITORY
  R241 KIO

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

To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns


D24372: Compile without deprecated foreach

2019-10-02 Thread Ahmad Samir
ahmadsamir created this revision.
ahmadsamir added a reviewer: dfaure.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  src/core/*
  src/urifilters/*
  src/widgets/*

TEST PLAN
  make && ctest

REPOSITORY
  R241 KIO

BRANCH
  ahmad/foreach-urifilters2 (branched from master)

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

AFFECTED FILES
  src/core/batchrenamejob.cpp
  src/core/connection.cpp
  src/core/copyjob.cpp
  src/core/desktopexecparser.cpp
  src/core/job.cpp
  src/core/kprotocolinfofactory.cpp
  src/core/kprotocolmanager.cpp
  src/core/ksslcertificatemanager.cpp
  src/core/ktcpsocket.cpp
  src/core/scheduler.cpp
  src/core/tcpslavebase.cpp
  src/urifilters/ikws/ikwsopts.cpp
  src/urifilters/ikws/searchproviderdlg.cpp
  src/urifilters/shorturi/kshorturifilter.cpp
  src/widgets/accessmanager.cpp
  src/widgets/clipboardupdater.cpp
  src/widgets/delegateanimationhandler.cpp
  src/widgets/dropjob.cpp
  src/widgets/kdesktopfileactions.cpp
  src/widgets/kdirmodel.cpp
  src/widgets/kfileitemactions.cpp
  src/widgets/kfileitemdelegate.cpp
  src/widgets/kpropertiesdialog.cpp
  src/widgets/krun_win.cpp
  src/widgets/ksslcertificatebox.cpp
  src/widgets/ksslinfodialog.cpp
  src/widgets/kurifiltersearchprovideractions.cpp
  src/widgets/previewjob.cpp
  src/widgets/sslui.cpp

To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns