From 3cf855ee19787000f7f363a7cf7c592e52085772 Mon Sep 17 00:00:00 2001 From: Lucy Liu Date: Fri, 19 Jul 2024 10:57:23 +1000 Subject: [PATCH] feat: Add `filter_keybinding` to `KeyBindingRegistry` (#212) * add filter * style: [pre-commit.ci] auto fixes [...] * typo * cruft * fix type * reivew * style: [pre-commit.ci] auto fixes [...] * fix * fix mac --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- src/app_model/registries/_keybindings_reg.py | 40 +++++++++- tests/test_registries.py | 79 +++++++++++++++++++- 2 files changed, 115 insertions(+), 4 deletions(-) diff --git a/src/app_model/registries/_keybindings_reg.py b/src/app_model/registries/_keybindings_reg.py index 6549c98..318383c 100644 --- a/src/app_model/registries/_keybindings_reg.py +++ b/src/app_model/registries/_keybindings_reg.py @@ -26,12 +26,33 @@ class _RegisteredKeyBinding(NamedTuple): class KeyBindingsRegistry: - """Registry for keybindings.""" + """Registry for keybindings. + + Attributes + ---------- + filter_keybinding : Callable[[KeyBinding], str] | None + Optional function for applying additional `KeyBinding` filtering. + Callable should accept a `KeyBinding` object and return an error message + (`str`) if `KeyBinding` is rejected, or empty string otherwise. + """ registered = Signal() def __init__(self) -> None: self._keybindings: list[_RegisteredKeyBinding] = [] + self._filter_keybinding: Callable[[KeyBinding], str] | None = None + + @property + def filter_keybinding(self) -> Callable[[KeyBinding], str] | None: + """Return the `filter_keybinding`.""" + return self._filter_keybinding + + @filter_keybinding.setter + def filter_keybinding(self, value: Callable[[KeyBinding], str] | None) -> None: + if callable(value) or value is None: + self._filter_keybinding = value + else: + raise TypeError("'filter_keybinding' must be a callable or None") def register_action_keybindings(self, action: Action) -> DisposeCallable | None: """Register all keybindings declared in `action.keybindings`. @@ -51,6 +72,7 @@ def register_action_keybindings(self, action: Action) -> DisposeCallable | None: return None disposers: list[Callable[[], None]] = [] + msg: list[str] = [] for keyb in keybindings: if action.enablement is not None: kwargs = keyb.model_dump() @@ -62,8 +84,16 @@ def register_action_keybindings(self, action: Action) -> DisposeCallable | None: _keyb = type(keyb)(**kwargs) else: _keyb = keyb - if d := self.register_keybinding_rule(action.id, _keyb): - disposers.append(d) + + try: + if d := self.register_keybinding_rule(action.id, _keyb): + disposers.append(d) + except ValueError as e: + msg.append(str(e)) + if msg: + raise ValueError( + "The following keybindings were not valid:\n" + "\n".join(msg) + ) if not disposers: # pragma: no cover return None @@ -93,6 +123,10 @@ def register_keybinding_rule( """ if plat_keybinding := rule._bind_to_current_platform(): keybinding = KeyBinding.validate(plat_keybinding) + if self._filter_keybinding: + msg = self._filter_keybinding(keybinding) + if msg: + raise ValueError(f"{keybinding}: {msg}") entry = _RegisteredKeyBinding( keybinding=keybinding, command_id=id, diff --git a/tests/test_registries.py b/tests/test_registries.py index c067842..80ef904 100644 --- a/tests/test_registries.py +++ b/tests/test_registries.py @@ -1,5 +1,14 @@ +import pytest + from app_model.registries import KeyBindingsRegistry, MenusRegistry -from app_model.types import MenuItem +from app_model.types import ( + Action, + KeyBinding, + KeyBindingRule, + KeyCode, + KeyMod, + MenuItem, +) def test_menus_registry() -> None: @@ -16,3 +25,71 @@ def test_menus_registry() -> None: def test_keybindings_registry() -> None: reg = KeyBindingsRegistry() assert "(0 bindings)" in repr(reg) + + +def test_register_keybinding_rule_filter_type() -> None: + """Check `_filter_keybinding` type checking when setting.""" + reg = KeyBindingsRegistry() + with pytest.raises(TypeError, match="'filter_keybinding' must be a callable"): + reg.filter_keybinding = "string" + + +def _filter_fun(kb: KeyBinding) -> str: + if kb.part0.is_modifier_key(): + return "modifier only sequences not allowed" + return "" + + +def test_register_keybinding_rule_filter_get() -> None: + """Check `_filter_keybinding` getter.""" + reg = KeyBindingsRegistry() + reg.filter_keybinding = _filter_fun + assert callable(reg.filter_keybinding) + + +def test_register_keybinding_rule_filter() -> None: + """Check `filter_keybinding` in `register_keybinding_rule`.""" + reg = KeyBindingsRegistry() + reg.filter_keybinding = _filter_fun + + # Valid keybinding + kb = KeyBindingRule(primary=KeyMod.CtrlCmd | KeyCode.KeyO) + reg.register_keybinding_rule("test", kb) + # Invalid keybinding + kb = KeyBindingRule(primary=KeyMod.Alt) + with pytest.raises(ValueError, match=r"Alt\+: modifier only"): + reg.register_keybinding_rule("test", kb) + + +@pytest.mark.parametrize( + "kb, msg", + [ + ( + [ + {"primary": KeyMod.CtrlCmd | KeyCode.KeyA}, + {"primary": KeyMod.Shift | KeyCode.KeyC}, + ], + "", + ), + ( + [{"primary": KeyMod.Alt}, {"primary": KeyMod.Shift}], + r"Alt\+: modifier only sequences not allowed\nShift\+: modifier", + ), + ], +) +def test_register_action_keybindings_filter(kb, msg) -> None: + """Check `filter_keybinding` in `register_action_keybindings`.""" + reg = KeyBindingsRegistry() + reg.filter_keybinding = _filter_fun + + action = Action( + id="cmd_id1", + title="title1", + callback=lambda: None, + keybindings=kb, + ) + if msg: + with pytest.raises(ValueError, match=msg): + reg.register_action_keybindings(action) + else: + reg.register_action_keybindings(action)