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


As documented, the shortcut(types) method allows to retrieve either the current 
shortcuts (like most other methods), or the *default* shortcuts, i.e. those 
that were set by the application developer initially, before the user could 
make changes.

This is done using dynamic properties on the action, so that the configuration 
dialog can handle both QActions and KActions, IIRC.
So it sounds to me like you'll have to change these dynamic properties, from 
"defaultPrimaryShortcut"/"defaultSecondaryShortcut" to a single property 
containing a list of default shortcuts.

I just saw your reply about plans for cleaning this up in kdelibs frameworks -- 
this would be MORE than welcome.
The only plan right now is 
http://article.gmane.org/gmane.comp.kde.devel.frameworks/350  but I think you 
have more details on the shortcut handling itself.

Ideally, I would like the handling of default shortcuts to go into Qt itself, 
so that we don't need KAction anymore in the simple case of a non-global 
action, and still have configurable shortcuts. Anyway, let's discuss this 
further on kde-frameworks-devel.

I agree with Michael Jansen that this patch alone isn't enough, but if you also 
plan to improve the shortcuts editor to support more than 2 alternates, and 
then look into Qt5/KF5, then I support this patch, as a first step in the right 
direction.


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

    Don't deprecate the method, it's still very much needed, for default 
shortcuts. So IMHO merge it with shortcut() again, i.e. remove the other 
method, the KDE_DEPRECATED, and put back the default argument.



kdeui/shortcuts/kshortcut.h
<http://git.reviewboard.kde.org/r/104801/#comment10830>

    Missing @since (same on the next method)


- David Faure


On May 9, 2012, 6:21 p.m., Mark Gaiser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104801/
> -----------------------------------------------------------
> 
> (Updated May 9, 2012, 6:21 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> So i was trying to fix this bug: https://bugs.kde.org/show_bug.cgi?id=181531 
> That only asked for one more shortcut. That issue seems to be a little more 
> complicated than it looks. Till this point KActions could only have a 
> "Primary" and a "Alternate" shortcut. 2 in total which is - in some 
> situations - not enough.
> 
> I fixed this by roughly restructuring nearly all of the KShortcut cpp file.
> 
> The only thing i'm not quite sure about is how "KShortcut 
> KAction::shortcut(ShortcutTypes type) const" looks right now.. If anyone has 
> some clarification on that one..?
> 
> 
> Diffs
> -----
> 
>   kdeui/actions/kaction.h d877554 
>   kdeui/actions/kaction.cpp 309cf82 
>   kdeui/shortcuts/kshortcut.h c720830 
>   kdeui/shortcuts/kshortcut.cpp e307ab0 
> 
> Diff: http://git.reviewboard.kde.org/r/104801/diff/
> 
> 
> Testing
> -------
> 
> I tested this by adding the missing bindings for Dolphin's back/forward and 
> it seems to be working just fine. I can use all available shortcuts.
> 
> 
> Thanks,
> 
> Mark Gaiser
> 
>

Reply via email to