experiment with finer-scoped reset in perform_op()

Basic proof of concept, where the 'delete note' operation in the
reviewer has been updated to use mw.perform_op(). Instead of manually
calling .reset() afterwards, a summary of the changes is returned as
part of the undo status query, and various parts of the GUI can listen
to gui_hooks.operation_did_execute and decide whether they want to
redraw based on the scope of the changes. This should allow the sidebar
to selectively redraw just the tags area in the future for example.

Currently we're just listing out all possible areas that might be changed;
in the future we could theoretically inspect the specific changes in the
undo log to provide a more accurate report (avoiding refreshing the tags
list when no tags were added for example).

You can test it out by opening the browse screen while studying, and
then deleting the current card - the browser should update to show (deleted)
on the cards due the earlier change.

If going ahead with this, aside from updating all the screens that currently
listen for resets, some thought will be required on how we can integrate
it with legacy code that expects to called when resets are made, and expects
to call .reset() when it makes changes.

Thoughts?

pylib/anki/collection.py

@@ -54,6 +54,7 @@ GraphPreferences = _pb.GraphPreferences
 BuiltinSort = _pb.SortOrder.Builtin
 Preferences = _pb.Preferences
 UndoStatus = _pb.UndoStatus
+StateChanges = _pb.StateChanges
 DefaultsForAdding = _pb.DeckAndNotetype
 
 

qt/aqt/browser.py

@@ -12,7 +12,7 @@ from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple, Union,
 import aqt
 import aqt.forms
 from anki.cards import Card
-from anki.collection import Collection, Config, SearchNode
+from anki.collection import Collection, Config, SearchNode, StateChanges
 from anki.consts import *
 from anki.errors import InvalidInput, NotFoundError
 from anki.lang import without_unicode_isolation
@@ -281,6 +281,15 @@ class DataModel(QAbstractTableModel):
         else:
             tv.selectRow(0)
 
+    def maybe_redraw_after_operation(self, changes: StateChanges) -> None:
+        if (
+            changes.card_modified
+            or changes.note_modified
+            or changes.deck_modified
+            or changes.notetype_modified
+        ):
+            self.reset()
+
     # Column data
     ######################################################################
 
@@ -434,8 +443,6 @@ class StatusDelegate(QItemDelegate):
 # Browser window
 ######################################################################
 
-# fixme: respond to reset+edit hooks
-
 
 class Browser(QMainWindow):
     model: DataModel
@@ -481,43 +488,14 @@ class Browser(QMainWindow):
         gui_hooks.browser_will_show(self)
         self.show()
 
-    def perform_op(
+    def on_operation_will_execute(self) -> None:
-        self,
+        # make sure the card list doesn't try to refresh itself during the operation,
-        op: Callable,
+        # as that will block the UI
-        on_done: Callable[[Future], None],
+        self.setUpdatesEnabled(False)
-        *,
-        reset_model: bool = True,
-    ) -> None:
-        """Run the provided operation on a background thread.
-        - Ensures any changes in the editor have been saved.
-        - 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
-        - If `reset_model` is true, calls beginReset()/endReset(), which will
-        refresh the displayed data, and update the editor's note. If the current search
-        has changed results, you will need to call .search() yourself in `on_done`.
 
-        Caller must run fut.result() in the on_done() callback to check for errors;
+    def on_operation_did_execute(self, changes: StateChanges) -> None:
-        if the operation returned a value, it will be returned by .result()
+        self.setUpdatesEnabled(True)
-        """
+        self.model.maybe_redraw_after_operation(changes)
-
-        def wrapped_op() -> None:
-            if reset_model:
-                self.model.beginReset()
-            self.setUpdatesEnabled(False)
-            op()
-
-        def wrapped_done(fut: Future) -> None:
-            self.setUpdatesEnabled(True)
-            on_done(fut)
-            if reset_model:
-                self.model.endReset()
-            self.mw.update_undo_actions()
-
-        self.editor.saveNow(
-            lambda: self.mw.taskman.with_progress(wrapped_op, wrapped_done)
-        )
 
     def setupMenus(self) -> None:
         # pylint: disable=unnecessary-lambda
