start migrating perform_op() into builder in separate file

By passing back the builder to the calling code to run, we don't need
to plumb extra arguments like success= and handler= through each
operation, and the ability to override the default tooltip behaviour
comes free on all operations

qt/aqt/browser.py

@@ -695,13 +695,8 @@ where id in %s"""
         if not (tags := tags or self._prompt_for_tags(tr.browsing_enter_tags_to_add())):
             return
         add_tags_to_notes(
-            mw=self.mw,
+            parent=self, note_ids=self.selected_notes(), space_separated_tags=tags
-            note_ids=self.selected_notes(),
+        ).run(handler=self)
-            space_separated_tags=tags,
-            success=lambda out: tooltip(
-                tr.browsing_notes_updated(count=out.count), parent=self
-            ),
-        )
 
     @ensure_editor_saved_on_trigger
     def remove_tags_from_selected_notes(self, tags: Optional[str] = None) -> None:
@@ -710,14 +705,10 @@ where id in %s"""
             tags := tags or self._prompt_for_tags(tr.browsing_enter_tags_to_delete())
         ):
             return
+
         remove_tags_from_notes(
-            mw=self.mw,
+            parent=self, note_ids=self.selected_notes(), space_separated_tags=tags
-            note_ids=self.selected_notes(),
+        ).run(handler=self)
-            space_separated_tags=tags,
-            success=lambda out: tooltip(
-                tr.browsing_notes_updated(count=out.count), parent=self
-            ),
-        )
 
     def _prompt_for_tags(self, prompt: str) -> Optional[str]:
         (tags, ok) = getTag(self, self.col, prompt)
