[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kopete-devel
Subject:    Re: [Kopete-devel] patch, and other stuff
From:       Martijn Klingens <klingens () kde ! org>
Date:       2002-05-05 12:47:58
[Download RAW message or body]

On Sunday 05 May 2002 14:26, Daniel Stone wrote:
> > Hmm, dropping the timestamp might be a good idea. I'm not so sure I like
> > your change from const KopeteContactList & to KopeteContact*, AFAIK
> > there's a default constructor for QPtrList that takes a single pointer as
> > argument.
>
> Yes, there is. This is how I had it. I suggest it stays that way.

And at the end of my message I noticed in the Qt API docs there is no such 
method at all. What is it now? ;-)

> > I'd vote for making the timestamp, foregroundColor, backgroundColor and
> > font properties and not mandatory parameters to the contructor btw. They
> > are all optional and should all default to the Kopete/user defaults and
> > not require additional code to support that. Dunca, would that be ok with
> > you?
>
> Um, they are - in kopetemessage.h. If you define the default param
> *again* in the actual source file, it generates a compiler warning. I
> cleaned all of these occurrences up.

Uhm... I meant kicking those parameters completely out of the constructor's 
prototype actually. If a plugin supports, say, fonts, it will then do 
explicitly msg->setFont( blah ). Without that it will by definition pick the 
default. If we decide that the default font is taken directly from kdeglobals 
we can change that later on this way. If instead the 'default' is hardwired 
into the .h file we can't. That's why it shouldn't be a parameter in the 
first place, but an explicit extra setXxxx() method call if you truly want to 
set it.

> > > +       void done() { emit done(*this); }; // call when a message is
> > > done
> >
> > This one sounds *very* redundant to me. Please don't...
>
> Umm.

Umm ?

> > Could you make that 'void sent( ... );' ? I think 'sent' is a better name
> > than 'done'
>
> I don't like the idea of a message emitting signals. At all.

Well, see the previous thread as well. I don't like it either, actually, nor 
do I think Andres does. But do you know of a better solution?

signal KopeteProtocol::messageSent( const KopeteMessage *msg );

instead? IIRC there was an argument against that as well. Andres?

> > > +       // What is this doing here ???? messages should be read when
> > > they arrive, not when they're sent
> > >         readMessages();
> >
> > No idea. Duncan?
>
> I put that there. The logic behind it is this:
> * You send a message.
> * Message goes into the queue.
> * Message then waits with all other unread messages until you click on
>   "read messages".
>
> Obviously, this sucks IMHO. If you send a message, then I dare say you
> want to read all the unread ones. So, I added this code in, to make sure
> the message queue got cleared; including the message you just sent.
> Pressing enter (well, ctrl+enter, but it should be enter, grumble
> grumble), and watching your message disappear would be nasty. :)

I think the message should be put on hold until the actual sent() signal is 
emitted (from whatever class we put the signal in ;-). In the slot that 
handles that signal we may indeed mark the message as read. Hmm, what about a 
property bool read(), void setRead( bool b ); in KopeteMessage? Just a (very) 
wild thought...

> > >         const KopeteContactList& members() const { return mContactList;
> > > }; /* Sorry, had to change this to members(), it was conflicting with
> > > kxContact */
> >
> > Huh? Those are two completely different classes...
>
> I'm buggered if I know what's going on here. AIMContact can't inherit
> both kxContact and KopeteContact, surely??

Well, we're talking about the class KopeteMessageManager :-)

I bet you now understand even less of this now :P (and so do I).

Martijn

_______________________________________________
Kopete-devel mailing list
Kopete-devel@mail.kde.org
http://mail.kde.org/mailman/listinfo/kopete-devel
[prev in list] [next in list] [prev in thread] [next in thread]