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

List:       kde-core-devel
Subject:    Re: PATCH: Re: kdelibs/kdeui
From:       Ellis Whitehead <kde () ellisw ! net>
Date:       2002-01-31 13:38:37
[Download RAW message or body]

On Thursday 31 January 2002 04:27, Simon Hausmann wrote:
> On Wed, Jan 30, 2002 at 12:03:31PM -0500, Ellis Whitehead wrote:
> > There was a discussion in early November (while I was at the ALS
> > conference and not keeping up on the list), but in the end the only
> > reason stated against calling unplugAll() from ~KAction() was about not
> > needing to make those calls during shut-down.
> > http://lists.kde.org/?l=kde-core-devel&m=95846925315076&w=2
> >
> > The safety and convenience is worth it, though, so that actions from a
> > dynamic widget can be used by a different persistent widget.  Without the
> > unplugAll() call in ~KAction(), attempts to handle this are very hacky.
>
> The problem I see is rather that it's an imcomplete feature, that
> can't be implemented the way it should be, ideally. The feature
> makes people think that they don't have to worry about calling
> unplug() anymore because the KAction destructor does it
> automatically for them. And this is the point, it's wrong. So we
> provide a feature that works only in the case that the raw KAction
> can handle a certain container. It will not work in all other cases
> (unless we duplicate code, and people writing own actions would have
> to, too) . And even this very case (that the original KAction knows
> how to deal with a certain container) is completely hidden to the
> user of the action object, the one calling plug (after all that's
> the whole point about the action concept :) .
>
> In other words: There is no way for a user of KAction to figure out
> when he needs to call unplugAll() on his own and when he can rely on
> ~KAction doing some work.  Unlike QAction (which provides this
> feature) in KDE there are quite a few classes derived from KAction.
> Most of them even being in kaction.cpp :)

I've attached the output from:
grep '::unplug(' -r kdelibs kdebase kdeaddons kdeartwork kdeedu kdegames 
kdegraphics kdemultimedia kdenetwork kdepim kdesdk kdesupport kdeutils 
kdevelop kdevelopHEAD koffice

It shows that unplug() is defined in the following files:

unrelated	kxmlguifactor_p.cpp
duplicate	konq_actions.cc
duplicate	kdeaddons/konq-plugins/sidebar/mediaplayer/controls.cpp
unrelated	kdegames/kmines/generic/gsettings.cpp
duplicate	kdemultimedia/noatun/library/controls.cpp
duplicate	kdemultimedia/kaboodle/controls.cpp
duplicate	kdenetwork/kdict/actions.cpp
duplicate	kdevelopHEAD/parts/classview/classactions.cpp

I looked at all of them, and "duplicate" means it's just a copy of 
KAction::unplug().  We could remove all instances of unplug() from the 
KAction children outside of kdebase.cpp and everything would continue to 
function as before.

In kaction.cpp, the following classes define unplug():
KActionMenu -- duplicate
KToolBarPopupAction -- adds QMenuBar check
KActionSeparator -- adds QMenuBar check

So we just need to either add a check in KAction()::unplug() for QMenuBar or 
call unplugAll() from ~KToolBarPopupAction() and ~KActionSeparator().  

If a class needs to define unplug(), then it needs to call unplugAll() from 
its destructor, and the application coder never does need to call unplug().  
Much simpler for the application code, and entirely reasonable for the 
KAction child coder -- especially considering that in practice overriding 
unplug() isn't necessary. 

Regards,
Ellis
["grep_unplug.txt" (text/plain)]

$ grep '::unplug(' -r kdelibs kdebase kdeaddons kdeartwork kdeedu kdegames \
kdegraphics kdemultimedia kdenetwork kdepim kdesdk kdesupport kdeutils kdevelop \
kdevelopHEAD koffice kdelibs/kdeui/kaction.cpp:void KAction::unplug( QWidget *w )
kdelibs/kdeui/kaction.cpp:void KActionMenu::unplug( QWidget* widget )
kdelibs/kdeui/kaction.cpp:    KAction::unplug( widget );
kdelibs/kdeui/kaction.cpp:void KToolBarPopupAction::unplug( QWidget *widget )
kdelibs/kdeui/kaction.cpp:  KAction::unplug( widget );
kdelibs/kdeui/kaction.cpp:void KActionSeparator::unplug( QWidget *widget )
kdelibs/kdeui/kxmlguifactory_p.cpp:void ActionList::unplug( QWidget *container ) \
const kdebase/konqueror/konq_actions.cc:void KonqComboAction::unplug( QWidget *w )
kdebase/konqueror/konq_actions.cc:void KonqLabelAction::unplug( QWidget *widget )
kdeaddons/konq-plugins/sidebar/mediaplayer/controls.cpp:void SliderAction::unplug( \
QWidget *w ) kdegames/kmines/generic/gsettings.cpp:void \
KSettingCollection::unplug(QObject *o) kdemultimedia/noatun/library/controls.cpp:void \
SliderAction::unplug( QWidget *w ) kdemultimedia/kaboodle/controls.cpp:void \
SliderAction::unplug( QWidget *w ) kdenetwork/kdict/actions.cpp:void \
DictComboAction::unplug( QWidget *widget ) kdenetwork/kdict/actions.cpp:void \
DictLabelAction::unplug( QWidget *widget ) kdenetwork/kdict/actions.cpp:void \
DictButtonAction::unplug( QWidget *widget ) \
kdevelopHEAD/parts/classview/classactions.cpp:void DelayedPopupAction::unplug(QWidget \
                *widget)
kdevelopHEAD/parts/classview/classactions.cpp:    KAction::unplug(widget);
$

unrelated	kxmlguifactor_p.cpp
duplicate	konq_actions.cc
duplicate	kdeaddons/konq-plugins/sidebar/mediaplayer/controls.cpp
unrelated	kdegames/kmines/generic/gsettings.cpp
duplicate	kdemultimedia/noatun/library/controls.cpp
duplicate	kdemultimedia/kaboodle/controls.cpp
duplicate	kdenetwork/kdict/actions.cpp
duplicate	kdevelopHEAD/parts/classview/classactions.cpp



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