Review Request: Implement simple Growl support in KNotify

[Sjors Gielen]

> On 2010-08-10 17:04:41, Olivier Goffart wrote:
> > This is a bit hackish (the hardcoded paths, and the QSystemTrayIcon stuff)  
> > But if it works, i'm ok with it.
> 
> Markus Slopianka wrote:
>     Amateur question: Wouldn't is make more sense if a "wrapper service" was implemented that calls Growl instead of calling Growl directly from KNotify? That way hackish code wouldn't hurt.
> 
> Sjors Gielen wrote:
>     This has been one of the options for me before I started on the patch, a DBus notification daemon that sends everything to Growl; however I rejected this option because it would be hard to ensure the daemon is installed correctly and running constantly. It would also be just another daemon for notifications to show correctly (would add up to 4). The daemon would have to run on all platforms with Growl installed, and from a normal KDE installation, without the packager having to worry about it and all.
>     
>     All in all, I thought implementing GNTP / Growl in KNotify itself was more reliable and easier. I discussed this with Olivier before starting on the patch. Thanks for your comment, though ;-)
>     
>     BTW: copying the response by e-mail from Olivier Goffart to the above comment here: "KNotify is already a wrapper service.  We do not need more :-)"

Committed to the KDE 4.6 branch as r1164056, with the fixed path for Windows and some fixed description the Growl team wasn't happy about (the Mac implementation of their protocol is not just "unstabilized", it's "non-existant")

Olivier, can I backport this to the KDE 4.5 branch for the 4.5.1 release or do you consider it 4.6-only?


- Sjors


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4962/#review6973
-----------------------------------------------------------


On 2010-08-10 15:05:45, Sjors Gielen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4962/
> -----------------------------------------------------------
> 
> (Updated 2010-08-10 15:05:45)
> 
> 
> Review request for kdelibs and Olivier Goffart.
> 
> 
> Summary
> -------
> 
> This patch implements, as promised, Growl support in KNotify. Support is added in a separate class with static methods, called sometimes in NotifyByPopup. It currently uses Growl via QApplication::message(), because the GNTP protocol using which icon sending and callbacks are possible isn't stabilized in the Mac versions yet. This patch makes it possible to display the title and the message, in stripped form as with the DBus daemons after my last patch, in a Growl notification, with a (quite random) Qt information icon.
> 
> Before calling QApplication::message(), it checks if Growl is installed using QFile::exists(). There is still a TODO in the source regarding the Windows path to Growl, I will fill that in soon, but it's trivial. With this patch applied, I expect no changes on any machine where Growl is not installed, since then the older mechanism of KPassivePopup is used. (DBus notification daemons are still preferred over Growl.)
> 
> Once the newest version of Growl has stable GNTP, I will implement it in NotifyByPopupGrowl and send in another patch.
> 
> 
> This addresses bug 212751.
>     https://bugs.kde.org/show_bug.cgi?id=212751
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/runtime/knotify/CMakeLists.txt 1155631 
>   /trunk/KDE/kdebase/runtime/knotify/notifybypopup.cpp 1155631 
>   /trunk/KDE/kdebase/runtime/knotify/notifybypopupgrowl.h PRE-CREATION 
>   /trunk/KDE/kdebase/runtime/knotify/notifybypopupgrowl.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/4962/diff
> 
> 
> Testing
> -------
> 
> Tested on Mac OS X. Nowhere else yet.
> 
> 
> Thanks,
> 
> Sjors
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20100815/ea00255e/attachment.htm>