On Monday 05 March 2007 05:17, Martijn Klingens wrote: > On Sunday 04 March 2007 19:53, Charles Connell wrote: > > I've got a patch here which reimplements the body of a Kopete::Message as > > a QTextDocument (before it was a QString). > > Just some notes on the performance bits, someone else should comment on the > rest ;) > > > 4. The infamous unescape() function is no longer necessary, although it > > is left in as a convenience function. escape() is no longer used as well. > > What you didn't properly port is the isRightToLeft() method -- you removed > the code that sets d->isRightToLeft. Either remove the function entirely > and change the call to it into the equivalent of > > plainBody().isRightToLeft(); // a QString method > > or readd the functionality. I have added the line: d->isRightToLeft = d->body->toPlainText().isRightToLeft();
> > > 2. QTextDocument cloning. QTextDocument's inherit QObject, so they have > > parents. Since Kopete::Message is not based on QObject, I have added a > > QObject for the QTextDocument to have as a parent. Every QTextDocument* > > received is cloned so that it has the parent we want it to have. I wish I > > could just reparent(), but only a QWidget can do that. > > Why not a null parent? Parent is not mandatory AFAICS. You just need to > manually delete in the destructor if you do that. > I have done just that. I had had a separate parent because I figured that if at a later time something else was to use the same bogus-parent-and-needs-to-be-deleted-in-destructor setup, I could use the parent I had set up to make things a little simpler. However, I seem to have forgotten to delete d->bodyParent anyways (I knew I had to, but I redid my changes a few times and I must have forgotten about it). Anyways, yes, d->body is now deleted in the destructor of Kopete::Message > > Known problems: > > 1. Speed? Unescape was also the bottleneck for performance, and I'm not > > sure if QTextDocument's equivalent functions are any better (or worse) > > I would still like to see a way to avoid unescape entirely. The version of > unescape that is performance-sensitive is used only for calling > isRightToLeft() and it doesn't feel well to do a complete > richtext-to-plaintext conversion just to figure out whether the thing is in > western left-to-right or Hebrew/Arabic RTL. > > There must be a way to figure that out without expensive conversions... I believe that unescape is never called in my code, only toPlainText(). I'm sincerely hoping that that isn't as expensive, because I use it in plainBody() obviously, and I know that that is called in many places throughout Kopete. Anyways, I'm back with a patch for the issues you raised. I would be happy to hear more from anyone about it. This is my first experience with an open source development team, and also with the kopete source code, so I'm bound to mess a few things up. Also, just a note: the attached diff is the diff between SVN trunk and my development copy. It is not a diff to my previous patch's merged code or anything funky like that. It may contain some entries I didn't cause because my development code is a few weeks old, and someone may have changed the SVN version of kopetemessage.cpp in that time. -- ~Charles Connell
Index: /home/kde-devel/kopete-devel/kopete/libkopete/kopetemessage.cpp
===================================================================
--- /home/kde-devel/kopete-devel/kopete/libkopete/kopetemessage.cpp (revision 638959)
+++ /home/kde-devel/kopete-devel/kopete/libkopete/kopetemessage.cpp (working copy)
@@ -26,7 +26,6 @@
#include <qtextcodec.h>
#include <QByteArray>
#include <QSharedData>
-#include <QPointer>
#include <QtCore/QLatin1String>
#include <kdebug.h>
@@ -73,7 +72,7 @@
QColor fgColor;
QColor bgColor;
- QString body;
+ QPointer<QTextDocument> body;
QString subject;
};
@@ -83,7 +82,7 @@
: from( const_cast<Contact*>(from) ), to( to ), manager( 0 ), direction( direction ), format( PlainText ), type( type ),
requestedPlugin( requestedPlugin ), importance( (to.count() <= 1) ? Normal : Low ),
bgOverride( false ), fgOverride( false ), rtfOverride( false ), isRightToLeft( false ),
- timeStamp( timeStamp ), body( QString::null ), subject( subject )
+ timeStamp( timeStamp ), ( new QTextDocument() ), subject( subject )
{
}
@@ -131,13 +130,49 @@
doSetBody( body, f );
}
+Message::Message( const Contact *fromKC, const QList<Contact*> &toKC, const QTextDocument *body,
+ MessageDirection direction, MessageFormat f, const QString &requestedPlugin, MessageType type )
+: d( new Private( QDateTime::currentDateTime(), fromKC, toKC, QString::null, direction, requestedPlugin, type ) )
+{
+ doSetBody( body, f );
+}
+
+Message::Message( const Contact *fromKC, const Contact *toKC, const QTextDocument *body,
+ MessageDirection direction, MessageFormat f, const QString &requestedPlugin, MessageType type )
+{
+ QList<Contact*> to;
+ to.append((Kopete::Contact*)toKC);
+ d = new Private( QDateTime::currentDateTime(), fromKC, to, QString::null, direction, requestedPlugin, type );
+ doSetBody( body, f );
+}
+
+Message::Message( const Contact *fromKC, const QList<Contact*> &toKC, const QTextDocument *body,
+ const QString &subject, MessageDirection direction, MessageFormat f, const QString &requestedPlugin, MessageType type )
+ : d( new Private( QDateTime::currentDateTime(), fromKC, toKC, subject, direction, requestedPlugin, type ) )
+{
+ doSetBody( body, f );
+}
+
+Message::Message( const QDateTime &timeStamp, const Contact *fromKC, const QList<Contact*> &toKC,
+ const QTextDocument *body, MessageDirection direction, MessageFormat f, const QString &requestedPlugin, MessageType type )
+ : d( new Private( timeStamp, fromKC, toKC, QString::null, direction, requestedPlugin, type ) )
+{
+ doSetBody( body, f );
+}
+
+
+Message::Message( const QDateTime &timeStamp, const Contact *fromKC, const QList<Contact*> &toKC,
+ const QTextDocument *body, const QString &subject, MessageDirection direction, MessageFormat f, const QString &requestedPlugin, MessageType type )
+ : d( new Private( timeStamp, fromKC, toKC, subject, direction, requestedPlugin, type ) )
+{
+ doSetBody( body, f );
+}
+
Message::Message( const Message &other )
: d(other.d)
{
}
-
-
Message& Message::operator=( const Message &other )
{
d = other.d;
@@ -146,6 +181,7 @@
Message::~Message()
{
+ delete d->body;
}
void Message::setBgOverride( bool enabled )
@@ -180,40 +216,34 @@
void Message::doSetBody( const QString &_body, Message::MessageFormat f )
{
- QString body = _body;
//TODO: move that in ChatTextEditPart::contents
if( f == RichText )
{
- //This is coming from the RichTextEditor component.
- //Strip off the containing HTML document
- body.replace( QRegExp( QLatin1String(".*<body[^>]*>(.*)</body>.*") ), QLatin1String("\\1") );
-
- //Strip <p> tags
- body.replace( QLatin1String("<p>"), QString() );
-
- //Replace </p> with a <br/>
- body.replace( QLatin1String("</p>"), QLatin1String("<br/>") );
-
- //Remove trailing </br>
- if ( body.endsWith( QLatin1String("<br/>") ) )
- body.truncate( body.length() - 5 );
-
- body.remove( QLatin1String("\n") );
- body.replace( QRegExp( QLatin1String( "\\s\\s" ) ), QLatin1String( " " ) );
+ d->body = new QTextDocument ();
+ d->body->setHtml(_body);
}
/*
else if( f == ParsedHTML )
{
kWarning( 14000 ) << k_funcinfo << "using ParsedHTML which is internal ! message: " << body << kdBacktrace() << endl;
}*/
-
- d->body = body;
+ else if (f == PlainText)
+ {
+ d->body = new QTextDocument ();
+ d->body->setPlainText(_body);
+ }
d->format = f;
// unescaping is very expensive, do it only once and cache the result
- d->isRightToLeft = ( f & RichText ? unescape( d->body ).isRightToLeft() : d->body.isRightToLeft() );
+ d->isRightToLeft = d->body->toPlainText().isRightToLeft();
}
+void Message::doSetBody (const QTextDocument *_body, Message::MessageFormat f)
+{
+ d->body = _body->clone();
+ d->format = f;
+}
+
void Message::setBody( const QString &body, MessageFormat f )
{
doSetBody( body, f );
@@ -313,42 +343,21 @@
QString Message::plainBody() const
{
- QString body=d->body;
- if( d->format & RichText )
- {
- body = unescape( body );
- }
- return body;
+ return d->body->toPlainText();
}
QString Message::escapedBody() const
{
- QString escapedBody=d->body;
// kDebug(14000) << k_funcinfo << escapedBody << " " << d->rtfOverride << endl;
- if( d->format & PlainText )
- {
- escapedBody=escape( escapedBody );
- }
- else if( d->format & RichText && d->rtfOverride)
- {
- //remove the rich text
- escapedBody = escape (unescape( escapedBody ) );
- }
+ return d->body->toHtml();
- return escapedBody;
}
QString Message::parsedBody() const
{
//kDebug(14000) << k_funcinfo << "messageformat: " << d->format << endl;
- if( d->format == ParsedHTML )
- {
- return d->body;
- }
- else
- {
#ifdef __GNUC__
#warning Disable Emoticon parsing for now, it make QString cause a ASSERT error. (DarkShock)
#endif
@@ -356,7 +365,6 @@
return Kopete::Emoticons::parseEmoticons(parseLinks(escapedBody(), RichText));
#endif
return escapedBody();
- }
}
static QString makeRegExp( const char *pattern )
@@ -477,6 +485,11 @@
return d->font;
}
+const QTextDocument * Message::body() const
+{
+ return d->body;
+}
+
QString Message::subject() const
{
return d->subject;
pgpHr3UMzrIDw.pgp
Description: PGP signature
_______________________________________________ kopete-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/kopete-devel
