Review Request 109161: Update Highlight filter

[Lasath Fernando]

> On Feb. 25, 2013, 11:57 p.m., David Edmundson wrote:
> > filters/highlight/highlight-filter.cpp, line 56
> > <http://git.reviewboard.kde.org/r/109161/diff/1/?file=115609#file115609line56>
> >
> >     I think it makes more sense to check just the relevant account.
> >     
> >     
> >     1) because I have a never have two account manager's in the same app rule. It's bad.
> >     
> >     2) It makes more sense. You have access to the relevant account in the context parameter.

To be honest, I doubt anyone's going to type "Lasath ???? Fernando" in chat, so it doesn't matter too much to me. I care more that it checks for my login username.


> On Feb. 25, 2013, 11:57 p.m., David Edmundson wrote:
> > filters/highlight/highlight-filter.cpp, line 67
> > <http://git.reviewboard.kde.org/r/109161/diff/1/?file=115609#file115609line67>
> >
> >     The main point of the highlight plugin isn't to put something in red but to emit a different notification. That's why we set the property. Ideally this means porting the text-ui notifyAboutIncomingMessage to take a KTp::Message object
> >     
> >     Putting your name in red doesn't really achieve anything, I know what my name looks like. Adium themes have a special class that should get set. 
> >
> 
> Lasath Fernando wrote:
>     How exactly do I set that class?
>     
>     And as for ChatWidget::notifyAboutIncomingMessage(), if I'm to make it work off a KTp::Message anyway, should I turn that into a Filter? I'm sure we can add a method in MessageProcessor to add this filter that can get called in the constructor of ChatWdiget. Or (if we decide to), we can let it run for the chat plasmoid too, since it doesn't emit any notifications whatsoever.

Actually, I'll do this in another patch (I don't like the idea of reviews being open for too long).

I'll keep track of it with: 
https://bugs.kde.org/show_bug.cgi?id=315790


- Lasath


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


On Feb. 25, 2013, 11:46 p.m., Lasath Fernando wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109161/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2013, 11:46 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> This one was fairly straight forward. Just had to fix some build issues and update the metadata file.
> 
> 
> Diffs
> -----
> 
>   filters/CMakeLists.txt ac25a17 
>   filters/highlight/CMakeLists.txt PRE-CREATION 
>   filters/highlight/highlight-filter.h PRE-CREATION 
>   filters/highlight/highlight-filter.cpp PRE-CREATION 
>   filters/highlight/ktptextui_message_filter_highlight.desktop PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/109161/diff/
> 
> 
> Testing
> -------
> 
> Wrote my name. Felt like in primary school.
> 
> 
> Thanks,
> 
> Lasath Fernando
> 
>

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