From kde-core-devel  Tue Apr 17 21:52:23 2007
From: Jakub Stachowski <stachowski () hypair ! net>
Date: Tue, 17 Apr 2007 21:52:23 +0000
To: kde-core-devel
Subject: Re: KShortcut and properties
Message-Id: <200704172352.23332.stachowski () hypair ! net>
X-MARC-Message: https://marc.info/?l=kde-core-devel&m=117685169401931

Dnia wtorek 17 kwiecień 2007, Thiago Macieira napisał:
> Jakub Stachowski wrote:
> > Hello,
> >
> > KAction has two properties of type KShortcut: shortcut and
> > globalShortcut. The
> > problem is that KShortcut is not registered with Qt metatype system so it
> > cannot really be used as type of property. This causes bug in
> > KXMLGUIFactory::configureAction - it reads shortcuts from .ui file (
> > ActionProperties node) and tries to set them using
> > setProperty("shortcut", [QString with shortcut definition]. This will
> > fail because bug mentioned above and because QString won't be casted to
> > KShortcut automatically. Attached is patch that fixes both
> > KShortcut-as-property problem (did i put qRegisterMetaType calls in
> > proper places?) and casting in KXMLGUIFactory.
>
> Patch looks good.
>
> Just the following:
>  #include <QtCore/QList>
>  #include <QtGui/QKeySequence>
> +#include <QMetaType>
>
> it should be QtCore/QMetaType.

Ok.

>
> While you're at it, here are some suggested improvements for KShortcut:
> * add operator QVariant() const { return qVariantFromValue(*this); }, just
> like QKeySequence has it.
>   Unlike KUrl, KShortcut's base class won't cause us problems
>
> * make qHash(const KShortcut &) non-inline.

This breaks BC, right?

> I don't think it's a good idea 
> to have qHash on a Qt type (QKeySequence) in one of our public header
> files. 
It was introduced in rev 614682 (log message "msvc compile") by Christian 
Ehrlicher. I have no windows machine to test it so i will leave this fragment 
to Christian.

> Actually, the whole qHash function looks dangerous to me the way it 
> is written.

Looks like it assumes that KShortcut is always two-key. I wonder why?