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/#review27994
-----------------------------------------------------------


Almost there! :-)


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

    Please remove the extra space at the end of this line.



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

    Please name the parameters and document them.



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

    Wouldn't it be better to use actionShortcuts here? At that point, you could know about an action because of a default shortcut but it could currently have no shortcut. I'd expect shortcut() and hasShortcut() to be consistent together, but right now they could be out of sync.
    
    Beside it would help you get rid of the const_cast which can't stay.
    
    Actually I think it could be a simple:
    return !shortcut(action).toList().isEmpty();


- Kevin Ottens


On Feb. 22, 2013, 10:59 p.m., Valentin Rusu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109019/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2013, 10:59 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
> -----
> 
>   KDE5PORTING.html e1b41d1 
>   kdeui/actions/kaction.h ddaa4de 
>   kdeui/actions/kaction.cpp ec63a72 
>   kdeui/actions/kaction_p.h b50e678 
>   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/20130225/1b8d4ef5/attachment.html>