From kde-pim Tue May 13 08:27:43 2008 From: Carlo Date: Tue, 13 May 2008 08:27:43 +0000 To: kde-pim Subject: Re: [Kde-pim] patch to use the emoticonslib in linklocator Message-Id: <3262b6180805130127g49cf3606rb279a9e4494725c8 () mail ! gmail ! com> X-MARC-Message: https://marc.info/?l=kde-pim&m=121066735318094 On Tue, May 13, 2008 at 9:42 AM, Tom Albers wrote: > Op dinsdag 13 mei 2008 01:31 schreef u: > > > > Hi, > > I've made a patch to use the emoticons lib inside linklocator, the > > emoticons library is currently in kdereview and it will be moved to > > kdelibs before the hard feature freeze > > > > Carlo > > Hi Carlo, > > Thanks for the patch. It looks good to me, except for a two concerns. > > 1. > You moved the "if ( flags & ReplaceSmileys ) {" 20 lines down, why is that (did not pull up the actual code). > > 2. > In the pimemoticons.kcfg there are a lot of commented out emoticons. Did you verify that none of them becomes active again with the new library? > > 3. > Don't forget to remove -lkemoticons if needed. > > 4. > Please check coding style, I at least spotted a lack of space after a ( > > Overall: nice work and thank you. Can you commit it after the move to kdelibs? If that is before beta 1, please make sure this patch goes in as well before beta 1, so we can have some testing. > > Best. > > Toma > _______________________________________________ > KDE PIM mailing list kde-pim@kde.org > https://mail.kde.org/mailman/listinfo/kde-pim > KDE PIM home page at http://pim.kde.org/ > 1)because KEmoticonsTheme::parseEmoticons parses the whole text, so I have to move it out of the parse loop, the other way could be to just copy the emoticonsMap from the theme into the one in linklocator and leave the rest as it is 2)a theme may or may not have those emoticons, but yeah I've forgot to exclude them too :P 3)yes that's just temporary 4)ok I'll look at that yes I'll commit this after moving the lib into kdelibs on May 18th Carlo _______________________________________________ KDE PIM mailing list kde-pim@kde.org https://mail.kde.org/mailman/listinfo/kde-pim KDE PIM home page at http://pim.kde.org/