Review Request: Fix KShortcut to really allow the usage of multiple shortcuts

Mark Gaiser markg85 at gmail.com
Wed May 2 19:19:46 BST 2012


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

(Updated May 2, 2012, 6:19 p.m.)


Review request for kdelibs.


Changes
-------

Update diff to reflect the suggestions made by David.
I also changed the idea behind this diff. It was my intention to simply deprecate everything related to primary/alternate and use prepend/append instead. However, it doesn't hurt to keep the current way without deprecating it and simply add the new prepend/append way to it.

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


Description (updated)
-------

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.

I have 2 issues remaining:
1. QList<QKeySequence> toList() const;
2. KShortcut shortcut() const;

If i enable those functions the compiler suddenly doesn't know which function to use.. The old ones or the new ones. Some suggestions on how to fix it would be welcome.


Diffs (updated)
-----

  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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20120502/93526f29/attachment.htm>


More information about the kde-core-devel mailing list