diff --git a/src/app_model/backends/qt/_qaction.py b/src/app_model/backends/qt/_qaction.py index 7b23eba9..74c2e80b 100644 --- a/src/app_model/backends/qt/_qaction.py +++ b/src/app_model/backends/qt/_qaction.py @@ -2,6 +2,7 @@ import contextlib from typing import TYPE_CHECKING, ClassVar, Mapping +from weakref import WeakValueDictionary from app_model import Application from app_model.expressions import Expr @@ -13,6 +14,7 @@ if TYPE_CHECKING: from PyQt6.QtGui import QAction from qtpy.QtCore import QObject + from typing_extensions import Self from app_model.types import CommandRule, MenuItem else: @@ -118,52 +120,64 @@ class QMenuItemAction(QCommandRuleAction): Mostly the same as a `CommandRuleAction`, but aware of the `menu_item.when` clause to toggle visibility. - """ - - _cache: ClassVar[dict[tuple[int, int], QMenuItemAction]] = {} - def __new__( - cls: type[QMenuItemAction], - menu_item: MenuItem, - app: Application | str, - parent: QObject | None = None, - *, - cache: bool = True, - ) -> QMenuItemAction: - """Create and cache a QMenuItemAction for the given menu item.""" - app = Application.get_or_create(app) if isinstance(app, str) else app - key = (id(app), hash(menu_item)) - if cache and key in cls._cache: - return cls._cache[key] + Parameters + ---------- + menu_item : MenuItem + `MenuItem` instance to create an action for. + app : Application | str + Application instance or name of application instance. + parent : QObject | None + Optional parent widget, by default None + """ - self = super().__new__(cls) - if cache: - cls._cache[key] = self - return self # type: ignore [no-any-return] + _cache: ClassVar[WeakValueDictionary[tuple[int, int], QMenuItemAction]] = ( + WeakValueDictionary() + ) def __init__( self, menu_item: MenuItem, app: Application | str, parent: QObject | None = None, - *, - cache: bool = True, # used in __new__ ): - initialized = False - with contextlib.suppress(RuntimeError): - initialized = getattr(self, "_initialized", False) - - if not initialized: - super().__init__(menu_item.command, app, parent) - self._menu_item = menu_item - key = (id(self._app), hash(menu_item)) - self.destroyed.connect(lambda: QMenuItemAction._cache.pop(key, None)) - self._app.destroyed.connect(lambda: QMenuItemAction._cache.pop(key, None)) - self._initialized = True - + super().__init__(menu_item.command, app, parent) + self._menu_item = menu_item with contextlib.suppress(NameError): self.update_from_context(self._app.context) + @staticmethod + def _cache_key(app: Application, menu_item: MenuItem) -> tuple[int, int]: + return (id(app), hash(menu_item)) + + @classmethod + def create( + cls, + menu_item: MenuItem, + app: Application | str, + parent: QObject | None = None, + ) -> Self: + """Create a new QMenuItemAction for the given menu item. + + Prefer this method over `__init__` to ensure that the cache is used, + so that: + + ```python + a1 = QMenuItemAction.create(action, full_app) + a2 = QMenuItemAction.create(action, full_app) + a1 is a2 # True + ``` + """ + app = Application.get_or_create(app) if isinstance(app, str) else app + cache_key = QMenuItemAction._cache_key(app, menu_item) + if cache_key in cls._cache: + res = cls._cache[cache_key] + res.setParent(parent) + return res + + cls._cache[cache_key] = obj = cls(menu_item, app, parent) + return obj + def update_from_context(self, ctx: Mapping[str, object]) -> None: """Update the enabled/visible state of this menu item from `ctx`.""" super().update_from_context(ctx) diff --git a/src/app_model/backends/qt/_qmenu.py b/src/app_model/backends/qt/_qmenu.py index 6b88ed0a..0ddc4068 100644 --- a/src/app_model/backends/qt/_qmenu.py +++ b/src/app_model/backends/qt/_qmenu.py @@ -305,7 +305,10 @@ def _rebuild( exclude: Collection[str] | None = None, ) -> None: """Rebuild menu by looking up `menu` in `Application`'s menu_registry.""" - menu.clear() + actions = menu.actions() + for action in actions: + menu.removeAction(action) + _exclude = exclude or set() groups = list(app.menus.iter_menu_groups(menu_id)) @@ -317,7 +320,7 @@ def _rebuild( submenu = QModelSubmenu(item, app, parent=menu) cast("QMenu", menu).addMenu(submenu) elif item.command.id not in _exclude: - action = QMenuItemAction(item, app=app, parent=menu) + action = QMenuItemAction.create(item, app=app, parent=menu) menu.addAction(action) if n < n_groups - 1: menu.addSeparator() diff --git a/tests/test_qt/test_qactions.py b/tests/test_qt/test_qactions.py index 8c82b1da..630d4346 100644 --- a/tests/test_qt/test_qactions.py +++ b/tests/test_qt/test_qactions.py @@ -13,8 +13,8 @@ def test_cache_qaction(qapp, full_app: "FullApp") -> None: action = next( i for k, items in full_app.menus for i in items if isinstance(i, MenuItem) ) - a1 = QMenuItemAction(action, full_app) - a2 = QMenuItemAction(action, full_app) + a1 = QMenuItemAction.create(action, full_app) + a2 = QMenuItemAction.create(action, full_app) assert a1 is a2 assert repr(a1).startswith("QMenuItemAction")