From 37791422bf670c41ebc8e7c9f0f5fb4e7e0a213a Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 28 Jun 2023 22:18:10 +0200 Subject: qt6: Make sure KeyInfo never has ints as members This used to be possible in some situations and was handled in somewhat unexpected places (e.g. .to_qt()). Instead, we now assume that KeyInfo is always "clean", and we handle the conversion from an int to a Qt.Key elsewhere. This only seems to affect tests, since otherwise we already made sure we get a Qt.Key and Qt.KeyboardModifier(s) e.g. in .from_event(). --- qutebrowser/keyinput/keyutils.py | 26 ++++++++++---------------- tests/unit/keyinput/key_data.py | 3 ++- tests/unit/keyinput/test_keyutils.py | 33 ++++++++++++++++++++++++--------- 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/qutebrowser/keyinput/keyutils.py b/qutebrowser/keyinput/keyutils.py index bd6d2db26..10f4d5378 100644 --- a/qutebrowser/keyinput/keyutils.py +++ b/qutebrowser/keyinput/keyutils.py @@ -347,7 +347,7 @@ def _unset_modifier_bits( https://github.com/python/cpython/issues/105497 """ if machinery.IS_QT5: - return cast(_ModifierType, modifiers & ~mask) + return Qt.KeyboardModifiers(modifiers & ~mask) # can lose type if it's 0 else: return Qt.KeyboardModifier(modifiers.value & ~mask.value) @@ -367,11 +367,14 @@ class KeyInfo: def __post_init__(self) -> None: """Run some validation on the key/modifier values.""" - # This is mainly useful while porting from Qt 5 to 6. - # FIXME:qt6 do we want to remove or keep this (and fix the remaining - # issues) when done? - # assert isinstance(self.key, Qt.Key), self.key - # assert isinstance(self.modifiers, Qt.KeyboardModifier), self.modifiers + # This changed with Qt 6, and e.g. to_qt() relies on this. + if machinery.IS_QT5: + modifier_classes = (Qt.KeyboardModifier, Qt.KeyboardModifiers) + elif machinery.IS_QT6: + modifier_classes = Qt.KeyboardModifier + assert isinstance(self.key, Qt.Key), self.key + assert isinstance(self.modifiers, modifier_classes), self.modifiers + _assert_plain_key(self.key) _assert_plain_modifier(self.modifiers) @@ -486,16 +489,7 @@ class KeyInfo: if machinery.IS_QT5: return int(self.key) | int(self.modifiers) else: - try: - # FIXME:qt6 We might want to consider only supporting KeyInfo to be - # instanciated with a real Qt.Key, not with ints. See __post_init__. - key = Qt.Key(self.key) - except ValueError as e: - # WORKAROUND for - # https://www.riverbankcomputing.com/pipermail/pyqt/2022-April/044607.html - raise InvalidKeyError(e) - - return QKeyCombination(self.modifiers, key) + return QKeyCombination(self.modifiers, self.key) def with_stripped_modifiers(self, modifiers: Qt.KeyboardModifier) -> "KeyInfo": mods = _unset_modifier_bits(self.modifiers, modifiers) diff --git a/tests/unit/keyinput/key_data.py b/tests/unit/keyinput/key_data.py index 3826d3ee9..5f151704a 100644 --- a/tests/unit/keyinput/key_data.py +++ b/tests/unit/keyinput/key_data.py @@ -24,6 +24,7 @@ import dataclasses from typing import Optional from qutebrowser.qt.core import Qt +from qutebrowser.keyinput import keyutils @dataclasses.dataclass(order=True) @@ -606,7 +607,7 @@ KEYS = [ Key('unknown', 'Unknown', qtest=False), # 0x0 is used by Qt for unknown keys... - Key(attribute='', name='nil', member=0x0, qtest=False), + Key(attribute='', name='nil', member=keyutils._NIL_KEY, qtest=False), ] diff --git a/tests/unit/keyinput/test_keyutils.py b/tests/unit/keyinput/test_keyutils.py index 2c0740c20..c3b6dc236 100644 --- a/tests/unit/keyinput/test_keyutils.py +++ b/tests/unit/keyinput/test_keyutils.py @@ -31,6 +31,16 @@ from qutebrowser.keyinput import keyutils from qutebrowser.utils import utils +pyqt_enum_workaround_skip = pytest.mark.skipif( + isinstance(keyutils._NIL_KEY, int), + reason="Can't create QKey for unknown keys with this PyQt version" +) +try: + OE_KEY = Qt.Key(ord('Œ')) +except ValueError: + OE_KEY = None # affected tests skipped + + @pytest.fixture(params=key_data.KEYS, ids=lambda k: k.attribute) def qt_key(request): """Get all existing keys from key_data.py. @@ -156,10 +166,14 @@ class TestKeyToString: (Qt.Key.Key_A, Qt.KeyboardModifier.ControlModifier | Qt.KeyboardModifier.AltModifier | Qt.KeyboardModifier.MetaModifier | Qt.KeyboardModifier.ShiftModifier, ''), - (ord('Œ'), Qt.KeyboardModifier.NoModifier, '<Œ>'), - (ord('Œ'), Qt.KeyboardModifier.ShiftModifier, ''), - (ord('Œ'), Qt.KeyboardModifier.GroupSwitchModifier, ''), - (ord('Œ'), Qt.KeyboardModifier.GroupSwitchModifier | Qt.KeyboardModifier.ShiftModifier, ''), + + pytest.param(OE_KEY, Qt.KeyboardModifier.NoModifier, '<Œ>', + marks=pyqt_enum_workaround_skip), + pytest.param(OE_KEY, Qt.KeyboardModifier.ShiftModifier, '', + marks=pyqt_enum_workaround_skip), + pytest.param(OE_KEY, Qt.KeyboardModifier.GroupSwitchModifier, '', + marks=pyqt_enum_workaround_skip), + pytest.param(OE_KEY, Qt.KeyboardModifier.GroupSwitchModifier | Qt.KeyboardModifier.ShiftModifier, ''), (Qt.Key.Key_Shift, Qt.KeyboardModifier.ShiftModifier, ''), (Qt.Key.Key_Shift, Qt.KeyboardModifier.ShiftModifier | Qt.KeyboardModifier.ControlModifier, ''), @@ -212,10 +226,10 @@ def test_surrogates(key, modifiers, text, expected, pyqt_enum_workaround): ([Qt.Key.Key_Shift, 0x29df6], '<𩷶>'), ([0x1f468, 0x200d, 0x1f468, 0x200d, 0x1f466], '<👨><‍><👨><‍><👦>'), ]) -def test_surrogate_sequences(keys, expected, pyqt_enum_workaround): - infos = [keyutils.KeyInfo(key) for key in keys] - with pyqt_enum_workaround(keyutils.KeyParseError): - seq = keyutils.KeySequence(*infos) +@pyqt_enum_workaround_skip +def test_surrogate_sequences(keys, expected): + infos = [keyutils.KeyInfo(Qt.Key(key)) for key in keys] + seq = keyutils.KeySequence(*infos) assert str(seq) == expected @@ -590,7 +604,8 @@ def test_key_info_to_qt(): (Qt.Key.Key_Return, False), (Qt.Key.Key_Enter, False), (Qt.Key.Key_Space, False), - (0x0, False), # Used by Qt for unknown keys + # Used by Qt for unknown keys + pytest.param(keyutils._NIL_KEY, False, marks=pyqt_enum_workaround_skip), (Qt.Key.Key_ydiaeresis, True), (Qt.Key.Key_X, True), -- cgit v1.2.3-54-g00ecf