Emoticonslib moved to kdereview

Olivier Goffart ogoffart at kde.org
Sun May 18 18:24:02 BST 2008


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.
KEmoticonProvider::Emoticon must not be part of the public API
A beter api wold be to have just KEmoticonProvider::addEmoticon


* 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

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

* My concens about html are still there.  parseEmoticons is ONLY for HTML, and 
the SkipHTML MUST to be set (by the parseEmoticons function)  

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

* in KEmoticonTheme::addEmoticon, the param text could maybe be a QStringList

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080518/05c3b504/attachment.sig>


More information about the kde-core-devel mailing list