Review Request: Cleanup : Move all gesture facilities of KAction in KGestureMap

[Kevin Ottens]

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


I didn't scrutinize the details of the patch yet, I just commented on the overall direction I think we have an opportunity to improve the API let's seize it.

I'll look more at the details in the next revisions of that patch as it might change quite a bit still.

In any case, thanks a lot for picking up that task!


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

    Since the end game is to move KAction in kde4support, we should try to keep its API unchanged.
    
    Please keep those methods in its API, but internally the implementation should just forward to the new implementation you're working on in KGestureMap.



kdeui/shortcuts/kgesturemap.h
<http://git.reviewboard.kde.org/r/107973/#comment18390>

    I don't really like the extra boolean which flips the semantic of addGesture... Looking at lxr.kde.org it turns out that addGesture and removeGesture are never called (except by KAction of course).
    
    That's good news as it means we can more freely modify KGestureMap API. :-)
    
    Seeing the existing calls in KAction basically always do a removeGesture followed by an addGesture, and that KAction itself exposed a setter API, what about simply removing them from the API and just have a setGesture + setDefaultGesture in the public API?


- Kevin Ottens


On Dec. 28, 2012, 11:32 a.m., Valentin Rusu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107973/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2012, 11:32 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> This changeset removes all gesture facilities from KAction and define them in KGestureMap. Thanks for the feedback.
> 
> 
> Diffs
> -----
> 
>   KDE5PORTING.html 86dfc1a 
>   kdeui/actions/kaction.h f45c94d 
>   kdeui/actions/kaction.cpp b78d1d6 
>   kdeui/actions/kaction_p.h f87272c 
>   kdeui/dialogs/kshortcutseditor.cpp 5653984 
>   kdeui/dialogs/kshortcutseditoritem.cpp e85a203 
>   kdeui/shortcuts/kgesturemap.h 56b42b6 
>   kdeui/shortcuts/kgesturemap.cpp 1350dbe 
> 
> Diff: http://git.reviewboard.kde.org/r/107973/diff/
> 
> 
> Testing
> -------
> 
> Code builds OK. KGestureMap is not actually used, though, as KGestureMap::eventFilter method return false right away.
> 
> 
> Thanks,
> 
> Valentin Rusu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20121228/0f875c7a/attachment.html>