Review Request: Make an effort not to steal existing action shortcuts in KAcceleratorManager
Thomas Friedrichsmeier
thomas.friedrichsmeier at ruhr-uni-bochum.de
Wed Apr 21 17:22:20 BST 2010
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3719/
-----------------------------------------------------------
Review request for kdelibs.
Summary
-------
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 represents a rather crude initial effort at that. I works for my use case, but has several shortcomings, which I hope somebody will step forward to help me fix them(*):
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) Converting the actions to string representation to find out which ones are Alt+<Printable Key>-combinations seems rather crude.
3) Accelerators would need to be re-checked after the shortcuts were configured, and probably after some other events as well.
(*): I do feel the patch is an improvement over the current situation even in its current state, so personally, I wouldn't feel bad about committing it right away. But that is not my call to make, and I expect others to disagree.
Diffs
-----
trunk/KDE/kdelibs/kdeui/shortcuts/kacceleratormanager.cpp 1117108
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
More information about the kde-core-devel
mailing list