----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103848/#review10284 -----------------------------------------------------------
Very nice. Incredibly awesomely designed :P. I need someone else to review as I helped write some of this. lib/filters.h <http://git.reviewboard.kde.org/r/103848/#comment8462> All of these leak. I would propose making AbstractMessageFilter inherit from QObject and pass a parent. You'll want that for when you go async. lib/filters.h <http://git.reviewboard.kde.org/r/103848/#comment8461> Defined, but never declared. lib/filters.h <http://git.reviewboard.kde.org/r/103848/#comment8464> This hasn't been fully removed from AdiumThemeView lib/image-filter.cpp <http://git.reviewboard.kde.org/r/103848/#comment8458> Change width: to max-width: (so you don't stretch small images) alt text needs going through i18n. lib/image-filter.cpp <http://git.reviewboard.kde.org/r/103848/#comment8459> break? What if there's two images? lib/message-processor.h <http://git.reviewboard.kde.org/r/103848/#comment8460> Passing non-const references is a bit wrong. I'm considering making Message explicitly shared. I.e it has a QExplicitlySharedDataPointer to it's private members, then we can pass it around properly. That or we pass a Message*. Can't decide which. lib/message.h <http://git.reviewboard.kde.org/r/103848/#comment8466> KTP_ needs removing lib/message.h <http://git.reviewboard.kde.org/r/103848/#comment8467> This should probably be inside Message::Private, which I'm starting to think should be a QSharedData. lib/message.h <http://git.reviewboard.kde.org/r/103848/#comment8465> we probably need an accessor for this, either directly or for the main properties. lib/message.cpp <http://git.reviewboard.kde.org/r/103848/#comment8463> What was the motivation behind changing this from QString to const char*. (not saying it's wrong, I just want the reasoning) - David Edmundson 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
