On Mar 5, 2007, at 8:07 PM, Charles Connell wrote:
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.
I will review the patch and provide comments tomorrow.
--
Matt
_______________________________________________
kopete-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kopete-devel
