feat: Update of shortcuts on menu rebuild (#258)

* add update of shortcuts on menu rebuild

* pytest.mark.usefixtures

* test draft

* Fix code to deturn proper tooltip and sort keybinding

* fix using text as tooltip

* fix pyright

* improve tests

* Update src/app_model/registries/_keybindings_reg.py

Co-authored-by: Talley Lambert <[email protected]>

* fix code

* Update src/app_model/backends/qt/_qaction.py

Co-authored-by: Talley Lambert <[email protected]>

---------

Co-authored-by: Talley Lambert <[email protected]>

Author: Czaki

src/app_model/backends/qt/_qaction.py

@@ -4,6 +4,8 @@
 from typing import TYPE_CHECKING, ClassVar
 from weakref import WeakValueDictionary
 
+from qtpy.QtGui import QKeySequence
+
 from app_model import Application
 from app_model.expressions import Expr
 from app_model.types import ToggleRule
@@ -52,6 +54,15 @@ def __init__(
             self._keybinding_tooltip = f"({kb.keybinding.to_text()})"
         self.triggered.connect(self._on_triggered)
 
+    def _update_keybinding(self) -> None:
+        shortcut = self.shortcut()
+        if kb := self._app.keybindings.get_keybinding(self._command_id):
+            self.setShortcut(QKeyBindingSequence(kb.keybinding))
+            self._keybinding_tooltip = f"({kb.keybinding.to_text()})"
+        elif not shortcut.isEmpty():
+            self.setShortcut(QKeySequence())
+            self._keybinding_tooltip = ""
+
     def _on_triggered(self, checked: bool) -> None:
         # execute_command returns a Future, for the sake of eventually being
         # asynchronous without breaking the API.  For now, we call result()
@@ -84,23 +95,29 @@ def __init__(
     ) -> None:
         super().__init__(command_rule.id, app, parent)
         self._cmd_rule = command_rule
+        self._tooltip = command_rule.tooltip or ""
         if use_short_title and command_rule.short_title:
             self.setText(command_rule.short_title)  # pragma: no cover
         else:
             self.setText(command_rule.title)
         if command_rule.icon:
             self.setIcon(to_qicon(command_rule.icon))
         self.setIconVisibleInMenu(command_rule.icon_visible_in_menu)
-        if command_rule.tooltip:
-            self.setToolTip(command_rule.tooltip)
         if command_rule.status_tip:
             self.setStatusTip(command_rule.status_tip)
         if command_rule.toggled is not None:
             self.setCheckable(True)
             self._refresh()
-        tooltip_with_keybinding = (
-            f"{self.toolTip()} {self._keybinding_tooltip}".rstrip()
-        )
+        tooltip_with_keybinding = f"{self._tooltip} {self._keybinding_tooltip}".rstrip()
+        self.setToolTip(tooltip_with_keybinding)
+
+    def setText(self, text: str | None) -> None:
+        super().setText(text)
+        self._tooltip = self._tooltip or text or ""
+
+    def _update_keybinding(self) -> None:
+        super()._update_keybinding()
+        tooltip_with_keybinding = f"{self._tooltip} {self._keybinding_tooltip}".rstrip()
         self.setToolTip(tooltip_with_keybinding)
 
     def update_from_context(self, ctx: Mapping[str, object]) -> None:
@@ -176,6 +193,7 @@ def create(
         cache_key = QMenuItemAction._cache_key(app, menu_item)
         if cache_key in cls._cache:
             res = cls._cache[cache_key]
+            res._update_keybinding()
             res.setParent(parent)
             return res
 

src/app_model/registries/_keybindings_reg.py

@@ -194,10 +194,9 @@ def __repr__(self) -> str:
     def get_keybinding(self, command_id: str) -> _RegisteredKeyBinding | None:
         """Return the first keybinding that matches the given command ID."""
         # TODO: improve me.
-        return next(
-            (entry for entry in self._keybindings if entry.command_id == command_id),
-            None,
-        )
+        matches = (kb for kb in self._keybindings if kb.command_id == command_id)
+        sorted_matches = sorted(matches, key=lambda x: x.source, reverse=True)
+        return next(iter(sorted_matches), None)
 
     def get_context_prioritized_keybinding(
         self, key: int, context: Mapping[str, object]

tests/test_qt/test_qactions.py

@@ -8,6 +8,7 @@
     Action,
     CommandRule,
     KeyBindingRule,
+    KeyBindingSource,
     KeyCode,
     MenuItem,
     ToggleRule,
@@ -18,7 +19,8 @@
     from conftest import FullApp
 
 
-def test_cache_qaction(qapp, full_app: "FullApp") -> None:
+@pytest.mark.usefixtures("qapp")
+def test_cache_qaction(full_app: "FullApp") -> None:
     action = next(
         i for k, items in full_app.menus for i in items if isinstance(i, MenuItem)
     )
@@ -28,7 +30,8 @@ def test_cache_qaction(qapp, full_app: "FullApp") -> None:
     assert repr(a1).startswith("QMenuItemAction")
 
 
-def test_toggle_qaction(qapp, simple_app: "Application") -> None:
+@pytest.mark.usefixtures("qapp")
+def test_toggle_qaction(simple_app: "Application") -> None:
     mock = Mock()
     x = False
 
@@ -75,6 +78,7 @@ def test_icon_visible_in_menu(qapp, simple_app: "Application") -> None:
     assert not q_action.isIconVisibleInMenu()
 
 
+@pytest.mark.usefixtures("qapp")
 @pytest.mark.parametrize(
     ("tooltip", "expected_tooltip"),
     [
@@ -83,7 +87,7 @@ def test_icon_visible_in_menu(qapp, simple_app: "Application") -> None:
     ],
 )
 def test_tooltip(
-    qapp, simple_app: "Application", tooltip: str, expected_tooltip: str
+    simple_app: "Application", tooltip: str, expected_tooltip: str
 ) -> None:
     action = Action(
         id="test.tooltip", title="Test tooltip", tooltip=tooltip, callback=lambda: None
@@ -93,6 +97,7 @@ def test_tooltip(
     assert q_action.toolTip() == expected_tooltip
 
 
+@pytest.mark.usefixtures("qapp")
 @pytest.mark.parametrize(
     ("tooltip", "tooltip_with_keybinding", "tooltip_without_keybinding"),
     [
@@ -105,7 +110,6 @@ def test_tooltip(
     ],
 )
 def test_keybinding_in_tooltip(
-    qapp,
     simple_app: "Application",
     tooltip: str,
     tooltip_with_keybinding: str,
@@ -127,3 +131,36 @@ def test_keybinding_in_tooltip(
     # check setting tooltip manually removes keybinding info
     q_action.setToolTip(tooltip)
     assert q_action.toolTip() == tooltip_without_keybinding
+
+
+@pytest.mark.usefixtures("qapp")
+def test_update_keybinding_in_tooltip(
+    simple_app: "Application",
+) -> None:
+    action = Action(
+        id="test.update.keybinding.tooltip",
+        title="Test update keybinding tooltip",
+        callback=lambda: None,
+        tooltip="Initial tooltip",
+        keybindings=[KeyBindingRule(primary=KeyCode.KeyK)],
+    )
+    dispose1 = simple_app.register_action(action)
+
+    q_action = QCommandRuleAction(action, simple_app)
+    assert q_action.toolTip() == "Initial tooltip (K)"
+
+    # Update the keybinding
+    dispose2 = simple_app.keybindings.register_keybinding_rule(
+        "test.update.keybinding.tooltip",
+        KeyBindingRule(primary=KeyCode.KeyL, source=KeyBindingSource.USER),
+    )
+    q_action._update_keybinding()
+    assert q_action.toolTip() == "Initial tooltip (L)"
+
+    dispose2()
+    q_action._update_keybinding()
+    assert q_action.toolTip() == "Initial tooltip (K)"
+
+    dispose1()
+    q_action._update_keybinding()
+    assert q_action.toolTip() == "Initial tooltip"