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

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to