more reset refactoring

'card modified' covers the common case where we need to rebuild the
study queue, but is also set when changing the card flags. We want to
avoid a queue rebuild in that case, as it causes UI flicker, and may
result in a different card being shown. Note marking doesn't trigger
a queue build, but still causes flicker, and may return the user back
to the front side when they were looking at the answer.

I still think entity-based change tracking is the simplest in the
common case, but to solve the above, I've introduced an enum describing
the last operation that was taken. This currently is not trying to list
out all possible operations, and just describes the ones we want to
special-case.

Other changes:

- Fire the old 'state_did_reset' hook after an operation is performed,
so legacy code can refresh itself after an operation is performed.
- Fire the new `operation_did_execute` hook when mw.reset() is called,
so that as the UI is updated to the use the new hook, it will still
be able to refresh after legacy code calls mw.reset()
- Update the deck browser, overview and review screens to listen to
the new hook, instead of relying on the main window to call moveToState()
- Add a 'set flag' backend action, so we can distinguish it from a
normal card update.
- Drop the separate added/modified entries in the change list in
favour of a single entry per entity.
- Add typing to mw.state
- Tweak perform_op()
- Convert a few more actions to use perform_op()

ftl/core/undo.ftl

@@ -18,3 +18,4 @@ undo-update-note = Update Note
 undo-update-card = Update Card
 undo-update-deck = Update Deck
 undo-forget-card = Forget Card
+undo-set-flag = Set Flag

pylib/anki/cards.py

@@ -196,7 +196,8 @@ class Card:
         return self.flags & 0b111
 
     def set_user_flag(self, flag: int) -> None:
-        assert 0 <= flag <= 7
+        print("use col.set_user_flag_for_cards() instead")
+        assert 0 <= flag <= 4
         self.flags = (self.flags & ~0b111) | flag
 
     # legacy

pylib/anki/collection.py

@@ -54,7 +54,7 @@ GraphPreferences = _pb.GraphPreferences
 BuiltinSort = _pb.SortOrder.Builtin
 Preferences = _pb.Preferences
 UndoStatus = _pb.UndoStatus
-StateChanges = _pb.StateChanges
+OperationInfo = _pb.OperationInfo
 DefaultsForAdding = _pb.DeckAndNotetype
 
 
@@ -783,8 +783,6 @@ table.review-log {{ {revlog_style} }}
             assert_exhaustive(self._undo)
             assert False
 
-        return status
-
     def clear_python_undo(self) -> None:
         """Clear the Python undo state.
         The backend will automatically clear backend undo state when
@@ -812,6 +810,11 @@ table.review-log {{ {revlog_style} }}
             assert_exhaustive(self._undo)
             assert False
 
+    def op_affects_study_queue(self, op: OperationInfo) -> bool:
+        if op.kind == op.SET_CARD_FLAG:
+            return False
+        return op.changes.card or op.changes.deck or op.changes.preference
+
     def _check_backend_undo_status(self) -> Optional[UndoStatus]:
         """Return undo status if undo available on backend.
         If backend has undo available, clear the Python undo state."""
@@ -981,21 +984,10 @@ table.review-log {{ {revlog_style} }}
         self._logHnd.close()
         self._logHnd = None
 
-    # Card Flags
     ##########################################################################
 
     def set_user_flag_for_cards(self, flag: int, cids: List[int]) -> None:
-        assert 0 <= flag <= 7
+        self._backend.set_flag(card_ids=cids, flag=flag)
-        self.db.execute(
-            "update cards set flags = (flags & ~?) | ?, usn=?, mod=? where id in %s"
-            % ids2str(cids),
-            0b111,
-            flag,
-            self.usn(),
-            intTime(),
-        )
-
-    ##########################################################################
 
     def set_wants_abort(self) -> None:
         self._backend.set_wants_abort()

qt/.pylintrc

@@ -8,6 +8,7 @@ ignored-modules=win32file,pywintypes,socket,win32pipe,winrt,pyaudio
 ignored-classes=
   SearchNode,
   Config,
+  OperationInfo
 
 [REPORTS]
 output-format=colorized

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, StateChanges
+from anki.collection import Collection, Config, OperationInfo, SearchNode
 from anki.consts import *
 from anki.errors import InvalidInput, NotFoundError
 from anki.lang import without_unicode_isolation
@@ -281,13 +281,8 @@ class DataModel(QAbstractTableModel):
         else:
             tv.selectRow(0)
 
-    def maybe_redraw_after_operation(self, changes: StateChanges) -> None:
+    def maybe_redraw_after_operation(self, op: OperationInfo) -> None:
-        if (
+        if op.changes.card or op.changes.note or op.changes.deck or op.changes.notetype:
-            changes.card_modified
-            or changes.note_modified
-            or changes.deck_modified
-            or changes.notetype_modified
-        ):
             self.reset()
 
     # Column data
@@ -493,9 +488,9 @@ class Browser(QMainWindow):
         # as that will block the UI
         self.setUpdatesEnabled(False)
 
-    def on_operation_did_execute(self, changes: StateChanges) -> None:
+    def on_operation_did_execute(self, op: OperationInfo) -> None:
         self.setUpdatesEnabled(True)
-        self.model.maybe_redraw_after_operation(changes)
+        self.model.maybe_redraw_after_operation(op)
 
     def setupMenus(self) -> None:
         # pylint: disable=unnecessary-lambda
@@ -1159,14 +1154,10 @@ where id in %s"""
         # select the next card if there is one
         self._onNextCard()
 
