Emoticonslib moved to kdereview

[Carlo]

On Sun, May 18, 2008 at 7:24 PM, Olivier Goffart <ogoffart at kde.org> wrote:
> Le dimanche 18 mai 2008, Carlo a écrit :
>> Ok so I've fixed some of the issue reported by Olivier Goffart and I'm
>> gonna move it  to kdelibs/kemoticons today, is it ok?
>>
>> Carlo
>
> More comments:
>
> * KEmoticonProvider still expose way to much internal stuff   (it returns
> pointer to QHash: bad)  making it impossible to eventualy change the matching
> algoritms later.
*emoticonsMap() is protected so I can modify the qhash inside each plugins
> KEmoticonProvider::Emoticon must not be part of the public API
> A beter api wold be to have just KEmoticonProvider::addEmoticon
I can make it private and make KEmoticonsTheme friend of KEmoticonsProvider
>
> * the KEmoticonTheme::Token class has a bad API.
>    Why do we need the html in there while it is used specialy when we don't
> use the html?
>    picPath should probably be named emoticonPath
the html code is copied from the Emoticon struct, so if I remove the
html code from the token I should either have a pointer to this struct
inside the token or generate the html when parseEmoticons is called
but this isn't really good
> * I still think the KEmoticons class has no interest (method should be moved
> in KEmoticonTheme as static,  or in a namespace.   It provides no interest
> (see the need of Kopete::Emoticons in your patch to Kopete)
I don't know how can I change this in a good way
> * My concens about html are still there.  parseEmoticons is ONLY for HTML, and
> the SkipHTML MUST to be set (by the parseEmoticons function)
So I just have to force SkipHtml inside parseEmoticons?
> * I don't like the exclude argument to the parseEmoticons function.  If you
> don't want some emoticons, just don't add them to the theme!!
but since users are dumb they will complain about things like (c) that
renders as a coffe cup in kmail but they want it with kopete since
it's used by msn and maybe other protocols
and anyway since it's an optional argument if you don't like it just
don't use it :P it shouldn't be a performance problem to lookup an
empty qlist
> * in KEmoticonTheme::addEmoticon, the param text could maybe be a QStringList
I can add an overloaded addEmoticon that takes a QStringList as parameter