Review Request 109181: Implement filter to handle notifications

Lasath Fernando kde at lasath.org
Tue Feb 26 17:26:00 UTC 2013



> On Feb. 26, 2013, 5:21 p.m., David Edmundson wrote:
> > lib/chat-widget.cpp, line 171
> > <http://git.reviewboard.kde.org/r/109181/diff/1/?file=115998#file115998line171>
> >
> >     If you have 5 tabs you have 5 notificationFilters. They won't be pointing to the correct chatroom.
> >     
> >     /could/ be worked around by checking if the channels match.. but it's a bit hacky.

Ah, I like that idea. 

I(someone else)'m going to have to refactor KTp::MessagesProcessor anyway so that each chat widget can have separate filters if we're going to make ChatWidget any cleaner.

For the sake of not letting this review go stale, we could merge with that hack and remove it later once the refactor is complete.


- Lasath


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


On Feb. 26, 2013, 5:14 p.m., Lasath Fernando wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109181/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2013, 5:14 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> While I'm quite content with the state of this patch, I'm not satisfied with the corresponding patch to ktp-common-internals (which I'll upload shortly).
> 
> There are 2 great advantages to having this filter than ChatWidget::notifyAboutIncomingMessage():
> 
> 1. It makes ChatWidget smaller and makes me that much happier. (Seriously, that class is way too HUUUUUGE and has waaaayyy too many responsibilities).
> 
> 2. Since it works off a KTp::Message rather than a Tp::RecievedMessage, it sends the correct notification based on whether the "highlight" property is set.
> 
> 
> This addresses bug 315790.
>     http://bugs.kde.org/show_bug.cgi?id=315790
> 
> 
> Diffs
> -----
> 
>   lib/notify-filter.h PRE-CREATION 
>   lib/notify-filter.cpp PRE-CREATION 
>   lib/chat-widget.cpp e7adc5a 
>   lib/CMakeLists.txt 5c4741d 
>   lib/chat-widget.h 2c85f3b 
> 
> Diff: http://git.reviewboard.kde.org/r/109181/diff/
> 
> 
> Testing
> -------
> 
> Sent messages to myself, and got the correct notifications.
> 
> 
> Thanks,
> 
> Lasath Fernando
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20130226/1e01b451/attachment-0001.html>


More information about the KDE-Telepathy mailing list