Review Request: Make an effort not to steal existing action shortcuts in KAcceleratorManager

[Thomas Friedrichsmeier]

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

(Updated 2010-06-08 11:49:52.257391)


Review request for kdelibs.


Changes
-------

Updated the diff for a slightly less crude detection of Alt+(single alphanum key) shortcuts.


Summary (updated)
-------

In RKWard we set default shortcuts Alt+0 through Alt+6 for navigating between our various tool windows and document windows. In addition, we have a tabbed document view, with filenames as tab-titles. Recently we found out that opening files named "a1.txt" or similar will lead to shortcut conflicts. What happens, here, is that KAcceleratorManager assigns a shortcut (in this case Alt+1) to the tab, which conflicts with our existing KAction-shortcuts.

It is possible to work around this issue using KAcceleratorManager::setNoAccel(). However:
a) That removes the accelerators from the tabbar.
b) Having never heard of KAcceleratorManager before, I had quite a hard time figuring out what was going on, and I'd like to save others that experience.
c) KAcceleratorManager should really make a better effort at respecting existing keybindings.

This patch is somewhat crude in some respects. It works for my use case, but has several shortcomings as listed below. Ideally, somebody will step forward to help me fix these. But personally, I feel that the patch is a significant improvement over the current situation as is, and is ready for committing, even in its current state. Also, seeing that this review request received no reviews for more than a month to date, perhaps there is no point in striving for perfection in this particular area. So - ok to commit?

Known shortcomings of the current patch:
1) This only works for actions inside KActionCollections. That covers KXMLGUI-applications, but if there is a more generic way to enumerate all actions/shortcuts, that would be much more elegant. Qt does seem to keep a - private - list of all assigned shortcuts, but I haven't figured out any way to get at that info.
2) Accelerators would need to be re-checked after the shortcuts were configured, and probably after some other events as well.


Diffs (updated)
-----

  trunk/KDE/kdelibs/kdeui/shortcuts/kacceleratormanager.cpp 1129152 

Diff: http://reviewboard.kde.org/r/3719/diff


Testing
-------

Works for RKWard: With the patch, KAcceleratorManager no longer steals the shortcuts that are already in use, but continues to assign free accelerator keys.

I also have a testcase-app at hand. Is there any good way to attach that in reviewboard?


Thanks,

Thomas