D24262: Modernize code: use range-based loops & algorithms in more places

2019-09-28 Thread Ahmad Samir
ahmadsamir added a comment. @kossebau: did you try the C++ Standard (working draft): https://isocpp.org/blog/2013/10/n3797-working-draft-standard-for-programming-language-c-stefanus-du-toit This ^ one is circa 2012. All the sources used to generate the C++ Standard drafts are

D24262: Modernize code: use range-based loops & algorithms in more places

2019-09-28 Thread Friedrich W. H. Kossebau
kossebau added a comment. Thanks for your teaching, appreciated :) Will have another look again when not tired. Just tried again to read on cppreference.com the stuff about lambda capturing, but still not digested what I read this afternoon, reread now did not help. So next try scheduled.

D24160: [KIO] Modernize the code to use range-for in more places

2019-09-28 Thread Friedrich W. H. Kossebau
kossebau accepted this revision. kossebau added a comment. This revision is now accepted and ready to land. Code changes look all fine to me. Only looked at code here with eyes (tired and fermentation products influenced ;) ), not built, run or tested though. REPOSITORY R241 KIO BRANCH

KDE CI: Frameworks » kwidgetsaddons » kf5-qt5 WindowsMSVCQt5.13 - Build # 18 - Still unstable!

2019-09-28 Thread CI System
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks/job/kwidgetsaddons/job/kf5-qt5%20WindowsMSVCQt5.13/18/ Project: kf5-qt5 WindowsMSVCQt5.13 Date of build: Sat, 28 Sep 2019 20:24:00 + Build duration: 11 min and counting JUnit Tests

KDE CI: Frameworks » kio » kf5-qt5 WindowsMSVCQt5.13 - Build # 84 - Fixed!

2019-09-28 Thread CI System
BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20WindowsMSVCQt5.13/84/ Project: kf5-qt5 WindowsMSVCQt5.13 Date of build: Sat, 28 Sep 2019 20:24:01 + Build duration: 5 min 26 sec and counting

