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

_______________________________________________
KDE-Telepathy mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-telepathy

Reply via email to