Review Request 109019: Move global shortcut facilities from KAction to KGlobalAccel

Kevin Ottens ervin at kde.org
Wed Feb 27 07:16:56 UTC 2013


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109019/#review28188
-----------------------------------------------------------


There's been a small misunderstanding I think. You introduced a slot which keep emitting actionGlobalShortcutChanged for actions alright *but* you introduced it in the wrong class. This slot should be in KAction and the connect for KGlobalAccel signal be done in KAction constructor, this way it is completely transparent to KGlobalAccel what KAction is doing.

Otherwise that's inserting an extra KGlobalAccel -> KAction dependency which will be in the way of later having a completely QAction based KGlobalAccel.


kdeui/shortcuts/kglobalaccel.h
<http://git.reviewboard.kde.org/r/109019/#comment21067>

    This should go in kaction.h



kdeui/shortcuts/kglobalaccel.cpp
<http://git.reviewboard.kde.org/r/109019/#comment21066>

    This should go in KAction ctor.



kdeui/shortcuts/kglobalaccel.cpp
<http://git.reviewboard.kde.org/r/109019/#comment21065>

    It should go in kaction.cpp. Of course it will become KAction::_k_emitActionGlobalShortcutChanged and the emit will occur only if action == this.



kdeui/shortcuts/kglobalaccel_p.h
<http://git.reviewboard.kde.org/r/109019/#comment21064>

    It should go in kaction_p.h.


- Kevin Ottens


On Feb. 26, 2013, 7:28 p.m., Valentin Rusu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109019/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2013, 7:28 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Kevin Ottens.
> 
> 
> Description
> -------
> 
> KDELibs cleanup task: move global shortcut facilities from KAction to KGlobalAccel. The second step, registering QActions instead of KActions, will be done after completing this review.
> 
> 
> Diffs
> -----
> 
>   kdeui/shortcuts/kglobalaccel_p.h 04feaba 
>   kdeui/shortcuts/kglobalaccel.cpp 36dbab7 
>   kdeui/actions/kaction_p.h b50e678 
>   kdeui/shortcuts/kglobalaccel.h ed68bba 
>   kdeui/actions/kaction.cpp ec63a72 
>   KDE5PORTING.html e1b41d1 
>   kdeui/actions/kaction.h ddaa4de 
> 
> Diff: http://git.reviewboard.kde.org/r/109019/diff/
> 
> 
> Testing
> -------
> 
> Code compiles
> 
> 
> Thanks,
> 
> Valentin Rusu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130227/b227b553/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list