Make sure all keyboard modifiers are handled correctly

This handles Qt.KeypadModifier (Num+...) correctly, adds tests for converting
modifiers to strings, and strips Qt.GroupSwitchModifier as QKeySequence doesn't
know about it.

Fixes #3675
This commit is contained in:
Florian Bruhin 2018-03-05 22:11:26 +01:00
parent 2ab270dfac
commit 29fdd1acc4
3 changed files with 121 additions and 25 deletions

View File

@ -51,6 +51,19 @@ def is_modifier_key(key):
return key in _MODIFIER_MAP
def _check_valid_utf8(s, data):
"""Make sure the given string is valid UTF-8.
Makes sure there are no chars where Qt did fall back to weird UTF-16
surrogates.
"""
try:
s.encode('utf-8')
except UnicodeEncodeError as e: # pragma: no cover
raise ValueError("Invalid encoding in 0x{:x} -> {}: {}"
.format(data, s, e))
def _key_to_string(key):
"""Convert a Qt::Key member to a meaningful name.
@ -131,7 +144,27 @@ def _key_to_string(key):
if key in special_names:
return special_names[key]
return QKeySequence(key).toString()
result = QKeySequence(key).toString()
_check_valid_utf8(result, key)
return result
def _modifiers_to_string(modifiers):
"""Convert the given Qt::KeyboardModifiers to a string.
Handles Qt.GroupSwitchModifier because Qt doesn't handle that as a
modifier.
"""
if modifiers & Qt.GroupSwitchModifier:
modifiers &= ~Qt.GroupSwitchModifier
result = 'AltGr+'
else:
result = ''
result += QKeySequence(modifiers).toString()
_check_valid_utf8(result, modifiers)
return result
class KeyParseError(Exception):
@ -191,7 +224,7 @@ def _parse_special_key(keystr):
for (orig, repl) in replacements:
keystr = keystr.replace(orig, repl)
for mod in ['ctrl', 'meta', 'alt', 'shift']:
for mod in ['ctrl', 'meta', 'alt', 'shift', 'num']:
keystr = keystr.replace(mod + '-', mod + '+')
return keystr
@ -246,7 +279,7 @@ class KeyInfo:
key_string = key_string.lower()
# "special" binding
modifier_string = QKeySequence(modifiers).toString()
modifier_string = _modifiers_to_string(modifiers)
return '<{}{}>'.format(modifier_string, key_string)
def text(self):
@ -411,33 +444,32 @@ class KeySequence:
raise utils.Unreachable("self={!r} other={!r}".format(self, other))
def append_event(self, ev):
"""Create a new KeySequence object with the given QKeyEvent added.
We need to do some sophisticated checking of modifiers here:
We don't care about a shift modifier with symbols (Shift-: should match
a : binding even though we typed it with a shift on an US-keyboard)
However, we *do* care about Shift being involved if we got an
upper-case letter, as Shift-A should match a Shift-A binding, but not
an "a" binding.
In addition, Shift also *is* relevant when other modifiers are
involved.
Shift-Ctrl-X should not be equivalent to Ctrl-X.
We also change Qt.Key_Backtab to Key_Tab here because nobody would
configure "Shift-Backtab" in their config.
"""
"""Create a new KeySequence object with the given QKeyEvent added."""
key = ev.key()
modifiers = ev.modifiers()
if key == 0x0:
raise KeyParseError(None, "Got nil key!")
# We always remove Qt.GroupSwitchModifier because QKeySequence has no
# way to mention that in a binding anyways...
modifiers &= ~Qt.GroupSwitchModifier
# We change Qt.Key_Backtab to Key_Tab here because nobody would
# configure "Shift-Backtab" in their config.
if modifiers & Qt.ShiftModifier and key == Qt.Key_Backtab:
key = Qt.Key_Tab
# We don't care about a shift modifier with symbols (Shift-: should
# match a : binding even though we typed it with a shift on an
# US-keyboard)
#
# However, we *do* care about Shift being involved if we got an
# upper-case letter, as Shift-A should match a Shift-A binding, but not
# an "a" binding.
#
# In addition, Shift also *is* relevant when other modifiers are
# involved. Shift-Ctrl-X should not be equivalent to Ctrl-X.
if (modifiers == Qt.ShiftModifier and
is_printable(ev.key()) and
not ev.text().isupper()):

View File

@ -37,7 +37,7 @@ class Key:
name: The name returned by str(KeyInfo) with that key.
text: The text returned by KeyInfo.text().
uppertext: The text returned by KeyInfo.text() with shift.
member: Filled by the test fixture, the numeric value.
member: The numeric value.
"""
attribute = attr.ib()
@ -54,6 +54,28 @@ class Key:
self.name = self.attribute
@attr.s
class Modifier:
"""A modifier with expected values.
Attributes:
attribute: The name of the Qt::KeyboardModifier attribute
('Shift' -> Qt.ShiftModifier)
name: The name returned by str(KeyInfo) with that modifier.
member: The numeric value.
"""
attribute = attr.ib()
name = attr.ib(None)
member = attr.ib(None)
def __attrs_post_init__(self):
self.member = getattr(Qt, self.attribute + 'Modifier')
if self.name is None:
self.name = self.attribute
# From enum Key in qt5/qtbase/src/corelib/global/qnamespace.h
KEYS = [
### misc keys
@ -589,3 +611,13 @@ KEYS = [
# 0x0 is used by Qt for unknown keys...
Key(attribute='', name='nil', member=0x0, qtest=False),
]
MODIFIERS = [
Modifier('Shift'),
Modifier('Control', 'Ctrl'),
Modifier('Alt'),
Modifier('Meta'),
Modifier('Keypad', 'Num'),
Modifier('GroupSwitch', 'AltGr'),
]

View File

@ -40,6 +40,14 @@ def qt_key(request):
return key
@pytest.fixture(params=key_data.MODIFIERS, ids=lambda m: m.attribute)
def qt_mod(request):
"""Get all existing modifiers from key_data.py."""
mod = request.param
assert mod.member is not None
return mod
@pytest.fixture(params=[key for key in key_data.KEYS if key.qtest],
ids=lambda k: k.attribute)
def qtest_key(request):
@ -47,7 +55,7 @@ def qtest_key(request):
return request.param
def test_key_data():
def test_key_data_keys():
"""Make sure all possible keys are in key_data.KEYS."""
key_names = {name[len("Key_"):]
for name, value in sorted(vars(Qt).items())
@ -57,6 +65,17 @@ def test_key_data():
assert not diff
def test_key_data_modifiers():
"""Make sure all possible modifiers are in key_data.MODIFIERS."""
mod_names = {name[:-len("Modifier")]
for name, value in sorted(vars(Qt).items())
if isinstance(value, Qt.KeyboardModifier) and
value not in [Qt.NoModifier, Qt.KeyboardModifierMask]}
mod_data_names = {mod.attribute for mod in sorted(key_data.MODIFIERS)}
diff = mod_names - mod_data_names
assert not diff
class KeyTesterWidget(QWidget):
"""Widget to get the text of QKeyPressEvents.
@ -113,6 +132,10 @@ class TestKeyToString:
def test_to_string(self, qt_key):
assert keyutils._key_to_string(qt_key.member) == qt_key.name
def test_modifiers_to_string(self, qt_mod):
expected = qt_mod.name + '+'
assert keyutils._modifiers_to_string(qt_mod.member) == expected
def test_missing(self, monkeypatch):
monkeypatch.delattr(keyutils.Qt, 'Key_AltGr')
# We don't want to test the key which is actually missing - we only
@ -160,6 +183,8 @@ def test_key_parse_error(keystr, expected):
('a<Ctrl+a>b', ['a', 'ctrl+a', 'b']),
('<Ctrl+a>a', ['ctrl+a', 'a']),
('a<Ctrl+a>', ['a', 'ctrl+a']),
('<Ctrl-a>', ['ctrl+a']),
('<Num-a>', ['num+a']),
])
def test_parse_keystr(keystr, parts):
assert list(keyutils._parse_keystring(keystr)) == parts
@ -337,6 +362,9 @@ class TestKeySequence:
('', Qt.Key_Backtab, Qt.ShiftModifier, '', '<Shift+Tab>'),
('', Qt.Key_Backtab, Qt.ControlModifier | Qt.ShiftModifier, '',
'<Control+Shift+Tab>'),
# Stripping of Qt.GroupSwitchModifier
('', Qt.Key_A, Qt.GroupSwitchModifier, 'a', 'a'),
])
def test_append_event(self, old, key, modifiers, text, expected):
seq = keyutils.KeySequence.parse(old)
@ -352,8 +380,6 @@ class TestKeySequence:
seq.append_event(event)
@pytest.mark.parametrize('keystr, expected', [
('<Control-x>', keyutils.KeySequence(Qt.ControlModifier | Qt.Key_X)),
('<Meta-x>', keyutils.KeySequence(Qt.MetaModifier | Qt.Key_X)),
('<Ctrl-Alt-y>',
keyutils.KeySequence(Qt.ControlModifier | Qt.AltModifier | Qt.Key_Y)),
('x', keyutils.KeySequence(Qt.Key_X)),
@ -364,6 +390,12 @@ class TestKeySequence:
keyutils.KeySequence(Qt.ControlModifier | Qt.Key_X,
Qt.MetaModifier | Qt.Key_Y)),
('<Shift-x>', keyutils.KeySequence(Qt.ShiftModifier | Qt.Key_X)),
('<Alt-x>', keyutils.KeySequence(Qt.AltModifier | Qt.Key_X)),
('<Control-x>', keyutils.KeySequence(Qt.ControlModifier | Qt.Key_X)),
('<Meta-x>', keyutils.KeySequence(Qt.MetaModifier | Qt.Key_X)),
('<Num-x>', keyutils.KeySequence(Qt.KeypadModifier | Qt.Key_X)),
('>', keyutils.KeySequence(Qt.Key_Greater)),
('<', keyutils.KeySequence(Qt.Key_Less)),
('a>', keyutils.KeySequence(Qt.Key_A, Qt.Key_Greater)),