KActionCollection: it's a jungle

[Aaron J. Seigo]

On March 17, 2007, Andreas Hartmetz wrote:
> Hi!
>
> In my kdelibs adventures, I met a strange animal called KActionCollection.
> It can do some some very useful things and some tricks nobody really wants
> to see.
> Back to the technical level...
> KActionCollection has many functions that can be summarized as "do one
> thing to all members of the collection". They are numerous, they clutter
> the documentation, and nobody uses them, according to lxr.kde.org.
> Some are really amusing, like addDocCollection() whose documentation goes
> like that:
> "Doc/View model. This lets you add the action collection of a document to a
> view's action collection."
> The action will be added to a special internal list and will then be
> treated like any other action but it won't be returned by any "query my
> actions" method, that is, except if you ask for it by name. yeah right.
>
> Therefore, I propose to remove all of these:
>
> * addDocCollection
>  a)huh?, b) used in one case in KHelpCenter. Can't be that useful.

looking at the code, it seems that the intention is to make it so that you can 
load a document with associated actions and add those actions to the 
application. however, if the application already has an action by the same 
name, then the app's action masks the doc's action. essentially, this seems 
to allow a doc to add unique actions to the view without clobbering the 
view's actions in the process.

sounds reasonable to me. that it's used only once makes it a bit odd to have 
that feature in kdelibs, but it does seem like the most elegant place for it.

> * setDefaultShortcutContext:
>  used three times, to set it to the default each time...

looks like a damage-during-porting sort of thing =) looks unecessary indeed 
now.

> * applyDefaultShortcutContext:
>  a)wtf?, b) unused

essentially a reset. that it's not used is interesting, and yes, it could be 
done with a foreach loop by the app. can probably go.

> * defaultShortcutContext:
>  then obsolete

agreed.

> * forgetEnabled:
>  a)wtf?, b)unused.

looks like an "api completeness" type of method. Unchanged is different than 
Enabled (see line 285). not a big deal, i wouldn't worry about it and leave 
it for completness.

> configIsGlobal() should be renamed to isConfigGlobal, for consistency with
> isEnabled() and isEmpty() and because it's a better name anyway.

hm.. i tried writing out a few possibilities and they all feel ... awkward =) 
i think it's the bool which makes it difficult to really pin this down. it 
reads so much more naturally, if a bit more verbose, when an enum is used:

enum ConfigurationContext { Global = 0, Application = 1};
void setConfigurationContext( ConfigurationContext context );
ConfigurationContext configurationContext() const;

so, this:

setConfigGlobal( false );
setConfigGlobal( true );

becomes:

setConfigurationContext( KActionCollection::Global );
setConfigurationContext( KActionCollection::Application );

that said, according to lxr.kde.org this is also unused. *shrug* it's a 
potentially useful feature, but then if nobody is using it... is it really? 
=)

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

Full time KDE developer sponsored by Trolltech (http://www.trolltech.com)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20070318/bc5b8b45/attachment.sig>