Review Request: MessageProcessor and Filters

[Lasath Fernando]

> On Feb. 2, 2012, 5:06 p.m., Daniele Elmo Domenichelli wrote:
> > lib/image-filter.cpp, line 35
> > <http://git.reviewboard.kde.org/r/103848/diff/1/?file=48496#file48496line35>
> >
> >     Perhaps you should resize the image if it is bigger than a certain size, or it will make the chat unreadable

style='max-width:100%;' does exactly that.


> On Feb. 2, 2012, 5:06 p.m., Daniele Elmo Domenichelli wrote:
> > lib/message-processor.cpp, line 46
> > <http://git.reviewboard.kde.org/r/103848/diff/1/?file=48498#file48498line46>
> >
> >     These are leaked, you should delete them in distructor

parented them now, so it should be all good.


> On Feb. 2, 2012, 5:06 p.m., Daniele Elmo Domenichelli wrote:
> > lib/message.cpp, lines 30-34
> > <http://git.reviewboard.kde.org/r/103848/diff/1/?file=48500#file48500line30>
> >
> >     Perhaps this could be a "cleanFilter"
> 
> David Edmundson wrote:
>     I considered that too, now you've said it too..lets do it.
>     
>     The only twist is we need to (eventually) work out if the original data is raw text or HTML and only do this as appropriate.

what's wrong with escaping raw text? It should be necessary anyway to prevent users from doing HTML injection. 


> On Feb. 2, 2012, 5:06 p.m., Daniele Elmo Domenichelli wrote:
> > lib/message.h, lines 42-46
> > <http://git.reviewboard.kde.org/r/103848/diff/1/?file=48499#file48499line42>
> >
> >     I'm not really sure if I understand why you are using a list with this enum here...
> >     So, you have a message, some filters can make some replacement to the main message and some other filters can append some parts.
> >     Why don't you use a QString for the main message and a QStingList for the appended parts?
> >
> 
> David Edmundson wrote:
>     makes sense. +1 from me.

coz we're _that_ clever :P


> On Feb. 2, 2012, 5:06 p.m., Daniele Elmo Domenichelli wrote:
> > lib/url-filter.cpp, line 2
> > <http://git.reviewboard.kde.org/r/103848/diff/1/?file=48501#file48501line2>
> >
> >     Since a large part this file seems to be taken from adium-theme-view.cpp you should probably add the copyright for the original file
> 
> David Edmundson wrote:
>     Yeah, credit me bitch.

fine then, be that way!
http://www.childperspective.com/wp-content/uploads/2009/05/photo_1565_200605151.jpg


> On Feb. 2, 2012, 5:06 p.m., Daniele Elmo Domenichelli wrote:
> > lib/url-filter.cpp, line 29
> > <http://git.reviewboard.kde.org/r/103848/diff/1/?file=48501#file48501line29>
> >
> >     If you use QList<KUrl> instead of this it will be easier later, for example when you search for the extention you should be sure to remove query items and fragments

Just did so. I don't think it made the code any cleaner though...


- Lasath


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


On Feb. 2, 2012, 2:42 p.m., Lasath Fernando wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103848/
> -----------------------------------------------------------
> 
> (Updated Feb. 2, 2012, 2:42 p.m.)
> 
> 
> Review request for Telepathy and David Edmundson.
> 
> 
> Description
> -------
> 
> Create an extensible framework for dealing with incomming messages in text-ui. This patch implements a MessageProcessor and 3 MessageFilters: Url and emoticon parsers, as well as an image embedding plugin (for funsizes :).
> Eventually these will be turned into actual plugins that are loaded at runtime rather than being hardcoded. But for now, they just make David's code cleaner.
> 
> 
> Diffs
> -----
> 
>   lib/CMakeLists.txt 31dadd2 
>   lib/adium-theme-message-info.cpp c5e0dfc 
>   lib/adium-theme-view.cpp a0e7ac1 
>   lib/chat-widget.cpp 75acef0 
>   lib/filters.h PRE-CREATION 
>   lib/image-filter.cpp PRE-CREATION 
>   lib/message-processor.h PRE-CREATION 
>   lib/message-processor.cpp PRE-CREATION 
>   lib/message.h PRE-CREATION 
>   lib/message.cpp PRE-CREATION 
>   lib/url-filter.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/103848/diff/diff
> 
> 
> Testing
> -------
> 
> Spammed meme images at david. He seemed to be happy and replied with lots of smiley faces.
> 
> 
> Thanks,
> 
> Lasath Fernando
> 
>

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