[knotifications] compile without foreach
ClosedPublic

Authored by mlaurent on Mar 5 2019, 8:07 PM.

Details

Summary

compile without foreach

Test Plan

autotest ok

Diff Detail

Repository
R289 KNotifications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mlaurent created this revision.Mar 5 2019, 8:07 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMar 5 2019, 8:07 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
mlaurent requested review of this revision.Mar 5 2019, 8:07 PM
dfaure requested changes to this revision.Mar 5 2019, 9:39 PM
dfaure added inline comments.
src/knotifyconfig.cpp
53–54

horribly slow, should use STL iterators

src/notifybypopup.cpp
422–423

same

This revision now requires changes to proceed.Mar 5 2019, 9:39 PM
mlaurent updated this revision to Diff 54388.Mar 20 2019, 7:13 AM

Use iterator here

mlaurent added inline comments.Mar 20 2019, 7:14 AM
src/knotifyconfig.cpp
53–54

How we can use iterator here ?
I didn't find a method for it.

pino added a subscriber: pino.Mar 20 2019, 7:43 AM
pino added inline comments.
src/notifybypopup.cpp
424

this loop now will run forever...

dfaure accepted this revision.Mar 20 2019, 8:49 AM
dfaure added inline comments.
src/knotifyconfig.cpp
53–54

Urgh, indeed. What kind of container is that....
Never mind then....

src/notifybypopup.cpp
426

This is why a for() loop is better than a while loop() for such usage.

This revision is now accepted and ready to land.Mar 20 2019, 8:49 AM
This revision was automatically updated to reflect the committed changes.
kossebau added a subscriber: kossebau.EditedMar 22 2019, 1:13 PM

This sadly breaks build with at least older versions of Phonon, as they have foreach loop in their public headers.

So this either needs a bump of the current

find_package(Phonon4Qt5 4.6.60 NO_MODULE)

or adding the QT_NO_FOREACH only if a newer working Phonon version is found.

Update: or removing the flag around the includes of Phonon for more hackery :)

This sadly breaks build with at least older versions of Phonon, as they have foreach loop in their public headers.

Ditto, the Mac build fails here now as well due to this.

For the record, Laurent commented out the QT_NO_FOREACH define in commit cdfaff8ce6aa359555d0b611f9141d2012ec9dde, so the build should be fixed again.