fix: Fix recursion check in Qt6

.github/workflows/ci.yml

@@ -54,9 +54,6 @@ jobs:
           - python-version: "3.10"
             os: "ubuntu-latest"
             qt: "PySide2~=5.15.0"
-          - python-version: "3.10"
-            os: "ubuntu-latest"
-            qt: "PyQt6~=6.2.0"
           - python-version: "3.10"
             os: "ubuntu-latest"
             qt: "PySide6~=6.3.0"

src/app_model/backends/qt/_qmenu.py

@@ -71,9 +71,7 @@ def findAction(self, object_name: str) -> QAction | QModelMenu | None:
         """
         return _find_action(self.actions(), object_name)
 
-    def update_from_context(
-        self, ctx: Mapping[str, object], _recurse: bool = True
-    ) -> None:
+    def update_from_context(self, ctx: Mapping[str, object]) -> None:
         """Update the enabled/visible state of each menu item with `ctx`.
 
         See `app_model.expressions` for details on expressions.
@@ -85,10 +83,8 @@ def update_from_context(
             `'when'` expressions provided for each action in the menu.
             *ALL variables used in these expressions must either be present in
             the `ctx` dict, or be builtins*.
-        _recurse : bool
-            recursion check, internal use only
         """
-        _update_from_context(self.actions(), ctx, _recurse=_recurse)
+        _update_from_context(self.actions(), ctx)
 
     def rebuild(
         self, include_submenus: bool = True, exclude: Collection[str] | None = None
@@ -145,11 +141,9 @@ def __init__(
         if submenu.icon:
             self.setIcon(to_qicon(submenu.icon))
 
-    def update_from_context(
-        self, ctx: Mapping[str, object], _recurse: bool = True
-    ) -> None:
+    def update_from_context(self, ctx: Mapping[str, object]) -> None:
         """Update the enabled state of this menu item from `ctx`."""
-        super().update_from_context(ctx, _recurse=_recurse)
+        super().update_from_context(ctx)
         self.setEnabled(expr.eval(ctx) if (expr := self._submenu.enablement) else True)
         # TODO: ... visibility needs to be controlled at the level of placement
         # in the submenu.  consider only using the `when` expression
@@ -213,9 +207,7 @@ def findAction(self, object_name: str) -> QAction | QModelMenu | None:
         """
         return _find_action(self.actions(), object_name)
 
-    def update_from_context(
-        self, ctx: Mapping[str, object], _recurse: bool = True
-    ) -> None:
+    def update_from_context(self, ctx: Mapping[str, object]) -> None:
         """Update the enabled/visible state of each menu item with `ctx`.
 
         See `app_model.expressions` for details on expressions.
@@ -227,10 +219,8 @@ def update_from_context(
             `'when'` expressions provided for each action in the menu.
             *ALL variables used in these expressions must either be present in
             the `ctx` dict, or be builtins*.
-        _recurse : bool
-            recursion check, internal use only
         """
-        _update_from_context(self.actions(), ctx, _recurse=_recurse)
+        _update_from_context(self.actions(), ctx)
 
     def rebuild(
         self, include_submenus: bool = True, exclude: Collection[str] | None = None
@@ -278,9 +268,7 @@ def __init__(
             id_, title = item if isinstance(item, tuple) else (item, item.title())
             self.addMenu(QModelMenu(id_, app, title, self))
 
-    def update_from_context(
-        self, ctx: Mapping[str, object], _recurse: bool = True
-    ) -> None:
+    def update_from_context(self, ctx: Mapping[str, object]) -> None:
         """Update the enabled/visible state of each menu item with `ctx`.
 
         See `app_model.expressions` for details on expressions.
@@ -292,10 +280,8 @@ def update_from_context(
             `'when'` expressions provided for each action in the menu.
             *ALL variables used in these expressions must either be present in
             the `ctx` dict, or be builtins*.
-        _recurse : bool
-            recursion check, internal use only
         """
-        _update_from_context(self.actions(), ctx, _recurse=_recurse)
+        _update_from_context(self.actions(), ctx)
 
 
 def _rebuild(
@@ -327,9 +313,7 @@ def _rebuild(
             menu.addSeparator()
 
 
-def _update_from_context(
-    actions: Iterable[QAction], ctx: Mapping[str, object], _recurse: bool = True
-) -> None:
+def _update_from_context(actions: Iterable[QAction], ctx: Mapping[str, object]) -> None:
     """Update the enabled/visible state of each menu item with `ctx`.
 
     See `app_model.expressions` for details on expressions.
@@ -343,23 +327,15 @@ def _update_from_context(
         `'when'` expressions provided for each action in the menu.
         *ALL variables used in these expressions must either be present in
         the `ctx` dict, or be builtins*.
-    _recurse : bool
-        recursion check, internal use only
     """
-    for action in actions:
-        if isinstance(action, QMenuItemAction):
-            action.update_from_context(ctx)
-        elif not QT6 and isinstance(menu := action.menu(), QModelMenu):
-            menu.update_from_context(ctx)
-        elif isinstance(parent := action.parent(), QModelMenu):
-            # FIXME: this is a hack for Qt6 that I don't entirely understand.
-            # QAction has lost the `.menu()` method, and it's a bit hard to find
-            # how to get to the parent menu now. Checking parent() seems to work,
-            # but I'm not sure if it's the right thing to do, and it leads to a
-            # recursion error.  I stop it with the _recurse flag here, but I wonder
-            # whether that will cause other problems.
-            if _recurse:
-                parent.update_from_context(ctx, _recurse=False)
+    try:
+        for action in actions:
+            if isinstance(action, QMenuItemAction):
+                action.update_from_context(ctx)
+            elif isinstance(menu := action.menu(), QModelMenu):
+                menu.update_from_context(ctx)
+    except AttributeError as e:  # pragma: no cover
+        raise AttributeError(f"This version of Qt is not supported: {e}") from e
 
 
 def _find_action(actions: Iterable[QAction], object_name: str) -> QAction | None:

tests/conftest.py

@@ -186,6 +186,7 @@ def build_app(name: str = "complete_test_app") -> FullApp:
             title="Open from B",
             callback=app.mocks.open_from_b,
             menus=[{"id": Menus.FILE_OPEN_FROM}],
+            enablement="sat",
         ),
         Action(
             id=Commands.UNIMPORTABLE,

tests/test_qt/test_qmenu.py

@@ -77,6 +77,9 @@ def test_submenu(qtbot: QtBot, full_app: FullApp) -> None:
     assert menu_texts == ["Open From...", "Open..."]
 
     submenu = menu.findChild(QModelMenu, app.Menus.FILE_OPEN_FROM)
+    submenu_action = submenu.findAction(app.Commands.OPEN_FROM_B)
+
+    assert submenu_action
     assert isinstance(submenu, QModelMenu)
     submenu.setVisible(True)
     assert submenu.isVisible()
@@ -86,21 +89,29 @@ def test_submenu(qtbot: QtBot, full_app: FullApp) -> None:
     # "not something_open" is the when clause
     # "friday" is the enablement clause
 
-    menu.update_from_context({"something_open": False, "friday": True})
+    menu.update_from_context({"something_open": False, "friday": True, "sat": True})
     assert submenu.isVisible()
     assert submenu.isEnabled()
+    assert submenu_action.isVisible()
+    assert submenu_action.isEnabled()
 
-    menu.update_from_context({"something_open": False, "friday": False})
+    menu.update_from_context({"something_open": False, "friday": False, "sat": True})
     assert submenu.isVisible()
     assert not submenu.isEnabled()
+    assert submenu_action.isVisible()
+    assert submenu_action.isEnabled()
 
-    menu.update_from_context({"something_open": True, "friday": False})
-    # assert not submenu.isVisible()
+    menu.update_from_context({"something_open": True, "friday": False, "sat": True})
     assert not submenu.isEnabled()
+    assert submenu_action.isEnabled()
+
+    menu.update_from_context({"something_open": True, "friday": True, "sat": True})
+    assert submenu.isEnabled()
+    assert submenu_action.isEnabled()
 
-    menu.update_from_context({"something_open": True, "friday": True})
-    # assert not submenu.isVisible()
+    menu.update_from_context({"something_open": True, "friday": True, "sat": False})
     assert submenu.isEnabled()
+    assert not submenu_action.isEnabled()
 
 
 @pytest.mark.filterwarnings("ignore:QPixmapCache.find:")