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