Re: Review Request 115157: Make better use of KWindowSystem in KPassivePopup

2014-01-21 Thread Alex Merry 

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

Re: Review Request 115157: Make better use of KWindowSystem in KPassivePopup

2014-01-21 Thread Commit Hook 

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

2014-01-21 Thread Martin Gräßlin 

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

2014-01-21 Thread Alex Merry 


> On Jan. 21, 2014, 6:42 a.m., Martin Gräßlin wrote:
> > src/kpassivepopup.cpp, lines 466-467
> > 
> >
> > 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

2014-01-20 Thread Martin Gräßlin 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115157/#review47854
---


Thanks for pointing out issues in KWindowSystem. I do hope that Windows and Mac 
developers will start fixing that


src/kpassivepopup.cpp


small suggestion:
if (QWidget *widget = QWidget::find(d->window)) {
...
}


- 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

Review Request 115157: Make better use of KWindowSystem in KPassivePopup

2014-01-20 Thread Alex Merry 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115157/
---

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