Re: Review Request 115157: Make better use of KWindowSystem in KPassivePopup
On Jan. 21, 2014, 6:42 a.m., Martin Gräßlin wrote:
src/kpassivepopup.cpp, lines 466-467
https://git.reviewboard.kde.org/r/115157/diff/1/?file=234862#file234862line466
small suggestion:
if (QWidget *widget = QWidget::find(d-window)) {
...
}
I have a strong dislike of assignments in if conditions...
- Alex
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115157/#review47854
---
On Jan. 20, 2014, 5:58 p.m., Alex Merry wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115157/
---
(Updated Jan. 20, 2014, 5:58 p.m.)
Review request for KDE Frameworks, Martin Gräßlin and Michael Palimaka.
Repository: knotifications
Description
---
This is an alternative approach to https://git.reviewboard.kde.org/r/115147/
where we avoid doing the cross platform support ourselves, and instead depend
on KWindowSystem.
Downsides: it looks like the implementations for KWindowInfo::geometry() and
KWindowInfo::frameGeometry are the wrong way round on windows, and they are
not implemented at all on Mac. However, the code we had here was never
tested either, so...
Make better use of KWindowSystem in KPassivePopup
This avoids having code that is compiled on non-X11 platforms but not on
X11 (the old non-X11 code did not compile).
Diffs
-
CMakeLists.txt 022cbcb7a12aa5ad9843019ffd73f1b3e117fb9b
src/kpassivepopup.cpp b41cb8dc8b3372346bd12c0413bf6bfa9a6fd00d
tests/CMakeLists.txt c7481362008e3cae10d0afcfbcaea5fe953ce62d
tests/kpassivepopuptest.cpp f457aed7e57904bcc00462a947bc5eaae7208792
Diff: https://git.reviewboard.kde.org/r/115157/diff/
Testing
---
Compiles and tests work when X11 is found. Compiles and tests do as well as
expected (ie: popups are placed next to the window instead of the taskbar
entry) when X11 is not found (specifically by commenting out
find_package(X11)).
Thanks,
Alex Merry
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115157: Make better use of KWindowSystem in KPassivePopup
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115157/#review47860 --- Ship it! Ship It! - Martin Gräßlin On Jan. 20, 2014, 6:58 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115157/ --- (Updated Jan. 20, 2014, 6:58 p.m.) Review request for KDE Frameworks, Martin Gräßlin and Michael Palimaka. Repository: knotifications Description --- This is an alternative approach to https://git.reviewboard.kde.org/r/115147/ where we avoid doing the cross platform support ourselves, and instead depend on KWindowSystem. Downsides: it looks like the implementations for KWindowInfo::geometry() and KWindowInfo::frameGeometry are the wrong way round on windows, and they are not implemented at all on Mac. However, the code we had here was never tested either, so... Make better use of KWindowSystem in KPassivePopup This avoids having code that is compiled on non-X11 platforms but not on X11 (the old non-X11 code did not compile). Diffs - CMakeLists.txt 022cbcb7a12aa5ad9843019ffd73f1b3e117fb9b src/kpassivepopup.cpp b41cb8dc8b3372346bd12c0413bf6bfa9a6fd00d tests/CMakeLists.txt c7481362008e3cae10d0afcfbcaea5fe953ce62d tests/kpassivepopuptest.cpp f457aed7e57904bcc00462a947bc5eaae7208792 Diff: https://git.reviewboard.kde.org/r/115157/diff/ Testing --- Compiles and tests work when X11 is found. Compiles and tests do as well as expected (ie: popups are placed next to the window instead of the taskbar entry) when X11 is not found (specifically by commenting out find_package(X11)). Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115157: Make better use of KWindowSystem in KPassivePopup
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115157/#review47861 --- This review has been submitted with commit 096faf3d2bf7786bbc54450f884050021cf582b6 by Alex Merry to branch master. - Commit Hook On Jan. 20, 2014, 5:58 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115157/ --- (Updated Jan. 20, 2014, 5:58 p.m.) Review request for KDE Frameworks, Martin Gräßlin and Michael Palimaka. Repository: knotifications Description --- This is an alternative approach to https://git.reviewboard.kde.org/r/115147/ where we avoid doing the cross platform support ourselves, and instead depend on KWindowSystem. Downsides: it looks like the implementations for KWindowInfo::geometry() and KWindowInfo::frameGeometry are the wrong way round on windows, and they are not implemented at all on Mac. However, the code we had here was never tested either, so... Make better use of KWindowSystem in KPassivePopup This avoids having code that is compiled on non-X11 platforms but not on X11 (the old non-X11 code did not compile). Diffs - CMakeLists.txt 022cbcb7a12aa5ad9843019ffd73f1b3e117fb9b src/kpassivepopup.cpp b41cb8dc8b3372346bd12c0413bf6bfa9a6fd00d tests/CMakeLists.txt c7481362008e3cae10d0afcfbcaea5fe953ce62d tests/kpassivepopuptest.cpp f457aed7e57904bcc00462a947bc5eaae7208792 Diff: https://git.reviewboard.kde.org/r/115157/diff/ Testing --- Compiles and tests work when X11 is found. Compiles and tests do as well as expected (ie: popups are placed next to the window instead of the taskbar entry) when X11 is not found (specifically by commenting out find_package(X11)). Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115157: Make better use of KWindowSystem in KPassivePopup
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115157/ --- (Updated Jan. 21, 2014, 12:24 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, Martin Gräßlin and Michael Palimaka. Repository: knotifications Description --- This is an alternative approach to https://git.reviewboard.kde.org/r/115147/ where we avoid doing the cross platform support ourselves, and instead depend on KWindowSystem. Downsides: it looks like the implementations for KWindowInfo::geometry() and KWindowInfo::frameGeometry are the wrong way round on windows, and they are not implemented at all on Mac. However, the code we had here was never tested either, so... Make better use of KWindowSystem in KPassivePopup This avoids having code that is compiled on non-X11 platforms but not on X11 (the old non-X11 code did not compile). Diffs - CMakeLists.txt 022cbcb7a12aa5ad9843019ffd73f1b3e117fb9b src/kpassivepopup.cpp b41cb8dc8b3372346bd12c0413bf6bfa9a6fd00d tests/CMakeLists.txt c7481362008e3cae10d0afcfbcaea5fe953ce62d tests/kpassivepopuptest.cpp f457aed7e57904bcc00462a947bc5eaae7208792 Diff: https://git.reviewboard.kde.org/r/115157/diff/ Testing --- Compiles and tests work when X11 is found. Compiles and tests do as well as expected (ie: popups are placed next to the window instead of the taskbar entry) when X11 is not found (specifically by commenting out find_package(X11)). Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