@@ -728,7 +719,7 @@ where id in %s"""
 
     @ensure_editor_saved_on_trigger
     def clear_unused_tags(self) -> None:
-        clear_unused_tags(mw=self.mw, parent=self)
+        clear_unused_tags(parent=self).run()
 
     addTags = add_tags_to_selected_notes
     deleteTags = remove_tags_from_selected_notes

qt/aqt/main.py

@@ -61,6 +61,11 @@ from aqt.emptycards import show_empty_cards
 from aqt.legacy import install_pylib_legacy
 from aqt.mediacheck import check_media_db
 from aqt.mediasync import MediaSyncer
+from aqt.operations import (
+    CollectionOpFailureCallback,
+    CollectionOpSuccessCallback,
+    ResultWithChanges,
+)
 from aqt.operations.collection import undo
 from aqt.profiles import ProfileManager as ProfileManagerType
 from aqt.qt import *
@@ -91,30 +96,6 @@ from aqt.utils import (
     tr,
 )
 
-
-class HasChangesProperty(Protocol):
-    changes: OpChanges
-
-
-# either an OpChanges object, or an object with .changes on it. This bound
-# doesn't actually work for protobuf objects, so new protobuf objects will
-# either need to be added here, or cast at call time
-ResultWithChanges = TypeVar(
-    "ResultWithChanges",
-    bound=Union[
-        OpChanges,
-        OpChangesWithCount,
-        OpChangesWithId,
-        OpChangesAfterUndo,
-        HasChangesProperty,
-    ],
-)
-
-T = TypeVar("T")
-
-PerformOpOptionalSuccessCallback = Optional[Callable[[ResultWithChanges], Any]]
-PerformOpOptionalFailureCallback = Optional[Callable[[Exception], Any]]
-
 install_pylib_legacy()
 
 MainWindowState = Literal[
@@ -122,6 +103,12 @@ MainWindowState = Literal[
 ]
 
 
+T = TypeVar("T")
+
+PerformOpOptionalSuccessCallback = Optional[CollectionOpSuccessCallback]
+PerformOpOptionalFailureCallback = Optional[CollectionOpFailureCallback]
+
+
 class AnkiQt(QMainWindow):
     col: Collection
     pm: ProfileManagerType

qt/aqt/operations/__init__.py

@@ -1,2 +1,133 @@
 # Copyright: Ankitects Pty Ltd and contributors
 # License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+from __future__ import annotations
+
+from concurrent.futures._base import Future
+from typing import Any, Callable, Generic, Optional, Protocol, TypeVar, Union
+
+import aqt
+from anki.collection import (
+    Collection,
+    OpChanges,
+    OpChangesAfterUndo,
+    OpChangesWithCount,
+    OpChangesWithId,
+)
+from aqt.qt import QWidget
+from aqt.utils import showWarning
+
+
+class HasChangesProperty(Protocol):
+    changes: OpChanges
+
+
+# either an OpChanges object, or an object with .changes on it. This bound
+# doesn't actually work for protobuf objects, so new protobuf objects will
+# either need to be added here, or cast at call time
+ResultWithChanges = TypeVar(
+    "ResultWithChanges",
+    bound=Union[
+        OpChanges,
+        OpChangesWithCount,
+        OpChangesWithId,
+        OpChangesAfterUndo,
+        HasChangesProperty,
+    ],
+)
+
+T = TypeVar("T")
+
+CollectionOpSuccessCallback = Callable[[ResultWithChanges], Any]
+CollectionOpFailureCallback = Optional[Callable[[Exception], Any]]
+
+
+class CollectionOp(Generic[ResultWithChanges]):
+    """Helper to perform a mutating DB operation on a background thread, and update UI.
+
+    `op` should either return OpChanges, or an object with a 'changes'
+    property. The changes will be passed to `operation_did_execute` so that
+    the UI can decide whether it needs to update itself.
+
+    - Shows progress popup for the duration of the op.
+    - Ensures the browser doesn't try to redraw during the operation, which can lead
+    to a frozen UI
+    - Updates undo state at the end of the operation
+    - Commits changes
+    - Fires the `operation_(will|did)_reset` hooks
+    - Fires the legacy `state_did_reset` hook
+
+    Be careful not to call any UI routines in `op`, as that may crash Qt.
+    This includes things select .selectedCards() in the browse screen.
+
+    `success` will be called with the return value of op().
+
+    If op() throws an exception, it will be shown in a popup, or
+    passed to `failure` if it is provided.
+    """
+
+    _success: Optional[CollectionOpSuccessCallback] = None
+    _failure: Optional[CollectionOpFailureCallback] = None
+
+    def __init__(self, parent: QWidget, op: Callable[[Collection], ResultWithChanges]):
+        self._parent = parent
+        self._op = op
+
+    def success(self, success: Optional[CollectionOpSuccessCallback]) -> CollectionOp:
+        self._success = success
+        return self
+
+    def failure(self, failure: Optional[CollectionOpFailureCallback]) -> CollectionOp:
+        self._failure = failure
+        return self
+
+    def run(self, *, handler: Optional[object] = None) -> None:
+        aqt.mw._increase_background_ops()
+
+        def wrapped_op() -> ResultWithChanges:
+            return self._op(aqt.mw.col)
+
+        def wrapped_done(future: Future) -> None:
+            aqt.mw._decrease_background_ops()
+            # did something go wrong?
+            if exception := future.exception():
+                if isinstance(exception, Exception):
+                    if self._failure:
+                        self._failure(exception)
+                    else:
+                        showWarning(str(exception), self._parent)
+                    return
+                else:
+                    # BaseException like SystemExit; rethrow it
+                    future.result()
+
+            result = future.result()
+            try:
+                if self._success:
+                    self._success(result)
+            finally:
+                # update undo status
+                status = aqt.mw.col.undo_status()
+                aqt.mw._update_undo_actions_for_status_and_save(status)
+                # fire change hooks
+                self._fire_change_hooks_after_op_performed(result, handler)
+
+        aqt.mw.taskman.with_progress(wrapped_op, wrapped_done)
+
+    def _fire_change_hooks_after_op_performed(
+        self,
+        result: ResultWithChanges,
+        handler: Optional[object],
+    ) -> None:
+        if isinstance(result, OpChanges):
+            changes = result
+        else:
+            changes = result.changes
+
+        # fire new hook
+        print("op changes:")
+        print(changes)
+        aqt.gui_hooks.operation_did_execute(changes, handler)
+        # fire legacy hook so old code notices changes
+        if aqt.mw.col.op_made_changes(changes):
+            aqt.gui_hooks.state_did_reset()

qt/aqt/operations/tag.py

@@ -3,94 +3,88 @@
 
 from __future__ import annotations
 
-from typing import Optional, Sequence
+from typing import Sequence
 
 from anki.collection import OpChangesWithCount
 from anki.notes import NoteId
 from aqt import AnkiQt, QWidget
-from aqt.main import PerformOpOptionalSuccessCallback
+from aqt.operations import CollectionOp
 from aqt.utils import showInfo, tooltip, tr
 
 
 def add_tags_to_notes(
     *,
-    mw: AnkiQt,
+    parent: QWidget,
     note_ids: Sequence[NoteId],
     space_separated_tags: str,
-    success: PerformOpOptionalSuccessCallback = None,
+) -> CollectionOp:
-    handler: Optional[object] = None,
+    return CollectionOp(
-) -> None:
+        parent, lambda col: col.tags.bulk_add(note_ids, space_separated_tags)
-    mw.perform_op(
+    ).success(
-        lambda: mw.col.tags.bulk_add(note_ids, space_separated_tags),
+        lambda out: tooltip(tr.browsing_notes_updated(count=out.count), parent=parent)
-        success=success,
-        handler=handler,
     )
 
 
 def remove_tags_from_notes(
     *,
-    mw: AnkiQt,
+    parent: QWidget,
     note_ids: Sequence[NoteId],
     space_separated_tags: str,
-    success: PerformOpOptionalSuccessCallback = None,
+) -> CollectionOp:
-    handler: Optional[object] = None,
+    return CollectionOp(
-) -> None:
+        parent, lambda col: col.tags.bulk_remove(note_ids, space_separated_tags)
-    mw.perform_op(
+    ).success(
-        lambda: mw.col.tags.bulk_remove(note_ids, space_separated_tags),
+        lambda out: tooltip(tr.browsing_notes_updated(count=out.count), parent=parent)
-        success=success,
-        handler=handler,
     )
 
 
-def clear_unused_tags(*, mw: AnkiQt, parent: QWidget) -> None:
+def clear_unused_tags(*, parent: QWidget) -> CollectionOp:
-    mw.perform_op(
+    return CollectionOp(parent, lambda col: col.tags.clear_unused_tags()).success(
-        mw.col.tags.clear_unused_tags,
+        lambda out: tooltip(
-        success=lambda out: tooltip(
             tr.browsing_removed_unused_tags_count(count=out.count), parent=parent
-        ),
+        )
     )
 
 
 def rename_tag(
     *,
-    mw: AnkiQt,
     parent: QWidget,
     current_name: str,
     new_name: str,
-) -> None:
+) -> CollectionOp:
     def success(out: OpChangesWithCount) -> None:
         if out.count:
             tooltip(tr.browsing_notes_updated(count=out.count), parent=parent)
         else:
             showInfo(tr.browsing_tag_rename_warning_empty(), parent=parent)
 
-    mw.perform_op(
+    return CollectionOp(
-        lambda: mw.col.tags.rename(old=current_name, new=new_name),
+        parent,
-        success=success,
+        lambda col: col.tags.rename(old=current_name, new=new_name),
-    )
+    ).success(success)
 
 
 def remove_tags_from_all_notes(
-    *, mw: AnkiQt, parent: QWidget, space_separated_tags: str
+    *, parent: QWidget, space_separated_tags: str
-) -> None:
+) -> CollectionOp:
-    mw.perform_op(
+    return CollectionOp(
-        lambda: mw.col.tags.remove(space_separated_tags=space_separated_tags),
+        parent, lambda col: col.tags.remove(space_separated_tags=space_separated_tags)
-        success=lambda out: tooltip(
+    ).success(
-            tr.browsing_notes_updated(count=out.count), parent=parent
+        lambda out: tooltip(tr.browsing_notes_updated(count=out.count), parent=parent)
-        ),
     )
 
 
 def reparent_tags(
-    *, mw: AnkiQt, parent: QWidget, tags: Sequence[str], new_parent: str
+    *, parent: QWidget, tags: Sequence[str], new_parent: str
-) -> None:
+) -> CollectionOp:
-    mw.perform_op(
+    return CollectionOp(
-        lambda: mw.col.tags.reparent(tags=tags, new_parent=new_parent),
+        parent, lambda col: col.tags.reparent(tags=tags, new_parent=new_parent)
-        success=lambda out: tooltip(
+    ).success(
-            tr.browsing_notes_updated(count=out.count), parent=parent
+        lambda out: tooltip(tr.browsing_notes_updated(count=out.count), parent=parent)
-        ),
     )
 
 
-def set_tag_collapsed(*, mw: AnkiQt, tag: str, collapsed: bool) -> None:
+def set_tag_collapsed(*, parent: QWidget, tag: str, collapsed: bool) -> CollectionOp:
-    mw.perform_op(lambda: mw.col.tags.set_collapsed(tag=tag, collapsed=collapsed))
+    return CollectionOp(
+        parent, lambda col: col.tags.set_collapsed(tag=tag, collapsed=collapsed)
+    )

qt/aqt/reviewer.py

@@ -851,20 +851,14 @@ time = %(time)d;
         note = self.card.note()
         if note.has_tag(MARKED_TAG):
             remove_tags_from_notes(
-                mw=self.mw,
+                parent=self.mw, note_ids=[note.id], space_separated_tags=MARKED_TAG
-                note_ids=[note.id],
+            ).success(redraw_mark).run(handler=self)
-                space_separated_tags=MARKED_TAG,
-                handler=self,
-                success=redraw_mark,
-            )
         else:
             add_tags_to_notes(
-                mw=self.mw,
+                parent=self.mw,
                 note_ids=[note.id],
                 space_separated_tags=MARKED_TAG,
-                handler=self,
+            ).success(redraw_mark).run(handler=self)
-                success=redraw_mark,
-            )
 
     def on_set_due(self) -> None:
         if self.mw.state != "review" or not self.card:

qt/aqt/sidebar.py

@@ -652,7 +652,7 @@ class SidebarTreeView(QTreeView):
         else:
             new_parent = target.full_name
 
-        reparent_tags(mw=self.mw, parent=self.browser, tags=tags, new_parent=new_parent)
+        reparent_tags(parent=self.browser, tags=tags, new_parent=new_parent).run()
 
         return True
 
@@ -947,8 +947,8 @@ class SidebarTreeView(QTreeView):
             def toggle_expand(node: TagTreeNode) -> Callable[[bool], None]:
                 full_name = head + node.name
                 return lambda expanded: set_tag_collapsed(
-                    mw=self.mw, tag=full_name, collapsed=not expanded
+                    parent=self, tag=full_name, collapsed=not expanded
-                )
+                ).run()
 
             for node in nodes:
                 item = SidebarItem(
@@ -1209,9 +1209,7 @@ class SidebarTreeView(QTreeView):
         tags = self.mw.col.tags.join(self._selected_tags())
         item.name = "..."
 
-        remove_tags_from_all_notes(
+        remove_tags_from_all_notes(parent=self.browser, space_separated_tags=tags).run()
-            mw=self.mw, parent=self.browser, space_separated_tags=tags
-        )
 
     def rename_tag(self, item: SidebarItem, new_name: str) -> None:
         if not new_name or new_name == item.name:
@@ -1226,11 +1224,10 @@ class SidebarTreeView(QTreeView):
         item.full_name = new_name
 
         rename_tag(
-            mw=self.mw,
             parent=self.browser,
             current_name=old_name,
             new_name=new_name,
-        )
+        ).run()
 
     # Saved searches
     ####################################

qt/tools/genhooks_gui.py

@@ -26,7 +26,6 @@ from anki.hooks import runFilter, runHook
 from anki.models import NotetypeDict
 from aqt.qt import QDialog, QEvent, QMenu, QWidget
 from aqt.tagedit import TagEdit
-import aqt.operations
 """
 
 # Hook list