-        def do_remove() -> None:
+        self.mw.perform_op(
-            self.col.remove_notes(nids)
+            lambda: self.col.remove_notes(nids),
-
+            success=lambda _: tooltip(tr(TR.BROWSING_NOTE_DELETED, count=len(nids))),
-        def on_done(fut: Future) -> None:
+        )
-            fut.result()
-            tooltip(tr(TR.BROWSING_NOTE_DELETED, count=len(nids)))
-
-        self.perform_op(do_remove, on_done, reset_model=True)
 
     # legacy
 
@@ -1196,14 +1187,7 @@ where id in %s"""
             return
         did = self.col.decks.id(ret.name)
 
-        def do_move() -> None:
+        self.mw.perform_op(lambda: self.col.set_deck(cids, did))
-            self.col.set_deck(cids, did)
-
-        def on_done(fut: Future) -> None:
-            fut.result()
-            self.mw.requireReset(reason=ResetReason.BrowserSetDeck, context=self)
-
-        self.perform_op(do_move, on_done)
 
     # legacy
 
@@ -1247,9 +1231,8 @@ where id in %s"""
             if not ok:
                 return
 
-        self.model.beginReset()
+        nids = self.selectedNotes()
-        func(self.selectedNotes(), tags)
+        self.mw.perform_op(lambda: func(nids, tags))
-        self.model.endReset()
         self.mw.requireReset(reason=ResetReason.BrowserAddTags, context=self)
 
     def clearUnusedTags(self) -> None:
@@ -1304,8 +1287,9 @@ where id in %s"""
         # flag needs toggling off?
         if n == self.card.user_flag():
             n = 0
-        self.col.set_user_flag_for_cards(n, self.selectedCards())
+
-        self.model.reset()
+        cids = self.selectedCards()
+        self.mw.perform_op(lambda: self.col.set_user_flag_for_cards(n, cids))
 
     def _updateFlagsMenu(self) -> None:
         flag = self.card and self.card.user_flag()
@@ -1382,11 +1366,6 @@ where id in %s"""
     # Scheduling
     ######################################################################
 
-    def _after_schedule(self) -> None:
-        self.model.reset()
-        # updates undo status
-        self.mw.requireReset(reason=ResetReason.BrowserReschedule, context=self)
-
     def set_due_date(self) -> None:
         self.editor.saveNow(
             lambda: set_due_date_dialog(
@@ -1394,7 +1373,6 @@ where id in %s"""
                 parent=self,
                 card_ids=self.selectedCards(),
                 config_key=Config.String.SET_DUE_BROWSER,
-                on_done=self._after_schedule,
             )
         )
 
@@ -1404,7 +1382,6 @@ where id in %s"""
                 mw=self.mw,
                 parent=self,
                 card_ids=self.selectedCards(),
-                on_done=self._after_schedule,
             )
         )
 

qt/aqt/deckbrowser.py

@@ -8,6 +8,7 @@ from dataclasses import dataclass
 from typing import Any
 
 import aqt
+from anki.collection import OperationInfo
 from anki.decks import DeckTreeNode
 from anki.errors import DeckIsFilteredError
 from anki.utils import intTime
@@ -61,6 +62,7 @@ class DeckBrowser:
         self.bottom = BottomBar(mw, mw.bottomWeb)
         self.scrollPos = QPoint(0, 0)
         self._v1_message_dismissed_at = 0
+        gui_hooks.operation_did_execute.append(self.on_operation_did_execute)
 
     def show(self) -> None:
         av_player.stop_and_clear_queue()
@@ -72,6 +74,13 @@ class DeckBrowser:
     def refresh(self) -> None:
         self._renderPage()
 
+    def on_operation_did_execute(self, op: OperationInfo) -> None:
+        if self.mw.state != "deckBrowser":
+            return
+
+        if self.mw.col.op_affects_study_queue(op):
+            self.refresh()
+
     # Event handlers
     ##########################################################################
 

qt/aqt/main.py