@@ -1466,6 +1444,8 @@ where id in %s"""
         gui_hooks.editor_did_unfocus_field.append(self.on_unfocus_field)
         gui_hooks.sidebar_should_refresh_decks.append(self.on_item_added)
         gui_hooks.sidebar_should_refresh_notetypes.append(self.on_item_added)
+        gui_hooks.operation_will_execute.append(self.on_operation_will_execute)
+        gui_hooks.operation_did_execute.append(self.on_operation_did_execute)
 
     def teardownHooks(self) -> None:
         gui_hooks.undo_state_did_change.remove(self.onUndoState)
@@ -1475,6 +1455,8 @@ where id in %s"""
         gui_hooks.editor_did_unfocus_field.remove(self.on_unfocus_field)
         gui_hooks.sidebar_should_refresh_decks.remove(self.on_item_added)
         gui_hooks.sidebar_should_refresh_notetypes.remove(self.on_item_added)
+        gui_hooks.operation_will_execute.remove(self.on_operation_will_execute)
+        gui_hooks.operation_did_execute.remove(self.on_operation_did_execute)
 
     def on_unfocus_field(self, changed: bool, note: Note, field_idx: int) -> None:
         self.refreshCurrentCard(note)

qt/aqt/main.py

@@ -14,7 +14,18 @@ import zipfile
 from argparse import Namespace
 from concurrent.futures import Future
 from threading import Thread
-from typing import Any, Callable, Dict, List, Optional, Sequence, TextIO, Tuple, cast
+from typing import (
+    Any,
+    Callable,
+    Dict,
+    List,
+    Optional,
+    Sequence,
+    TextIO,
+    Tuple,
+    TypeVar,
+    cast,
+)
 
 import anki
 import aqt
@@ -34,6 +45,7 @@ from anki.collection import (
     Config,
     ReviewUndo,
     UndoResult,
+    UndoStatus,
 )
 from anki.decks import Deck
 from anki.hooks import runHook
@@ -74,6 +86,8 @@ from aqt.utils import (
     tr,
 )
 
+T = TypeVar("T")
+
 install_pylib_legacy()
 
 
@@ -694,6 +708,44 @@ class AnkiQt(QMainWindow):
     # Resetting state
     ##########################################################################
 
+    def perform_op(
+        self,
+        op: Callable[[], T],
+        on_success: Optional[Callable[[T], None]] = None,
+        on_exception: Optional[Callable[[BaseException], None]] = None,
+    ) -> None:
+        """Run the provided operation on a background thread.
+        - Ensures any changes in the editor have been saved.
+        - 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
+
+        on_success() will be called with the return value of op()
+        if op() threw an exception, on_exception() will be called with it,
+        if it was provided
+
+        """
+
+        gui_hooks.operation_will_execute()
+
+        def wrapped_done(future: Future) -> None:
+            try:
+                if exception := future.exception():
+                    if on_exception:
+                        on_exception(exception)
+                    else:
+                        showWarning(str(exception))
+                else:
+                    if on_success:
+                        on_success(future.result())
+            finally:
+                status = self.col.undo_status()
+                self._update_undo_actions_for_status(status)
+                gui_hooks.operation_did_execute(status.changes)
+
+        self.taskman.with_progress(op, wrapped_done)
+
     def reset(self, guiOnly: bool = False) -> None:
         "Called for non-trivial edits. Rebuilds queue and updates UI."
         if self.col:
