Review Request 115157: Make better use of KWindowSystem in KPassivePopup

Alex Merry kde at randomguy3.me.uk
Tue Jan 21 11:13:21 UTC 2014



> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140121/b16ff133/attachment.html>


More information about the Kde-frameworks-devel mailing list