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

[Kevin Ottens]

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



kdeui/actions/kaction.h
<http://git.reviewboard.kde.org/r/109019/#comment20825>

    Please don't remove this enum from KAction, it should really be a copy to KGlobalAccel not a move as we try to maintain KAction source compatibility.



kdeui/actions/kaction.h
<http://git.reviewboard.kde.org/r/109019/#comment20826>

    Of course means this one should go back to use KAction's enum.



kdeui/actions/kaction.h
<http://git.reviewboard.kde.org/r/109019/#comment20827>

    And this one too.



kdeui/actions/kaction_p.h
<http://git.reviewboard.kde.org/r/109019/#comment20828>

    Why this include? Probably unnecessary, you didn't introduce a KAuthAction use there AFAICT.



kdeui/actions/kactioncollection.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20829>

    Unnecessary change if you keep the enum in KAction.



kdeui/actions/kactioncollection.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20830>

    Unnecessary change if you keep the enum in KAction.



kdeui/dialogs/kshortcutseditoritem.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20831>

    Unnecessary change if you keep the enum in KAction.



kdeui/dialogs/kshortcutseditoritem.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20832>

    Unnecessary change if you keep the enum in KAction.



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

    Please add apidox for those methods, don't forget the @since 5.0.



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

    Hm... I'm wondering, should this signal have a twin (both emitted at the same time) where we indicate the action linked to the shortcut when it changed?
    
    The old one being tied to KAction we had that information (knew the emitter), here the information is lost.



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

    Drop the const, it's obviously not const since you'r forced to use a const_cast later on...



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

    Should not be const as you call non const methods on instances of KAction stored there.



kdeui/tests/kglobalshortcuttest.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20837>

    Unnecessary change if you keep the enum in KAction.



kdeui/tests/kglobalshortcuttest.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20838>

    Unnecessary change if you keep the enum in KAction.



kdeui/tests/kglobalshortcuttest.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20839>

    Unnecessary change if you keep the enum in KAction.



kdeui/tests/kglobalshortcuttest.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20840>

    Unnecessary change if you keep the enum in KAction.



kdeui/tests/kglobalshortcuttest.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20841>

    Unnecessary change if you keep the enum in KAction.



kdeui/tests/kglobalshortcuttest.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20842>

    Unnecessary change if you keep the enum in KAction.



kdeui/tests/kglobalshortcuttest.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20843>

    Unnecessary change if you keep the enum in KAction.



kdeui/tests/kglobalshortcuttest.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20844>

    Unnecessary change if you keep the enum in KAction.



kdeui/tests/kglobalshortcuttest.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20845>

    Unnecessary change if you keep the enum in KAction.



kdeui/tests/kglobalshortcuttest.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20846>

    Unnecessary change if you keep the enum in KAction.



kdeui/tests/kglobalshortcuttest.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20847>

    Unnecessary change if you keep the enum in KAction.



kdeui/tests/kglobalshortcuttest.cpp
<http://git.reviewboard.kde.org/r/109019/#comment20848>

    Unnecessary change if you keep the enum in KAction.


- Kevin Ottens


On Feb. 20, 2013, 10:32 p.m., Valentin Rusu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109019/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2013, 10:32 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/tests/kglobalshortcuttest.cpp 49fb8ec 
>   kdeui/actions/kaction.h ddaa4de 
>   kdeui/actions/kaction.cpp ec63a72 
>   kdeui/actions/kaction_p.h b50e678 
>   kdeui/actions/kactioncollection.cpp 555b204 
>   kdeui/dialogs/kshortcutseditoritem.cpp 5aa8437 
>   kdeui/shortcuts/kglobalaccel.h ed68bba 
>   kdeui/shortcuts/kglobalaccel.cpp 36dbab7 
>   kdeui/shortcuts/kglobalaccel_p.h 04feaba 
> 
> 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/20130221/626a684c/attachment-0001.html>