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

List:       kde-core-devel
Subject:    Re: PATCH KGuiItem
From:       Holger Freyther <freyther () gmx ! net>
Date:       2001-11-13 15:17:28
[Download RAW message or body]

On Tuesday 13 November 2001 13:34, Martijn Klingens wrote:
> On Monday 12 November 2001 23:33, you wrote:
> > the KGuiItem is on the Todo List. Actually I created the KGuiItem class
> > and changed KAction to use it.  So basicly I don't know if everything
> > necessary is in this class so speak up if you need some more.
>
> Well... there is a bit too *much* in KAction now :-)
>
> You duplicated the methods to get/set the fields that KGuiItem provides.
> That somehow sounds a bit redundant to me, since your modified KAction
> inherits the methods from KGuiItem.
I need to do this. KAction is doing somesthing special and it's also 
necessary because of setFooBar(int id, FooBar * ). According to tronical it's 
a feature of c++ that it'll hide a function like setFooBar( FooBar * ).
I don't know if you get my point but's obvoiulsy necessary.

> Also, I think you should have a d pointer in KGuiItem and KAction. Removing
> those as you do now is a very bad idea IMO for kdelibs code.
Could you explain it please. IIRC I chatted with tronical and he mentioned he 
would prefer inheritance over the d->pointer stuff. But maybe I got him 
wrong. Actually I'm very flexible with it
> > I'm going to patch KDialogBase and KmessageBox to make use of this
> > KGuiItem class as soon as it is accepted
>
> Somehow I have the feeling you make me unemployed with regards to what I
> wanted to do on KDE with the little spare time I have ;-)
I'm sorry I can stop if you want ;-}
> > Further steps could be to add these things to KPopupMenu, KPushButton,
> > KComboBox...
>
> KPopupMenu is a bit questionable, since you can also plug KActions in menus
> (and even in *Q* popup menus, instead of the 'K' version), but KPushButton
> definitely could benefit from this.
You're right with the Q/KPopupMenu
> > TODO: I'm going to add signals to KGuiItem to do the following,
> > KGuiItem *item = new KStdAction::ok();
> > item->setText(i18n("&Ok!"));
> > KPushButton *button = new KPushButton(item );
> > item->setText(i18n("&Save") );
> > So with these signals I'm able to "autoupdate" the items.
>
> Yes! I already dearly missed them in KDE 2.x's KAction, aand with KGuiItem
> they are really needed IMO.
> I don't know what the general consensus is, but I'd favour the signal to
> emit a pointer to itself as last parameter:
>
> emit( textChanged( m_text, this ) );
ok will do this. Ididn't know about this option :)
>
> Qt's connect allows to connect with discarding the second parameter and in
> a number of applications it might be very useful to have this pointer at
> hand (share a single slot, optionally, to handle multiple actions/gui
> items).
>
> Martijn
>
> PS: I didn't have time to look at your code too closely, so I might find
> more issues that I'd like to discuss, but on the whole your patch looks
> pretty good already. Nice work!
there are some details. I forgot to initilaize some parameters (shame on me). 
At my first start of kde everything crashed in setAccell becuase I forgot to 
set m_kaccel to zero and then it didn't crash but every action was disabled 
:(((( and but on my ondisk copy everything is fine by now.
Martijn thanks for your reply.
regards Holger

PS:Could you explain me the point of d pointers?
-- 
Kwebsuite http://kwebsuite.sf.net Holger Freyther alias zecke(123)
freyther@yahoo.com
freyther@gmx.net

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