D24171: [src/filewidgets/*] replace deprecated foreach with range for

2019-09-28 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 67008. ahmadsamir added a comment. Don't iterate over the temp. container returned by qhash.keys(), rather over the qhash itself REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24171?vs=67007=67008 BRANCH

D24171: [src/filewidgets/*] replace deprecated foreach with range for

2019-09-28 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > kfilepreviewgenerator.cpp:733 > // to use their default MIME type icon. > -foreach (const QUrl , m_cutItemsCache.keys()) { > -const QModelIndex index = dirModel->indexForUrl(url); > +auto it = m_cutItemsCache.keys().cbegin(); >

D24171: [src/filewidgets/*] replace deprecated foreach with range for

2019-09-28 Thread Ahmad Samir
ahmadsamir updated this revision to Diff 67007. ahmadsamir added a comment. Change according to comments REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24171?vs=66899=67007 BRANCH ahmad/foreach-filewidgets (branched from master) REVISION DETAIL

D24171: [src/filewidgets/*] replace deprecated foreach with range for

2019-09-28 Thread Ahmad Samir
ahmadsamir marked an inline comment as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D24171 To: ahmadsamir, kde-frameworks-devel, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24262: Modernize code: use range-based loops & algorithms in more places

2019-09-28 Thread Dominik Haumann
dhaumann added a comment. In D24262#539317 , @dfaure wrote: > It's actually quite clear in my head, because I imagine the generated class. A captured variable in a lambda becomes a member variable. If it's a capture by value (which is what

D24262: Modernize code: use range-based loops & algorithms in more places

2019-09-28 Thread Dominik Haumann
dhaumann added inline comments. INLINE COMMENTS > kossebau wrote in plugin.cpp:196 > Confirmed by experiments. Still not yet found a document where explicitly it > is mentioned that the copy constructor will be invoked to generate a copy of > the object for any captured variables only being of

D24262: Modernize code: use range-based loops & algorithms in more places

2019-09-28 Thread David Faure
dfaure added a comment. It's actually quite clear in my head, because I imagine the generated class. A captured variable in a lambda becomes a member variable. If it's a capture by value (which is what happens with [library]), it's a "plain value" member. So: bool

D24287: cleanup dbus related objects early enough to avoid hang on program exit

2019-09-28 Thread Christoph Cullmann
This revision was automatically updated to reflect the committed changes. Closed by commit R288:301b33daff31: cleanup dbus related objects early enough to avoid hang on program exit (authored by cullmann). REPOSITORY R288 KJobWidgets CHANGES SINCE LAST UPDATE

D24287: cleanup dbus related objects early enough to avoid hang on program exit

2019-09-28 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R288 KJobWidgets BRANCH master REVISION DETAIL https://phabricator.kde.org/D24287 To: cullmann, dfaure, kfunk Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24171: [src/filewidgets/*] replace deprecated foreach with range for

2019-09-28 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > ahmadsamir wrote in kurlnavigator.cpp:803 > I thought button->deleteLater() may change m_navButtons, doesn't it? How would it? It doesn't know that container. All it does is posting an event to that object. The container isn't modified. [and

D24171: [src/filewidgets/*] replace deprecated foreach with range for

2019-09-28 Thread Ahmad Samir
ahmadsamir marked 2 inline comments as done. ahmadsamir added inline comments. INLINE COMMENTS > dfaure wrote in kurlnavigator.cpp:803 > Why not a range for here? It should be just fine. I thought button->deleteLater() may change m_navButtons, doesn't it? REPOSITORY R241 KIO REVISION DETAIL

D24287: cleanup dbus related objects early enough to avoid hang on program exit

2019-09-28 Thread Christoph Cullmann
cullmann added a comment. This should be a replacement for D2545 . REPOSITORY R288 KJobWidgets REVISION DETAIL https://phabricator.kde.org/D24287 To: cullmann, dfaure, kfunk Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24287: cleanup dbus related objects early enough to avoid hang on program exit

2019-09-28 Thread Christoph Cullmann
cullmann created this revision. cullmann added reviewers: dfaure, kfunk. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. cullmann requested review of this revision. REVISION SUMMARY workaround for issue seen on Windows that at the moment requires Qt patch

D24210: KadeModeMenuList: fix memory leaks and others

2019-09-28 Thread Christoph Cullmann
cullmann requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D24210 To: nibags, #ktexteditor, dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh,

D24210: KadeModeMenuList: fix memory leaks and others

2019-09-28 Thread Christoph Cullmann
cullmann added a comment. I would prefer just some QString m_searchName, strings are shared anyways, no need to try to save there a few bytes but then introduce more heap allocated things. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D24210 To: nibags,

D23789: RFC: Add ECMGenerateExportHeaders, for improved handling of deprecated API

2019-09-28 Thread Christian Ehrlicher
chehrlic added a comment. In D23789#538985 , @kossebau wrote: > >> - why has all Qt code not yet been adapted to QT_DEPRECATED_VERSION/QT_DEPRECATED_VERSION_X, are there places where those macros should not be used, but the version-less ones?

KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.13 - Build # 107 - Still Unstable!

2019-09-28 Thread CI System
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/107/ Project: kf5-qt5 FreeBSDQt5.13 Date of build: Sat, 28 Sep 2019 15:32:18 + Build duration: 8 min 36 sec and counting JUnit Tests Name:

KDE CI: Frameworks » kio » kf5-qt5 WindowsMSVCQt5.13 - Build # 83 - Failure!

2019-09-28 Thread CI System
BUILD FAILURE Build URL https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20WindowsMSVCQt5.13/83/ Project: kf5-qt5 WindowsMSVCQt5.13 Date of build: Sat, 28 Sep 2019 15:32:18 + Build duration: 3 min 36 sec and counting CONSOLE OUTPUT

D24262: Modernize code: use range-based loops & algorithms in more places

2019-09-28 Thread Friedrich W. H. Kossebau
kossebau added inline comments. INLINE COMMENTS > kossebau wrote in plugin.cpp:196 > Meh, I need to check quite some code of mine then, I got that wrong and > thought that the values of the actual variables listed are captured (i.e. for > a reference type the reference "pointer"), and not that

D24262: Modernize code: use range-based loops & algorithms in more places

2019-09-28 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 66996. kossebau added a comment. properly mark captured variable to be captured by reference as intended REPOSITORY R306 KParts CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24262?vs=66950=66996 BRANCH morerangebased REVISION DETAIL

D24262: Modernize code: use range-based loops & algorithms in more places

2019-09-28 Thread Friedrich W. H. Kossebau
kossebau added inline comments. INLINE COMMENTS > dfaure wrote in plugin.cpp:196 > STL is fine by definition, it's the C++ standard. > > But yes, no need for cbegin/cend on a const container. > > Catching `library` by reference makes sense, just like you wouldn't pass it > by value to a

D24261: Modernize code: use range-based for loop in more places

2019-09-28 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > kossebau wrote in kgesture.cpp:131 > Why the sizeof==8 rule? > > Good catch about the 1, slipped me, will drop change here. sizeof(QPoint)==8 indeed, i.e. passing a QPoint by value is like passing two ints by value, which is fine. > kossebau

D24262: Modernize code: use range-based loops & algorithms in more places

2019-09-28 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > kossebau wrote in plugin.cpp:196 > Do we have rules in KF whether stl-like names liks cbegin() & cend() are fine? > Then plugins is const, so begin()/end() are giving use the const iterators > already. > > Does it make sense to catch the

D24160: [KIO] Modernize the code to use range-for in more places

2019-09-28 Thread David Faure
dfaure updated this revision to Diff 66991. dfaure marked 4 inline comments as done. dfaure added a comment. Make changes suggested by Friedrich's code review (thanks!) REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24160?vs=5=66991 BRANCH

KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.13 - Build # 106 - Still Unstable!

2019-09-28 Thread CI System
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/106/ Project: kf5-qt5 FreeBSDQt5.13 Date of build: Sat, 28 Sep 2019 12:57:43 + Build duration: 6 min 22 sec and counting JUnit Tests Name:

D24160: [KIO] Modernize the code to use range-for in more places

2019-09-28 Thread David Faure
dfaure marked 18 inline comments as done. dfaure added inline comments. INLINE COMMENTS > kossebau wrote in kmountpoint.cpp:344 > Why change away from auto? Consistency with all other range for loops in kio, and https://wiki.qt.io/Coding_Conventions#auto_Keyword > kossebau wrote in

KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.13 - Build # 105 - Still Unstable!

2019-09-28 Thread CI System
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/105/ Project: kf5-qt5 FreeBSDQt5.13 Date of build: Sat, 28 Sep 2019 12:23:00 + Build duration: 8 min 44 sec and counting JUnit Tests Name:

KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.13 - Build # 104 - Still Unstable!

2019-09-28 Thread CI System
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/104/ Project: kf5-qt5 FreeBSDQt5.13 Date of build: Sat, 28 Sep 2019 12:14:19 + Build duration: 8 min 36 sec and counting JUnit Tests Name:

KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.13 - Build # 103 - Still Unstable!

2019-09-28 Thread CI System
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/103/ Project: kf5-qt5 FreeBSDQt5.13 Date of build: Sat, 28 Sep 2019 12:05:31 + Build duration: 8 min 44 sec and counting JUnit Tests Name:

KDE CI: Frameworks » kio » kf5-qt5 SUSEQt5.12 - Build # 233 - Fixed!

2019-09-28 Thread CI System
BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.12/233/ Project: kf5-qt5 SUSEQt5.12 Date of build: Sat, 28 Sep 2019 11:56:12 + Build duration: 13 min and counting BUILD ARTIFACTS

KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.13 - Build # 102 - Still Unstable!

2019-09-28 Thread CI System
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/102/ Project: kf5-qt5 FreeBSDQt5.13 Date of build: Sat, 28 Sep 2019 11:56:13 + Build duration: 9 min 14 sec and counting JUnit Tests Name:

KDE CI: Frameworks » kwidgetsaddons » kf5-qt5 WindowsMSVCQt5.13 - Build # 17 - Failure!

2019-09-28 Thread CI System
BUILD FAILURE Build URL https://build.kde.org/job/Frameworks/job/kwidgetsaddons/job/kf5-qt5%20WindowsMSVCQt5.13/17/ Project: kf5-qt5 WindowsMSVCQt5.13 Date of build: Sat, 28 Sep 2019 11:12:13 + Build duration: 52 min and counting CONSOLE OUTPUT

D24171: [src/filewidgets/*] replace deprecated foreach with range for

2019-09-28 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kfilepreviewgenerator.cpp:733 > // to use their default MIME type icon. > -foreach (const QUrl , m_cutItemsCache.keys()) { > +const QList list =

D24254: [KCollapsibleGroupBox] Fix QTimeLine::start warning at runtime

2019-09-28 Thread David Faure
dfaure closed this revision. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D24254 To: dfaure, cfeck, ngraham, elvisangelaccio Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24262: Modernize code: use range-based loops & algorithms in more places

2019-09-28 Thread Friedrich W. H. Kossebau
kossebau added inline comments. INLINE COMMENTS > dhaumann wrote in plugin.cpp:196 > cbegin() + cend() > Catch library by reference? [& library] Do we have rules in KF whether stl-like names liks cbegin() & cend() are fine? Then plugins is const, so begin()/end() are giving use the const

D24261: Modernize code: use range-based for loop in more places

2019-09-28 Thread Friedrich W. H. Kossebau
kossebau updated this revision to Diff 66989. kossebau added a comment. update to @dhaumann's first review: - drop wrong change for loop starting at 1 - use plain array for shortcutTitleToColumnMap REPOSITORY R263 KXmlGui CHANGES SINCE LAST UPDATE

D24261: Modernize code: use range-based for loop in more places

2019-09-28 Thread Friedrich W. H. Kossebau
kossebau added a comment. Thanks for review work, @dhaumann :) Seems we have some differences though, so let's try to align knowledge: When it comes to by-value or by reference of the loop range element value, I learned that similar rules as for arguments are valid: small size in bytes

D24245: Add support for passing Unix file descriptors

2019-09-28 Thread Alexander Volkov
volkov added inline comments. INLINE COMMENTS > aacid wrote in kauthunixfiledescriptor.h:33 > Does this class really need to be exported and its header installed? > > Seems like this is implementation detail? Who would use this header? It's needed to mark file descriptors in QVariantMap. Usage

D23388: add a border for the toolbars WIP

2019-09-28 Thread Björn Feber
GB_2 added a comment. Ping. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D23388 To: camiloh, #plasma, mart, ngraham, apol Cc: GB_2, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D24245: Add support for passing Unix file descriptors

2019-09-28 Thread Albert Astals Cid
aacid added inline comments. INLINE COMMENTS > kauthunixfiledescriptor.h:33 > + > +class KAUTH_EXPORT UnixFileDescriptor > +{ Does this class really need to be exported and its header installed? Seems like this is implementation detail? Who would use this header? REPOSITORY R283 KAuth

D24262: Modernize code: use range-based loops & algorithms in more places

2019-09-28 Thread Dominik Haumann
dhaumann added inline comments. INLINE COMMENTS > plugin.cpp:196 > > -QObjectList::ConstIterator it = plugins.begin(); > -for (; it != plugins.end(); ++it) { > -Plugin *plugin = qobject_cast(*it); > -if (plugin && plugin->d->m_library == library) { > -return

D24261: Modernize code: use range-based for loop in more places

2019-09-28 Thread Dominik Haumann
dhaumann added inline comments. INLINE COMMENTS > kedittoolbar.cpp:1566 > { > -XmlDataList::Iterator xit = m_xmlFiles.begin(); > -for (; xit != m_xmlFiles.end(); ++xit) { > -if ((*xit).type() == XmlData::Merged) { > +for (auto& xmlFile : m_xmlFiles) { > +if