@@ -1108,6 +1160,21 @@ title="%s" %s>%s</button>""" % (
             self.form.actionUndo.setEnabled(False)
             gui_hooks.undo_state_did_change(False)
 
+    def _update_undo_actions_for_status(self, status: UndoStatus) -> None:
+        """Update menu text and enable/disable menu item as appropriate.
+        Plural as this may handle redo in the future too."""
+        undo_action = status.undo
+
+        if undo_action:
+            undo_action = tr(TR.UNDO_UNDO_ACTION, val=undo_action)
+            self.form.actionUndo.setText(undo_action)
+            self.form.actionUndo.setEnabled(True)
+            gui_hooks.undo_state_did_change(True)
+        else:
+            self.form.actionUndo.setText(tr(TR.UNDO_UNDO))
+            self.form.actionUndo.setEnabled(False)
+            gui_hooks.undo_state_did_change(False)
+
     def checkpoint(self, name: str) -> None:
         self.col.save(name)
         self.update_undo_actions()

qt/aqt/reviewer.py

@@ -13,7 +13,7 @@ from PyQt5.QtCore import Qt
 
 from anki import hooks
 from anki.cards import Card
-from anki.collection import Config
+from anki.collection import Config, StateChanges
 from anki.utils import stripHTML
 from aqt import AnkiQt, gui_hooks
 from aqt.profiles import VideoDriver
@@ -63,6 +63,7 @@ class Reviewer:
         self.state: Optional[str] = None
         self.bottom = BottomBar(mw, mw.bottomWeb)
         hooks.card_did_leech.append(self.onLeech)
+        gui_hooks.operation_did_execute.append(self.on_operation_did_execute)
 
     def show(self) -> None:
         self.mw.col.reset()
@@ -86,6 +87,18 @@ class Reviewer:
         gui_hooks.reviewer_will_end()
         self.card = None
 
+    def on_operation_did_execute(self, changes: StateChanges) -> None:
+        need_queue_rebuild = (
+            changes.card_added
+            or changes.card_modified
+            or changes.deck_modified
+            or changes.preference_modified
+        )
+
+        if need_queue_rebuild:
+            self.mw.col.reset()
+            self.nextCard()
+
     # Fetching a card
     ##########################################################################
 
@@ -839,9 +852,11 @@ time = %(time)d;
         if self.mw.state != "review" or not self.card:
             return
         cnt = len(self.card.note().cards())
-        self.mw.col.remove_notes([self.card.note().id])
+
-        self.mw.reset()
+        self.mw.perform_op(
-        tooltip(tr(TR.STUDYING_NOTE_AND_ITS_CARD_DELETED, count=cnt))
+            lambda: self.mw.col.remove_notes([self.card.note().id]),
+            lambda _: tooltip(tr(TR.STUDYING_NOTE_AND_ITS_CARD_DELETED, count=cnt)),
+        )
 
     def onRecordVoice(self) -> None:
         def after_record(path: str) -> None:

qt/tools/genhooks_gui.py

@@ -365,8 +365,9 @@ hooks = [
         args=["context: aqt.browser.SearchContext"],
         doc="""Allows you to modify the list of returned card ids from a search.""",
     ),
-    # States
+    # Main window states
     ###################
+    # these refer to things like deckbrowser, overview and reviewer state,
     Hook(
         name="state_will_change",
         args=["new_state: str", "old_state: str"],
@@ -382,6 +383,8 @@ hooks = [
         name="state_shortcuts_will_change",
         args=["state: str", "shortcuts: List[Tuple[str, Callable]]"],
     ),
+    # UI state/refreshing
+    ###################
     Hook(
         name="state_did_revert",
         args=["action: str"],
@@ -393,6 +396,20 @@ hooks = [
         legacy_hook="reset",
         doc="Called when the interface needs to be redisplayed after non-trivial changes have been made.",
     ),
+    Hook(
+        name="operation_will_execute",
+        doc="""Called before an operation is executed with mw.perform_op().
+        Subscribers can use this to ensure they don't try to access the collection until the operation completes,
+        as doing so on the main thread will temporarily freeze the UI.""",
+    ),
+    Hook(
+        name="operation_did_execute",
+        args=[
+            "changes: anki.collection.StateChanges",
+        ],
+        doc="""Called after an operation completes.
+        Changes can be inspected to determine whether the UI needs updating.""",
+    ),
     # Webview
     ###################
     Hook(

rslib/src/ops.rs

@@ -61,6 +61,7 @@ impl Op {
             Op::AddNote => StateChanges {
                 card_added: true,
                 note_added: true,
+                tag_modified: true,
                 ..default()
             },
             Op::RemoveDeck => StateChanges {
@@ -86,6 +87,8 @@ impl Op {
                 note_modified: true,
                 // edits may result in new cards being generated
                 card_added: true,
+                // and may result in new tags being added
+                tag_modified: true,
                 ..default()
             },
             Op::UpdatePreferences => StateChanges {