@@ -19,6 +19,7 @@ from typing import (
     Callable,
     Dict,
     List,
+    Literal,
     Optional,
     Sequence,
     TextIO,
@@ -43,6 +44,7 @@ from anki.collection import (
     Checkpoint,
     Collection,
     Config,
+    OperationInfo,
     ReviewUndo,
     UndoResult,
     UndoStatus,
@@ -112,6 +114,11 @@ class ResetRequired:
         self.mw = mw
 
 
+MainWindowState = Literal[
+    "startup", "deckBrowser", "overview", "review", "resetRequired", "profileManager"
+]
+
+
 class AnkiQt(QMainWindow):
     col: Collection
     pm: ProfileManagerType
@@ -128,7 +135,7 @@ class AnkiQt(QMainWindow):
     ) -> None:
         QMainWindow.__init__(self)
         self.backend = backend
-        self.state = "startup"
+        self.state: MainWindowState = "startup"
         self.opts = opts
         self.col: Optional[Collection] = None
         self.taskman = TaskManager(self)
@@ -664,12 +671,12 @@ class AnkiQt(QMainWindow):
         self.pm.save()
         self.progress.finish()
 
-    # State machine
+    # Tracking main window state (deck browser, reviewer, etc)
     ##########################################################################
 
-    def moveToState(self, state: str, *args: Any) -> None:
+    def moveToState(self, state: MainWindowState, *args: Any) -> None:
         # print("-> move from", self.state, "to", state)
-        oldState = self.state or "dummy"
+        oldState = self.state
         cleanup = getattr(self, f"_{oldState}Cleanup", None)
         if cleanup:
             # pylint: disable=not-callable
@@ -711,20 +718,27 @@ class AnkiQt(QMainWindow):
     def perform_op(
         self,
         op: Callable[[], T],
-        on_success: Optional[Callable[[T], None]] = None,
+        *,
-        on_exception: Optional[Callable[[BaseException], None]] = None,
+        success: Optional[Callable[[T], None]] = None,
+        failure: 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
+        - Commits changes
+        - Fires the `operation_(will|did)_reset` hooks
+        - Fires the legacy `state_did_reset` hook
 
-        on_success() will be called with the return value of op()
+        Be careful not to call any UI routines in `op`, as that may crash Qt.
-        if op() threw an exception, on_exception() will be called with it,
+        This includes things select .selectedCards() in the browse screen.
-        if it was provided
 
+        on_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 on_exception() if it is provided.
         """
 
         gui_hooks.operation_will_execute()
@@ -732,28 +746,48 @@ class AnkiQt(QMainWindow):
         def wrapped_done(future: Future) -> None:
             try:
                 if exception := future.exception():
-                    if on_exception:
+                    if failure:
-                        on_exception(exception)
+                        failure(exception)
                     else:
                         showWarning(str(exception))
                 else:
-                    if on_success:
+                    if success:
-                        on_success(future.result())
+                        success(future.result())
             finally:
                 status = self.col.undo_status()
-                self._update_undo_actions_for_status(status)
+                self._update_undo_actions_for_status_and_save(status)
-                gui_hooks.operation_did_execute(status.changes)
+                print("last op", status.last_op)
+                gui_hooks.operation_did_execute(status.last_op)
+                # fire legacy hook so old code notices changes
+                gui_hooks.state_did_reset()
 
         self.taskman.with_progress(op, wrapped_done)
 
-    def reset(self, guiOnly: bool = False) -> None:
+    def _synthesize_op_did_execute_from_reset(self) -> None:
-        "Called for non-trivial edits. Rebuilds queue and updates UI."
+        """Fire the `operation_did_execute` hook with everything marked as changed,
+        after legacy code has called .reset()"""
+        op = OperationInfo()
+        for field in op.changes.DESCRIPTOR.fields:
+            setattr(op.changes, field.name, True)
+        gui_hooks.operation_did_execute(op)
+
+    def reset(self, unused_arg: bool = False) -> None:
+        """Legacy method of telling UI to refresh after changes made to DB.
+
+        New code should use mw.perform_op() instead."""
+
         if self.col:
-            if not guiOnly:
+            # fire new `operation_did_execute` hook first. If the overview
-                self.col.reset()
+            # or review screen are currently open, they will rebuild the study
+            # queues (via mw.col.reset())
+            self._synthesize_op_did_execute_from_reset()
+            # fire the old reset hook
             gui_hooks.state_did_reset()
+
             self.update_undo_actions()
-            self.moveToState(self.state)
+
+            # fixme: double-check
+            # self.moveToState(self.state)
 
     def requireReset(
         self,
@@ -784,7 +818,7 @@ class AnkiQt(QMainWindow):
         # windows
         self.progress.timer(100, self.maybeReset, False)
 
-    def _resetRequiredState(self, oldState: str) -> None:
+    def _resetRequiredState(self, oldState: MainWindowState) -> None:
         if oldState != "resetRequired":
             self.returnState = oldState
         if self.resetModal:
@@ -1160,7 +1194,7 @@ 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:
+    def _update_undo_actions_for_status_and_save(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
@@ -1175,6 +1209,8 @@ title="%s" %s>%s</button>""" % (
             self.form.actionUndo.setEnabled(False)
             gui_hooks.undo_state_did_change(False)
 
+        self.col.autosave()
+
     def checkpoint(self, name: str) -> None:
         self.col.save(name)
         self.update_undo_actions()

qt/aqt/overview.py

@@ -6,6 +6,7 @@ from dataclasses import dataclass
 from typing import Any, Callable, Dict, List, Optional, Tuple
 
 import aqt
+from anki.collection import OperationInfo
 from aqt import gui_hooks
 from aqt.sound import av_player
 from aqt.toolbar import BottomBar
@@ -42,6 +43,7 @@ class Overview:
         self.mw = mw
         self.web = mw.web
         self.bottom = BottomBar(mw, mw.bottomWeb)
+        gui_hooks.operation_did_execute.append(self.on_operation_did_execute)
 
     def show(self) -> None:
         av_player.stop_and_clear_queue()
@@ -56,6 +58,14 @@ class Overview:
         self.mw.web.setFocus()
         gui_hooks.overview_did_refresh(self)
 
+    def on_operation_did_execute(self, op: OperationInfo) -> None:
+        if self.mw.state != "overview":
+            return
+
+        if self.mw.col.op_affects_study_queue(op):
+            # will also cover the deck description modified case
+            self.refresh()
+
     # Handlers
     ############################################################
 

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, StateChanges
+from anki.collection import Config, OperationInfo
 from anki.utils import stripHTML
 from aqt import AnkiQt, gui_hooks
 from aqt.profiles import VideoDriver
@@ -87,17 +87,26 @@ class Reviewer:
         gui_hooks.reviewer_will_end()
         self.card = None
 
-    def on_operation_did_execute(self, changes: StateChanges) -> None:
+    def on_operation_did_execute(self, op: OperationInfo) -> None:
-        need_queue_rebuild = (
+        if self.mw.state != "review":
-            changes.card_added
+            return
-            or changes.card_modified
-            or changes.deck_modified
-            or changes.preference_modified
-        )
 
-        if need_queue_rebuild:
+        if op.kind == OperationInfo.UPDATE_NOTE_TAGS:
+            self.card.load()
+            self._update_mark_icon()
+        elif op.kind == OperationInfo.SET_CARD_FLAG:
+            # fixme: v3 mtime check
+            self.card.load()
+            self._update_flag_icon()
+        elif self.mw.col.op_affects_study_queue(op):
+            # need queue rebuild
             self.mw.col.reset()
             self.nextCard()
+            return
+        elif op.changes.note or op.changes.notetype or op.changes.tag:
+            # need redraw of current card
+            self.card.load()
+            self._showQuestion()
 
     # Fetching a card
     ##########################################################################
@@ -795,24 +804,27 @@ time = %(time)d;
     def onOptions(self) -> None:
         self.mw.onDeckConf(self.mw.col.decks.get(self.card.odid or self.card.did))
 
-    def set_flag_on_current_card(self, flag: int) -> None:
+    def set_flag_on_current_card(self, desired_flag: int) -> None:
-        # need to toggle off?
+        def op() -> None:
-        if self.card.user_flag() == flag:
+            # need to toggle off?
-            flag = 0
+            if self.card.user_flag() == desired_flag:
-        self.card.set_user_flag(flag)
+                flag = 0
-        self.mw.col.update_card(self.card)
+            else:
-        self.mw.update_undo_actions()
+                flag = desired_flag
-        self._update_flag_icon()
+            self.mw.col.set_user_flag_for_cards(flag, [self.card.id])
+
+        self.mw.perform_op(op)
 
     def toggle_mark_on_current_note(self) -> None:
-        note = self.card.note()
+        def op() -> None:
-        if note.has_tag("marked"):
+            tag = "marked"
-            note.remove_tag("marked")
+            note = self.card.note()
-        else:
+            if note.has_tag(tag):
-            note.add_tag("marked")
+                self.mw.col.tags.bulk_remove([note.id], tag)
-        self.mw.col.update_note(note)
+            else:
-        self.mw.update_undo_actions()
+                self.mw.col.tags.bulk_add([note.id], tag)
-        self._update_mark_icon()
+
+        self.mw.perform_op(op)
 
     def on_set_due(self) -> None:
         if self.mw.state != "review" or not self.card:
@@ -823,28 +835,33 @@ time = %(time)d;
             parent=self.mw,
             card_ids=[self.card.id],
             config_key=Config.String.SET_DUE_REVIEWER,
-            on_done=self.mw.reset,
         )
 
     def suspend_current_note(self) -> None:
-        self.mw.col.sched.suspend_cards([c.id for c in self.card.note().cards()])
+        self.mw.perform_op(
-        self.mw.reset()
+            lambda: self.mw.col.sched.suspend_cards(
-        tooltip(tr(TR.STUDYING_NOTE_SUSPENDED))
+                [c.id for c in self.card.note().cards()]
+            ),
+            success=lambda _: tooltip(tr(TR.STUDYING_NOTE_SUSPENDED)),
+        )
 
     def suspend_current_card(self) -> None:
-        self.mw.col.sched.suspend_cards([self.card.id])
+        self.mw.perform_op(
-        self.mw.reset()
+            lambda: self.mw.col.sched.suspend_cards([self.card.id]),
-        tooltip(tr(TR.STUDYING_CARD_SUSPENDED))
+            success=lambda _: tooltip(tr(TR.STUDYING_CARD_SUSPENDED)),
+        )
 
     def bury_current_card(self) -> None:
-        self.mw.col.sched.bury_cards([self.card.id])
+        self.mw.perform_op(
-        self.mw.reset()
+            lambda: self.mw.col.sched.bury_cards([self.card.id]),
-        tooltip(tr(TR.STUDYING_CARD_BURIED))
+            success=lambda _: tooltip(tr(TR.STUDYING_CARD_BURIED)),
+        )
 
     def bury_current_note(self) -> None:
-        self.mw.col.sched.bury_note(self.card.note())
+        self.mw.perform_op(
-        self.mw.reset()
+            lambda: self.mw.col.sched.bury_note(self.card.note()),
-        tooltip(tr(TR.STUDYING_NOTE_BURIED))
+            success=lambda _: tooltip(tr(TR.STUDYING_NOTE_BURIED)),
+        )
 
     def delete_current_note(self) -> None:
         # need to check state because the shortcut is global to the main
@@ -855,7 +872,9 @@ time = %(time)d;
 
         self.mw.perform_op(
             lambda: self.mw.col.remove_notes([self.card.note().id]),
-            lambda _: tooltip(tr(TR.STUDYING_NOTE_AND_ITS_CARD_DELETED, count=cnt)),
+            success=lambda _: tooltip(
+                tr(TR.STUDYING_NOTE_AND_ITS_CARD_DELETED, count=cnt)
+            ),
         )
 
     def onRecordVoice(self) -> None:

qt/aqt/scheduling.py

@@ -3,14 +3,13 @@
 
 from __future__ import annotations
 
-from concurrent.futures import Future
 from typing import List, Optional
 
 import aqt
 from anki.collection import Config
 from anki.lang import TR
 from aqt.qt import *
-from aqt.utils import getText, showWarning, tooltip, tr
+from aqt.utils import getText, tooltip, tr
 
 
 def set_due_date_dialog(
@@ -19,12 +18,13 @@ def set_due_date_dialog(
     parent: QDialog,
     card_ids: List[int],
     config_key: Optional[Config.String.Key.V],
-    on_done: Callable[[], None],
 ) -> None:
     if not card_ids:
         return
 
-    default = mw.col.get_config_string(config_key) if config_key is not None else ""
+    default_text = (
+        mw.col.get_config_string(config_key) if config_key is not None else ""
+    )
     prompt = "\n".join(
         [
             tr(TR.SCHEDULING_SET_DUE_DATE_PROMPT, cards=len(card_ids)),
@@ -34,49 +34,28 @@ def set_due_date_dialog(
     (days, success) = getText(
         prompt=prompt,
         parent=parent,
-        default=default,
+        default=default_text,
         title=tr(TR.ACTIONS_SET_DUE_DATE),
     )
     if not success or not days.strip():
         return
 
-    def set_due() -> None:
+    mw.perform_op(
-        mw.col.sched.set_due_date(card_ids, days, config_key)
+        lambda: mw.col.sched.set_due_date(card_ids, days, config_key),
-
+        success=lambda _: tooltip(
-    def after_set(fut: Future) -> None:
-        try:
-            fut.result()
-        except Exception as e:
-            showWarning(str(e))
-            on_done()
-            return
-
-        tooltip(
             tr(TR.SCHEDULING_SET_DUE_DATE_DONE, cards=len(card_ids)),
             parent=parent,
-        )
+        ),
-
+    )
-        on_done()
-
-    mw.taskman.with_progress(set_due, after_set)
 
 
-def forget_cards(
+def forget_cards(*, mw: aqt.AnkiQt, parent: QDialog, card_ids: List[int]) -> None:
-    *, mw: aqt.AnkiQt, parent: QDialog, card_ids: List[int], on_done: Callable[[], None]
-) -> None:
     if not card_ids:
         return
 
-    def on_done_wrapper(fut: Future) -> None:
+    mw.perform_op(
-        try:
+        lambda: mw.col.sched.schedule_cards_as_new(card_ids),
-            fut.result()
+        success=lambda _: tooltip(
-        except Exception as e:
+            tr(TR.SCHEDULING_FORGOT_CARDS, cards=len(card_ids)), parent=parent
-            showWarning(str(e))
+        ),
-        else:
-            tooltip(tr(TR.SCHEDULING_FORGOT_CARDS, cards=len(card_ids)), parent=parent)
-
-        on_done()
-
-    mw.taskman.with_progress(
-        lambda: mw.col.sched.schedule_cards_as_new(card_ids), on_done_wrapper
     )

qt/tools/genhooks_gui.py

@@ -394,7 +394,10 @@ hooks = [
     Hook(
         name="state_did_reset",
         legacy_hook="reset",
-        doc="Called when the interface needs to be redisplayed after non-trivial changes have been made.",
+        doc="""Legacy 'reset' hook. Called by mw.reset() and mw.perform_op() to redraw the UI.
+        
+        New code should use `operation_did_execute` instead.
+        """,
     ),
     Hook(
         name="operation_will_execute",
@@ -405,10 +408,14 @@ hooks = [
     Hook(
         name="operation_did_execute",
         args=[
-            "changes: anki.collection.StateChanges",
+            "op: anki.collection.OperationInfo",
         ],
         doc="""Called after an operation completes.
-        Changes can be inspected to determine whether the UI needs updating.""",
+        Changes can be inspected to determine whether the UI needs updating.
+        
+        This will also be called when the legacy mw.reset() is used. When called via
+        mw.reset(), `operation_will_execute` will not be called.
+        """,
     ),
     # Webview
     ###################

rslib/backend.proto

@@ -267,6 +267,7 @@ service CardsService {
   rpc UpdateCard(UpdateCardIn) returns (Empty);
   rpc RemoveCards(RemoveCardsIn) returns (Empty);
   rpc SetDeck(SetDeckIn) returns (Empty);
+  rpc SetFlag(SetFlagIn) returns (Empty);
 }
 
 // Protobuf stored in .anki2 files
@@ -1442,22 +1443,30 @@ message GetQueuedCardsOut {
   }
 }
 
-message StateChanges {
+message OperationInfo {
-  bool card_added = 1;
+  message Changes {
-  bool card_modified = 2;
+    bool card = 1;
-  bool note_added = 3;
+    bool note = 2;
-  bool note_modified = 4;
+    bool deck = 3;
-  bool deck_added = 5;
+    bool tag = 4;
-  bool deck_modified = 6;
+    bool notetype = 5;
-  bool tag_modified = 7;
+    bool preference = 6;
-  bool notetype_modified = 8;
+  }
-  bool preference_modified = 9;
+  // this is not an exhaustive list; we can add more cases as we need them
+  enum Kind {
+    OTHER = 0;
+    UPDATE_NOTE_TAGS = 1;
+    SET_CARD_FLAG = 2;
+  }
+
+  Kind kind = 1;
+  Changes changes = 2;
 }
 
 message UndoStatus {
   string undo = 1;
   string redo = 2;
-  StateChanges changes = 3;
+  OperationInfo last_op = 3;
 }
 
 message DefaultsForAddingIn {
@@ -1472,4 +1481,9 @@ message DeckAndNotetype {
 message RenameDeckIn {
   int64 deck_id = 1;
   string new_name = 2;
-}
+}
+
+message SetFlagIn {
+  repeated int64 card_ids = 1;
+  uint32 flag = 2;
+}

rslib/src/backend/card.rs

@@ -54,6 +54,13 @@ impl CardsService for Backend {
         let deck_id = input.deck_id.into();
         self.with_col(|col| col.set_deck(&cids, deck_id).map(Into::into))
     }
+
+    fn set_flag(&self, input: pb::SetFlagIn) -> Result<pb::Empty> {
+        self.with_col(|col| {
+            col.set_card_flag(&to_card_ids(input.card_ids), input.flag)
+                .map(Into::into)
+        })
+    }
 }
 
 impl TryFrom<pb::Card> for Card {
@@ -111,3 +118,7 @@ impl From<Card> for pb::Card {
         }
     }
 }
+
+fn to_card_ids(v: Vec<i64>) -> Vec<CardID> {
+    v.into_iter().map(CardID).collect()
+}

rslib/src/backend/collection.rs

@@ -85,20 +85,20 @@ impl CollectionService for Backend {
     }
 
     fn get_undo_status(&self, _input: pb::Empty) -> Result<pb::UndoStatus> {
-        self.with_col(|col| Ok(col.undo_status()))
+        self.with_col(|col| Ok(col.undo_status().into_protobuf(&col.i18n)))
     }
 
     fn undo(&self, _input: pb::Empty) -> Result<pb::UndoStatus> {
         self.with_col(|col| {
             col.undo()?;
-            Ok(col.undo_status())
+            Ok(col.undo_status().into_protobuf(&col.i18n))
         })
     }
 
     fn redo(&self, _input: pb::Empty) -> Result<pb::UndoStatus> {
         self.with_col(|col| {
             col.redo()?;
-            Ok(col.undo_status())
+            Ok(col.undo_status().into_protobuf(&col.i18n))
         })
     }
 }

rslib/src/backend/notes.rs

@@ -95,7 +95,7 @@ impl NotesService for Backend {
 
     fn add_note_tags(&self, input: pb::AddNoteTagsIn) -> Result<pb::UInt32> {
         self.with_col(|col| {
-            col.add_tags_to_notes(&to_nids(input.nids), &input.tags)
+            col.add_tags_to_notes(&to_note_ids(input.nids), &input.tags)
                 .map(|n| n as u32)
         })
         .map(Into::into)
@@ -104,7 +104,7 @@ impl NotesService for Backend {
     fn update_note_tags(&self, input: pb::UpdateNoteTagsIn) -> Result<pb::UInt32> {
         self.with_col(|col| {
             col.replace_tags_for_notes(
-                &to_nids(input.nids),
+                &to_note_ids(input.nids),
                 &input.tags,
                 &input.replacement,
                 input.regex,
@@ -127,7 +127,7 @@ impl NotesService for Backend {
         self.with_col(|col| {
             col.transact(None, |col| {
                 col.after_note_updates(
-                    &to_nids(input.nids),
+                    &to_note_ids(input.nids),
                     input.generate_cards,
                     input.mark_notes_modified,
                 )?;
@@ -167,6 +167,6 @@ impl NotesService for Backend {
     }
 }
 
-fn to_nids(ids: Vec<i64>) -> Vec<NoteID> {
+fn to_note_ids(ids: Vec<i64>) -> Vec<NoteID> {
     ids.into_iter().map(NoteID).collect()
 }

rslib/src/backend/ops.rs

@@ -1,20 +1,48 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
-use crate::{backend_proto as pb, ops::StateChanges};
+use pb::operation_info::{Changes, Kind};
 
-impl From<StateChanges> for pb::StateChanges {
+use crate::{backend_proto as pb, ops::StateChanges, prelude::*, undo::UndoStatus};
+
+impl From<StateChanges> for Changes {
     fn from(c: StateChanges) -> Self {
-        pb::StateChanges {
+        Changes {
-            card_added: c.card_added,
+            card: c.card,
-            card_modified: c.card_modified,
+            note: c.note,
-            note_added: c.note_added,
+            deck: c.deck,
-            note_modified: c.note_modified,
+            tag: c.tag,
-            deck_added: c.deck_added,
+            notetype: c.notetype,
-            deck_modified: c.deck_modified,
+            preference: c.preference,
-            tag_modified: c.tag_modified,
+        }
-            notetype_modified: c.notetype_modified,
+    }
-            preference_modified: c.preference_modified,
+}
+
+impl From<Op> for Kind {
+    fn from(o: Op) -> Self {
+        match o {
+            Op::SetFlag => Kind::SetCardFlag,
+            Op::UpdateTag => Kind::UpdateNoteTags,
+            _ => Kind::Other,
+        }
+    }
+}
+
+impl From<Op> for pb::OperationInfo {
+    fn from(op: Op) -> Self {
+        pb::OperationInfo {
+            changes: Some(op.state_changes().into()),
+            kind: Kind::from(op) as i32,
+        }
+    }
+}
+
+impl UndoStatus {
+    pub(crate) fn into_protobuf(self, i18n: &I18n) -> pb::UndoStatus {
+        pb::UndoStatus {
+            undo: self.undo.map(|op| op.describe(i18n)).unwrap_or_default(),
+            redo: self.redo.map(|op| op.describe(i18n)).unwrap_or_default(),
+            last_op: self.undo.map(Into::into),
         }
     }
 }

rslib/src/card/mod.rs

@@ -111,6 +111,15 @@ impl Card {
         self.deck_id = deck;
     }
 
+    fn set_flag(&mut self, flag: u8) {
+        // we currently only allow 4 flags
+        assert!(flag < 5);
+
+        // but reserve space for 7, preserving the rest of
+        // the flags (up to a byte)
+        self.flags = (self.flags & !0b111) | flag
+    }
+
     /// Return the total number of steps left to do, ignoring the
     /// "steps today" number packed into the DB representation.
     pub fn remaining_steps(&self) -> u32 {
@@ -221,6 +230,24 @@ impl Collection {
         })
     }
 
+    pub fn set_card_flag(&mut self, cards: &[CardID], flag: u32) -> Result<()> {
+        if flag > 4 {
+            return Err(AnkiError::invalid_input("invalid flag"));
+        }
+        let flag = flag as u8;
+
+        self.storage.set_search_table_to_card_ids(cards, false)?;
+        let usn = self.usn()?;
+        self.transact(Some(Op::SetFlag), |col| {
+            for mut card in col.storage.all_searched_cards()? {
+                let original = card.clone();
+                card.set_flag(flag);
+                col.update_card_inner(&mut card, original, usn)?;
+            }
+            Ok(())
+        })
+    }
+
     /// Get deck config for the given card. If missing, return default values.
     #[allow(dead_code)]
     pub(crate) fn deck_config_for_card(&mut self, card: &Card) -> Result<DeckConf> {

rslib/src/ops.rs

@@ -13,7 +13,9 @@ pub enum Op {
     RemoveNote,
     RenameDeck,
     ScheduleAsNew,
+    SetDeck,
     SetDueDate,
+    SetFlag,
     Suspend,
     UnburyUnsuspend,
     UpdateCard,
@@ -21,91 +23,11 @@ pub enum Op {
     UpdateNote,
     UpdatePreferences,
     UpdateTag,
-    SetDeck,
 }
 
 impl Op {
-    /// Used internally to decide whether the study queues need to be invalidated.
+    pub fn describe(self, i18n: &I18n) -> String {
-    pub(crate) fn needs_study_queue_reset(self) -> bool {
+        let key = match self {
-        let changes = self.state_changes();
-        self != Op::AnswerCard
-            && (changes.card_added
-                || changes.card_modified
-                || changes.deck_modified
-                || changes.preference_modified)
-    }
-
-    pub fn state_changes(self) -> StateChanges {
-        let default = Default::default;
-        match self {
-            Op::ScheduleAsNew
-            | Op::SetDueDate
-            | Op::Suspend
-            | Op::UnburyUnsuspend
-            | Op::UpdateCard
-            | Op::SetDeck
-            | Op::Bury => StateChanges {
-                card_modified: true,
-                ..default()
-            },
-            Op::AnswerCard => StateChanges {
-                card_modified: true,
-                // this also modifies the daily counts stored in the
-                // deck, but the UI does not care about that
-                ..default()
-            },
-            Op::AddDeck => StateChanges {
-                deck_added: true,
-                ..default()
-            },
-            Op::AddNote => StateChanges {
-                card_added: true,
-                note_added: true,
-                tag_modified: true,
-                ..default()
-            },
-            Op::RemoveDeck => StateChanges {
-                card_modified: true,
-                note_modified: true,
-                deck_modified: true,
-                ..default()
-            },
-            Op::RemoveNote => StateChanges {
-                card_modified: true,
-                note_modified: true,
-                ..default()
-            },
-            Op::RenameDeck => StateChanges {
-                deck_modified: true,
-                ..default()
-            },
-            Op::UpdateDeck => StateChanges {
-                deck_modified: true,
-                ..default()
-            },
-            Op::UpdateNote => StateChanges {
-                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 {
-                preference_modified: true,
-                ..default()
-            },
-            Op::UpdateTag => StateChanges {
-                tag_modified: true,
-                ..default()
-            },
-        }
-    }
-}
-
-impl Collection {
-    pub fn describe_op_kind(&self, op: Op) -> String {
-        let key = match op {
             Op::AddDeck => TR::UndoAddDeck,
             Op::AddNote => TR::UndoAddNote,
             Op::AnswerCard => TR::UndoAnswerCard,
@@ -123,21 +45,94 @@ impl Collection {
             Op::UpdatePreferences => TR::PreferencesPreferences,
             Op::UpdateTag => TR::UndoUpdateTag,
             Op::SetDeck => TR::BrowsingChangeDeck,
+            Op::SetFlag => TR::UndoSetFlag,
         };
 
-        self.i18n.tr(key).to_string()
+        i18n.tr(key).to_string()
+    }
+
+    /// Used internally to decide whether the study queues need to be invalidated.
+    pub(crate) fn needs_study_queue_reset(self) -> bool {
+        let changes = self.state_changes();
+        self != Op::AnswerCard && (changes.card || changes.deck || changes.preference)
+    }
+
+    pub fn state_changes(self) -> StateChanges {
+        let default = Default::default;
+        match self {
+            Op::ScheduleAsNew
+            | Op::SetDueDate
+            | Op::Suspend
+            | Op::UnburyUnsuspend
+            | Op::UpdateCard
+            | Op::SetDeck
+            | Op::Bury
+            | Op::SetFlag => StateChanges {
+                card: true,
+                ..default()
+            },
+            Op::AnswerCard => StateChanges {
+                card: true,
+                // this also modifies the daily counts stored in the
+                // deck, but the UI does not care about that
+                ..default()
+            },
+            Op::AddDeck => StateChanges {
+                deck: true,
+                ..default()
+            },
+            Op::AddNote => StateChanges {
+                card: true,
+                note: true,
+                tag: true,
+                ..default()
+            },
+            Op::RemoveDeck => StateChanges {
+                card: true,
+                note: true,
+                deck: true,
+                ..default()
+            },
+            Op::RemoveNote => StateChanges {
+                card: true,
+                note: true,
+                ..default()
+            },
+            Op::RenameDeck => StateChanges {
+                deck: true,
+                ..default()
+            },
+            Op::UpdateDeck => StateChanges {
+                deck: true,
+                ..default()
+            },
+            Op::UpdateNote => StateChanges {
+                note: true,
+                // edits may result in new cards being generated
+                card: true,
+                // and may result in new tags being added
+                tag: true,
+                ..default()
+            },
+            Op::UpdatePreferences => StateChanges {
+                preference: true,
+                ..default()
+            },
+            Op::UpdateTag => StateChanges {
+                note: true,
+                tag: true,
+                ..default()
+            },
+        }
     }
 }
 
 #[derive(Debug, Default, Clone, Copy)]
 pub struct StateChanges {
-    pub card_added: bool,
+    pub card: bool,
-    pub card_modified: bool,
+    pub note: bool,
-    pub note_added: bool,
+    pub deck: bool,
-    pub note_modified: bool,
+    pub tag: bool,
-    pub deck_added: bool,
+    pub notetype: bool,
-    pub deck_modified: bool,
+    pub preference: bool,
-    pub tag_modified: bool,
-    pub notetype_modified: bool,
-    pub preference_modified: bool,
 }

rslib/src/undo/mod.rs

@@ -6,7 +6,6 @@ mod changes;
 pub use crate::ops::Op;
 pub(crate) use changes::UndoableChange;
 
-use crate::backend_proto as pb;
 use crate::prelude::*;
 use std::collections::VecDeque;
 
@@ -32,6 +31,11 @@ impl Default for UndoMode {
     }
 }
 
+pub struct UndoStatus {
+    pub undo: Option<Op>,
+    pub redo: Option<Op>,
+}
+
 #[derive(Debug, Default)]
 pub(crate) struct UndoManager {
     // undo steps are added to the front of a double-ended queue, so we can
@@ -75,6 +79,8 @@ impl UndoManager {
                     self.undo_steps.truncate(UNDO_LIMIT - 1);
                     self.undo_steps.push_front(step);
                 }
+            } else {
+                println!("no undo changes, discarding step");
             }
         }
         println!("ended, undo steps count now {}", self.undo_steps.len());
@@ -141,22 +147,10 @@ impl Collection {
         Ok(())
     }
 
-    pub fn undo_status(&self) -> pb::UndoStatus {
+    pub fn undo_status(&self) -> UndoStatus {
-        pb::UndoStatus {
+        UndoStatus {
-            undo: self
+            undo: self.can_undo(),
-                .can_undo()
+            redo: self.can_redo(),
-                .map(|op| self.describe_op_kind(op))
-                .unwrap_or_default(),
-            redo: self
-                .can_redo()
-                .map(|op| self.describe_op_kind(op))
-                .unwrap_or_default(),
-            changes: Some(
-                self.can_undo()
-                    .map(|op| op.state_changes())
-                    .unwrap_or_default()
-                    .into(),
-            ),
         }
     }