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

List:       kfm-devel
Subject:    Re: PATCH adds PluginInterface to KonqPopupMenu
From:       Simon Hausmann <hausmann () kde ! org>
Date:       2001-10-29 8:25:11
[Download RAW message or body]

On Mon, Oct 29, 2001 at 12:38:17AM +0100, Holger Freyther wrote:
> Moin,
> this patch adds a plugin interface to KonqPopupmenu. If found the current way 
> was too static. 
> This PATCH consists of two parts.
> 1. It adds some public function to make information public to 
> KonqPopupMenuPlugin Classes.
> 2. it adds a new class called KonqPopupMenuPlugin. You need to inherit this 
> one and KLibLoader to create your own plugins.
> The Plugin class has two choices. Either use KonqPopupMenu::addAction( ) or 
> it can use KonqPopupMenu directly.
> In some situations the XML GUI stuff is too limited then you could add a 
> dummy entry and use the slotXMLGUIFinished()  to manipulate the menu.
> A couple of plugins are still to come.
> One is a so called quick copy and quick move. It adds something similiar as 
> the disknavigator to a popupmenu.
> Then you would right click go to the quick copy submenu and choose the path.
> I would be happy if this patch would be applied

Nice idea. Some comments about the implementation :)

I don't like that we need to make KonqPopupMenu 'public' for that.
Yes, it is already public in the sense that it gets installed, but
now that I see it I think the API is much to open for a library
class IMHO. Maybe we should close it more in terms of functional methods
like the add* ones. However having accessor methods for some of the
attributes sounds like a good idea to me (but please no getFoo()
style :-} It's so horribly inconsistent to Qt/kdelibs, and
inconsistency is what third party developers hate) . Also I don't
think we need a KonqPopupMenuPlugin base class. Given that it
provides no functionality (only empty stubs) I think the same
functionality can be implemented without it. Just ask the factory to
create a QObject and pass the KonqPopupMenu as parent (like it's
done now) . 

The popupmenu implementation does not appear to be interested querying 
any information off the plugin, so I don't think we need an interface for 
that. And the plugin implementation doesn't really need any other information 
from the popupmenu other than those url attributes and the like. In fact...  I 
guess we could even pass these parameters as part of the (QStringList) args, so 
the plugin gets them right away, without the need for D-casting the
parent to KonqPopupMenu and calling accessors. But it should be no
problem to provide both, of course.

As for the add* methods. I think we should make them private and let
the plugin be a KXMLGUIClient component that the popupmenu simply
adds to the GUI. That leaves complete freedom to the plugin and in
fact it should make it simpler (no need to call any strange add*
methods, use the standard xmlgui 'API' (xml format) instead) .

What do you think?

Simon

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