From 09abb3d596d426fe5fca0243ff75bdab9184959c Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Wed, 6 Aug 2025 15:06:00 +0200 Subject: [PATCH 01/10] add update of shortcuts on menu rebuild --- src/app_model/backends/qt/_qaction.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/app_model/backends/qt/_qaction.py b/src/app_model/backends/qt/_qaction.py index 9dd5a65..cc6e7d2 100644 --- a/src/app_model/backends/qt/_qaction.py +++ b/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() @@ -103,6 +114,13 @@ def __init__( ) self.setToolTip(tooltip_with_keybinding) + 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: """Update the enabled state of this menu item from `ctx`.""" self.setEnabled(expr.eval(ctx) if (expr := self._cmd_rule.enablement) else True) @@ -176,6 +194,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 From 80f298ecf8ac63aded30bab8ca3c36c11a502ea9 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Wed, 6 Aug 2025 15:23:22 +0200 Subject: [PATCH 02/10] pytest.mark.usefixtures --- tests/test_qt/test_qactions.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/test_qt/test_qactions.py b/tests/test_qt/test_qactions.py index 3c8a160..f1dc42f 100644 --- a/tests/test_qt/test_qactions.py +++ b/tests/test_qt/test_qactions.py @@ -18,7 +18,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 +29,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 +77,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 +86,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 +96,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 +109,6 @@ def test_tooltip( ], ) def test_keybinding_in_tooltip( - qapp, simple_app: "Application", tooltip: str, tooltip_with_keybinding: str, From ec859d4e82fd7746b9c88a5d7dc16da911964ca9 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Wed, 6 Aug 2025 15:39:09 +0200 Subject: [PATCH 03/10] test draft --- tests/test_qt/test_qactions.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/test_qt/test_qactions.py b/tests/test_qt/test_qactions.py index f1dc42f..c046ddf 100644 --- a/tests/test_qt/test_qactions.py +++ b/tests/test_qt/test_qactions.py @@ -130,3 +130,27 @@ 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)], + ) + simple_app.register_action(action) + + q_action = QCommandRuleAction(action, simple_app) + assert q_action.toolTip() == "Initial tooltip (K)" + + # Update the keybinding + simple_app.keybindings.register_keybinding_rule( + "test.update.keybinding.tooltip", KeyBindingRule(primary=KeyCode.KeyL) + ) + q_action._update_keybinding() + assert q_action.toolTip() == "Initial tooltip (L)" From 285c58d999b7c66bdc1ca44a3ddb496913182929 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Thu, 7 Aug 2025 09:55:58 +0200 Subject: [PATCH 04/10] Fix code to deturn proper tooltip and sort keybinding --- src/app_model/backends/qt/_qaction.py | 12 ++++-------- src/app_model/registries/_keybindings_reg.py | 13 ++++++++++++- tests/test_qt/test_qactions.py | 4 +++- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/app_model/backends/qt/_qaction.py b/src/app_model/backends/qt/_qaction.py index cc6e7d2..d67374e 100644 --- a/src/app_model/backends/qt/_qaction.py +++ b/src/app_model/backends/qt/_qaction.py @@ -95,6 +95,7 @@ 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: @@ -102,23 +103,18 @@ def __init__( 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) + self.setToolTip(self._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 _update_keybinding(self) -> None: super()._update_keybinding() - 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 update_from_context(self, ctx: Mapping[str, object]) -> None: diff --git a/src/app_model/registries/_keybindings_reg.py b/src/app_model/registries/_keybindings_reg.py index 1e04dc3..f2dc76f 100644 --- a/src/app_model/registries/_keybindings_reg.py +++ b/src/app_model/registries/_keybindings_reg.py @@ -2,6 +2,7 @@ from bisect import insort_left from collections import defaultdict +from operator import attrgetter from typing import TYPE_CHECKING, Callable, NamedTuple from psygnal import Signal @@ -195,7 +196,17 @@ 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), + iter( + sorted( + ( + entry + for entry in self._keybindings + if entry.command_id == command_id + ), + key=attrgetter("source"), + reverse=True, + ) + ), None, ) diff --git a/tests/test_qt/test_qactions.py b/tests/test_qt/test_qactions.py index c046ddf..cf4deaa 100644 --- a/tests/test_qt/test_qactions.py +++ b/tests/test_qt/test_qactions.py @@ -8,6 +8,7 @@ Action, CommandRule, KeyBindingRule, + KeyBindingSource, KeyCode, MenuItem, ToggleRule, @@ -150,7 +151,8 @@ def test_update_keybinding_in_tooltip( # Update the keybinding simple_app.keybindings.register_keybinding_rule( - "test.update.keybinding.tooltip", KeyBindingRule(primary=KeyCode.KeyL) + "test.update.keybinding.tooltip", + KeyBindingRule(primary=KeyCode.KeyL, source=KeyBindingSource.USER), ) q_action._update_keybinding() assert q_action.toolTip() == "Initial tooltip (L)" From edf4b80943ff8eb9b58650e29b6fb384eaec6179 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Thu, 7 Aug 2025 10:32:01 +0200 Subject: [PATCH 05/10] fix using text as tooltip --- src/app_model/backends/qt/_qaction.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/app_model/backends/qt/_qaction.py b/src/app_model/backends/qt/_qaction.py index d67374e..b633d11 100644 --- a/src/app_model/backends/qt/_qaction.py +++ b/src/app_model/backends/qt/_qaction.py @@ -103,7 +103,8 @@ def __init__( if command_rule.icon: self.setIcon(to_qicon(command_rule.icon)) self.setIconVisibleInMenu(command_rule.icon_visible_in_menu) - self.setToolTip(self._tooltip) + if self._tooltip: + self.setToolTip(self._tooltip) if command_rule.status_tip: self.setStatusTip(command_rule.status_tip) if command_rule.toggled is not None: @@ -112,6 +113,10 @@ def __init__( tooltip_with_keybinding = f"{self._tooltip} {self._keybinding_tooltip}".rstrip() self.setToolTip(tooltip_with_keybinding) + def setText(self, text: str) -> None: + super().setText(text) + self._tooltip = self._tooltip or text + def _update_keybinding(self) -> None: super()._update_keybinding() tooltip_with_keybinding = f"{self._tooltip} {self._keybinding_tooltip}".rstrip() From 81c8f92100f50440ec10e16d2bd5b858bbb9de6a Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Thu, 7 Aug 2025 10:34:18 +0200 Subject: [PATCH 06/10] fix pyright --- src/app_model/backends/qt/_qaction.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app_model/backends/qt/_qaction.py b/src/app_model/backends/qt/_qaction.py index b633d11..30648e9 100644 --- a/src/app_model/backends/qt/_qaction.py +++ b/src/app_model/backends/qt/_qaction.py @@ -113,9 +113,9 @@ def __init__( tooltip_with_keybinding = f"{self._tooltip} {self._keybinding_tooltip}".rstrip() self.setToolTip(tooltip_with_keybinding) - def setText(self, text: str) -> None: + def setText(self, text: str | None) -> None: super().setText(text) - self._tooltip = self._tooltip or text + self._tooltip = self._tooltip or text or "" def _update_keybinding(self) -> None: super()._update_keybinding() From 3bab6303616ef4269eec9856def0f8b3906a3a59 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Thu, 7 Aug 2025 10:59:28 +0200 Subject: [PATCH 07/10] improve tests --- tests/test_qt/test_qactions.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/test_qt/test_qactions.py b/tests/test_qt/test_qactions.py index cf4deaa..e27d37c 100644 --- a/tests/test_qt/test_qactions.py +++ b/tests/test_qt/test_qactions.py @@ -144,15 +144,23 @@ def test_update_keybinding_in_tooltip( tooltip="Initial tooltip", keybindings=[KeyBindingRule(primary=KeyCode.KeyK)], ) - simple_app.register_action(action) + dispose1 = simple_app.register_action(action) q_action = QCommandRuleAction(action, simple_app) assert q_action.toolTip() == "Initial tooltip (K)" # Update the keybinding - simple_app.keybindings.register_keybinding_rule( + 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" From 94b694cf3836e3114c2117ac41ec18d1ec3e9291 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Tue, 12 Aug 2025 20:50:25 +0200 Subject: [PATCH 08/10] Update src/app_model/registries/_keybindings_reg.py Co-authored-by: Talley Lambert --- src/app_model/registries/_keybindings_reg.py | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/app_model/registries/_keybindings_reg.py b/src/app_model/registries/_keybindings_reg.py index f2dc76f..ce1b03d 100644 --- a/src/app_model/registries/_keybindings_reg.py +++ b/src/app_model/registries/_keybindings_reg.py @@ -196,19 +196,9 @@ def get_keybinding(self, command_id: str) -> _RegisteredKeyBinding | None: """Return the first keybinding that matches the given command ID.""" # TODO: improve me. return next( - iter( - sorted( - ( - entry - for entry in self._keybindings - if entry.command_id == command_id - ), - key=attrgetter("source"), - reverse=True, - ) - ), - None, - ) + matches = (kb for kb in self._keybindings if kb.command_id == command_id) + sorted_matches = sorted(matches, key=attrgetter("source"), reverse=True) + return next(iter(sorted_matches), None) def get_context_prioritized_keybinding( self, key: int, context: Mapping[str, object] From 371027f6278501451e03c9036ba9a9f4d7630d6d Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Tue, 12 Aug 2025 20:54:48 +0200 Subject: [PATCH 09/10] fix code --- src/app_model/registries/_keybindings_reg.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/app_model/registries/_keybindings_reg.py b/src/app_model/registries/_keybindings_reg.py index ce1b03d..f21736e 100644 --- a/src/app_model/registries/_keybindings_reg.py +++ b/src/app_model/registries/_keybindings_reg.py @@ -2,7 +2,6 @@ from bisect import insort_left from collections import defaultdict -from operator import attrgetter from typing import TYPE_CHECKING, Callable, NamedTuple from psygnal import Signal @@ -195,9 +194,8 @@ 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( matches = (kb for kb in self._keybindings if kb.command_id == command_id) - sorted_matches = sorted(matches, key=attrgetter("source"), reverse=True) + sorted_matches = sorted(matches, key=lambda x: x.source, reverse=True) return next(iter(sorted_matches), None) def get_context_prioritized_keybinding( From 18b6b52d5d2017d0f89b4cafb10dcf58d7a09eb7 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Tue, 12 Aug 2025 21:03:46 +0200 Subject: [PATCH 10/10] Update src/app_model/backends/qt/_qaction.py Co-authored-by: Talley Lambert --- src/app_model/backends/qt/_qaction.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/app_model/backends/qt/_qaction.py b/src/app_model/backends/qt/_qaction.py index 30648e9..fbec462 100644 --- a/src/app_model/backends/qt/_qaction.py +++ b/src/app_model/backends/qt/_qaction.py @@ -103,8 +103,6 @@ def __init__( if command_rule.icon: self.setIcon(to_qicon(command_rule.icon)) self.setIconVisibleInMenu(command_rule.icon_visible_in_menu) - if self._tooltip: - self.setToolTip(self._tooltip) if command_rule.status_tip: self.setStatusTip(command_rule.status_tip) if command_rule.toggled is not None: