RFC: rename or kill KShortcut / add new triggers to KAction

[Hamish Rodda]

On Friday 03 November 2006 20:47, David Faure wrote:
> On Fri Nov 3 2006, Andreas Hartmetz wrote:
> > Hi!
> >
> > I never liked the mess of KAction, KShortcut and KAccel. Hamish Rodda
> > did a nice job cleaning it up and I still don't like the parts that
> > are left.
> > KShortcut is not much more than a QList<QKeySequence> now and I see no
> > good reason to keep it. Applications may need to adapt, but a quick
> > grep through kdebase didn't find any code that really needs KShortcut.
>
> I'm afraid your grep program is broken :-)
>
> Just one example from konqueror/konq_mainwindow.cc:
>   KShortcut reloadShortcut = KStdAccel::shortcut(KStdAccel::Reload);
>   reloadShortcut.append(Qt::CTRL + Qt::Key_R);
>   m_paReload = new KAction(KIcon("reload"),  i18n( "&Reload" ),
> actionCollection(), "reload" );
>
> > Additionally, QShortcut exists now (since Qt 4.0) and it is something
> > very different from KShortcut. This violates the principle of least
> > surprise.
>
> Well it's only "very different" because it supports multiple alternative
> shortcuts, no?
>
> The above code with QList<QKeySequence> would look a bit uglier.

Actually, with QKeySequence's constructors not being explicit, wouldn't that 
just work with a typedef or a grep & replace?  I actually thought about 
making the change to QList.

> Maybe KShortcut should be KShortcuts? ;)

Or KShortcutGroup or KShortcutList...

> > What I propose is to:
> > either
> > -remove KShortcut
> > or
> > -rename KShortcut to KActionTriggerSet and make its API very similar
> > to KAction's shortcuts manipulation API. KActionTriggerSet should hold
> > exactly the triggers of one KAction including its default triggers,
> > not more and not less.

Other than the default/custom confusion which I am simply yet to rename, I 
don't particularly see why we should move global shortcuts etc all out to 
KShortcut.  To change a global shortcut, you'd need to retrieve the old 
shortcut set, change it, and set it back again if we did this.

> Hmm. I guess this proposal alone wouldn't make much sense (more complicated
> name) but if that class also holds mouse gestures then I guess it does make
> sense.

Well, I think we could add mouse gestures to KAction.  But I'd like to see the 
code first...

Cheers,
Hamish.
-------------- 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/20061103/6d2fc843/attachment.sig>