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

List:       kde-core-devel
Subject:    Re: KShortcut and properties
From:       Thiago Macieira <thiago () kde ! org>
Date:       2007-04-18 4:52:13
Message-ID: 200704180652.22688.thiago () kde ! org
[Download RAW message or body]


Jakub Stachowski wrote:
>> 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?

De-inlining a method isn't BC, provided calling the older method is 
acceptable.

If you want, you can commit this on the next Monday, to be sure.

>> 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.

It's only there because of the inlined KShortcut just above it. IMHO, it 
has to go.

>> 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?

I wonder the same.

-- 
  Thiago Macieira  -  thiago (AT) macieira.info - thiago (AT) kde.org
    PGP/GPG: 0x6EF45358; fingerprint:
    E067 918B B660 DBD1 105C  966C 33F5 F005 6EF4 5358

[Attachment #3 (application/pgp-signature)]

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