Merge pull request #1072 from ankitects/refresh

Experimental changes to resetting

ftl/core/browsing.ftl

@@ -140,3 +140,14 @@ browsing-edited-today = Edited
 browsing-sidebar-due-today = Due
 browsing-sidebar-untagged = Untagged
 browsing-sidebar-overdue = Overdue
+browsing-row-deleted = (deleted)
+browsing-removed-unused-tags-count =
+    { $count ->
+        [one] Removed { $count } unused tag.
+       *[other] Removed { $count } unused tags.
+    }
+browsing-changed-new-position =
+    { $count ->
+        [one] Changed position of { $count } new card.
+       *[other] Changed position of { $count } new cards.
+    }

ftl/core/undo.ftl

@@ -18,3 +18,6 @@ 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
+# when dragging/dropping tags and decks in the sidebar
+undo-reparent = Change Parent

ftl/qt/qt-misc.ftl

@@ -36,7 +36,6 @@ qt-misc-please-select-a-deck = Please select a deck.
 qt-misc-please-use-fileimport-to-import-this = Please use File>Import to import this file.
 qt-misc-processing = Processing...
 qt-misc-replace-your-collection-with-an-earlier = Replace your collection with an earlier backup?
-qt-misc-resume-now = Resume Now
 qt-misc-revert-to-backup = Revert to backup
 qt-misc-reverted-to-state-prior-to = Reverted to state prior to '{ $val }'.
 qt-misc-segoe-ui = "Segoe UI"
@@ -56,7 +55,6 @@ qt-misc-unable-to-move-existing-file-to = Unable to move existing file to trash
 qt-misc-undo = Undo
 qt-misc-undo2 = Undo { $val }
 qt-misc-unexpected-response-code = Unexpected response code: { $val }
-qt-misc-waiting-for-editing-to-finish = Waiting for editing to finish.
 qt-misc-would-you-like-to-download-it = Would you like to download it now?
 qt-misc-your-collection-file-appears-to-be = Your collection file appears to be corrupt. This can happen when the file is copied or moved while Anki is open, or when the collection is stored on a network or cloud drive. If problems persist after restarting your computer, please open an automatic backup from the profile screen.
 qt-misc-your-computers-storage-may-be-full = Your computer's storage may be full. Please delete some unneeded files, then try again.

pip/pyqt5/install_pyqt5.py

@@ -91,7 +91,17 @@ def copy_and_fix_pyi(source, dest):
     with open(source) as input_file:
         with open(dest, "w") as output_file:
             for line in input_file.readlines():
+                # assigning to None is a syntax error
                 line = fix_none.sub(r"\1_ =", line)
+                # inheriting from the missing sip.sipwrapper definition
+                # causes missing attributes not to be detected, as it's treating
+                # the class as inheriting from Any
+                line = line.replace("sip.simplewrapper", "object")
+                line = line.replace("sip.wrapper", "object")
+                # remove blanket getattr in QObject which also causes missing
+                # attributes not to be detected
+                if "def __getattr__(self, name: str) -> typing.Any" in line:
+                    continue
                 output_file.write(line)
 
 

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

@@ -3,6 +3,22 @@
 
 from __future__ import annotations
 
+from typing import Any, List, Literal, Optional, Sequence, Tuple, Union
+
+import anki._backend.backend_pb2 as _pb
+
+# protobuf we publicly export - listed first to avoid circular imports
+SearchNode = _pb.SearchNode
+Progress = _pb.Progress
+EmptyCardsReport = _pb.EmptyCardsReport
+GraphPreferences = _pb.GraphPreferences
+BuiltinSort = _pb.SortOrder.Builtin
+Preferences = _pb.Preferences
+UndoStatus = _pb.UndoStatus
+OpChanges = _pb.OpChanges
+OpChangesWithCount = _pb.OpChangesWithCount
+DefaultsForAdding = _pb.DeckAndNotetype
+
 import copy
 import os
 import pprint
@@ -12,12 +28,8 @@ import time
 import traceback
 import weakref
 from dataclasses import dataclass, field
-from typing import Any, List, Literal, Optional, Sequence, Tuple, Union
 
-import anki._backend.backend_pb2 as _pb
+import anki.latex
-import anki.find
-import anki.latex  # sets up hook
-import anki.template
 from anki import hooks
 from anki._backend import RustBackend
 from anki.cards import Card
@@ -45,16 +57,10 @@ from anki.utils import (
     stripHTMLMedia,
 )
 
-# public exports
+anki.latex.setup_hook()
-SearchNode = _pb.SearchNode
+
+
 SearchJoiner = Literal["AND", "OR"]
-Progress = _pb.Progress
-EmptyCardsReport = _pb.EmptyCardsReport
-GraphPreferences = _pb.GraphPreferences
-BuiltinSort = _pb.SortOrder.Builtin
-Preferences = _pb.Preferences
-UndoStatus = _pb.UndoStatus
-DefaultsForAdding = _pb.DeckAndNotetype
 
 
 @dataclass
@@ -195,23 +201,17 @@ class Collection:
 
     flush = setMod
 
-    def modified_after_begin(self) -> bool:
+    def modified_by_backend(self) -> bool:
         # Until we can move away from long-running transactions, the Python
-        # code needs to know if transaction should be committed, so we need
+        # code needs to know if the transaction should be committed, so we need
         # to check if the backend updated the modification time.
         return self.db.last_begin_at != self.mod
 
     def save(self, name: Optional[str] = None, trx: bool = True) -> None:
         "Flush, commit DB, and take out another write lock if trx=True."
         # commit needed?
-        if self.db.modified_in_python or self.modified_after_begin():
+        if self.db.modified_in_python or self.modified_by_backend():
-            if self.db.modified_in_python:
+            self.db.modified_in_python = False
-                self.db.execute("update col set mod = ?", intTime(1000))
-                self.db.modified_in_python = False
-            else:
-                # modifications made by the backend will have already bumped
-                # mtime
-                pass
             self.db.commit()
             if trx:
                 self.db.begin()
@@ -328,10 +328,12 @@ class Collection:
     def get_note(self, id: int) -> Note:
         return Note(self, id=id)
 
-    def update_note(self, note: Note) -> None:
+    def update_note(self, note: Note) -> OpChanges:
         """Save note changes to database, and add an undo entry.
         Unlike note.flush(), this will invalidate any current checkpoint."""
-        self._backend.update_note(note=note._to_backend_note(), skip_undo_entry=False)
+        return self._backend.update_note(
+            note=note._to_backend_note(), skip_undo_entry=False
+        )
 
     getCard = get_card
     getNote = get_note
@@ -366,12 +368,14 @@ class Collection:
     def new_note(self, notetype: NoteType) -> Note:
         return Note(self, notetype)
 
-    def add_note(self, note: Note, deck_id: int) -> None:
+    def add_note(self, note: Note, deck_id: int) -> OpChanges:
-        note.id = self._backend.add_note(note=note._to_backend_note(), deck_id=deck_id)
+        out = self._backend.add_note(note=note._to_backend_note(), deck_id=deck_id)
+        note.id = out.note_id
+        return out.changes
 
-    def remove_notes(self, note_ids: Sequence[int]) -> None:
+    def remove_notes(self, note_ids: Sequence[int]) -> OpChanges:
         hooks.notes_will_be_deleted(self, note_ids)
-        self._backend.remove_notes(note_ids=note_ids, card_ids=[])
+        return self._backend.remove_notes(note_ids=note_ids, card_ids=[])
 
     def remove_notes_by_card(self, card_ids: List[int]) -> None:
         if hooks.notes_will_be_deleted.count():
@@ -445,8 +449,8 @@ class Collection:
         "You probably want .remove_notes_by_card() instead."
         self._backend.remove_cards(card_ids=card_ids)
 
-    def set_deck(self, card_ids: List[int], deck_id: int) -> None:
+    def set_deck(self, card_ids: Sequence[int], deck_id: int) -> OpChanges:
-        self._backend.set_deck(card_ids=card_ids, deck_id=deck_id)
+        return self._backend.set_deck(card_ids=card_ids, deck_id=deck_id)
 
     def get_empty_cards(self) -> EmptyCardsReport:
         return self._backend.get_empty_cards()
@@ -536,14 +540,26 @@ class Collection:
 
     def find_and_replace(
         self,
-        nids: List[int],
+        *,
-        src: str,
+        note_ids: Sequence[int],
-        dst: str,
+        search: str,
-        regex: Optional[bool] = None,
+        replacement: str,
-        field: Optional[str] = None,
+        regex: bool = False,
-        fold: bool = True,
+        field_name: Optional[str] = None,
-    ) -> int:
+        match_case: bool = False,
-        return anki.find.findReplace(self, nids, src, dst, regex, field, fold)
+    ) -> OpChangesWithCount:
+        "Find and replace fields in a note. Returns changed note count."
+        return self._backend.find_and_replace(
+            nids=note_ids,
+            search=search,
+            replacement=replacement,
+            regex=regex,
+            match_case=match_case,
+            field_name=field_name or "",
+        )
+
+    def field_names_for_note_ids(self, nids: Sequence[int]) -> Sequence[str]:
+        return self._backend.field_names_for_notes(nids)
 
     # returns array of ("dupestr", [nids])
     def findDupes(self, fieldName: str, search: str = "") -> List[Tuple[Any, list]]:
@@ -788,8 +804,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
@@ -817,6 +831,18 @@ table.review-log {{ {revlog_style} }}
             assert_exhaustive(self._undo)
             assert False
 
+    def op_affects_study_queue(self, changes: OpChanges) -> bool:
+        if changes.kind == changes.SET_CARD_FLAG:
+            return False
+        return changes.card or changes.deck or changes.preference
+
+    def op_made_changes(self, changes: OpChanges) -> bool:
+        for field in changes.DESCRIPTOR.fields:
+            if field.name != "kind":
+                if getattr(changes, field.name, False):
+                    return True
+        return False
+
     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."""
@@ -986,21 +1012,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:
+    def set_user_flag_for_cards(self, flag: int, cids: Sequence[int]) -> OpChanges:
-        assert 0 <= flag <= 7
+        return 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()

pylib/anki/decks.py

@@ -11,6 +11,7 @@ from typing import Any, Dict, Iterable, List, Optional, Sequence, Tuple, Union
 
 import anki  # pylint: disable=unused-import
 import anki._backend.backend_pb2 as _pb
+from anki.collection import OpChangesWithCount
 from anki.consts import *
 from anki.errors import NotFoundError
 from anki.utils import from_json_bytes, ids2str, intTime, legacy_func, to_json_bytes
@@ -138,7 +139,7 @@ class DeckManager:
         assert cardsToo and childrenToo
         self.remove([did])
 
-    def remove(self, dids: List[int]) -> int:
+    def remove(self, dids: Sequence[int]) -> OpChangesWithCount:
         return self.col._backend.remove_decks(dids)
 
     def all_names_and_ids(

pylib/anki/find.py

@@ -37,18 +37,19 @@ def findReplace(
     fold: bool = True,
 ) -> int:
     "Find and replace fields in a note. Returns changed note count."
-    return col._backend.find_and_replace(
+    print("use col.find_and_replace() instead of findReplace()")
-        nids=nids,
+    return col.find_and_replace(
+        note_ids=nids,
         search=src,
         replacement=dst,
         regex=regex,
         match_case=not fold,
         field_name=field,
-    )
+    ).count
 
 
 def fieldNamesForNotes(col: Collection, nids: List[int]) -> List[str]:
-    return list(col._backend.field_names_for_notes(nids))
+    return list(col.field_names_for_note_ids(nids))
 
 
 # Find duplicates

pylib/anki/latex.py

@@ -178,4 +178,5 @@ def _errMsg(col: anki.collection.Collection, type: str, texpath: str) -> Any:
     return msg
 
 
-hooks.card_did_render.append(on_card_did_render)
+def setup_hook() -> None:
+    hooks.card_did_render.append(on_card_did_render)

pylib/anki/scheduler/base.py

@@ -5,6 +5,7 @@ from __future__ import annotations
 
 import anki
 import anki._backend.backend_pb2 as _pb
+from anki.collection import OpChanges, OpChangesWithCount
 from anki.config import Config
 
 SchedTimingToday = _pb.SchedTimingTodayOut
@@ -96,11 +97,11 @@ select id from cards where did in %s and queue = {QUEUE_TYPE_REV} and due <= ? l
     # Suspending & burying
     ##########################################################################
 
-    def unsuspend_cards(self, ids: List[int]) -> None:
+    def unsuspend_cards(self, ids: Sequence[int]) -> OpChanges:
-        self.col._backend.restore_buried_and_suspended_cards(ids)
+        return self.col._backend.restore_buried_and_suspended_cards(ids)
 
-    def unbury_cards(self, ids: List[int]) -> None:
+    def unbury_cards(self, ids: List[int]) -> OpChanges:
-        self.col._backend.restore_buried_and_suspended_cards(ids)
+        return self.col._backend.restore_buried_and_suspended_cards(ids)
 
     def unbury_cards_in_current_deck(
         self,
@@ -108,17 +109,17 @@ select id from cards where did in %s and queue = {QUEUE_TYPE_REV} and due <= ? l
     ) -> None:
         self.col._backend.unbury_cards_in_current_deck(mode)
 
-    def suspend_cards(self, ids: Sequence[int]) -> None:
+    def suspend_cards(self, ids: Sequence[int]) -> OpChanges:
-        self.col._backend.bury_or_suspend_cards(
+        return self.col._backend.bury_or_suspend_cards(
             card_ids=ids, mode=BuryOrSuspend.SUSPEND
         )
 
-    def bury_cards(self, ids: Sequence[int], manual: bool = True) -> None:
+    def bury_cards(self, ids: Sequence[int], manual: bool = True) -> OpChanges:
         if manual:
             mode = BuryOrSuspend.BURY_USER
         else:
             mode = BuryOrSuspend.BURY_SCHED
-        self.col._backend.bury_or_suspend_cards(card_ids=ids, mode=mode)
+        return self.col._backend.bury_or_suspend_cards(card_ids=ids, mode=mode)
 
     def bury_note(self, note: Note) -> None:
         self.bury_cards(note.card_ids())
@@ -126,16 +127,16 @@ select id from cards where did in %s and queue = {QUEUE_TYPE_REV} and due <= ? l
     # Resetting/rescheduling
     ##########################################################################
 
-    def schedule_cards_as_new(self, card_ids: List[int]) -> None:
+    def schedule_cards_as_new(self, card_ids: List[int]) -> OpChanges:
         "Put cards at the end of the new queue."
-        self.col._backend.schedule_cards_as_new(card_ids=card_ids, log=True)
+        return self.col._backend.schedule_cards_as_new(card_ids=card_ids, log=True)
 
     def set_due_date(
         self,
         card_ids: List[int],
         days: str,
         config_key: Optional[Config.String.Key.V] = None,
-    ) -> None:
+    ) -> OpChanges:
         """Set cards to be due in `days`, turning them into review cards if necessary.
         `days` can be of the form '5' or '5..7'
         If `config_key` is provided, provided days will be remembered in config."""
@@ -143,7 +144,9 @@ select id from cards where did in %s and queue = {QUEUE_TYPE_REV} and due <= ? l
             key = Config.String(key=config_key)
         else:
             key = None
-        self.col._backend.set_due_date(card_ids=card_ids, days=days, config_key=key)
+        return self.col._backend.set_due_date(
+            card_ids=card_ids, days=days, config_key=key
+        )
 
     def resetCards(self, ids: List[int]) -> None:
         "Completely reset cards for export."
@@ -164,20 +167,20 @@ select id from cards where did in %s and queue = {QUEUE_TYPE_REV} and due <= ? l
     # Repositioning new cards
     ##########################################################################
 
-    def sortCards(
+    def reposition_new_cards(
         self,
-        cids: List[int],
+        card_ids: Sequence[int],
-        start: int = 1,
+        starting_from: int,
-        step: int = 1,
+        step_size: int,
-        shuffle: bool = False,
+        randomize: bool,
-        shift: bool = False,
+        shift_existing: bool,
-    ) -> None:
+    ) -> OpChangesWithCount:
-        self.col._backend.sort_cards(
+        return self.col._backend.sort_cards(
-            card_ids=cids,
+            card_ids=card_ids,
-            starting_from=start,
+            starting_from=starting_from,
-            step_size=step,
+            step_size=step_size,
-            randomize=shuffle,
+            randomize=randomize,
-            shift_existing=shift,
+            shift_existing=shift_existing,
         )
 
     def randomizeCards(self, did: int) -> None:
@@ -201,3 +204,14 @@ select id from cards where did in %s and queue = {QUEUE_TYPE_REV} and due <= ? l
         # in order due?
         if conf["new"]["order"] == NEW_CARDS_RANDOM:
             self.randomizeCards(did)
+
+    # legacy
+    def sortCards(
+        self,
+        cids: List[int],
+        start: int = 1,
+        step: int = 1,
+        shuffle: bool = False,
+        shift: bool = False,
+    ) -> None:
+        self.reposition_new_cards(cids, start, step, shuffle, shift)

pylib/anki/tags.py

@@ -18,10 +18,12 @@ from typing import Collection, List, Match, Optional, Sequence
 import anki  # pylint: disable=unused-import
 import anki._backend.backend_pb2 as _pb
 import anki.collection
+from anki.collection import OpChangesWithCount
 from anki.utils import ids2str
 
 # public exports
 TagTreeNode = _pb.TagTreeNode
+MARKED_TAG = "marked"
 
 
 class TagManager:
@@ -43,17 +45,8 @@ class TagManager:
     # Registering and fetching tags
     #############################################################
 
-    def register(
+    def clear_unused_tags(self) -> OpChangesWithCount:
-        self, tags: Collection[str], usn: Optional[int] = None, clear: bool = False
+        return self.col._backend.clear_unused_tags()
-    ) -> None:
-        print("tags.register() is deprecated and no longer works")
-
-    def registerNotes(self, nids: Optional[List[int]] = None) -> None:
-        "Clear unused tags and add any missing tags from notes to the tag list."
-        self.clear_unused_tags()
-
-    def clear_unused_tags(self) -> None:
-        self.col._backend.clear_unused_tags()
 
     def byDeck(self, did: int, children: bool = False) -> List[str]:
         basequery = "select n.tags from cards c, notes n WHERE c.nid = n.id"
@@ -72,52 +65,53 @@ class TagManager:
         "Set browser expansion state for tag, registering the tag if missing."
         self.col._backend.set_tag_expanded(name=tag, expanded=expanded)
 
-    # Bulk addition/removal from notes
+    # Bulk addition/removal from specific notes
     #############################################################
 
-    def bulk_add(self, nids: List[int], tags: str) -> int:
+    def bulk_add(self, note_ids: Sequence[int], tags: str) -> OpChangesWithCount:
         """Add space-separate tags to provided notes, returning changed count."""
-        return self.col._backend.add_note_tags(nids=nids, tags=tags)
+        return self.col._backend.add_note_tags(note_ids=note_ids, tags=tags)
 
-    def bulk_update(
+    def bulk_remove(self, note_ids: Sequence[int], tags: str) -> OpChangesWithCount:
-        self, nids: Sequence[int], tags: str, replacement: str, regex: bool
+        return self.col._backend.remove_note_tags(note_ids=note_ids, tags=tags)
-    ) -> int:
+
-        """Replace space-separated tags, returning changed count.
+    # Find&replace
-        Tags replaced with an empty string will be removed."""
+    #############################################################
-        return self.col._backend.update_note_tags(
+
-            nids=nids, tags=tags, replacement=replacement, regex=regex
+    def find_and_replace(
+        self,
+        note_ids: Sequence[int],
+        search: str,
+        replacement: str,
+        regex: bool,
+        match_case: bool,
+    ) -> OpChangesWithCount:
+        """Replace instances of 'search' with 'replacement' in tags.
+        Each tag is matched separately. If the replacement results in an empty string,
+        the tag will be removed."""
+        return self.col._backend.find_and_replace_tag(
+            note_ids=note_ids,
+            search=search,
+            replacement=replacement,
+            regex=regex,
+            match_case=match_case,
         )
 
-    def bulk_remove(self, nids: Sequence[int], tags: str) -> int:
+    # Bulk addition/removal based on tag
-        return self.bulk_update(nids, tags, "", False)
+    #############################################################
 
-    def rename(self, old: str, new: str) -> int:
+    def rename(self, old: str, new: str) -> OpChangesWithCount:
-        "Rename provided tag, returning number of changed notes."
+        "Rename provided tag and its children, returning number of changed notes."
-        nids = self.col.find_notes(anki.collection.SearchNode(tag=old))
+        return self.col._backend.rename_tags(current_prefix=old, new_prefix=new)
-        if not nids:
-            return 0
-        escaped_name = re.sub(r"[*_\\]", r"\\\g<0>", old)
-        return self.bulk_update(nids, escaped_name, new, False)
 
-    def remove(self, tag: str) -> None:
+    def remove(self, space_separated_tags: str) -> OpChangesWithCount:
-        self.col._backend.clear_tag(tag)
+        "Remove the provided tag(s) and their children from notes and the tag list."
+        return self.col._backend.remove_tags(val=space_separated_tags)
 
-    def drag_drop(self, source_tags: List[str], target_tag: str) -> None:
+    def reparent(self, tags: Sequence[str], new_parent: str) -> OpChangesWithCount:
-        """Rename one or more source tags that were dropped on `target_tag`.
+        """Change the parent of the provided tags.
-        If target_tag is "", tags will be placed at the top level."""
+        If new_parent is empty, tags will be reparented to the top-level."""
-        self.col._backend.drag_drop_tags(source_tags=source_tags, target_tag=target_tag)
+        return self.col._backend.reparent_tags(tags=tags, new_parent=new_parent)
-
-    # legacy routines
-
-    def bulkAdd(self, ids: List[int], tags: str, add: bool = True) -> None:
-        "Add tags in bulk. TAGS is space-separated."
-        if add:
-            self.bulk_add(ids, tags)
-        else:
-            self.bulk_update(ids, tags, "", False)
-
-    def bulkRem(self, ids: List[int], tags: str) -> None:
-        self.bulkAdd(ids, tags, False)
 
     # String-based utilities
     ##########################################################################
@@ -169,3 +163,24 @@ class TagManager:
     def inList(self, tag: str, tags: List[str]) -> bool:
         "True if TAG is in TAGS. Ignore case."
         return tag.lower() in [t.lower() for t in tags]
+
+    # legacy
+    ##########################################################################
+
+    def registerNotes(self, nids: Optional[List[int]] = None) -> None:
+        self.clear_unused_tags()
+
+    def register(
+        self, tags: Collection[str], usn: Optional[int] = None, clear: bool = False
+    ) -> None:
+        print("tags.register() is deprecated and no longer works")
+
+    def bulkAdd(self, ids: List[int], tags: str, add: bool = True) -> None:
+        "Add tags in bulk. TAGS is space-separated."
+        if add:
+            self.bulk_add(ids, tags)
+        else:
+            self.bulk_remove(ids, tags)
+
+    def bulkRem(self, ids: List[int], tags: str) -> None:
+        self.bulkAdd(ids, tags, False)

pylib/tests/test_find.py

@@ -243,24 +243,40 @@ def test_findReplace():
     col.addNote(note2)
     nids = [note.id, note2.id]
     # should do nothing
-    assert col.findReplace(nids, "abc", "123") == 0
+    assert (
+        col.find_and_replace(note_ids=nids, search="abc", replacement="123").count == 0
+    )
     # global replace
-    assert col.findReplace(nids, "foo", "qux") == 2
+    assert (
+        col.find_and_replace(note_ids=nids, search="foo", replacement="qux").count == 2
+    )
     note.load()
     assert note["Front"] == "qux"
     note2.load()
     assert note2["Back"] == "qux"
     # single field replace
-    assert col.findReplace(nids, "qux", "foo", field="Front") == 1
+    assert (
+        col.find_and_replace(
+            note_ids=nids, search="qux", replacement="foo", field_name="Front"
+        ).count
+        == 1
+    )
     note.load()
     assert note["Front"] == "foo"
     note2.load()
     assert note2["Back"] == "qux"
     # regex replace
-    assert col.findReplace(nids, "B.r", "reg") == 0
+    assert (
+        col.find_and_replace(note_ids=nids, search="B.r", replacement="reg").count == 0
+    )
     note.load()
     assert note["Back"] != "reg"
-    assert col.findReplace(nids, "B.r", "reg", regex=True) == 1
+    assert (
+        col.find_and_replace(
+            note_ids=nids, search="B.r", replacement="reg", regex=True
+        ).count
+        == 1
+    )
     note.load()
     assert note["Back"] == "reg"
 

pylib/tests/test_schedv1.py

@@ -1023,62 +1023,6 @@ def test_deckFlow():
         col.sched.answerCard(c, 2)
 
 
-def test_reorder():
-    col = getEmptyCol()
-    # add a note with default deck
-    note = col.newNote()
-    note["Front"] = "one"
-    col.addNote(note)
-    note2 = col.newNote()
-    note2["Front"] = "two"
-    col.addNote(note2)
-    assert note2.cards()[0].due == 2
-    found = False
-    # 50/50 chance of being reordered
-    for i in range(20):
-        col.sched.randomizeCards(1)
-        if note.cards()[0].due != note.id:
-            found = True
-            break
-    assert found
-    col.sched.orderCards(1)
-    assert note.cards()[0].due == 1
-    # shifting
-    note3 = col.newNote()
-    note3["Front"] = "three"
-    col.addNote(note3)
-    note4 = col.newNote()
-    note4["Front"] = "four"
-    col.addNote(note4)
-    assert note.cards()[0].due == 1
-    assert note2.cards()[0].due == 2
-    assert note3.cards()[0].due == 3
-    assert note4.cards()[0].due == 4
-    col.sched.sortCards([note3.cards()[0].id, note4.cards()[0].id], start=1, shift=True)
-    assert note.cards()[0].due == 3
-    assert note2.cards()[0].due == 4
-    assert note3.cards()[0].due == 1
-    assert note4.cards()[0].due == 2
-
-
-def test_forget():
-    col = getEmptyCol()
-    note = col.newNote()
-    note["Front"] = "one"
-    col.addNote(note)
-    c = note.cards()[0]
-    c.queue = QUEUE_TYPE_REV
-    c.type = CARD_TYPE_REV
-    c.ivl = 100
-    c.due = 0
-    c.flush()
-    col.reset()
-    assert col.sched.counts() == (0, 0, 1)
-    col.sched.forgetCards([c.id])
-    col.reset()
-    assert col.sched.counts() == (1, 0, 0)
-
-
 def test_norelearn():
     col = getEmptyCol()
     # add a note

pylib/tests/test_schedv2.py

@@ -1211,7 +1211,13 @@ def test_reorder():
     assert note2.cards()[0].due == 2
     assert note3.cards()[0].due == 3
     assert note4.cards()[0].due == 4
-    col.sched.sortCards([note3.cards()[0].id, note4.cards()[0].id], start=1, shift=True)
+    col.sched.reposition_new_cards(
+        [note3.cards()[0].id, note4.cards()[0].id],
+        starting_from=1,
+        shift_existing=True,
+        step_size=1,
+        randomize=False,
+    )
     assert note.cards()[0].due == 3
     assert note2.cards()[0].due == 4
     assert note3.cards()[0].due == 1

qt/.pylintrc

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

qt/aqt/__init__.py

@@ -10,7 +10,7 @@ import os
 import sys
 import tempfile
 import traceback
-from typing import Any, Callable, Dict, List, Optional, Tuple, Union
+from typing import Any, Callable, Dict, List, Optional, Tuple, Union, cast
 
 import anki.lang
 from anki import version as _version
@@ -299,7 +299,7 @@ class AnkiApp(QApplication):
         if not sock.waitForReadyRead(self.TMOUT):
             sys.stderr.write(sock.errorString())
             return
-        path = bytes(sock.readAll()).decode("utf8")
+        path = bytes(cast(bytes, sock.readAll())).decode("utf8")
         self.appMsg.emit(path)  # type: ignore
         sock.disconnectFromServer()
 

qt/aqt/addcards.py

@@ -6,12 +6,12 @@ from typing import Callable, List, Optional
 import aqt.deckchooser
 import aqt.editor
 import aqt.forms
-from anki.collection import SearchNode
+from anki.collection import OpChanges, SearchNode
 from anki.consts import MODEL_CLOZE
 from anki.notes import DuplicateOrEmptyResult, Note
 from anki.utils import htmlToTextLine, isMac
 from aqt import AnkiQt, gui_hooks
-from aqt.main import ResetReason
+from aqt.note_ops import add_note
 from aqt.notetypechooser import NoteTypeChooser
 from aqt.qt import *
 from aqt.sound import av_player
@@ -104,10 +104,10 @@ class AddCards(QDialog):
         self.historyButton = b
 
     def setAndFocusNote(self, note: Note) -> None:
-        self.editor.setNote(note, focusTo=0)
+        self.editor.set_note(note, focusTo=0)
 
     def show_notetype_selector(self) -> None:
-        self.editor.saveNow(self.notetype_chooser.choose_notetype)
+        self.editor.call_after_note_saved(self.notetype_chooser.choose_notetype)
 
     def on_notetype_change(self, notetype_id: int) -> None:
         # need to adjust current deck?
@@ -182,7 +182,7 @@ class AddCards(QDialog):
         aqt.dialogs.open("Browser", self.mw, search=(SearchNode(nid=nid),))
 
     def add_current_note(self) -> None:
-        self.editor.saveNow(self._add_current_note)
+        self.editor.call_after_note_saved(self._add_current_note)
 
     def _add_current_note(self) -> None:
         note = self.editor.note
@@ -191,23 +191,24 @@ class AddCards(QDialog):
             return
 
         target_deck_id = self.deck_chooser.selected_deck_id
-        self.mw.col.add_note(note, target_deck_id)
 
-        # only used for detecting changed sticky fields on close
+        def on_success(changes: OpChanges) -> None:
-        self._last_added_note = note
+            # only used for detecting changed sticky fields on close
+            self._last_added_note = note
 
-        self.addHistory(note)
+            self.addHistory(note)
-        self.mw.requireReset(reason=ResetReason.AddCardsAddNote, context=self)
 
-        # workaround for PyQt focus bug
+            # workaround for PyQt focus bug
-        self.editor.hideCompleters()
+            self.editor.hideCompleters()
 
-        tooltip(tr(TR.ADDING_ADDED), period=500)
+            tooltip(tr(TR.ADDING_ADDED), period=500)
-        av_player.stop_and_clear_queue()
+            av_player.stop_and_clear_queue()
-        self._load_new_note(sticky_fields_from=note)
+            self._load_new_note(sticky_fields_from=note)
-        self.mw.col.autosave()  # fixme:
+            gui_hooks.add_cards_did_add_note(note)
 
-        gui_hooks.add_cards_did_add_note(note)
+        add_note(
+            mw=self.mw, note=note, target_deck_id=target_deck_id, success=on_success
+        )
 
     def _note_can_be_added(self, note: Note) -> bool:
         result = note.duplicate_or_empty()
@@ -258,7 +259,7 @@ class AddCards(QDialog):
             if ok:
                 onOk()
 
-        self.editor.saveNow(afterSave)
+        self.editor.call_after_note_saved(afterSave)
 
     def closeWithCallback(self, cb: Callable[[], None]) -> None:
         def doClose() -> None:

qt/aqt/addons.py

@@ -715,7 +715,7 @@ class AddonsDialog(QDialog):
         gui_hooks.addons_dialog_will_show(self)
         self.show()
 
-    def dragEnterEvent(self, event: QEvent) -> None:
+    def dragEnterEvent(self, event: QDragEnterEvent) -> None:
         mime = event.mimeData()
         if not mime.hasUrls():
             return None
@@ -724,7 +724,7 @@ class AddonsDialog(QDialog):
         if all(url.toLocalFile().endswith(ext) for url in urls):
             event.acceptProposedAction()
 
-    def dropEvent(self, event: QEvent) -> None:
+    def dropEvent(self, event: QDropEvent) -> None:
         mime = event.mimeData()
         paths = []
         for url in mime.urls():
@@ -908,7 +908,7 @@ class AddonsDialog(QDialog):
 
 
 class GetAddons(QDialog):
-    def __init__(self, dlg: QDialog) -> None:
+    def __init__(self, dlg: AddonsDialog) -> None:
         QDialog.__init__(self, dlg)
         self.addonsDlg = dlg
         self.mgr = dlg.mgr
@@ -1079,7 +1079,9 @@ class DownloaderInstaller(QObject):
 
         self.on_done = on_done
 
-        self.mgr.mw.progress.start(immediate=True, parent=self.parent())
+        parent = self.parent()
+        assert isinstance(parent, QWidget)
+        self.mgr.mw.progress.start(immediate=True, parent=parent)
         self.mgr.mw.taskman.run_in_background(self._download_all, self._download_done)
 
     def _progress_callback(self, up: int, down: int) -> None:
@@ -1438,7 +1440,7 @@ def prompt_to_update(
 
 
 class ConfigEditor(QDialog):
-    def __init__(self, dlg: QDialog, addon: str, conf: Dict) -> None:
+    def __init__(self, dlg: AddonsDialog, addon: str, conf: Dict) -> None:
         super().__init__(dlg)
         self.addon = addon
         self.conf = conf
@@ -1506,7 +1508,7 @@ class ConfigEditor(QDialog):
         txt = gui_hooks.addon_config_editor_will_save_json(txt)
         try:
             new_conf = json.loads(txt)
-            jsonschema.validate(new_conf, self.parent().mgr._addon_schema(self.addon))
+            jsonschema.validate(new_conf, self.mgr._addon_schema(self.addon))
         except ValidationError as e:
             # The user did edit the configuration and entered a value
             # which can not be interpreted.

qt/aqt/card_ops.py

@@ -0,0 +1,16 @@
+# 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 typing import Sequence
+
+from aqt import AnkiQt
+
+
+def set_card_deck(*, mw: AnkiQt, card_ids: Sequence[int], deck_id: int) -> None:
+    mw.perform_op(lambda: mw.col.set_deck(card_ids, deck_id))
+
+
+def set_card_flag(*, mw: AnkiQt, card_ids: Sequence[int], flag: int) -> None:
+    mw.perform_op(lambda: mw.col.set_user_flag_for_cards(flag, card_ids))

qt/aqt/clayout.py

@@ -795,7 +795,7 @@ class CardLayout(QDialog):
                 showWarning(str(e))
                 return
             self.mw.reset()
-            tooltip(tr(TR.CARD_TEMPLATES_CHANGES_SAVED), parent=self.parent())
+            tooltip(tr(TR.CARD_TEMPLATES_CHANGES_SAVED), parent=self.parentWidget())
             self.cleanup()
             gui_hooks.sidebar_should_refresh_notetypes()
             return QDialog.accept(self)

qt/aqt/deck_ops.py

@@ -0,0 +1,24 @@
+# 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 typing import Sequence
+
+from anki.lang import TR
+from aqt import AnkiQt, QWidget
+from aqt.utils import tooltip, tr
+
+
+def remove_decks(
+    *,
+    mw: AnkiQt,
+    parent: QWidget,
+    deck_ids: Sequence[int],
+) -> None:
+    mw.perform_op(
+        lambda: mw.col.decks.remove(deck_ids),
+        success=lambda out: tooltip(
+            tr(TR.BROWSING_CARDS_DELETED, count=out.count), parent=parent
+        ),
+    )

qt/aqt/deckbrowser.py

@@ -8,10 +8,12 @@ from dataclasses import dataclass
 from typing import Any
 
 import aqt
+from anki.collection import OpChanges
 from anki.decks import DeckTreeNode
 from anki.errors import DeckIsFilteredError
 from anki.utils import intTime
 from aqt import AnkiQt, gui_hooks
+from aqt.deck_ops import remove_decks
 from aqt.qt import *
 from aqt.sound import av_player
 from aqt.toolbar import BottomBar
@@ -23,7 +25,6 @@ from aqt.utils import (
     shortcut,
     showInfo,
     showWarning,
-    tooltip,
     tr,
 )
 
@@ -61,6 +62,7 @@ class DeckBrowser:
         self.bottom = BottomBar(mw, mw.bottomWeb)
         self.scrollPos = QPoint(0, 0)
         self._v1_message_dismissed_at = 0
+        self._refresh_needed = False
 
     def show(self) -> None:
         av_player.stop_and_clear_queue()
@@ -68,9 +70,24 @@ class DeckBrowser:
         self._renderPage()
         # redraw top bar for theme change
         self.mw.toolbar.redraw()
+        self.refresh()
 
     def refresh(self) -> None:
         self._renderPage()
+        self._refresh_needed = False
+
+    def refresh_if_needed(self) -> None:
+        if self._refresh_needed:
+            self.refresh()
+
+    def op_executed(self, changes: OpChanges, focused: bool) -> bool:
+        if self.mw.col.op_affects_study_queue(changes):
+            self._refresh_needed = True
+
+        if focused:
+            self.refresh_if_needed()
+
+        return self._refresh_needed
 
     # Event handlers
     ##########################################################################
@@ -145,7 +162,6 @@ class DeckBrowser:
             ],
             context=self,
         )
-        self.web.key = "deckBrowser"
         self._drawButtons()
         if offset is not None:
             self._scrollToOffset(offset)
@@ -305,15 +321,7 @@ class DeckBrowser:
         self.mw.taskman.with_progress(process, on_done)
 
     def _delete(self, did: int) -> None:
-        def do_delete() -> int:
+        remove_decks(mw=self.mw, parent=self.mw, deck_ids=[did])
-            return self.mw.col.decks.remove([did])
-
-        def on_done(fut: Future) -> None:
-            self.mw.update_undo_actions()
-            self.show()
-            tooltip(tr(TR.BROWSING_CARDS_DELETED, count=fut.result()))
-
-        self.mw.taskman.with_progress(do_delete, on_done)
 
     # Top buttons
     ######################################################################

qt/aqt/editcurrent.py

@@ -1,10 +1,12 @@
 # Copyright: Ankitects Pty Ltd and contributors
 # License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
 import aqt.editor
+from anki.collection import OpChanges
+from anki.errors import NotFoundError
 from aqt import gui_hooks
-from aqt.main import ResetReason
 from aqt.qt import *
-from aqt.utils import TR, disable_help_button, restoreGeom, saveGeom, tooltip, tr
+from aqt.utils import TR, disable_help_button, restoreGeom, saveGeom, tr
 
 
 class EditCurrent(QDialog):
@@ -23,59 +25,53 @@ class EditCurrent(QDialog):
         )
         self.editor = aqt.editor.Editor(self.mw, self.form.fieldsArea, self)
         self.editor.card = self.mw.reviewer.card
-        self.editor.setNote(self.mw.reviewer.card.note(), focusTo=0)
+        self.editor.set_note(self.mw.reviewer.card.note(), focusTo=0)
         restoreGeom(self, "editcurrent")
-        gui_hooks.state_did_reset.append(self.onReset)
+        gui_hooks.operation_did_execute.append(self.on_operation_did_execute)
-        self.mw.requireReset(reason=ResetReason.EditCurrentInit, context=self)
         self.show()
-        # reset focus after open, taking care not to retain webview
-        # pylint: disable=unnecessary-lambda
-        self.mw.progress.timer(100, lambda: self.editor.web.setFocus(), False)
 
-    def onReset(self) -> None:
+    def on_operation_did_execute(self, changes: OpChanges) -> None:
-        # lazy approach for now: throw away edits
+        if not (changes.note or changes.notetype):
-        try:
-            n = self.editor.note
-            n.load()  # reload in case the model changed
-        except:
-            # card's been deleted
-            gui_hooks.state_did_reset.remove(self.onReset)
-            self.editor.setNote(None)
-            self.mw.reset()
-            aqt.dialogs.markClosed("EditCurrent")
-            self.close()
             return
-        self.editor.setNote(n)
+        if self.editor.is_updating_note():
+            return
+
+        # reload note
+        note = self.editor.note
+        try:
+            note.load()
+        except NotFoundError:
+            # note's been deleted
+            self.cleanup_and_close()
+            return
+
+        self.editor.set_note(note)
+
+    def cleanup_and_close(self) -> None:
+        gui_hooks.operation_did_execute.remove(self.on_operation_did_execute)
+        self.editor.cleanup()
+        saveGeom(self, "editcurrent")
+        aqt.dialogs.markClosed("EditCurrent")
+        QDialog.reject(self)
 
     def reopen(self, mw: aqt.AnkiQt) -> None:
-        tooltip("Please finish editing the existing card first.")
+        if card := self.mw.reviewer.card:
-        self.onReset()
+            self.editor.set_note(card.note())
 
     def reject(self) -> None:
         self.saveAndClose()
 
     def saveAndClose(self) -> None:
-        self.editor.saveNow(self._saveAndClose)
+        self.editor.call_after_note_saved(self._saveAndClose)
 
     def _saveAndClose(self) -> None:
-        gui_hooks.state_did_reset.remove(self.onReset)
+        self.cleanup_and_close()
-        r = self.mw.reviewer
-        try:
-            r.card.load()
-        except:
-            # card was removed by clayout
-            pass
-        else:
-            self.mw.reviewer.cardQueue.append(self.mw.reviewer.card)
-        self.editor.cleanup()
-        self.mw.moveToState("review")
-        saveGeom(self, "editcurrent")
-        aqt.dialogs.markClosed("EditCurrent")
-        QDialog.reject(self)
 
     def closeWithCallback(self, onsuccess: Callable[[], None]) -> None:
         def callback() -> None:
             self._saveAndClose()
             onsuccess()
 
-        self.editor.saveNow(callback)
+        self.editor.call_after_note_saved(callback)
+
+    onReset = on_operation_did_execute

qt/aqt/editor.py

@@ -1,5 +1,8 @@
 # Copyright: Ankitects Pty Ltd and contributors
 # License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+from __future__ import annotations
+
 import base64
 import html
 import itertools
@@ -24,16 +27,17 @@ from anki.collection import Config, SearchNode
 from anki.consts import MODEL_CLOZE
 from anki.hooks import runFilter
 from anki.httpclient import HttpClient
-from anki.notes import Note
+from anki.notes import DuplicateOrEmptyResult, Note
 from anki.utils import checksum, isLin, isWin, namedtmp
 from aqt import AnkiQt, colors, gui_hooks
-from aqt.main import ResetReason
+from aqt.note_ops import update_note
 from aqt.qt import *
 from aqt.sound import av_player
 from aqt.theme import theme_manager
 from aqt.utils import (
     TR,
     HelpPage,
+    KeyboardModifiersPressed,
     disable_help_button,
     getFile,
     openHelp,
@@ -90,8 +94,17 @@ _html = """
 </div>
 """
 
-# caller is responsible for resetting note on reset
+
 class Editor:
+    """The screen that embeds an editing widget should listen for changes via
+    the `operation_did_execute` hook, and call set_note() when the editor needs
+    redrawing.
+
+    The editor will cause that hook to be fired when it saves changes. To avoid
+    an unwanted refresh, the parent widget should call editor.is_updating_note(),
+    and avoid re-setting the note if it returns true.
+    """
+
     def __init__(
         self, mw: AnkiQt, widget: QWidget, parentWindow: QWidget, addMode: bool = False
     ) -> None:
@@ -101,6 +114,7 @@ class Editor:
         self.note: Optional[Note] = None
         self.addMode = addMode
         self.currentField: Optional[int] = None
+        self._is_updating_note = False
         # current card, for card layout
         self.card: Optional[Card] = None
         self.setupOuter()
@@ -399,7 +413,7 @@ class Editor:
         return checkFocus
 
     def onFields(self) -> None:
-        self.saveNow(self._onFields)
+        self.call_after_note_saved(self._onFields)
 
     def _onFields(self) -> None:
         from aqt.fields import FieldDialog
@@ -407,7 +421,7 @@ class Editor:
         FieldDialog(self.mw, self.note.model(), parent=self.parentWindow)
 
     def onCardLayout(self) -> None:
-        self.saveNow(self._onCardLayout)
+        self.call_after_note_saved(self._onCardLayout)
 
     def _onCardLayout(self) -> None:
         from aqt.clayout import CardLayout
@@ -450,7 +464,6 @@ class Editor:
 
             if not self.addMode:
                 self._save_current_note()
-                self.mw.requireReset(reason=ResetReason.EditorBridgeCmd, context=self)
             if type == "blur":
                 self.currentField = None
                 # run any filters
@@ -459,10 +472,10 @@ class Editor:
                     # event has had time to fire
                     self.mw.progress.timer(100, self.loadNoteKeepingFocus, False)
                 else:
-                    self.checkValid()
+                    self._check_and_update_duplicate_display_async()
             else:
                 gui_hooks.editor_did_fire_typing_timer(self.note)
-                self.checkValid()
+                self._check_and_update_duplicate_display_async()
 
         # focused into field?
         elif cmd.startswith("focus"):
@@ -492,7 +505,7 @@ class Editor:
     # Setting/unsetting the current note
     ######################################################################
 
-    def setNote(
+    def set_note(
         self, note: Optional[Note], hide: bool = True, focusTo: Optional[int] = None
     ) -> None:
         "Make NOTE the current note."
@@ -519,11 +532,15 @@ class Editor:
         self.widget.show()
         self.updateTags()
 
+        dupe_status = self.note.duplicate_or_empty()
+
         def oncallback(arg: Any) -> None:
             if not self.note:
                 return
             self.setupForegroundButton()
-            self.checkValid()
+            # we currently do this synchronously to ensure we load before the
+            # sidebar on browser startup
+            self._update_duplicate_display(dupe_status)
             if focusTo is not None:
                 self.web.setFocus()
             gui_hooks.editor_did_load_note(self)
@@ -544,7 +561,14 @@ class Editor:
 
     def _save_current_note(self) -> None:
         "Call after note is updated with data from webview."
-        self.mw.col.update_note(self.note)
+        self._is_updating_note = True
+        update_note(mw=self.mw, note=self.note, after_hooks=self._after_updating_note)
+
+    def _after_updating_note(self) -> None:
+        self._is_updating_note = False
+
+    def is_updating_note(self) -> bool:
+        return self._is_updating_note
 
     def fonts(self) -> List[Tuple[str, int, bool]]:
         return [
@@ -552,19 +576,34 @@ class Editor:
             for f in self.note.model()["flds"]
         ]
 
-    def saveNow(self, callback: Callable, keepFocus: bool = False) -> None:
+    def call_after_note_saved(
+        self, callback: Callable, keepFocus: bool = False
+    ) -> None:
         "Save unsaved edits then call callback()."
         if not self.note:
             # calling code may not expect the callback to fire immediately
             self.mw.progress.timer(10, callback, False)
             return
-        self.saveTags()
+        self.blur_tags_if_focused()
         self.web.evalWithCallback("saveNow(%d)" % keepFocus, lambda res: callback())
 
-    def checkValid(self) -> None:
+    saveNow = call_after_note_saved
+
+    def _check_and_update_duplicate_display_async(self) -> None:
+        note = self.note
+
+        def on_done(result: DuplicateOrEmptyResult.V) -> None:
+            if self.note != note:
+                return
+            self._update_duplicate_display(result)
+
+        self.mw.query_op(self.note.duplicate_or_empty, success=on_done)
+
+    checkValid = _check_and_update_duplicate_display_async
+
+    def _update_duplicate_display(self, result: DuplicateOrEmptyResult.V) -> None:
         cols = [""] * len(self.note.fields)
-        err = self.note.duplicate_or_empty()
+        if result == DuplicateOrEmptyResult.DUPLICATE:
-        if err == 2:
             cols[0] = "dupe"
 
         self.web.eval(f"setBackgrounds({json.dumps(cols)});")
@@ -597,16 +636,20 @@ class Editor:
         return True
 
     def cleanup(self) -> None:
-        self.setNote(None)
+        self.set_note(None)
         # prevent any remaining evalWithCallback() events from firing after C++ object deleted
         self.web = None
 
+    # legacy
+
+    setNote = set_note
+
     # HTML editing
     ######################################################################
 
     def onHtmlEdit(self) -> None:
         field = self.currentField
-        self.saveNow(lambda: self._onHtmlEdit(field))
+        self.call_after_note_saved(lambda: self._onHtmlEdit(field))
 
     def _onHtmlEdit(self, field: int) -> None:
         d = QDialog(self.widget, Qt.Window)
@@ -656,7 +699,7 @@ class Editor:
         l = QLabel(tr(TR.EDITING_TAGS))
         tb.addWidget(l, 1, 0)
         self.tags = aqt.tagedit.TagEdit(self.widget)
-        qconnect(self.tags.lostFocus, self.saveTags)
+        qconnect(self.tags.lostFocus, self.on_tag_focus_lost)
         self.tags.setToolTip(
             shortcut(tr(TR.EDITING_JUMP_TO_TAGS_WITH_CTRLANDSHIFTANDT))
         )
@@ -672,13 +715,17 @@ class Editor:
         if not self.tags.text() or not self.addMode:
             self.tags.setText(self.note.stringTags().strip())
 
-    def saveTags(self) -> None:
+    def on_tag_focus_lost(self) -> None:
-        if not self.note:
-            return
         self.note.tags = self.mw.col.tags.split(self.tags.text())
+        gui_hooks.editor_did_update_tags(self.note)
         if not self.addMode:
             self._save_current_note()
-        gui_hooks.editor_did_update_tags(self.note)
+
+    def blur_tags_if_focused(self) -> None:
+        if not self.note:
+            return
+        if self.tags.hasFocus():
+            self.widget.setFocus()
 
     def hideCompleters(self) -> None:
         self.tags.hideCompleter()
@@ -687,9 +734,12 @@ class Editor:
         self.tags.setFocus()
 
     # legacy
+
     def saveAddModeVars(self) -> None:
         pass
 
+    saveTags = blur_tags_if_focused
+
     # Format buttons
     ######################################################################
 
@@ -712,7 +762,7 @@ class Editor:
         self.web.eval("setFormat('removeFormat');")
 
     def onCloze(self) -> None:
-        self.saveNow(self._onCloze, keepFocus=True)
+        self.call_after_note_saved(self._onCloze, keepFocus=True)
 
     def _onCloze(self) -> None:
         # check that the model is set up for cloze deletion
@@ -729,7 +779,7 @@ class Editor:
             if m:
                 highest = max(highest, sorted([int(x) for x in m])[-1])
         # reuse last?
-        if not self.mw.app.keyboardModifiers() & Qt.AltModifier:
+        if not KeyboardModifiersPressed().alt:
             highest += 1
         # must start at 1
         highest = max(1, highest)
@@ -1106,7 +1156,7 @@ class EditorWebView(AnkiWebView):
         strip_html = self.editor.mw.col.get_config_bool(
             Config.Bool.PASTE_STRIPS_FORMATTING
         )
-        if self.editor.mw.app.queryKeyboardModifiers() & Qt.ShiftModifier:
+        if KeyboardModifiersPressed().shift:
             strip_html = not strip_html
         return strip_html
 

qt/aqt/errors.py

@@ -4,7 +4,7 @@ import html
 import re
 import sys
 import traceback
-from typing import Optional
+from typing import Optional, TextIO, cast
 
 from markdown import markdown
 
@@ -37,7 +37,7 @@ class ErrorHandler(QObject):
         qconnect(self.errorTimer, self._setTimer)
         self.pool = ""
         self._oldstderr = sys.stderr
-        sys.stderr = self
+        sys.stderr = cast(TextIO, self)
 
     def unload(self) -> None:
         sys.stderr = self._oldstderr

qt/aqt/fields.py

@@ -26,7 +26,7 @@ from aqt.utils import (
 
 class FieldDialog(QDialog):
     def __init__(
-        self, mw: AnkiQt, nt: NoteType, parent: Optional[QDialog] = None
+        self, mw: AnkiQt, nt: NoteType, parent: Optional[QWidget] = None
     ) -> None:
         QDialog.__init__(self, parent or mw)
         self.mw = mw

qt/aqt/find_and_replace.py

@@ -0,0 +1,182 @@
+# 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 typing import List, Optional, Sequence
+
+import aqt
+from anki.lang import TR
+from aqt import AnkiQt, QWidget
+from aqt.qt import QDialog, Qt
+from aqt.utils import (
+    HelpPage,
+    disable_help_button,
+    openHelp,
+    qconnect,
+    restore_combo_history,
+    restore_combo_index_for_session,
+    restore_is_checked,
+    restoreGeom,
+    save_combo_history,
+    save_combo_index_for_session,
+    save_is_checked,
+    saveGeom,
+    show_invalid_search_error,
+    tooltip,
+    tr,
+)
+
+
+def find_and_replace(
+    *,
+    mw: AnkiQt,
+    parent: QWidget,
+    note_ids: Sequence[int],
+    search: str,
+    replacement: str,
+    regex: bool,
+    field_name: Optional[str],
+    match_case: bool,
+) -> None:
+    mw.perform_op(
+        lambda: mw.col.find_and_replace(
+            note_ids=note_ids,
+            search=search,
+            replacement=replacement,
+            regex=regex,
+            field_name=field_name,
+            match_case=match_case,
+        ),
+        success=lambda out: tooltip(
+            tr(TR.FINDREPLACE_NOTES_UPDATED, changed=out.count, total=len(note_ids)),
+            parent=parent,
+        ),
+        failure=lambda exc: show_invalid_search_error(exc, parent=parent),
+    )
+
+
+def find_and_replace_tag(
+    *,
+    mw: AnkiQt,
+    parent: QWidget,
+    note_ids: Sequence[int],
+    search: str,
+    replacement: str,
+    regex: bool,
+    match_case: bool,
+) -> None:
+    mw.perform_op(
+        lambda: mw.col.tags.find_and_replace(
+            note_ids=note_ids,
+            search=search,
+            replacement=replacement,
+            regex=regex,
+            match_case=match_case,
+        ),
+        success=lambda out: tooltip(
+            tr(TR.FINDREPLACE_NOTES_UPDATED, changed=out.count, total=len(note_ids)),
+            parent=parent,
+        ),
+        failure=lambda exc: show_invalid_search_error(exc, parent=parent),
+    )
+
+
+class FindAndReplaceDialog(QDialog):
+    COMBO_NAME = "BrowserFindAndReplace"
+
+    def __init__(self, parent: QWidget, *, mw: AnkiQt, note_ids: Sequence[int]) -> None:
+        super().__init__(parent)
+        self.mw = mw
+        self.note_ids = note_ids
+        self.field_names: List[str] = []
+
+        # fetch field names and then show
+        mw.query_op(
+            lambda: mw.col.field_names_for_note_ids(note_ids),
+            success=self._show,
+        )
+
+    def _show(self, field_names: Sequence[str]) -> None:
+        # add "all fields" and "tags" to the top of the list
+        self.field_names = [
+            tr(TR.BROWSING_ALL_FIELDS),
+            tr(TR.EDITING_TAGS),
+        ] + list(field_names)
+
+        disable_help_button(self)
+        self.form = aqt.forms.findreplace.Ui_Dialog()
+        self.form.setupUi(self)
+        self.setWindowModality(Qt.WindowModal)
+
+        self._find_history = restore_combo_history(
+            self.form.find, self.COMBO_NAME + "Find"
+        )
+        self.form.find.completer().setCaseSensitivity(True)
+        self._replace_history = restore_combo_history(
+            self.form.replace, self.COMBO_NAME + "Replace"
+        )
+        self.form.replace.completer().setCaseSensitivity(True)
+
+        restore_is_checked(self.form.re, self.COMBO_NAME + "Regex")
+        restore_is_checked(self.form.ignoreCase, self.COMBO_NAME + "ignoreCase")
+
+        self.form.field.addItems(self.field_names)
+        restore_combo_index_for_session(
+            self.form.field, self.field_names, self.COMBO_NAME + "Field"
+        )
+
+        qconnect(self.form.buttonBox.helpRequested, self.show_help)
+
+        restoreGeom(self, "findreplace")
+        self.show()
+        self.form.find.setFocus()
+
+    def accept(self) -> None:
+        saveGeom(self, "findreplace")
+        save_combo_index_for_session(self.form.field, self.COMBO_NAME + "Field")
+
+        search = save_combo_history(
+            self.form.find, self._find_history, self.COMBO_NAME + "Find"
+        )
+        replace = save_combo_history(
+            self.form.replace, self._replace_history, self.COMBO_NAME + "Replace"
+        )
+        regex = self.form.re.isChecked()
+        match_case = not self.form.ignoreCase.isChecked()
+        save_is_checked(self.form.re, self.COMBO_NAME + "Regex")
+        save_is_checked(self.form.ignoreCase, self.COMBO_NAME + "ignoreCase")
+
+        if self.form.field.currentIndex() == 1:
+            # tags
+            find_and_replace_tag(
+                mw=self.mw,
+                parent=self.parentWidget(),
+                note_ids=self.note_ids,
+                search=search,
+                replacement=replace,
+                regex=regex,
+                match_case=match_case,
+            )
+            return
+
+        if self.form.field.currentIndex() == 0:
+            field = None
+        else:
+            field = self.field_names[self.form.field.currentIndex() - 2]
+
+        find_and_replace(
+            mw=self.mw,
+            parent=self.parentWidget(),
+            note_ids=self.note_ids,
+            search=search,
+            replacement=replace,
+            regex=regex,
+            field_name=field,
+            match_case=match_case,
+        )
+
+        super().accept()
+
+    def show_help(self) -> None:
+        openHelp(HelpPage.BROWSING_FIND_AND_REPLACE)

qt/aqt/main.py

@@ -14,7 +14,20 @@ 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,
+    Literal,
+    Optional,
+    Protocol,
+    Sequence,
+    TextIO,
+    Tuple,
+    TypeVar,
+    cast,
+)
 
 import anki
 import aqt
@@ -32,8 +45,11 @@ from anki.collection import (
     Checkpoint,
     Collection,
     Config,
+    OpChanges,
+    OpChangesWithCount,
     ReviewUndo,
     UndoResult,
+    UndoStatus,
 )
 from anki.decks import Deck
 from anki.hooks import runHook
@@ -56,8 +72,10 @@ from aqt.theme import theme_manager
 from aqt.utils import (
     TR,
     HelpPage,
+    KeyboardModifiersPressed,
     askUser,
     checkInvalidFilename,
+    current_top_level_widget,
     disable_help_button,
     getFile,
     getOnlyText,
@@ -71,31 +89,29 @@ from aqt.utils import (
     showInfo,
     showWarning,
     tooltip,
+    top_level_widget,
     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, HasChangesProperty]
+)
+
+PerformOpOptionalSuccessCallback = Optional[Callable[[ResultWithChanges], Any]]
+
 install_pylib_legacy()
 
-
+MainWindowState = Literal[
-class ResetReason(enum.Enum):
+    "startup", "deckBrowser", "overview", "review", "resetRequired", "profileManager"
-    Unknown = "unknown"
+]
-    AddCardsAddNote = "addCardsAddNote"
-    EditCurrentInit = "editCurrentInit"
-    EditorBridgeCmd = "editorBridgeCmd"
-    BrowserSetDeck = "browserSetDeck"
-    BrowserAddTags = "browserAddTags"
-    BrowserRemoveTags = "browserRemoveTags"
-    BrowserSuspend = "browserSuspend"
-    BrowserReposition = "browserReposition"
-    BrowserReschedule = "browserReschedule"
-    BrowserFindReplace = "browserFindReplace"
-    BrowserTagDupes = "browserTagDupes"
-    BrowserDeleteDeck = "browserDeleteDeck"
-
-
-class ResetRequired:
-    def __init__(self, mw: AnkiQt) -> None:
-        self.mw = mw
 
 
 class AnkiQt(QMainWindow):
@@ -106,7 +122,7 @@ class AnkiQt(QMainWindow):
 
     def __init__(
         self,
-        app: QApplication,
+        app: aqt.AnkiApp,
         profileManager: ProfileManagerType,
         backend: _RustBackend,
         opts: Namespace,
@@ -114,7 +130,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)
@@ -123,9 +139,7 @@ class AnkiQt(QMainWindow):
         self.app = app
         self.pm = profileManager
         # init rest of app
-        self.safeMode = (
+        self.safeMode = (KeyboardModifiersPressed().shift) or self.opts.safemode
-            self.app.queryKeyboardModifiers() & Qt.ShiftModifier
-        ) or self.opts.safemode
         try:
             self.setupUI()
             self.setupAddons(args)
@@ -173,6 +187,7 @@ class AnkiQt(QMainWindow):
         self.setupHooks()
         self.setup_timers()
         self.updateTitleBar()
+        self.setup_focus()
         # screens
         self.setupDeckBrowser()
         self.setupOverview()
@@ -201,6 +216,12 @@ class AnkiQt(QMainWindow):
         "Shortcut to create a weak reference that doesn't break code completion."
         return weakref.proxy(self)  # type: ignore
 
+    def setup_focus(self) -> None:
+        qconnect(self.app.focusChanged, self.on_focus_changed)
+
+    def on_focus_changed(self, old: QWidget, new: QWidget) -> None:
+        gui_hooks.focus_did_change(new, old)
+
     # Profiles
     ##########################################################################
 
@@ -650,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
@@ -694,66 +715,214 @@ class AnkiQt(QMainWindow):
     # Resetting state
     ##########################################################################
 
-    def reset(self, guiOnly: bool = False) -> None:
+    def query_op(
-        "Called for non-trivial edits. Rebuilds queue and updates UI."
+        self,
+        op: Callable[[], Any],
+        *,
+        success: Callable[[Any], Any] = None,
+        failure: Optional[Callable[[Exception], Any]] = None,
+    ) -> None:
+        """Run an operation that queries the DB on a background thread.
+
+        Similar interface to perform_op(), but intended to be used for operations
+        that do not change collection state. Undo status will not be changed,
+        and `operation_did_execute` will not fire. No progress window will
+        be shown either.
+
+        `operations_will|did_execute` will still fire, so the UI can defer
+        updates during a background task.
+        """
+
+        def wrapped_done(future: Future) -> None:
+            self._decrease_background_ops()
+            # did something go wrong?
+            if exception := future.exception():
+                if isinstance(exception, Exception):
+                    if failure:
+                        failure(exception)
+                    else:
+                        showWarning(str(exception))
+                    return
+                else:
+                    # BaseException like SystemExit; rethrow it
+                    future.result()
+
+            result = future.result()
+            if success:
+                success(result)
+
+        self._increase_background_ops()
+        self.taskman.run_in_background(op, wrapped_done)
+
+    # Resetting state
+    ##########################################################################
+
+    def perform_op(
+        self,
+        op: Callable[[], ResultWithChanges],
+        *,
+        success: PerformOpOptionalSuccessCallback = None,
+        failure: Optional[Callable[[Exception], Any]] = None,
+        after_hooks: Optional[Callable[[], None]] = None,
+    ) -> None:
+        """Run the provided operation on a background thread.
+
+        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.
+
+        after_hooks() will be called after hooks are fired, if it is provided.
+        Components can use this to ignore change notices generated by operations
+        they invoke themselves, or perform some subsequent action.
+        """
+
+        self._increase_background_ops()
+
+        def wrapped_done(future: Future) -> None:
+            self._decrease_background_ops()
+            # did something go wrong?
+            if exception := future.exception():
+                if isinstance(exception, Exception):
+                    if failure:
+                        failure(exception)
+                    else:
+                        showWarning(str(exception))
+                    return
+                else:
+                    # BaseException like SystemExit; rethrow it
+                    future.result()
+
+            result = future.result()
+            try:
+                if success:
+                    success(result)
+            finally:
+                # update undo status
+                status = self.col.undo_status()
+                self._update_undo_actions_for_status_and_save(status)
+                # fire change hooks
+                self._fire_change_hooks_after_op_performed(result, after_hooks)
+
+        self.taskman.with_progress(op, wrapped_done)
+
+    def _increase_background_ops(self) -> None:
+        if not self._background_op_count:
+            gui_hooks.backend_will_block()
+        self._background_op_count += 1
+
+    def _decrease_background_ops(self) -> None:
+        self._background_op_count -= 1
+        if not self._background_op_count:
+            gui_hooks.backend_did_block()
+        assert self._background_op_count >= 0
+
+    def _fire_change_hooks_after_op_performed(
+        self, result: ResultWithChanges, after_hooks: Optional[Callable[[], None]]
+    ) -> None:
+        if isinstance(result, OpChanges):
+            changes = result
+        else:
+            changes = result.changes
+
+        # fire new hook
+        print("op changes:")
+        print(changes)
+        gui_hooks.operation_did_execute(changes)
+        # fire legacy hook so old code notices changes
+        if self.col.op_made_changes(changes):
+            gui_hooks.state_did_reset()
+        if after_hooks:
+            after_hooks()
+
+    def _synthesize_op_did_execute_from_reset(self) -> None:
+        """Fire the `operation_did_execute` hook with everything marked as changed,
+        after legacy code has called .reset()"""
+        op = OpChanges()
+        for field in op.DESCRIPTOR.fields:
+            if field.name != "kind":
+                setattr(op, field.name, True)
+        gui_hooks.operation_did_execute(op)
+
+    def on_operation_did_execute(self, changes: OpChanges) -> None:
+        "Notify current screen of changes."
+        focused = current_top_level_widget() == self
+        if self.state == "review":
+            dirty = self.reviewer.op_executed(changes, focused)
+        elif self.state == "overview":
+            dirty = self.overview.op_executed(changes, focused)
+        elif self.state == "deckBrowser":
+            dirty = self.deckBrowser.op_executed(changes, focused)
+        else:
+            dirty = False
+
+        if not focused and dirty:
+            self.fade_out_webview()
+
+    def on_focus_did_change(
+        self, new_focus: Optional[QWidget], _old: Optional[QWidget]
+    ) -> None:
+        "If main window has received focus, ensure current UI state is updated."
+        if new_focus and top_level_widget(new_focus) == self:
+            if self.state == "review":
+                self.reviewer.refresh_if_needed()
+            elif self.state == "overview":
+                self.overview.refresh_if_needed()
+            elif self.state == "deckBrowser":
+                self.deckBrowser.refresh_if_needed()
+
+    def fade_out_webview(self) -> None:
+        self.web.eval("document.body.style.opacity = 0.3")
+
+    def fade_in_webview(self) -> None:
+        self.web.eval("document.body.style.opacity = 1")
+
+    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)
+
+    # legacy
 
     def requireReset(
         self,
         modal: bool = False,
-        reason: ResetReason = ResetReason.Unknown,
+        reason: Any = None,
         context: Any = None,
     ) -> None:
-        "Signal queue needs to be rebuilt when edits are finished or by user."
+        self.reset()
-        self.autosave()
-        self.resetModal = modal
-        if gui_hooks.main_window_should_require_reset(
-            self.interactiveState(), reason, context
-        ):
-            self.moveToState("resetRequired")
-
-    def interactiveState(self) -> bool:
-        "True if not in profile manager, syncing, etc."
-        return self.state in ("overview", "review", "deckBrowser")
 
     def maybeReset(self) -> None:
-        self.autosave()
+        pass
-        if self.state == "resetRequired":
-            self.state = self.returnState
-            self.reset()
 
     def delayedMaybeReset(self) -> None:
-        # if we redraw the page in a button click event it will often crash on
+        pass
-        # windows
-        self.progress.timer(100, self.maybeReset, False)
 
-    def _resetRequiredState(self, oldState: str) -> None:
+    def _resetRequiredState(self, oldState: MainWindowState) -> None:
-        if oldState != "resetRequired":
+        pass
-            self.returnState = oldState
-        if self.resetModal:
-            # we don't have to change the webview, as we have a covering window
-            return
-        web_context = ResetRequired(self)
-        self.web.set_bridge_command(lambda url: self.delayedMaybeReset(), web_context)
-        i = tr(TR.QT_MISC_WAITING_FOR_EDITING_TO_FINISH)
-        b = self.button("refresh", tr(TR.QT_MISC_RESUME_NOW), id="resume")
-        self.web.stdHtml(
-            f"""
-<center><div style="height: 100%">
-<div style="position:relative; vertical-align: middle;">
-{i}<br><br>
-{b}</div></div></center>
-<script>$('#resume').focus()</script>
-""",
-            context=web_context,
-        )
-        self.bottomWeb.hide()
-        self.web.setFocus()
 
     # HTML helpers
     ##########################################################################
@@ -812,10 +981,8 @@ title="%s" %s>%s</button>""" % (
 
         # force webengine processes to load before cwd is changed
         if isWin:
-            for o in self.web, self.bottomWeb:
+            for webview in self.web, self.bottomWeb:
-                o.requiresCol = False
+                webview.force_load_hack()
-                o._domReady = False
-                o._page.setContent(bytes("", "ascii"))
 
     def closeAllWindows(self, onsuccess: Callable) -> None:
         aqt.dialogs.closeAll(onsuccess)
@@ -879,6 +1046,7 @@ title="%s" %s>%s</button>""" % (
 
     def setupThreads(self) -> None:
         self._mainThread = QThread.currentThread()
+        self._background_op_count = 0
 
     def inMainThread(self) -> bool:
         return self._mainThread == QThread.currentThread()
@@ -988,8 +1156,7 @@ title="%s" %s>%s</button>""" % (
             ("y", self.on_sync_button_clicked),
         ]
         self.applyShortcuts(globalShortcuts)
-
+        self.stateShortcuts: List[QShortcut] = []
-        self.stateShortcuts: Sequence[Tuple[str, Callable]] = []
 
     def applyShortcuts(
         self, shortcuts: Sequence[Tuple[str, Callable]]
@@ -1087,6 +1254,8 @@ title="%s" %s>%s</button>""" % (
             if on_done:
                 on_done(result)
 
+        # fixme: perform_op? -> needs to save
+        # fixme: parent
         self.taskman.with_progress(self.col.undo, on_done_outer)
 
     def update_undo_actions(self) -> None:
@@ -1108,6 +1277,23 @@ title="%s" %s>%s</button>""" % (
             self.form.actionUndo.setEnabled(False)
             gui_hooks.undo_state_did_change(False)
 
+    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
+
+        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)
+
+        self.col.autosave()
+
     def checkpoint(self, name: str) -> None:
         self.col.save(name)
         self.update_undo_actions()
@@ -1149,7 +1335,7 @@ title="%s" %s>%s</button>""" % (
         deck = self._selectedDeck()
         if not deck:
             return
-        want_old = self.app.queryKeyboardModifiers() & Qt.ShiftModifier
+        want_old = KeyboardModifiersPressed().shift
         if want_old:
             aqt.dialogs.open("DeckStats", self)
         else:
@@ -1300,7 +1486,7 @@ title="%s" %s>%s</button>""" % (
         if elap > minutes * 60:
             self.maybe_auto_sync_media()
 
-    # Permanent libanki hooks
+    # Permanent hooks
     ##########################################################################
 
     def setupHooks(self) -> None:
@@ -1310,6 +1496,8 @@ title="%s" %s>%s</button>""" % (
 
         gui_hooks.av_player_will_play.append(self.on_av_player_will_play)
         gui_hooks.av_player_did_end_playing.append(self.on_av_player_did_end_playing)
+        gui_hooks.operation_did_execute.append(self.on_operation_did_execute)
+        gui_hooks.focus_did_change.append(self.on_focus_did_change)
 
         self._activeWindowOnPlay: Optional[QWidget] = None
 
@@ -1404,13 +1592,14 @@ title="%s" %s>%s</button>""" % (
         frm = self.debug_diag_form = aqt.forms.debug.Ui_Dialog()
 
         class DebugDialog(QDialog):
+            silentlyClose = True
+
             def reject(self) -> None:
                 super().reject()
                 saveSplitter(frm.splitter, "DebugConsoleWindow")
                 saveGeom(self, "DebugConsoleWindow")
 
         d = self.debugDiag = DebugDialog()
-        d.silentlyClose = True
         disable_help_button(d)
         frm.setupUi(d)
         restoreGeom(d, "DebugConsoleWindow")
@@ -1574,7 +1763,8 @@ title="%s" %s>%s</button>""" % (
         if not self.hideMenuAccels:
             return
         tgt = tgt or self
-        for action in tgt.findChildren(QAction):
+        for action_ in tgt.findChildren(QAction):
+            action = cast(QAction, action_)
             txt = str(action.text())
             m = re.match(r"^(.+)\(&.+\)(.+)?", txt)
             if m:
@@ -1582,7 +1772,7 @@ title="%s" %s>%s</button>""" % (
 
     def hideStatusTips(self) -> None:
         for action in self.findChildren(QAction):
-            action.setStatusTip("")
+            cast(QAction, action).setStatusTip("")
 
     def onMacMinimize(self) -> None:
         self.setWindowState(self.windowState() | Qt.WindowMinimized)  # type: ignore
@@ -1645,6 +1835,10 @@ title="%s" %s>%s</button>""" % (
     def _isAddon(self, buf: str) -> bool:
         return buf.endswith(self.addonManager.ext)
 
+    def interactiveState(self) -> bool:
+        "True if not in profile manager, syncing, etc."
+        return self.state in ("overview", "review", "deckBrowser")
+
     # GC
     ##########################################################################
     # The default Python garbage collection can trigger on any thread. This can
@@ -1700,3 +1894,20 @@ title="%s" %s>%s</button>""" % (
 
     def serverURL(self) -> str:
         return "http://127.0.0.1:%d/" % self.mediaServer.getPort()
+
+
+# legacy
+class ResetReason(enum.Enum):
+    Unknown = "unknown"
+    AddCardsAddNote = "addCardsAddNote"
+    EditCurrentInit = "editCurrentInit"
+    EditorBridgeCmd = "editorBridgeCmd"
+    BrowserSetDeck = "browserSetDeck"
+    BrowserAddTags = "browserAddTags"
+    BrowserRemoveTags = "browserRemoveTags"
+    BrowserSuspend = "browserSuspend"
+    BrowserReposition = "browserReposition"
+    BrowserReschedule = "browserReschedule"
+    BrowserFindReplace = "browserFindReplace"
+    BrowserTagDupes = "browserTagDupes"
+    BrowserDeleteDeck = "browserDeleteDeck"

qt/aqt/models.py

@@ -31,7 +31,7 @@ class Models(QDialog):
     def __init__(
         self,
         mw: AnkiQt,
-        parent: Optional[QDialog] = None,
+        parent: Optional[QWidget] = None,
         fromMain: bool = False,
         selected_notetype_id: Optional[int] = None,
     ):

qt/aqt/note_ops.py

@@ -0,0 +1,36 @@
+# 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 typing import Callable, Sequence
+
+from anki.notes import Note
+from aqt import AnkiQt
+from aqt.main import PerformOpOptionalSuccessCallback
+
+
+def add_note(
+    *,
+    mw: AnkiQt,
+    note: Note,
+    target_deck_id: int,
+    success: PerformOpOptionalSuccessCallback = None,
+) -> None:
+    mw.perform_op(lambda: mw.col.add_note(note, target_deck_id), success=success)
+
+
+def update_note(*, mw: AnkiQt, note: Note, after_hooks: Callable[[], None]) -> None:
+    mw.perform_op(
+        lambda: mw.col.update_note(note),
+        after_hooks=after_hooks,
+    )
+
+
+def remove_notes(
+    *,
+    mw: AnkiQt,
+    note_ids: Sequence[int],
+    success: PerformOpOptionalSuccessCallback = None,
+) -> None:
+    mw.perform_op(lambda: mw.col.remove_notes(note_ids), success=success)

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 OpChanges
 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)
+        self._refresh_needed = False
 
     def show(self) -> None:
         av_player.stop_and_clear_queue()
@@ -55,6 +57,20 @@ class Overview:
         self._renderBottom()
         self.mw.web.setFocus()
         gui_hooks.overview_did_refresh(self)
+        self._refresh_needed = False
+
+    def refresh_if_needed(self) -> None:
+        if self._refresh_needed:
+            self.refresh()
+
+    def op_executed(self, changes: OpChanges, focused: bool) -> bool:
+        if self.mw.col.op_affects_study_queue(changes):
+            self._refresh_needed = True
+
+        if focused:
+            self.refresh_if_needed()
+
+        return self._refresh_needed
 
     # Handlers
     ############################################################

qt/aqt/previewer.py

@@ -1,11 +1,14 @@
 # Copyright: Ankitects Pty Ltd and contributors
 # License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
-# mypy: check-untyped-defs
+
+from __future__ import annotations
+
 import json
 import re
 import time
 from typing import Any, Callable, Optional, Tuple, Union
 
+import aqt.browser
 from anki.cards import Card
 from anki.collection import Config
 from aqt import AnkiQt, gui_hooks
@@ -300,6 +303,12 @@ class MultiCardPreviewer(Previewer):
 
 class BrowserPreviewer(MultiCardPreviewer):
     _last_card_id = 0
+    _parent: Optional[aqt.browser.Browser]
+
+    def __init__(
+        self, parent: aqt.browser.Browser, mw: AnkiQt, on_close: Callable[[], None]
+    ) -> None:
+        super().__init__(parent=parent, mw=mw, on_close=on_close)
 
     def card(self) -> Optional[Card]:
         if self._parent.singleCard:
@@ -317,12 +326,12 @@ class BrowserPreviewer(MultiCardPreviewer):
             return changed
 
     def _on_prev_card(self) -> None:
-        self._parent.editor.saveNow(
+        self._parent.editor.call_after_note_saved(
             lambda: self._parent._moveCur(QAbstractItemView.MoveUp)
         )
 
     def _on_next_card(self) -> None:
-        self._parent.editor.saveNow(
+        self._parent.editor.call_after_note_saved(
             lambda: self._parent._moveCur(QAbstractItemView.MoveDown)
         )
 

qt/aqt/progress.py

@@ -16,10 +16,11 @@ from aqt.utils import TR, disable_help_button, tr
 class ProgressManager:
     def __init__(self, mw: aqt.AnkiQt) -> None:
         self.mw = mw
-        self.app = QApplication.instance()
+        self.app = mw.app
         self.inDB = False
         self.blockUpdates = False
         self._show_timer: Optional[QTimer] = None
+        self._busy_cursor_timer: Optional[QTimer] = None
         self._win: Optional[ProgressDialog] = None
         self._levels = 0
 
@@ -74,7 +75,7 @@ class ProgressManager:
         max: int = 0,
         min: int = 0,
         label: Optional[str] = None,
-        parent: Optional[QDialog] = None,
+        parent: Optional[QWidget] = None,
         immediate: bool = False,
     ) -> Optional[ProgressDialog]:
         self._levels += 1
@@ -94,14 +95,15 @@ class ProgressManager:
         self._win.setWindowTitle("Anki")
         self._win.setWindowModality(Qt.ApplicationModal)
         self._win.setMinimumWidth(300)
-        self._setBusy()
+        self._busy_cursor_timer = QTimer(self.mw)
+        self._busy_cursor_timer.setSingleShot(True)
+        self._busy_cursor_timer.start(300)
+        qconnect(self._busy_cursor_timer.timeout, self._set_busy_cursor)
         self._shown: float = 0
         self._counter = min
         self._min = min
         self._max = max
         self._firstTime = time.time()
-        self._lastUpdate = time.time()
-        self._updating = False
         self._show_timer = QTimer(self.mw)
         self._show_timer.setSingleShot(True)
         self._show_timer.start(immediate and 100 or 600)
@@ -120,13 +122,10 @@ class ProgressManager:
         if not self.mw.inMainThread():
             print("progress.update() called on wrong thread")
             return
-        if self._updating:
-            return
         if maybeShow:
             self._maybeShow()
         if not self._shown:
             return
-        elapsed = time.time() - self._lastUpdate
         if label:
             self._win.form.label.setText(label)
 
@@ -136,19 +135,16 @@ class ProgressManager:
             self._counter = value or (self._counter + 1)
             self._win.form.progressBar.setValue(self._counter)
 
-        if process and elapsed >= 0.2:
-            self._updating = True
-            self.app.processEvents()  # type: ignore #possibly related to https://github.com/python/mypy/issues/6910
-            self._updating = False
-            self._lastUpdate = time.time()
-
     def finish(self) -> None:
         self._levels -= 1
         self._levels = max(0, self._levels)
         if self._levels == 0:
             if self._win:
                 self._closeWin()
-            self._unsetBusy()
+            if self._busy_cursor_timer:
+                self._busy_cursor_timer.stop()
+                self._busy_cursor_timer = None
+            self._restore_cursor()
             if self._show_timer:
                 self._show_timer.stop()
                 self._show_timer = None
@@ -183,14 +179,17 @@ class ProgressManager:
                 if elap >= 0.5:
                     break
                 self.app.processEvents(QEventLoop.ExcludeUserInputEvents)  # type: ignore #possibly related to https://github.com/python/mypy/issues/6910
-        self._win.cancel()
+        # if the parent window has been deleted, the progress dialog may have
+        # already been dropped; delete it if it hasn't been
+        if not sip.isdeleted(self._win):
+            self._win.cancel()
         self._win = None
         self._shown = 0
 
-    def _setBusy(self) -> None:
+    def _set_busy_cursor(self) -> None:
         self.mw.app.setOverrideCursor(QCursor(Qt.WaitCursor))
 
-    def _unsetBusy(self) -> None:
+    def _restore_cursor(self) -> None:
         self.app.restoreOverrideCursor()
 
     def busy(self) -> int:

qt/aqt/reviewer.py

@@ -7,19 +7,30 @@ import html
 import json
 import re
 import unicodedata as ucd
+from enum import Enum, auto
 from typing import Any, Callable, List, Match, Optional, Sequence, Tuple, Union
 
 from PyQt5.QtCore import Qt
 
 from anki import hooks
 from anki.cards import Card
-from anki.collection import Config
+from anki.collection import Config, OpChanges
+from anki.tags import MARKED_TAG
 from anki.utils import stripHTML
 from aqt import AnkiQt, gui_hooks
+from aqt.card_ops import set_card_flag
+from aqt.note_ops import remove_notes
 from aqt.profiles import VideoDriver
 from aqt.qt import *
-from aqt.scheduling import set_due_date_dialog
+from aqt.scheduling_ops import (
+    bury_cards,
+    bury_note,
+    set_due_date_dialog,
+    suspend_cards,
+    suspend_note,
+)
 from aqt.sound import av_player, play_clicked_audio, record_audio
+from aqt.tag_ops import add_tags, remove_tags_for_notes
 from aqt.theme import theme_manager
 from aqt.toolbar import BottomBar
 from aqt.utils import (
@@ -33,6 +44,12 @@ from aqt.utils import (
 from aqt.webview import AnkiWebView
 
 
+class RefreshNeeded(Enum):
+    NO = auto()
+    NOTE_TEXT = auto()
+    QUEUES = auto()
+
+
 class ReviewerBottomBar:
     def __init__(self, reviewer: Reviewer) -> None:
         self.reviewer = reviewer
@@ -61,16 +78,17 @@ class Reviewer:
         self._recordedAudio: Optional[str] = None
         self.typeCorrect: str = None  # web init happens before this is set
         self.state: Optional[str] = None
+        self._refresh_needed = RefreshNeeded.NO
         self.bottom = BottomBar(mw, mw.bottomWeb)
         hooks.card_did_leech.append(self.onLeech)
 
     def show(self) -> None:
-        self.mw.col.reset()
         self.mw.setStateShortcuts(self._shortcutKeys())  # type: ignore
         self.web.set_bridge_command(self._linkHandler, self)
         self.bottom.web.set_bridge_command(self._linkHandler, ReviewerBottomBar(self))
         self._reps: int = None
-        self.nextCard()
+        self._refresh_needed = RefreshNeeded.QUEUES
+        self.refresh_if_needed()
 
     def lastCard(self) -> Optional[Card]:
         if self._answeredIds:
@@ -86,6 +104,42 @@ class Reviewer:
         gui_hooks.reviewer_will_end()
         self.card = None
 
+    def refresh_if_needed(self) -> None:
+        if self._refresh_needed is RefreshNeeded.QUEUES:
+            self.mw.col.reset()
+            self.nextCard()
+            self.mw.fade_in_webview()
+            self._refresh_needed = RefreshNeeded.NO
+        elif self._refresh_needed is RefreshNeeded.NOTE_TEXT:
+            self._redraw_current_card()
+            self.mw.fade_in_webview()
+            self._refresh_needed = RefreshNeeded.NO
+
+    def op_executed(self, changes: OpChanges, focused: bool) -> bool:
+        if changes.note and changes.kind == OpChanges.UPDATE_NOTE_TAGS:
+            self.card.load()
+            self._update_mark_icon()
+        elif changes.card and changes.kind == OpChanges.SET_CARD_FLAG:
+            # fixme: v3 mtime check
+            self.card.load()
+            self._update_flag_icon()
+        elif self.mw.col.op_affects_study_queue(changes):
+            self._refresh_needed = RefreshNeeded.QUEUES
+        elif changes.note or changes.notetype or changes.tag:
+            self._refresh_needed = RefreshNeeded.NOTE_TEXT
+
+        if focused and self._refresh_needed is not RefreshNeeded.NO:
+            self.refresh_if_needed()
+
+        return self._refresh_needed is not RefreshNeeded.NO
+
+    def _redraw_current_card(self) -> None:
+        self.card.load()
+        if self.state == "answer":
+            self._showAnswer()
+        else:
+            self._showQuestion()
+
     # Fetching a card
     ##########################################################################
 
@@ -219,7 +273,7 @@ class Reviewer:
         self.web.eval(f"_drawFlag({self.card.user_flag()});")
 
     def _update_mark_icon(self) -> None:
-        self.web.eval(f"_drawMark({json.dumps(self.card.note().has_tag('marked'))});")
+        self.web.eval(f"_drawMark({json.dumps(self.card.note().has_tag(MARKED_TAG))});")
 
     _drawMark = _update_mark_icon
     _drawFlag = _update_flag_icon
@@ -782,24 +836,23 @@ 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?
-        if self.card.user_flag() == flag:
+        if self.card.user_flag() == desired_flag:
             flag = 0
-        self.card.set_user_flag(flag)
+        else:
-        self.mw.col.update_card(self.card)
+            flag = desired_flag
-        self.mw.update_undo_actions()
+
-        self._update_flag_icon()
+        set_card_flag(mw=self.mw, card_ids=[self.card.id], flag=flag)
 
     def toggle_mark_on_current_note(self) -> None:
         note = self.card.note()
-        if note.has_tag("marked"):
+        if note.has_tag(MARKED_TAG):
-            note.remove_tag("marked")
+            remove_tags_for_notes(
+                mw=self.mw, note_ids=[note.id], space_separated_tags=MARKED_TAG
+            )
         else:
-            note.add_tag("marked")
+            add_tags(mw=self.mw, note_ids=[note.id], space_separated_tags=MARKED_TAG)
-        self.mw.col.update_note(note)
-        self.mw.update_undo_actions()
-        self._update_mark_icon()
 
     def on_set_due(self) -> None:
         if self.mw.state != "review" or not self.card:
@@ -810,38 +863,52 @@ 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()])
+        suspend_note(
-        self.mw.reset()
+            mw=self.mw,
-        tooltip(tr(TR.STUDYING_NOTE_SUSPENDED))
+            note_id=self.card.nid,
+            success=lambda _: tooltip(tr(TR.STUDYING_NOTE_SUSPENDED)),
+        )
 
     def suspend_current_card(self) -> None:
-        self.mw.col.sched.suspend_cards([self.card.id])
+        suspend_cards(
-        self.mw.reset()
+            mw=self.mw,
-        tooltip(tr(TR.STUDYING_CARD_SUSPENDED))
+            card_ids=[self.card.id],
-
+            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.reset()
-        tooltip(tr(TR.STUDYING_CARD_BURIED))
 
     def bury_current_note(self) -> None:
-        self.mw.col.sched.bury_note(self.card.note())
+        bury_note(
-        self.mw.reset()
+            mw=self.mw,
-        tooltip(tr(TR.STUDYING_NOTE_BURIED))
+            note_id=self.card.nid,
+            success=lambda _: tooltip(tr(TR.STUDYING_NOTE_BURIED)),
+        )
+
+    def bury_current_card(self) -> None:
+        bury_cards(
+            mw=self.mw,
+            card_ids=[self.card.id],
+            success=lambda _: tooltip(tr(TR.STUDYING_CARD_BURIED)),
+        )
 
     def delete_current_note(self) -> None:
         # need to check state because the shortcut is global to the main
         # window
         if self.mw.state != "review" or not self.card:
             return
+
+        # fixme: pass this back from the backend method instead
         cnt = len(self.card.note().cards())
-        self.mw.col.remove_notes([self.card.note().id])
+
-        self.mw.reset()
+        remove_notes(
-        tooltip(tr(TR.STUDYING_NOTE_AND_ITS_CARD_DELETED, count=cnt))
+            mw=self.mw,
+            note_ids=[self.card.nid],
+            success=lambda _: tooltip(
+                tr(TR.STUDYING_NOTE_AND_ITS_CARD_DELETED, count=cnt)
+            ),
+        )
 
     def onRecordVoice(self) -> None:
         def after_record(path: str) -> None:

qt/aqt/scheduling.py

@@ -1,82 +0,0 @@
-# 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 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
-
-
-def set_due_date_dialog(
-    *,
-    mw: aqt.AnkiQt,
-    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 ""
-    prompt = "\n".join(
-        [
-            tr(TR.SCHEDULING_SET_DUE_DATE_PROMPT, cards=len(card_ids)),
-            tr(TR.SCHEDULING_SET_DUE_DATE_PROMPT_HINT),
-        ]
-    )
-    (days, success) = getText(
-        prompt=prompt,
-        parent=parent,
-        default=default,
-        title=tr(TR.ACTIONS_SET_DUE_DATE),
-    )
-    if not success or not days.strip():
-        return
-
-    def set_due() -> None:
-        mw.col.sched.set_due_date(card_ids, days, config_key)
-
-    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(
-    *, 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:
-        try:
-            fut.result()
-        except Exception as e:
-            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/aqt/scheduling_ops.py

@@ -0,0 +1,175 @@
+# 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 typing import List, Optional, Sequence
+
+import aqt
+from anki.collection import CARD_TYPE_NEW, Config
+from anki.lang import TR
+from aqt import AnkiQt
+from aqt.main import PerformOpOptionalSuccessCallback
+from aqt.qt import *
+from aqt.utils import disable_help_button, getText, tooltip, tr
+
+
+def set_due_date_dialog(
+    *,
+    mw: aqt.AnkiQt,
+    parent: QWidget,
+    card_ids: List[int],
+    config_key: Optional[Config.String.Key.V],
+) -> None:
+    if not card_ids:
+        return
+
+    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)),
+            tr(TR.SCHEDULING_SET_DUE_DATE_PROMPT_HINT),
+        ]
+    )
+    (days, success) = getText(
+        prompt=prompt,
+        parent=parent,
+        default=default_text,
+        title=tr(TR.ACTIONS_SET_DUE_DATE),
+    )
+    if not success or not days.strip():
+        return
+
+    mw.perform_op(
+        lambda: mw.col.sched.set_due_date(card_ids, days, config_key),
+        success=lambda _: tooltip(
+            tr(TR.SCHEDULING_SET_DUE_DATE_DONE, cards=len(card_ids)),
+            parent=parent,
+        ),
+    )
+
+
+def forget_cards(*, mw: aqt.AnkiQt, parent: QWidget, card_ids: List[int]) -> None:
+    if not card_ids:
+        return
+
+    mw.perform_op(
+        lambda: mw.col.sched.schedule_cards_as_new(card_ids),
+        success=lambda _: tooltip(
+            tr(TR.SCHEDULING_FORGOT_CARDS, cards=len(card_ids)), parent=parent
+        ),
+    )
+
+
+def reposition_new_cards_dialog(
+    *, mw: AnkiQt, parent: QWidget, card_ids: Sequence[int]
+) -> None:
+    assert mw.col.db
+    row = mw.col.db.first(
+        f"select min(due), max(due) from cards where type={CARD_TYPE_NEW} and odid=0"
+    )
+    assert row
+    (min_position, max_position) = row
+    min_position = max(min_position or 0, 0)
+    max_position = max_position or 0
+
+    d = QDialog(parent)
+    disable_help_button(d)
+    d.setWindowModality(Qt.WindowModal)
+    frm = aqt.forms.reposition.Ui_Dialog()
+    frm.setupUi(d)
+
+    txt = tr(TR.BROWSING_QUEUE_TOP, val=min_position)
+    txt += "\n" + tr(TR.BROWSING_QUEUE_BOTTOM, val=max_position)
+    frm.label.setText(txt)
+
+    frm.start.selectAll()
+    if not d.exec_():
+        return
+
+    start = frm.start.value()
+    step = frm.step.value()
+    randomize = frm.randomize.isChecked()
+    shift = frm.shift.isChecked()
+
+    reposition_new_cards(
+        mw=mw,
+        parent=parent,
+        card_ids=card_ids,
+        starting_from=start,
+        step_size=step,
+        randomize=randomize,
+        shift_existing=shift,
+    )
+
+
+def reposition_new_cards(
+    *,
+    mw: AnkiQt,
+    parent: QWidget,
+    card_ids: Sequence[int],
+    starting_from: int,
+    step_size: int,
+    randomize: bool,
+    shift_existing: bool,
+) -> None:
+    mw.perform_op(
+        lambda: mw.col.sched.reposition_new_cards(
+            card_ids=card_ids,
+            starting_from=starting_from,
+            step_size=step_size,
+            randomize=randomize,
+            shift_existing=shift_existing,
+        ),
+        success=lambda out: tooltip(
+            tr(TR.BROWSING_CHANGED_NEW_POSITION, count=out.count), parent=parent
+        ),
+    )
+
+
+def suspend_cards(
+    *,
+    mw: AnkiQt,
+    card_ids: Sequence[int],
+    success: PerformOpOptionalSuccessCallback = None,
+) -> None:
+    mw.perform_op(lambda: mw.col.sched.suspend_cards(card_ids), success=success)
+
+
+def suspend_note(
+    *,
+    mw: AnkiQt,
+    note_id: int,
+    success: PerformOpOptionalSuccessCallback = None,
+) -> None:
+    mw.taskman.run_in_background(
+        lambda: mw.col.card_ids_of_note(note_id),
+        lambda future: suspend_cards(mw=mw, card_ids=future.result(), success=success),
+    )
+
+
+def unsuspend_cards(*, mw: AnkiQt, card_ids: Sequence[int]) -> None:
+    mw.perform_op(lambda: mw.col.sched.unsuspend_cards(card_ids))
+
+
+def bury_cards(
+    *,
+    mw: AnkiQt,
+    card_ids: Sequence[int],
+    success: PerformOpOptionalSuccessCallback = None,
+) -> None:
+    mw.perform_op(lambda: mw.col.sched.bury_cards(card_ids), success=success)
+
+
+def bury_note(
+    *,
+    mw: AnkiQt,
+    note_id: int,
+    success: PerformOpOptionalSuccessCallback = None,
+) -> None:
+    mw.taskman.run_in_background(
+        lambda: mw.col.card_ids_of_note(note_id),
+        lambda future: bury_cards(mw=mw, card_ids=future.result(), success=success),
+    )

qt/aqt/sidebar.py

@@ -8,7 +8,7 @@ from enum import Enum, auto
 from typing import Dict, Iterable, List, Optional, Tuple, cast
 
 import aqt
-from anki.collection import Config, SearchJoiner, SearchNode
+from anki.collection import Config, OpChanges, SearchJoiner, SearchNode
 from anki.decks import DeckTreeNode
 from anki.errors import DeckIsFilteredError, InvalidInput
 from anki.notes import Note
@@ -16,18 +16,18 @@ from anki.tags import TagTreeNode
 from anki.types import assert_exhaustive
 from aqt import colors, gui_hooks
 from aqt.clayout import CardLayout
-from aqt.main import ResetReason
+from aqt.deck_ops import remove_decks
 from aqt.models import Models
 from aqt.qt import *
+from aqt.tag_ops import remove_tags_for_all_notes, rename_tag, reparent_tags
 from aqt.theme import ColoredIcon, theme_manager
 from aqt.utils import (
     TR,
+    KeyboardModifiersPressed,
     askUser,
     getOnlyText,
     show_invalid_search_error,
-    showInfo,
     showWarning,
-    tooltip,
     tr,
 )
 
@@ -253,9 +253,7 @@ class SidebarModel(QAbstractItemModel):
             return QVariant(item.tooltip)
         return QVariant(theme_manager.icon_from_resources(item.icon))
 
-    def setData(
+    def setData(self, index: QModelIndex, text: str, _role: int = Qt.EditRole) -> bool:
-        self, index: QModelIndex, text: QVariant, _role: int = Qt.EditRole
-    ) -> bool:
         return self.sidebar._on_rename(index.internalPointer(), text)
 
     def supportedDropActions(self) -> Qt.DropActions:
@@ -353,6 +351,10 @@ def _want_right_border() -> bool:
     return not isMac or theme_manager.night_mode
 
 
+# fixme: we should have a top-level Sidebar class inheriting from QWidget that
+# handles the treeview, search bar and so on. Currently the treeview embeds the
+# search bar which is wrong, and the layout code is handled in browser.py instead
+# of here
 class SidebarTreeView(QTreeView):
     def __init__(self, browser: aqt.browser.Browser) -> None:
         super().__init__()
@@ -361,6 +363,7 @@ class SidebarTreeView(QTreeView):
         self.col = self.mw.col
         self.current_search: Optional[str] = None
         self.valid_drop_types: Tuple[SidebarItemType, ...] = ()
+        self._refresh_needed = False
 
         self.setContextMenuPolicy(Qt.CustomContextMenu)
         self.customContextMenuRequested.connect(self.onContextMenu)  # type: ignore
@@ -388,6 +391,10 @@ class SidebarTreeView(QTreeView):
 
         self.setStyleSheet("QTreeView { %s }" % ";".join(styles))
 
+        # these do not really belong here, they should be in a higher-level class
+        self.toolbar = SidebarToolbar(self)
+        self.searchBar = SidebarSearchBar(self)
+
     @property
     def tool(self) -> SidebarTool:
         return self._tool
@@ -408,7 +415,21 @@ class SidebarTreeView(QTreeView):
         self.setExpandsOnDoubleClick(double_click_expands)
 
     def model(self) -> SidebarModel:
-        return super().model()
+        return cast(SidebarModel, super().model())
+
+    # Refreshing
+    ###########################
+
+    def op_executed(self, op: OpChanges, focused: bool) -> None:
+        if op.tag or op.notetype or op.deck:
+            self._refresh_needed = True
+        if focused:
+            self.refresh_if_needed()
+
+    def refresh_if_needed(self) -> None:
+        if self._refresh_needed:
+            self.refresh()
+            self._refresh_needed = False
 
     def refresh(
         self, is_current: Optional[Callable[[SidebarItem], bool]] = None
@@ -417,16 +438,17 @@ class SidebarTreeView(QTreeView):
         if not self.isVisible():
             return
 
-        def on_done(fut: Future) -> None:
+        def on_done(root: SidebarItem) -> None:
-            self.setUpdatesEnabled(True)
+            # user may have closed browser
-            root = fut.result()
+            if sip.isdeleted(self):
+                return
+
+            # block repainting during refreshing to avoid flickering
+            self.setUpdatesEnabled(False)
+
             model = SidebarModel(self, root)
-
-            # from PyQt5.QtTest import QAbstractItemModelTester
-            # tester = QAbstractItemModelTester(model)
-
             self.setModel(model)
-            qconnect(self.selectionModel().selectionChanged, self._on_selection_changed)
+
             if self.current_search:
                 self.search_for(self.current_search)
             else:
@@ -434,9 +456,12 @@ class SidebarTreeView(QTreeView):
             if is_current:
                 self.restore_current(is_current)
 
-        # block repainting during refreshing to avoid flickering
+            self.setUpdatesEnabled(True)
-        self.setUpdatesEnabled(False)
+
-        self.mw.taskman.run_in_background(self._root_tree, on_done)
+            # needs to be set after changing model
+            qconnect(self.selectionModel().selectionChanged, self._on_selection_changed)
+
+        self.mw.query_op(self._root_tree, success=on_done)
 
     def restore_current(self, is_current: Callable[[SidebarItem], bool]) -> None:
         if current := self.find_item(is_current):
@@ -496,22 +521,22 @@ class SidebarTreeView(QTreeView):
         joiner: SearchJoiner = "AND",
     ) -> None:
         """Modify the current search string based on modifier keys, then refresh."""
-        mods = self.mw.app.keyboardModifiers()
+        mods = KeyboardModifiersPressed()
         previous = SearchNode(parsable_text=self.browser.current_search())
         current = self.mw.col.group_searches(*terms, joiner=joiner)
 
         # if Alt pressed, invert
-        if mods & Qt.AltModifier:
+        if mods.alt:
             current = SearchNode(negated=current)
 
         try:
-            if mods & Qt.ControlModifier and mods & Qt.ShiftModifier:
+            if mods.control and mods.shift:
                 # If Ctrl+Shift, replace searches nodes of the same type.
                 search = self.col.replace_in_search_node(previous, current)
-            elif mods & Qt.ControlModifier:
+            elif mods.control:
                 # If Ctrl, AND with previous
                 search = self.col.join_searches(previous, current, "AND")
-            elif mods & Qt.ShiftModifier:
+            elif mods.shift:
                 # If Shift, OR with previous
                 search = self.col.join_searches(previous, current, "OR")
             else:
@@ -602,39 +627,27 @@ class SidebarTreeView(QTreeView):
                 lambda: self.col.decks.drag_drop_decks(source_ids, target.id), on_done
             )
 
-        self.browser.editor.saveNow(on_save)
+        self.browser.editor.call_after_note_saved(on_save)
         return True
 
     def _handle_drag_drop_tags(
         self, sources: List[SidebarItem], target: SidebarItem
     ) -> bool:
-        source_ids = [
+        tags = [
             source.full_name
             for source in sources
             if source.item_type == SidebarItemType.TAG
         ]
-        if not source_ids:
+        if not tags:
             return False
 
-        def on_done(fut: Future) -> None:
-            self.mw.requireReset(reason=ResetReason.BrowserAddTags, context=self)
-            self.browser.model.endReset()
-            fut.result()
-            self.refresh()
-
         if target.item_type == SidebarItemType.TAG_ROOT:
-            target_name = ""
+            new_parent = ""
         else:
-            target_name = target.full_name
+            new_parent = target.full_name
 
-        def on_save() -> None:
+        reparent_tags(mw=self.mw, parent=self.browser, tags=tags, new_parent=new_parent)
-            self.mw.checkpoint(tr(TR.ACTIONS_RENAME_TAG))
-            self.browser.model.beginReset()
-            self.mw.taskman.with_progress(
-                lambda: self.col.tags.drag_drop(source_ids, target_name), on_done
-            )
 
-        self.browser.editor.saveNow(on_save)
         return True
 
     def _on_search(self, index: QModelIndex) -> None:
@@ -693,7 +706,9 @@ class SidebarTreeView(QTreeView):
         for stage in SidebarStage:
             if stage == SidebarStage.ROOT:
                 root = SidebarItem("", "", item_type=SidebarItemType.ROOT)
-            handled = gui_hooks.browser_will_build_tree(False, root, stage, self)
+            handled = gui_hooks.browser_will_build_tree(
+                False, root, stage, self.browser
+            )
             if not handled:
                 self._build_stage(root, stage)
 
@@ -1166,78 +1181,40 @@ class SidebarTreeView(QTreeView):
         self.mw.update_undo_actions()
 
     def delete_decks(self, _item: SidebarItem) -> None:
-        self.browser.editor.saveNow(self._delete_decks)
+        remove_decks(mw=self.mw, parent=self.browser, deck_ids=self._selected_decks())
-
-    def _delete_decks(self) -> None:
-        def do_delete() -> int:
-            return self.mw.col.decks.remove(dids)
-
-        def on_done(fut: Future) -> None:
-            self.mw.requireReset(reason=ResetReason.BrowserDeleteDeck, context=self)
-            self.browser.search()
-            self.browser.model.endReset()
-            tooltip(tr(TR.BROWSING_CARDS_DELETED, count=fut.result()), parent=self)
-            self.refresh()
-
-        dids = self._selected_decks()
-        self.browser.model.beginReset()
-        self.mw.taskman.with_progress(do_delete, on_done)
 
     # Tags
     ###########################
 
     def remove_tags(self, item: SidebarItem) -> None:
-        self.browser.editor.saveNow(lambda: self._remove_tags(item))
+        tags = self.mw.col.tags.join(self._selected_tags())
+        item.name = "..."
 
-    def _remove_tags(self, _item: SidebarItem) -> None:
+        remove_tags_for_all_notes(
-        tags = self._selected_tags()
+            mw=self.mw, parent=self.browser, space_separated_tags=tags
-
+        )
-        def do_remove() -> int:
-            return self.col._backend.expunge_tags(" ".join(tags))
-
-        def on_done(fut: Future) -> None:
-            self.mw.requireReset(reason=ResetReason.BrowserRemoveTags, context=self)
-            self.browser.model.endReset()
-            tooltip(tr(TR.BROWSING_NOTES_UPDATED, count=fut.result()), parent=self)
-            self.refresh()
-
-        self.mw.checkpoint(tr(TR.ACTIONS_REMOVE_TAG))
-        self.browser.model.beginReset()
-        self.mw.taskman.with_progress(do_remove, on_done)
 
     def rename_tag(self, item: SidebarItem, new_name: str) -> None:
-        new_name = new_name.replace(" ", "")
+        if not new_name or new_name == item.name:
-        if new_name and new_name != item.name:
+            return
-            # block repainting until collection is updated
+
-            self.setUpdatesEnabled(False)
+        new_name_base = new_name
-            self.browser.editor.saveNow(lambda: self._rename_tag(item, new_name))
 
-    def _rename_tag(self, item: SidebarItem, new_name: str) -> None:
         old_name = item.full_name
         new_name = item.name_prefix + new_name
 
-        def do_rename() -> int:
+        item.name = new_name_base
-            self.mw.col.tags.remove(old_name)
-            return self.col.tags.rename(old_name, new_name)
 
-        def on_done(fut: Future) -> None:
+        rename_tag(
-            self.setUpdatesEnabled(True)
+            mw=self.mw,
-            self.mw.requireReset(reason=ResetReason.BrowserAddTags, context=self)
+            parent=self.browser,
-            self.browser.model.endReset()
+            current_name=old_name,
-
+            new_name=new_name,
-            count = fut.result()
+            after_rename=lambda: self.refresh(
-            if not count:
+                lambda item: item.item_type == SidebarItemType.TAG
-                showInfo(tr(TR.BROWSING_TAG_RENAME_WARNING_EMPTY))
+                and item.full_name == new_name
-            else:
+            ),
-                tooltip(tr(TR.BROWSING_NOTES_UPDATED, count=count), parent=self)
+        )
-                self.refresh(
-                    lambda item: item.item_type == SidebarItemType.TAG
-                    and item.full_name == new_name
-                )
-
-        self.mw.checkpoint(tr(TR.ACTIONS_RENAME_TAG))
-        self.browser.model.beginReset()
-        self.mw.taskman.with_progress(do_rename, on_done)
 
     # Saved searches
     ####################################

qt/aqt/sound.py

@@ -15,7 +15,7 @@ import wave
 from abc import ABC, abstractmethod
 from concurrent.futures import Future
 from operator import itemgetter
-from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Tuple
+from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Tuple, cast
 
 import aqt
 from anki import hooks
@@ -568,7 +568,7 @@ class QtAudioInputRecorder(Recorder):
         super().start(on_done)
 
     def _on_read_ready(self) -> None:
-        self._buffer += self._iodevice.readAll()
+        self._buffer += cast(bytes, self._iodevice.readAll())
 
     def stop(self, on_done: Callable[[str], None]) -> None:
         def on_stop_timer() -> None:

qt/aqt/studydeck.py

@@ -33,9 +33,9 @@ class StudyDeck(QDialog):
         help: HelpPageArgument = HelpPage.KEYBOARD_SHORTCUTS,
         current: Optional[str] = None,
         cancel: bool = True,
-        parent: Optional[QDialog] = None,
+        parent: Optional[QWidget] = None,
         dyn: bool = False,
-        buttons: Optional[List[str]] = None,
+        buttons: Optional[List[Union[str, QPushButton]]] = None,
         geomKey: str = "default",
     ) -> None:
         QDialog.__init__(self, parent or mw)
@@ -53,8 +53,10 @@ class StudyDeck(QDialog):
                 self.form.buttonBox.button(QDialogButtonBox.Cancel)
             )
         if buttons is not None:
-            for b in buttons:
+            for button_or_label in buttons:
-                self.form.buttonBox.addButton(b, QDialogButtonBox.ActionRole)
+                self.form.buttonBox.addButton(
+                    button_or_label, QDialogButtonBox.ActionRole
+                )
         else:
             b = QPushButton(tr(TR.ACTIONS_ADD))
             b.setShortcut(QKeySequence("Ctrl+N"))
@@ -89,7 +91,7 @@ class StudyDeck(QDialog):
         self.exec_()
 
     def eventFilter(self, obj: QObject, evt: QEvent) -> bool:
-        if evt.type() == QEvent.KeyPress:
+        if isinstance(evt, QKeyEvent) and evt.type() == QEvent.KeyPress:
             new_row = current_row = self.form.list.currentRow()
             rows_count = self.form.list.count()
             key = evt.key()
@@ -98,7 +100,10 @@ class StudyDeck(QDialog):
                 new_row = current_row - 1
             elif key == Qt.Key_Down:
                 new_row = current_row + 1
-            elif evt.modifiers() & Qt.ControlModifier and Qt.Key_1 <= key <= Qt.Key_9:
+            elif (
+                int(evt.modifiers()) & Qt.ControlModifier
+                and Qt.Key_1 <= key <= Qt.Key_9
+            ):
                 row_index = key - Qt.Key_1
                 if row_index < rows_count:
                     new_row = row_index

qt/aqt/tag_ops.py

@@ -0,0 +1,88 @@
+# 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 typing import Callable, Sequence
+
+from anki.collection import OpChangesWithCount
+from anki.lang import TR
+from aqt import AnkiQt, QWidget
+from aqt.main import PerformOpOptionalSuccessCallback
+from aqt.utils import showInfo, tooltip, tr
+
+
+def add_tags(
+    *,
+    mw: AnkiQt,
+    note_ids: Sequence[int],
+    space_separated_tags: str,
+    success: PerformOpOptionalSuccessCallback = None,
+) -> None:
+    mw.perform_op(
+        lambda: mw.col.tags.bulk_add(note_ids, space_separated_tags), success=success
+    )
+
+
+def remove_tags_for_notes(
+    *,
+    mw: AnkiQt,
+    note_ids: Sequence[int],
+    space_separated_tags: str,
+    success: PerformOpOptionalSuccessCallback = None,
+) -> None:
+    mw.perform_op(
+        lambda: mw.col.tags.bulk_remove(note_ids, space_separated_tags), success=success
+    )
+
+
+def clear_unused_tags(*, mw: AnkiQt, parent: QWidget) -> None:
+    mw.perform_op(
+        mw.col.tags.clear_unused_tags,
+        success=lambda out: tooltip(
+            tr(TR.BROWSING_REMOVED_UNUSED_TAGS_COUNT, count=out.count), parent=parent
+        ),
+    )
+
+
+def rename_tag(
+    *,
+    mw: AnkiQt,
+    parent: QWidget,
+    current_name: str,
+    new_name: str,
+    after_rename: Callable[[], None],
+) -> None:
+    def success(out: OpChangesWithCount) -> None:
+        if out.count:
+            tooltip(tr(TR.BROWSING_NOTES_UPDATED, count=out.count), parent=parent)
+        else:
+            showInfo(tr(TR.BROWSING_TAG_RENAME_WARNING_EMPTY), parent=parent)
+
+    mw.perform_op(
+        lambda: mw.col.tags.rename(old=current_name, new=new_name),
+        success=success,
+        after_hooks=after_rename,
+    )
+
+
+def remove_tags_for_all_notes(
+    *, mw: AnkiQt, parent: QWidget, space_separated_tags: str
+) -> None:
+    mw.perform_op(
+        lambda: mw.col.tags.remove(space_separated_tags=space_separated_tags),
+        success=lambda out: tooltip(
+            tr(TR.BROWSING_NOTES_UPDATED, count=out.count), parent=parent
+        ),
+    )
+
+
+def reparent_tags(
+    *, mw: AnkiQt, parent: QWidget, tags: Sequence[str], new_parent: str
+) -> None:
+    mw.perform_op(
+        lambda: mw.col.tags.reparent(tags=tags, new_parent=new_parent),
+        success=lambda out: tooltip(
+            tr(TR.BROWSING_NOTES_UPDATED, count=out.count), parent=parent
+        ),
+    )

qt/aqt/tagedit.py

@@ -12,24 +12,24 @@ from aqt.qt import *
 
 
 class TagEdit(QLineEdit):
-    completer: Union[QCompleter, TagCompleter]
+    _completer: Union[QCompleter, TagCompleter]
 
     lostFocus = pyqtSignal()
 
     # 0 = tags, 1 = decks
-    def __init__(self, parent: QDialog, type: int = 0) -> None:
+    def __init__(self, parent: QWidget, type: int = 0) -> None:
         QLineEdit.__init__(self, parent)
         self.col: Optional[Collection] = None
         self.model = QStringListModel()
         self.type = type
         if type == 0:
-            self.completer = TagCompleter(self.model, parent, self)
+            self._completer = TagCompleter(self.model, parent, self)
         else:
-            self.completer = QCompleter(self.model, parent)
+            self._completer = QCompleter(self.model, parent)
-        self.completer.setCompletionMode(QCompleter.PopupCompletion)
+        self._completer.setCompletionMode(QCompleter.PopupCompletion)
-        self.completer.setCaseSensitivity(Qt.CaseInsensitive)
+        self._completer.setCaseSensitivity(Qt.CaseInsensitive)
-        self.completer.setFilterMode(Qt.MatchContains)
+        self._completer.setFilterMode(Qt.MatchContains)
-        self.setCompleter(self.completer)
+        self.setCompleter(self._completer)
 
     def setCol(self, col: Collection) -> None:
         "Set the current col, updating list of available tags."
@@ -47,29 +47,29 @@ class TagEdit(QLineEdit):
     def keyPressEvent(self, evt: QKeyEvent) -> None:
         if evt.key() in (Qt.Key_Up, Qt.Key_Down):
             # show completer on arrow key up/down
-            if not self.completer.popup().isVisible():
+            if not self._completer.popup().isVisible():
                 self.showCompleter()
             return
-        if evt.key() == Qt.Key_Tab and evt.modifiers() & Qt.ControlModifier:
+        if evt.key() == Qt.Key_Tab and int(evt.modifiers()) & Qt.ControlModifier:
             # select next completion
-            if not self.completer.popup().isVisible():
+            if not self._completer.popup().isVisible():
                 self.showCompleter()
-            index = self.completer.currentIndex()
+            index = self._completer.currentIndex()
-            self.completer.popup().setCurrentIndex(index)
+            self._completer.popup().setCurrentIndex(index)
             cur_row = index.row()
-            if not self.completer.setCurrentRow(cur_row + 1):
+            if not self._completer.setCurrentRow(cur_row + 1):
-                self.completer.setCurrentRow(0)
+                self._completer.setCurrentRow(0)
             return
         if (
             evt.key() in (Qt.Key_Enter, Qt.Key_Return)
-            and self.completer.popup().isVisible()
+            and self._completer.popup().isVisible()
         ):
             # apply first completion if no suggestion selected
-            selected_row = self.completer.popup().currentIndex().row()
+            selected_row = self._completer.popup().currentIndex().row()
             if selected_row == -1:
-                self.completer.setCurrentRow(0)
+                self._completer.setCurrentRow(0)
-                index = self.completer.currentIndex()
+                index = self._completer.currentIndex()
-                self.completer.popup().setCurrentIndex(index)
+                self._completer.popup().setCurrentIndex(index)
             self.hideCompleter()
             QWidget.keyPressEvent(self, evt)
             return
@@ -90,18 +90,18 @@ class TagEdit(QLineEdit):
         gui_hooks.tag_editor_did_process_key(self, evt)
 
     def showCompleter(self) -> None:
-        self.completer.setCompletionPrefix(self.text())
+        self._completer.setCompletionPrefix(self.text())
-        self.completer.complete()
+        self._completer.complete()
 
     def focusOutEvent(self, evt: QFocusEvent) -> None:
         QLineEdit.focusOutEvent(self, evt)
         self.lostFocus.emit()  # type: ignore
-        self.completer.popup().hide()
+        self._completer.popup().hide()
 
     def hideCompleter(self) -> None:
-        if sip.isdeleted(self.completer):
+        if sip.isdeleted(self._completer):
             return
-        self.completer.popup().hide()
+        self._completer.popup().hide()
 
 
 class TagCompleter(QCompleter):

qt/aqt/taglimit.py

@@ -15,8 +15,8 @@ class TagLimit(QDialog):
         self.tags: str = ""
         self.tags_list: List[str] = []
         self.mw = mw
-        self.parent: Optional[QWidget] = parent
+        self.parent_: Optional[CustomStudy] = parent
-        self.deck = self.parent.deck
+        self.deck = self.parent_.deck
         self.dialog = aqt.forms.taglimit.Ui_Dialog()
         self.dialog.setupUi(self)
         disable_help_button(self)

qt/aqt/taskman.py

@@ -3,6 +3,8 @@
 
 """
 Helper for running tasks on background threads.
+
+See mw.query_op() and mw.perform_op() for slightly higher-level routines.
 """
 
 from __future__ import annotations
@@ -49,6 +51,14 @@ class TaskManager(QObject):
         the completed future.
 
         Args if provided will be passed on as keyword arguments to the task callable."""
+        # Before we launch a background task, ensure any pending on_done closure are run on
+        # main. Qt's signal/slot system will have posted a notification, but it may
+        # not have been processed yet. The on_done() closures may make small queries
+        # to the database that we want to run first - if we delay them until after the
+        # background task starts, and it takes out a long-running lock on the database,
+        # the UI thread will hang until the end of the op.
+        self._on_closures_pending()
+
         if args is None:
             args = {}
 

qt/aqt/theme.py

@@ -86,12 +86,12 @@ class ThemeManager:
         else:
             # specified colours
             icon = QIcon(path.path)
-            img = icon.pixmap(16)
+            pixmap = icon.pixmap(16)
-            painter = QPainter(img)
+            painter = QPainter(pixmap)
             painter.setCompositionMode(QPainter.CompositionMode_SourceIn)
-            painter.fillRect(img.rect(), QColor(path.current_color(self.night_mode)))
+            painter.fillRect(pixmap.rect(), QColor(path.current_color(self.night_mode)))
             painter.end()
-            icon = QIcon(img)
+            icon = QIcon(pixmap)
             return icon
 
         return cache.setdefault(path, icon)

qt/aqt/utils.py

@@ -7,6 +7,7 @@ import re
 import subprocess
 import sys
 from enum import Enum
+from functools import wraps
 from typing import (
     TYPE_CHECKING,
     Any,
@@ -110,7 +111,7 @@ def openHelp(section: HelpPageArgument) -> None:
     openLink(link)
 
 
-def openLink(link: str) -> None:
+def openLink(link: Union[str, QUrl]) -> None:
     tooltip(tr(TR.QT_MISC_LOADING), period=1000)
     with noBundledLibs():
         QDesktopServices.openUrl(QUrl(link))
@@ -118,7 +119,7 @@ def openLink(link: str) -> None:
 
 def showWarning(
     text: str,
-    parent: Optional[QDialog] = None,
+    parent: Optional[QWidget] = None,
     help: HelpPageArgument = "",
     title: str = "Anki",
     textFormat: Optional[TextFormat] = None,
@@ -138,17 +139,17 @@ def showCritical(
     return showInfo(text, parent, help, "critical", title=title, textFormat=textFormat)
 
 
-def show_invalid_search_error(err: Exception) -> None:
+def show_invalid_search_error(err: Exception, parent: Optional[QWidget] = None) -> None:
     "Render search errors in markdown, then display a warning."
     text = str(err)
     if isinstance(err, InvalidInput):
         text = markdown(text)
-    showWarning(text)
+    showWarning(text, parent=parent)
 
 
 def showInfo(
     text: str,
-    parent: Union[Literal[False], QDialog] = False,
+    parent: Optional[QWidget] = None,
     help: HelpPageArgument = "",
     type: str = "info",
     title: str = "Anki",
@@ -157,7 +158,7 @@ def showInfo(
 ) -> int:
     "Show a small info window with an OK button."
     parent_widget: QWidget
-    if parent is False:
+    if parent is None:
         parent_widget = aqt.mw.app.activeWindow() or aqt.mw
     else:
         parent_widget = parent
@@ -213,6 +214,7 @@ def showText(
     disable_help_button(diag)
     layout = QVBoxLayout(diag)
     diag.setLayout(layout)
+    text: Union[QPlainTextEdit, QTextBrowser]
     if plain_text_edit:
         # used by the importer
         text = QPlainTextEdit()
@@ -221,10 +223,10 @@ def showText(
     else:
         text = QTextBrowser()
         text.setOpenExternalLinks(True)
-    if type == "text":
+        if type == "text":
-        text.setPlainText(txt)
+            text.setPlainText(txt)
-    else:
+        else:
-        text.setHtml(txt)
+            text.setHtml(txt)
     layout.addWidget(text)
     box = QDialogButtonBox(QDialogButtonBox.Close)
     layout.addWidget(box)
@@ -262,7 +264,7 @@ def showText(
 
 def askUser(
     text: str,
-    parent: QDialog = None,
+    parent: QWidget = None,
     help: HelpPageArgument = None,
     defaultno: bool = False,
     msgfunc: Optional[Callable] = None,
@@ -295,7 +297,7 @@ class ButtonedDialog(QMessageBox):
         self,
         text: str,
         buttons: List[str],
-        parent: Optional[QDialog] = None,
+        parent: Optional[QWidget] = None,
         help: HelpPageArgument = None,
         title: str = "Anki",
     ):
@@ -328,7 +330,7 @@ class ButtonedDialog(QMessageBox):
 def askUserDialog(
     text: str,
     buttons: List[str],
-    parent: Optional[QDialog] = None,
+    parent: Optional[QWidget] = None,
     help: HelpPageArgument = None,
     title: str = "Anki",
 ) -> ButtonedDialog:
@@ -341,7 +343,7 @@ def askUserDialog(
 class GetTextDialog(QDialog):
     def __init__(
         self,
-        parent: Optional[QDialog],
+        parent: Optional[QWidget],
         question: str,
         help: HelpPageArgument = None,
         edit: Optional[QLineEdit] = None,
@@ -388,7 +390,7 @@ class GetTextDialog(QDialog):
 
 def getText(
     prompt: str,
-    parent: Optional[QDialog] = None,
+    parent: Optional[QWidget] = None,
     help: HelpPageArgument = None,
     edit: Optional[QLineEdit] = None,
     default: str = "",
@@ -445,7 +447,7 @@ def chooseList(
 
 
 def getTag(
-    parent: QDialog, deck: Collection, question: str, **kwargs: Any
+    parent: QWidget, deck: Collection, question: str, **kwargs: Any
 ) -> Tuple[str, int]:
     from aqt.tagedit import TagEdit
 
@@ -458,7 +460,8 @@ def getTag(
 
 def disable_help_button(widget: QWidget) -> None:
     "Disable the help button in the window titlebar."
-    flags = cast(Qt.WindowType, widget.windowFlags() & ~Qt.WindowContextHelpButtonHint)
+    flags_int = int(widget.windowFlags()) & ~Qt.WindowContextHelpButtonHint
+    flags = Qt.WindowFlags(flags_int)  # type: ignore
     widget.setWindowFlags(flags)
 
 
@@ -467,7 +470,7 @@ def disable_help_button(widget: QWidget) -> None:
 
 
 def getFile(
-    parent: QDialog,
+    parent: QWidget,
     title: str,
     # single file returned unless multi=True
     cb: Optional[Callable[[Union[str, Sequence[str]]], None]],
@@ -547,9 +550,9 @@ def getSaveFile(
     return file
 
 
-def saveGeom(widget: QDialog, key: str) -> None:
+def saveGeom(widget: QWidget, key: str) -> None:
     key += "Geom"
-    if isMac and widget.windowState() & Qt.WindowFullScreen:
+    if isMac and int(widget.windowState()) & Qt.WindowFullScreen:
         geom = None
     else:
         geom = widget.saveGeometry()
@@ -599,12 +602,12 @@ def ensureWidgetInScreenBoundaries(widget: QWidget) -> None:
         widget.move(x, y)
 
 
-def saveState(widget: QFileDialog, key: str) -> None:
+def saveState(widget: Union[QFileDialog, QMainWindow], key: str) -> None:
     key += "State"
     aqt.mw.pm.profile[key] = widget.saveState()
 
 
-def restoreState(widget: Union[aqt.AnkiQt, QFileDialog], key: str) -> None:
+def restoreState(widget: Union[QFileDialog, QMainWindow], key: str) -> None:
     key += "State"
     if aqt.mw.pm.profile.get(key):
         widget.restoreState(aqt.mw.pm.profile[key])
@@ -632,12 +635,12 @@ def restoreHeader(widget: QHeaderView, key: str) -> None:
         widget.restoreState(aqt.mw.pm.profile[key])
 
 
-def save_is_checked(widget: QWidget, key: str) -> None:
+def save_is_checked(widget: QCheckBox, key: str) -> None:
     key += "IsChecked"
     aqt.mw.pm.profile[key] = widget.isChecked()
 
 
-def restore_is_checked(widget: QWidget, key: str) -> None:
+def restore_is_checked(widget: QCheckBox, key: str) -> None:
     key += "IsChecked"
     if aqt.mw.pm.profile.get(key) is not None:
         widget.setChecked(aqt.mw.pm.profile[key])
@@ -718,8 +721,9 @@ def maybeHideClose(bbox: QDialogButtonBox) -> None:
 def addCloseShortcut(widg: QDialog) -> None:
     if not isMac:
         return
-    widg._closeShortcut = QShortcut(QKeySequence("Ctrl+W"), widg)
+    shortcut = QShortcut(QKeySequence("Ctrl+W"), widg)
-    qconnect(widg._closeShortcut.activated, widg.reject)
+    qconnect(shortcut.activated, widg.reject)
+    setattr(widg, "_closeShortcut", shortcut)
 
 
 def downArrow() -> str:
@@ -729,6 +733,20 @@ def downArrow() -> str:
     return "▾"
 
 
+def top_level_widget(widget: QWidget) -> QWidget:
+    window = None
+    while widget := widget.parentWidget():
+        window = widget
+    return window
+
+
+def current_top_level_widget() -> Optional[QWidget]:
+    if widget := QApplication.focusWidget():
+        return top_level_widget(widget)
+    else:
+        return None
+
+
 # Tooltips
 ######################################################################
 
@@ -739,7 +757,7 @@ _tooltipLabel: Optional[QLabel] = None
 def tooltip(
     msg: str,
     period: int = 3000,
-    parent: Optional[aqt.AnkiQt] = None,
+    parent: Optional[QWidget] = None,
     x_offset: int = 0,
     y_offset: int = 100,
 ) -> None:
@@ -974,3 +992,50 @@ def startup_info() -> Any:
     si = subprocess.STARTUPINFO()  # pytype: disable=module-attr
     si.dwFlags |= subprocess.STARTF_USESHOWWINDOW  # pytype: disable=module-attr
     return si
+
+
+def ensure_editor_saved(func: Callable) -> Callable:
+    """Ensure the current editor's note is saved before running the wrapped function.
+
+    Must be used on functions that may be invoked from a shortcut key while the
+    editor has focus. For functions that can't be activated while the editor has
+    focus, you don't need this.
+
+    Will look for the editor as self.editor.
+    """
+
+    @wraps(func)
+    def decorated(self: Any, *args: Any, **kwargs: Any) -> None:
+        self.editor.call_after_note_saved(lambda: func(self, *args, **kwargs))
+
+    return decorated
+
+
+def ensure_editor_saved_on_trigger(func: Callable) -> Callable:
+    """Like ensure_editor_saved(), but tells Qt this function takes no args.
+
+    This ensures PyQt doesn't attempt to pass a `toggled` arg
+    into functions connected to a `triggered` signal.
+    """
+    return pyqtSlot()(ensure_editor_saved(func))  # type: ignore
+
+
+class KeyboardModifiersPressed:
+    "Util for type-safe checks of currently-pressed modifier keys."
+
+    def __init__(self) -> None:
+        from aqt import mw
+
+        self._modifiers = int(mw.app.keyboardModifiers())
+
+    @property
+    def shift(self) -> bool:
+        return bool(self._modifiers & Qt.ShiftModifier)
+
+    @property
+    def control(self) -> bool:
+        return bool(self._modifiers & Qt.ControlModifier)
+
+    @property
+    def alt(self) -> bool:
+        return bool(self._modifiers & Qt.AltModifier)

qt/aqt/webview.py

@@ -1,5 +1,6 @@
 # Copyright: Ankitects Pty Ltd and contributors
 # License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
 import dataclasses
 import json
 import re
@@ -31,12 +32,15 @@ class AnkiWebPage(QWebEnginePage):
 
     def _setupBridge(self) -> None:
         class Bridge(QObject):
+            def __init__(self, bridge_handler: Callable[[str], Any]) -> None:
+                super().__init__()
+                self.onCmd = bridge_handler
+
             @pyqtSlot(str, result=str)  # type: ignore
             def cmd(self, str: str) -> Any:
                 return json.dumps(self.onCmd(str))
 
-        self._bridge = Bridge()
+        self._bridge = Bridge(self._onCmd)
-        self._bridge.onCmd = self._onCmd
 
         self._channel = QWebChannel(self)
         self._channel.registerObject("py", self._bridge)
@@ -46,7 +50,7 @@ class AnkiWebPage(QWebEnginePage):
         jsfile = QFile(qwebchannel)
         if not jsfile.open(QIODevice.ReadOnly):
             print(f"Error opening '{qwebchannel}': {jsfile.error()}", file=sys.stderr)
-        jstext = bytes(jsfile.readAll()).decode("utf-8")
+        jstext = bytes(cast(bytes, jsfile.readAll())).decode("utf-8")
         jsfile.close()
 
         script = QWebEngineScript()
@@ -131,7 +135,7 @@ class AnkiWebPage(QWebEnginePage):
         openLink(url)
         return False
 
-    def _onCmd(self, str: str) -> None:
+    def _onCmd(self, str: str) -> Any:
         return self._onBridgeCmd(str)
 
     def javaScriptAlert(self, url: QUrl, text: str) -> None:
@@ -252,7 +256,7 @@ class AnkiWebView(QWebEngineView):
         # disable pinch to zoom gesture
         if isinstance(evt, QNativeGestureEvent):
             return True
-        elif evt.type() == QEvent.MouseButtonRelease:
+        elif isinstance(evt, QMouseEvent) and evt.type() == QEvent.MouseButtonRelease:
             if evt.button() == Qt.MidButton and isLin:
                 self.onMiddleClickPaste()
                 return True
@@ -273,7 +277,9 @@ class AnkiWebView(QWebEngineView):
                     w.close()
                 else:
                     # in the main window, removes focus from type in area
-                    self.parent().setFocus()
+                    parent = self.parent()
+                    assert isinstance(parent, QWidget)
+                    parent.setFocus()
                 break
             w = w.parent()
 
@@ -315,15 +321,16 @@ class AnkiWebView(QWebEngineView):
         self.set_open_links_externally(True)
 
     def _setHtml(self, html: str) -> None:
-        app = QApplication.instance()
+        from aqt import mw
-        oldFocus = app.focusWidget()
+
+        oldFocus = mw.app.focusWidget()
         self._domDone = False
         self._page.setHtml(html)
         # work around webengine stealing focus on setHtml()
         if oldFocus:
             oldFocus.setFocus()
 
-    def load(self, url: QUrl) -> None:
+    def load_url(self, url: QUrl) -> None:
         # allow queuing actions when loading url directly
         self._domDone = False
         super().load(url)
@@ -641,5 +648,12 @@ document.head.appendChild(style);
         else:
             extra = ""
         self.hide_while_preserving_layout()
-        self.load(QUrl(f"{mw.serverURL()}_anki/pages/{name}.html{extra}"))
+        self.load_url(QUrl(f"{mw.serverURL()}_anki/pages/{name}.html{extra}"))
         self.inject_dynamic_style_and_show()
+
+    def force_load_hack(self) -> None:
+        """Force process to initialize.
+        Must be done on Windows prior to changing current working directory."""
+        self.requiresCol = False
+        self._domReady = False
+        self._page.setContent(bytes("", "ascii"))

qt/mypy.ini

@@ -9,6 +9,19 @@ check_untyped_defs = true
 disallow_untyped_defs = True
 strict_equality = true
 
+[mypy-aqt.scheduling_ops]
+no_strict_optional = false
+[mypy-aqt.note_ops]
+no_strict_optional = false
+[mypy-aqt.card_ops]
+no_strict_optional = false
+[mypy-aqt.deck_ops]
+no_strict_optional = false
+[mypy-aqt.find_and_replace]
+no_strict_optional = false
+[mypy-aqt.tag_ops]
+no_strict_optional = false
+
 [mypy-aqt.winpaths]
 disallow_untyped_defs=false
 [mypy-aqt.mpv]

qt/tools/genhooks_gui.py

@@ -24,7 +24,7 @@ from anki.cards import Card
 from anki.decks import Deck, DeckConfig
 from anki.hooks import runFilter, runHook
 from anki.models import NoteType
-from aqt.qt import QDialog, QEvent, QMenu
+from aqt.qt import QDialog, QEvent, QMenu, QWidget
 from aqt.tagedit import TagEdit
 """
 
@@ -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"],
@@ -391,7 +394,46 @@ 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_did_execute",
+        args=[
+            "changes: anki.collection.OpChanges",
+        ],
+        doc="""Called after an operation completes.
+        Changes can be inspected to determine whether the UI needs updating.
+        
+        This will also be called when the legacy mw.reset() is used.
+        """,
+    ),
+    Hook(
+        name="focus_did_change",
+        args=[
+            "new: Optional[QWidget]",
+            "old: Optional[QWidget]",
+        ],
+        doc="""Called each time the focus changes. Can be used to defer updates from
+        `operation_did_execute` until a window is brought to the front.""",
+    ),
+    Hook(
+        name="backend_will_block",
+        doc="""Called before one or more operations are executed with mw.perform_op().
+        
+        Subscribers can use this to set a flag to avoid DB queries until the operation
+        completes, as doing so will freeze the UI until the long-running operation
+        completes.
+        """,
+    ),
+    Hook(
+        name="backend_did_block",
+        doc="""Called after one or more operations are executed with mw.perform_op().
+        Called regardless of the success of individual operations, and only called when
+        there are no outstanding ops.
+        """,
     ),
     # Webview
     ###################

rslib/backend.proto

@@ -45,6 +45,11 @@ message StringList {
   repeated string vals = 1;
 }
 
+message OpChangesWithCount {
+  uint32 count = 1;
+  OpChanges changes = 2;
+}
+
 // IDs used in RPC calls
 ///////////////////////////////////////////////////////////
 
@@ -108,19 +113,19 @@ service SchedulingService {
   rpc ExtendLimits(ExtendLimitsIn) returns (Empty);
   rpc CountsForDeckToday(DeckID) returns (CountsForDeckTodayOut);
   rpc CongratsInfo(Empty) returns (CongratsInfoOut);
-  rpc RestoreBuriedAndSuspendedCards(CardIDs) returns (Empty);
+  rpc RestoreBuriedAndSuspendedCards(CardIDs) returns (OpChanges);
   rpc UnburyCardsInCurrentDeck(UnburyCardsInCurrentDeckIn) returns (Empty);
-  rpc BuryOrSuspendCards(BuryOrSuspendCardsIn) returns (Empty);
+  rpc BuryOrSuspendCards(BuryOrSuspendCardsIn) returns (OpChanges);
   rpc EmptyFilteredDeck(DeckID) returns (Empty);
   rpc RebuildFilteredDeck(DeckID) returns (UInt32);
-  rpc ScheduleCardsAsNew(ScheduleCardsAsNewIn) returns (Empty);
+  rpc ScheduleCardsAsNew(ScheduleCardsAsNewIn) returns (OpChanges);
-  rpc SetDueDate(SetDueDateIn) returns (Empty);
+  rpc SetDueDate(SetDueDateIn) returns (OpChanges);
-  rpc SortCards(SortCardsIn) returns (Empty);
+  rpc SortCards(SortCardsIn) returns (OpChangesWithCount);
-  rpc SortDeck(SortDeckIn) returns (Empty);
+  rpc SortDeck(SortDeckIn) returns (OpChangesWithCount);
   rpc GetNextCardStates(CardID) returns (NextCardStates);
   rpc DescribeNextStates(NextCardStates) returns (StringList);
   rpc StateIsLeech(SchedulingState) returns (Bool);
-  rpc AnswerCard(AnswerCardIn) returns (Empty);
+  rpc AnswerCard(AnswerCardIn) returns (OpChanges);
   rpc UpgradeScheduler(Empty) returns (Empty);
   rpc GetQueuedCards(GetQueuedCardsIn) returns (GetQueuedCardsOut);
 }
@@ -134,23 +139,21 @@ service DecksService {
   rpc GetDeckLegacy(DeckID) returns (Json);
   rpc GetDeckNames(GetDeckNamesIn) returns (DeckNames);
   rpc NewDeckLegacy(Bool) returns (Json);
-  rpc RemoveDecks(DeckIDs) returns (UInt32);
+  rpc RemoveDecks(DeckIDs) returns (OpChangesWithCount);
-  rpc DragDropDecks(DragDropDecksIn) returns (Empty);
+  rpc DragDropDecks(DragDropDecksIn) returns (OpChanges);
-  rpc RenameDeck(RenameDeckIn) returns (Empty);
+  rpc RenameDeck(RenameDeckIn) returns (OpChanges);
 }
 
 service NotesService {
   rpc NewNote(NoteTypeID) returns (Note);
-  rpc AddNote(AddNoteIn) returns (NoteID);
+  rpc AddNote(AddNoteIn) returns (AddNoteOut);
   rpc DefaultsForAdding(DefaultsForAddingIn) returns (DeckAndNotetype);
   rpc DefaultDeckForNotetype(NoteTypeID) returns (DeckID);
-  rpc UpdateNote(UpdateNoteIn) returns (Empty);
+  rpc UpdateNote(UpdateNoteIn) returns (OpChanges);
   rpc GetNote(NoteID) returns (Note);
-  rpc RemoveNotes(RemoveNotesIn) returns (Empty);
+  rpc RemoveNotes(RemoveNotesIn) returns (OpChanges);
-  rpc AddNoteTags(AddNoteTagsIn) returns (UInt32);
-  rpc UpdateNoteTags(UpdateNoteTagsIn) returns (UInt32);
   rpc ClozeNumbersInNote(Note) returns (ClozeNumbersInNoteOut);
-  rpc AfterNoteUpdates(AfterNoteUpdatesIn) returns (Empty);
+  rpc AfterNoteUpdates(AfterNoteUpdatesIn) returns (OpChanges);
   rpc FieldNamesForNotes(FieldNamesForNotesIn) returns (FieldNamesForNotesOut);
   rpc NoteIsDuplicateOrEmpty(Note) returns (NoteIsDuplicateOrEmptyOut);
   rpc CardsOfNote(NoteID) returns (CardIDs);
@@ -179,7 +182,7 @@ service ConfigService {
   rpc GetConfigString(Config.String) returns (String);
   rpc SetConfigString(SetConfigStringIn) returns (Empty);
   rpc GetPreferences(Empty) returns (Preferences);
-  rpc SetPreferences(Preferences) returns (Empty);
+  rpc SetPreferences(Preferences) returns (OpChanges);
 }
 
 service NoteTypesService {
@@ -212,13 +215,16 @@ service DeckConfigService {
 }
 
 service TagsService {
-  rpc ClearUnusedTags(Empty) returns (Empty);
+  rpc ClearUnusedTags(Empty) returns (OpChangesWithCount);
   rpc AllTags(Empty) returns (StringList);
-  rpc ExpungeTags(String) returns (UInt32);
+  rpc RemoveTags(String) returns (OpChangesWithCount);
   rpc SetTagExpanded(SetTagExpandedIn) returns (Empty);
-  rpc ClearTag(String) returns (Empty);
   rpc TagTree(Empty) returns (TagTreeNode);
-  rpc DragDropTags(DragDropTagsIn) returns (Empty);
+  rpc ReparentTags(ReparentTagsIn) returns (OpChangesWithCount);
+  rpc RenameTags(RenameTagsIn) returns (OpChangesWithCount);
+  rpc AddNoteTags(NoteIDsAndTagsIn) returns (OpChangesWithCount);
+  rpc RemoveNoteTags(NoteIDsAndTagsIn) returns (OpChangesWithCount);
+  rpc FindAndReplaceTag(FindAndReplaceTagIn) returns (OpChangesWithCount);
 }
 
 service SearchService {
@@ -227,7 +233,7 @@ service SearchService {
   rpc SearchNotes(SearchNotesIn) returns (SearchNotesOut);
   rpc JoinSearchNodes(JoinSearchNodesIn) returns (String);
   rpc ReplaceSearchNode(ReplaceSearchNodeIn) returns (String);
-  rpc FindAndReplace(FindAndReplaceIn) returns (UInt32);
+  rpc FindAndReplace(FindAndReplaceIn) returns (OpChangesWithCount);
 }
 
 service StatsService {
@@ -264,9 +270,10 @@ service CollectionService {
 
 service CardsService {
   rpc GetCard(CardID) returns (Card);
-  rpc UpdateCard(UpdateCardIn) returns (Empty);
+  rpc UpdateCard(UpdateCardIn) returns (OpChanges);
   rpc RemoveCards(RemoveCardsIn) returns (Empty);
-  rpc SetDeck(SetDeckIn) returns (Empty);
+  rpc SetDeck(SetDeckIn) returns (OpChanges);
+  rpc SetFlag(SetFlagIn) returns (OpChanges);
 }
 
 // Protobuf stored in .anki2 files
@@ -919,9 +926,14 @@ message TagTreeNode {
   bool expanded = 4;
 }
 
-message DragDropTagsIn {
+message ReparentTagsIn {
-  repeated string source_tags = 1;
+  repeated string tags = 1;
-  string target_tag = 2;
+  string new_parent = 2;
+}
+
+message RenameTagsIn {
+  string current_prefix = 1;
+  string new_prefix = 2;
 }
 
 message SetConfigJsonIn {
@@ -970,6 +982,11 @@ message AddNoteIn {
   int64 deck_id = 2;
 }
 
+message AddNoteOut {
+  int64 note_id = 1;
+  OpChanges changes = 2;
+}
+
 message UpdateNoteIn {
   Note note = 1;
   bool skip_undo_entry = 2;
@@ -1027,16 +1044,17 @@ message AfterNoteUpdatesIn {
   bool generate_cards = 3;
 }
 
-message AddNoteTagsIn {
+message NoteIDsAndTagsIn {
-  repeated int64 nids = 1;
+  repeated int64 note_ids = 1;
   string tags = 2;
 }
 
-message UpdateNoteTagsIn {
+message FindAndReplaceTagIn {
-  repeated int64 nids = 1;
+  repeated int64 note_ids = 1;
-  string tags = 2;
+  string search = 2;
   string replacement = 3;
   bool regex = 4;
+  bool match_case = 5;
 }
 
 message CheckDatabaseOut {
@@ -1442,6 +1460,24 @@ message GetQueuedCardsOut {
   }
 }
 
+message OpChanges {
+  // 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;
+    UPDATE_NOTE = 3;
+  }
+
+  Kind kind = 1;
+  bool card = 2;
+  bool note = 3;
+  bool deck = 4;
+  bool tag = 5;
+  bool notetype = 6;
+  bool preference = 7;
+}
+
 message UndoStatus {
   string undo = 1;
   string redo = 2;
@@ -1459,4 +1495,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

@@ -21,22 +21,17 @@ impl CardsService for Backend {
         })
     }
 
-    fn update_card(&self, input: pb::UpdateCardIn) -> Result<pb::Empty> {
+    fn update_card(&self, input: pb::UpdateCardIn) -> Result<pb::OpChanges> {
         self.with_col(|col| {
-            let op = if input.skip_undo_entry {
-                None
-            } else {
-                Some(UndoableOpKind::UpdateCard)
-            };
             let mut card: Card = input.card.ok_or(AnkiError::NotFound)?.try_into()?;
-            col.update_card_with_op(&mut card, op)
+            col.update_card_maybe_undoable(&mut card, !input.skip_undo_entry)
         })
         .map(Into::into)
     }
 
     fn remove_cards(&self, input: pb::RemoveCardsIn) -> Result<pb::Empty> {
         self.with_col(|col| {
-            col.transact(None, |col| {
+            col.transact_no_undo(|col| {
                 col.remove_cards_and_orphaned_notes(
                     &input
                         .card_ids
@@ -49,11 +44,18 @@ impl CardsService for Backend {
         })
     }
 
-    fn set_deck(&self, input: pb::SetDeckIn) -> Result<pb::Empty> {
+    fn set_deck(&self, input: pb::SetDeckIn) -> Result<pb::OpChanges> {
         let cids: Vec<_> = input.card_ids.into_iter().map(CardID).collect();
         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::OpChanges> {
+        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 +113,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/config.rs

@@ -61,7 +61,7 @@ impl ConfigService for Backend {
 
     fn set_config_json(&self, input: pb::SetConfigJsonIn) -> Result<pb::Empty> {
         self.with_col(|col| {
-            col.transact(None, |col| {
+            col.transact_no_undo(|col| {
                 // ensure it's a well-formed object
                 let val: Value = serde_json::from_slice(&input.value_json)?;
                 col.set_config(input.key.as_str(), &val)
@@ -71,7 +71,7 @@ impl ConfigService for Backend {
     }
 
     fn remove_config(&self, input: pb::String) -> Result<pb::Empty> {
-        self.with_col(|col| col.transact(None, |col| col.remove_config(input.val.as_str())))
+        self.with_col(|col| col.transact_no_undo(|col| col.remove_config(input.val.as_str())))
             .map(Into::into)
     }
 
@@ -92,8 +92,10 @@ impl ConfigService for Backend {
     }
 
     fn set_config_bool(&self, input: pb::SetConfigBoolIn) -> Result<pb::Empty> {
-        self.with_col(|col| col.transact(None, |col| col.set_bool(input.key().into(), input.value)))
+        self.with_col(|col| {
-            .map(Into::into)
+            col.transact_no_undo(|col| col.set_bool(input.key().into(), input.value))
+        })
+        .map(Into::into)
     }
 
     fn get_config_string(&self, input: pb::config::String) -> Result<pb::String> {
@@ -106,7 +108,7 @@ impl ConfigService for Backend {
 
     fn set_config_string(&self, input: pb::SetConfigStringIn) -> Result<pb::Empty> {
         self.with_col(|col| {
-            col.transact(None, |col| col.set_string(input.key().into(), &input.value))
+            col.transact_no_undo(|col| col.set_string(input.key().into(), &input.value))
         })
         .map(Into::into)
     }
@@ -115,7 +117,7 @@ impl ConfigService for Backend {
         self.with_col(|col| col.get_preferences())
     }
 
-    fn set_preferences(&self, input: pb::Preferences) -> Result<pb::Empty> {
+    fn set_preferences(&self, input: pb::Preferences) -> Result<pb::OpChanges> {
         self.with_col(|col| col.set_preferences(input))
             .map(Into::into)
     }

rslib/src/backend/dbproxy.rs

@@ -75,7 +75,7 @@ pub(super) fn db_command_bytes(col: &mut Collection, input: &[u8]) -> Result<Vec
             args,
             first_row_only,
         } => {
-            maybe_clear_undo(col, &sql);
+            update_state_after_modification(col, &sql);
             if first_row_only {
                 db_query_row(&col.storage, &sql, &args)?
             } else {
@@ -87,6 +87,10 @@ pub(super) fn db_command_bytes(col: &mut Collection, input: &[u8]) -> Result<Vec
             DBResult::None
         }
         DBRequest::Commit => {
+            if col.state.modified_by_dbproxy {
+                col.storage.set_modified()?;
+                col.state.modified_by_dbproxy = false;
+            }
             col.storage.commit_trx()?;
             DBResult::None
         }
@@ -96,17 +100,17 @@ pub(super) fn db_command_bytes(col: &mut Collection, input: &[u8]) -> Result<Vec
             DBResult::None
         }
         DBRequest::ExecuteMany { sql, args } => {
-            maybe_clear_undo(col, &sql);
+            update_state_after_modification(col, &sql);
             db_execute_many(&col.storage, &sql, &args)?
         }
     };
     Ok(serde_json::to_vec(&resp)?)
 }
 
-fn maybe_clear_undo(col: &mut Collection, sql: &str) {
+fn update_state_after_modification(col: &mut Collection, sql: &str) {
     if !is_dql(sql) {
         println!("clearing undo+study due to {}", sql);
-        col.discard_undo_and_study_queues();
+        col.update_state_after_dbproxy_modification();
     }
 }
 

rslib/src/backend/deckconfig.rs

@@ -17,7 +17,7 @@ impl DeckConfigService for Backend {
         let conf: DeckConfSchema11 = serde_json::from_slice(&input.config)?;
         let mut conf: DeckConf = conf.into();
         self.with_col(|col| {
-            col.transact(None, |col| {
+            col.transact_no_undo(|col| {
                 col.add_or_update_deck_config(&mut conf, input.preserve_usn_and_mtime)?;
                 Ok(pb::DeckConfigId { dcid: conf.id.0 })
             })
@@ -54,7 +54,7 @@ impl DeckConfigService for Backend {
     }
 
     fn remove_deck_config(&self, input: pb::DeckConfigId) -> Result<pb::Empty> {
-        self.with_col(|col| col.transact(None, |col| col.remove_deck_config(input.into())))
+        self.with_col(|col| col.transact_no_undo(|col| col.remove_deck_config(input.into())))
             .map(Into::into)
     }
 }

rslib/src/backend/decks.rs

@@ -15,7 +15,7 @@ impl DecksService for Backend {
             let schema11: DeckSchema11 = serde_json::from_slice(&input.deck)?;
             let mut deck: Deck = schema11.into();
             if input.preserve_usn_and_mtime {
-                col.transact(None, |col| {
+                col.transact_no_undo(|col| {
                     let usn = col.usn()?;
                     col.add_or_update_single_deck_with_existing_id(&mut deck, usn)
                 })?;
@@ -109,12 +109,12 @@ impl DecksService for Backend {
             .map(Into::into)
     }
 
-    fn remove_decks(&self, input: pb::DeckIDs) -> Result<pb::UInt32> {
+    fn remove_decks(&self, input: pb::DeckIDs) -> Result<pb::OpChangesWithCount> {
         self.with_col(|col| col.remove_decks_and_child_decks(&Into::<Vec<DeckID>>::into(input)))
             .map(Into::into)
     }
 
-    fn drag_drop_decks(&self, input: pb::DragDropDecksIn) -> Result<pb::Empty> {
+    fn drag_drop_decks(&self, input: pb::DragDropDecksIn) -> Result<pb::OpChanges> {
         let source_dids: Vec<_> = input.source_deck_ids.into_iter().map(Into::into).collect();
         let target_did = if input.target_deck_id == 0 {
             None
@@ -125,7 +125,7 @@ impl DecksService for Backend {
             .map(Into::into)
     }
 
-    fn rename_deck(&self, input: pb::RenameDeckIn) -> Result<pb::Empty> {
+    fn rename_deck(&self, input: pb::RenameDeckIn) -> Result<pb::OpChanges> {
         self.with_col(|col| col.rename_deck(input.deck_id.into(), &input.new_name))
             .map(Into::into)
     }

rslib/src/backend/generic.rs

@@ -80,3 +80,12 @@ impl From<Vec<String>> for pb::StringList {
         pb::StringList { vals }
     }
 }
+
+impl From<OpOutput<usize>> for pb::OpChangesWithCount {
+    fn from(out: OpOutput<usize>) -> Self {
+        pb::OpChangesWithCount {
+            count: out.output as u32,
+            changes: Some(out.changes.into()),
+        }
+    }
+}

rslib/src/backend/media.rs

@@ -19,7 +19,7 @@ impl MediaService for Backend {
             move |progress| handler.update(Progress::MediaCheck(progress as u32), true);
         self.with_col(|col| {
             let mgr = MediaManager::new(&col.media_folder, &col.media_db)?;
-            col.transact(None, |ctx| {
+            col.transact_no_undo(|ctx| {
                 let mut checker = MediaChecker::new(ctx, &mgr, progress_fn);
                 let mut output = checker.check()?;
 
@@ -62,7 +62,7 @@ impl MediaService for Backend {
 
         self.with_col(|col| {
             let mgr = MediaManager::new(&col.media_folder, &col.media_db)?;
-            col.transact(None, |ctx| {
+            col.transact_no_undo(|ctx| {
                 let mut checker = MediaChecker::new(ctx, &mgr, progress_fn);
 
                 checker.empty_trash()
@@ -78,7 +78,7 @@ impl MediaService for Backend {
         self.with_col(|col| {
             let mgr = MediaManager::new(&col.media_folder, &col.media_db)?;
 
-            col.transact(None, |ctx| {
+            col.transact_no_undo(|ctx| {
                 let mut checker = MediaChecker::new(ctx, &mgr, progress_fn);
 
                 checker.restore_trash()

rslib/src/backend/mod.rs

@@ -15,6 +15,7 @@ mod i18n;
 mod media;
 mod notes;
 mod notetypes;
+mod ops;
 mod progress;
 mod scheduler;
 mod search;

rslib/src/backend/notes.rs

@@ -12,9 +12,6 @@ use crate::{
 pub(super) use pb::notes_service::Service as NotesService;
 
 impl NotesService for Backend {
-    // notes
-    //-------------------------------------------------------------------
-
     fn new_note(&self, input: pb::NoteTypeId) -> Result<pb::Note> {
         self.with_col(|col| {
             let nt = col.get_notetype(input.into())?.ok_or(AnkiError::NotFound)?;
@@ -22,11 +19,14 @@ impl NotesService for Backend {
         })
     }
 
-    fn add_note(&self, input: pb::AddNoteIn) -> Result<pb::NoteId> {
+    fn add_note(&self, input: pb::AddNoteIn) -> Result<pb::AddNoteOut> {
         self.with_col(|col| {
             let mut note: Note = input.note.ok_or(AnkiError::NotFound)?.into();
-            col.add_note(&mut note, DeckID(input.deck_id))
+            let changes = col.add_note(&mut note, DeckID(input.deck_id))?;
-                .map(|_| pb::NoteId { nid: note.id.0 })
+            Ok(pb::AddNoteOut {
+                note_id: note.id.0,
+                changes: Some(changes.into()),
+            })
         })
     }
 
@@ -46,15 +46,10 @@ impl NotesService for Backend {
         })
     }
 
-    fn update_note(&self, input: pb::UpdateNoteIn) -> Result<pb::Empty> {
+    fn update_note(&self, input: pb::UpdateNoteIn) -> Result<pb::OpChanges> {
         self.with_col(|col| {
-            let op = if input.skip_undo_entry {
-                None
-            } else {
-                Some(UndoableOpKind::UpdateNote)
-            };
             let mut note: Note = input.note.ok_or(AnkiError::NotFound)?.into();
-            col.update_note_with_op(&mut note, op)
+            col.update_note_maybe_undoable(&mut note, !input.skip_undo_entry)
         })
         .map(Into::into)
     }
@@ -68,7 +63,7 @@ impl NotesService for Backend {
         })
     }
 
-    fn remove_notes(&self, input: pb::RemoveNotesIn) -> Result<pb::Empty> {
+    fn remove_notes(&self, input: pb::RemoveNotesIn) -> Result<pb::OpChanges> {
         self.with_col(|col| {
             if !input.note_ids.is_empty() {
                 col.remove_notes(
@@ -77,9 +72,8 @@ impl NotesService for Backend {
                         .into_iter()
                         .map(Into::into)
                         .collect::<Vec<_>>(),
-                )?;
+                )
-            }
+            } else {
-            if !input.card_ids.is_empty() {
                 let nids = col.storage.note_ids_of_cards(
                     &input
                         .card_ids
@@ -87,29 +81,9 @@ impl NotesService for Backend {
                         .map(Into::into)
                         .collect::<Vec<_>>(),
                 )?;
-                col.remove_notes(&nids.into_iter().collect::<Vec<_>>())?
+                col.remove_notes(&nids.into_iter().collect::<Vec<_>>())
             }
-            Ok(().into())
+            .map(Into::into)
-        })
-    }
-
-    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)
-                .map(|n| n as u32)
-        })
-        .map(Into::into)
-    }
-
-    fn update_note_tags(&self, input: pb::UpdateNoteTagsIn) -> Result<pb::UInt32> {
-        self.with_col(|col| {
-            col.replace_tags_for_notes(
-                &to_nids(input.nids),
-                &input.tags,
-                &input.replacement,
-                input.regex,
-            )
-            .map(|n| (n as u32).into())
         })
     }
 
@@ -123,16 +97,14 @@ impl NotesService for Backend {
         })
     }
 
-    fn after_note_updates(&self, input: pb::AfterNoteUpdatesIn) -> Result<pb::Empty> {
+    fn after_note_updates(&self, input: pb::AfterNoteUpdatesIn) -> Result<pb::OpChanges> {
         self.with_col(|col| {
-            col.transact(None, |col| {
+            col.after_note_updates(
-                col.after_note_updates(
+                &to_note_ids(input.nids),
-                    &to_nids(input.nids),
+                input.generate_cards,
-                    input.generate_cards,
+                input.mark_notes_modified,
-                    input.mark_notes_modified,
+            )
-                )?;
+            .map(Into::into)
-                Ok(pb::Empty {})
-            })
         })
     }
 
@@ -167,6 +139,6 @@ impl NotesService for Backend {
     }
 }
 
-fn to_nids(ids: Vec<i64>) -> Vec<NoteID> {
+pub(super) fn to_note_ids(ids: Vec<i64>) -> Vec<NoteID> {
     ids.into_iter().map(NoteID).collect()
 }

rslib/src/backend/ops.rs

@@ -0,0 +1,46 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+use pb::op_changes::Kind;
+
+use crate::{backend_proto as pb, ops::OpChanges, prelude::*, undo::UndoStatus};
+
+impl From<Op> for Kind {
+    fn from(o: Op) -> Self {
+        match o {
+            Op::SetFlag => Kind::SetCardFlag,
+            Op::UpdateTag => Kind::UpdateNoteTags,
+            Op::UpdateNote => Kind::UpdateNote,
+            _ => Kind::Other,
+        }
+    }
+}
+
+impl From<OpChanges> for pb::OpChanges {
+    fn from(c: OpChanges) -> Self {
+        pb::OpChanges {
+            kind: Kind::from(c.op) as i32,
+            card: c.changes.card,
+            note: c.changes.note,
+            deck: c.changes.deck,
+            tag: c.changes.tag,
+            notetype: c.changes.notetype,
+            preference: c.changes.preference,
+        }
+    }
+}
+
+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(),
+        }
+    }
+}
+
+impl From<OpOutput<()>> for pb::OpChanges {
+    fn from(o: OpOutput<()>) -> Self {
+        o.changes.into()
+    }
+}

rslib/src/backend/scheduler/mod.rs

@@ -39,7 +39,7 @@ impl SchedulingService for Backend {
 
     fn update_stats(&self, input: pb::UpdateStatsIn) -> Result<pb::Empty> {
         self.with_col(|col| {
-            col.transact(None, |col| {
+            col.transact_no_undo(|col| {
                 let today = col.current_due_day(0)?;
                 let usn = col.usn()?;
                 col.update_deck_stats(today, usn, input).map(Into::into)
@@ -49,7 +49,7 @@ impl SchedulingService for Backend {
 
     fn extend_limits(&self, input: pb::ExtendLimitsIn) -> Result<pb::Empty> {
         self.with_col(|col| {
-            col.transact(None, |col| {
+            col.transact_no_undo(|col| {
                 let today = col.current_due_day(0)?;
                 let usn = col.usn()?;
                 col.extend_limits(
@@ -72,7 +72,7 @@ impl SchedulingService for Backend {
         self.with_col(|col| col.congrats_info())
     }
 
-    fn restore_buried_and_suspended_cards(&self, input: pb::CardIDs) -> Result<pb::Empty> {
+    fn restore_buried_and_suspended_cards(&self, input: pb::CardIDs) -> Result<pb::OpChanges> {
         let cids: Vec<_> = input.into();
         self.with_col(|col| col.unbury_or_unsuspend_cards(&cids).map(Into::into))
     }
@@ -87,7 +87,7 @@ impl SchedulingService for Backend {
         })
     }
 
-    fn bury_or_suspend_cards(&self, input: pb::BuryOrSuspendCardsIn) -> Result<pb::Empty> {
+    fn bury_or_suspend_cards(&self, input: pb::BuryOrSuspendCardsIn) -> Result<pb::OpChanges> {
         self.with_col(|col| {
             let mode = input.mode();
             let cids: Vec<_> = input.card_ids.into_iter().map(CardID).collect();
@@ -103,7 +103,7 @@ impl SchedulingService for Backend {
         self.with_col(|col| col.rebuild_filtered_deck(input.did.into()).map(Into::into))
     }
 
-    fn schedule_cards_as_new(&self, input: pb::ScheduleCardsAsNewIn) -> Result<pb::Empty> {
+    fn schedule_cards_as_new(&self, input: pb::ScheduleCardsAsNewIn) -> Result<pb::OpChanges> {
         self.with_col(|col| {
             let cids: Vec<_> = input.card_ids.into_iter().map(CardID).collect();
             let log = input.log;
@@ -111,14 +111,14 @@ impl SchedulingService for Backend {
         })
     }
 
-    fn set_due_date(&self, input: pb::SetDueDateIn) -> Result<pb::Empty> {
+    fn set_due_date(&self, input: pb::SetDueDateIn) -> Result<pb::OpChanges> {
         let config = input.config_key.map(Into::into);
         let days = input.days;
         let cids: Vec<_> = input.card_ids.into_iter().map(CardID).collect();
         self.with_col(|col| col.set_due_date(&cids, &days, config).map(Into::into))
     }
 
-    fn sort_cards(&self, input: pb::SortCardsIn) -> Result<pb::Empty> {
+    fn sort_cards(&self, input: pb::SortCardsIn) -> Result<pb::OpChangesWithCount> {
         let cids: Vec<_> = input.card_ids.into_iter().map(CardID).collect();
         let (start, step, random, shift) = (
             input.starting_from,
@@ -137,7 +137,7 @@ impl SchedulingService for Backend {
         })
     }
 
-    fn sort_deck(&self, input: pb::SortDeckIn) -> Result<pb::Empty> {
+    fn sort_deck(&self, input: pb::SortDeckIn) -> Result<pb::OpChangesWithCount> {
         self.with_col(|col| {
             col.sort_deck(input.deck_id.into(), input.randomize)
                 .map(Into::into)
@@ -161,13 +161,13 @@ impl SchedulingService for Backend {
         Ok(state.leeched().into())
     }
 
-    fn answer_card(&self, input: pb::AnswerCardIn) -> Result<pb::Empty> {
+    fn answer_card(&self, input: pb::AnswerCardIn) -> Result<pb::OpChanges> {
         self.with_col(|col| col.answer_card(&input.into()))
             .map(Into::into)
     }
 
     fn upgrade_scheduler(&self, _input: pb::Empty) -> Result<pb::Empty> {
-        self.with_col(|col| col.transact(None, |col| col.upgrade_to_v2_scheduler()))
+        self.with_col(|col| col.transact_no_undo(|col| col.upgrade_to_v2_scheduler()))
             .map(Into::into)
     }
 

rslib/src/backend/search.rs

@@ -68,7 +68,7 @@ impl SearchService for Backend {
         Ok(replace_search_node(existing, replacement).into())
     }
 
-    fn find_and_replace(&self, input: pb::FindAndReplaceIn) -> Result<pb::UInt32> {
+    fn find_and_replace(&self, input: pb::FindAndReplaceIn) -> Result<pb::OpChangesWithCount> {
         let mut search = if input.regex {
             input.search
         } else {
@@ -86,7 +86,7 @@ impl SearchService for Backend {
         let repl = input.replacement;
         self.with_col(|col| {
             col.find_and_replace(nids, &search, &repl, field_name)
-                .map(|cnt| pb::UInt32 { val: cnt as u32 })
+                .map(Into::into)
         })
     }
 }

rslib/src/backend/tags.rs

@@ -1,13 +1,13 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
-use super::Backend;
+use super::{notes::to_note_ids, Backend};
 use crate::{backend_proto as pb, prelude::*};
 pub(super) use pb::tags_service::Service as TagsService;
 
 impl TagsService for Backend {
-    fn clear_unused_tags(&self, _input: pb::Empty) -> Result<pb::Empty> {
+    fn clear_unused_tags(&self, _input: pb::Empty) -> Result<pb::OpChangesWithCount> {
-        self.with_col(|col| col.transact(None, |col| col.clear_unused_tags().map(Into::into)))
+        self.with_col(|col| col.transact_no_undo(|col| col.clear_unused_tags().map(Into::into)))
     }
 
     fn all_tags(&self, _input: pb::Empty) -> Result<pb::StringList> {
@@ -23,40 +23,66 @@ impl TagsService for Backend {
         })
     }
 
-    fn expunge_tags(&self, tags: pb::String) -> Result<pb::UInt32> {
+    fn remove_tags(&self, tags: pb::String) -> Result<pb::OpChangesWithCount> {
-        self.with_col(|col| col.expunge_tags(tags.val.as_str()).map(Into::into))
+        self.with_col(|col| col.remove_tags(tags.val.as_str()).map(Into::into))
     }
 
     fn set_tag_expanded(&self, input: pb::SetTagExpandedIn) -> Result<pb::Empty> {
         self.with_col(|col| {
-            col.transact(None, |col| {
+            col.transact_no_undo(|col| {
                 col.set_tag_expanded(&input.name, input.expanded)?;
                 Ok(().into())
             })
         })
     }
 
-    fn clear_tag(&self, tag: pb::String) -> Result<pb::Empty> {
-        self.with_col(|col| {
-            col.transact(None, |col| {
-                col.storage.clear_tag_and_children(tag.val.as_str())?;
-                Ok(().into())
-            })
-        })
-    }
-
     fn tag_tree(&self, _input: pb::Empty) -> Result<pb::TagTreeNode> {
         self.with_col(|col| col.tag_tree())
     }
 
-    fn drag_drop_tags(&self, input: pb::DragDropTagsIn) -> Result<pb::Empty> {
+    fn reparent_tags(&self, input: pb::ReparentTagsIn) -> Result<pb::OpChangesWithCount> {
-        let source_tags = input.source_tags;
+        let source_tags = input.tags;
-        let target_tag = if input.target_tag.is_empty() {
+        let target_tag = if input.new_parent.is_empty() {
             None
         } else {
-            Some(input.target_tag)
+            Some(input.new_parent)
         };
-        self.with_col(|col| col.drag_drop_tags(&source_tags, target_tag))
+        self.with_col(|col| col.reparent_tags(&source_tags, target_tag))
             .map(Into::into)
     }
+
+    fn rename_tags(&self, input: pb::RenameTagsIn) -> Result<pb::OpChangesWithCount> {
+        self.with_col(|col| col.rename_tag(&input.current_prefix, &input.new_prefix))
+            .map(Into::into)
+    }
+
+    fn add_note_tags(&self, input: pb::NoteIDsAndTagsIn) -> Result<pb::OpChangesWithCount> {
+        self.with_col(|col| {
+            col.add_tags_to_notes(&to_note_ids(input.note_ids), &input.tags)
+                .map(Into::into)
+        })
+    }
+
+    fn remove_note_tags(&self, input: pb::NoteIDsAndTagsIn) -> Result<pb::OpChangesWithCount> {
+        self.with_col(|col| {
+            col.remove_tags_from_notes(&to_note_ids(input.note_ids), &input.tags)
+                .map(Into::into)
+        })
+    }
+
+    fn find_and_replace_tag(
+        &self,
+        input: pb::FindAndReplaceTagIn,
+    ) -> Result<pb::OpChangesWithCount> {
+        self.with_col(|col| {
+            col.find_and_replace_tag(
+                &to_note_ids(input.note_ids),
+                &input.search,
+                &input.replacement,
+                input.regex,
+                input.match_case,
+            )
+            .map(Into::into)
+        })
+    }
 }

rslib/src/card/mod.rs

@@ -6,9 +6,10 @@ pub(crate) mod undo;
 use crate::err::{AnkiError, Result};
 use crate::notes::NoteID;
 use crate::{
-    collection::Collection, config::SchedulerVersion, timestamp::TimestampSecs, types::Usn,
+    collection::Collection, config::SchedulerVersion, prelude::*, timestamp::TimestampSecs,
+    types::Usn,
 };
-use crate::{define_newtype, undo::UndoableOpKind};
+use crate::{define_newtype, ops::StateChanges};
 
 use crate::{deckconf::DeckConf, decks::DeckID};
 use num_enum::TryFromPrimitive;
@@ -110,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 {
@@ -139,13 +149,31 @@ impl Card {
 }
 
 impl Collection {
-    pub(crate) fn update_card_with_op(
+    pub(crate) fn update_card_maybe_undoable(
         &mut self,
         card: &mut Card,
-        op: Option<UndoableOpKind>,
+        undoable: bool,
-    ) -> Result<()> {
+    ) -> Result<OpOutput<()>> {
         let existing = self.storage.get_card(card.id)?.ok_or(AnkiError::NotFound)?;
-        self.transact(op, |col| col.update_card_inner(card, existing, col.usn()?))
+        if undoable {
+            self.transact(Op::UpdateCard, |col| {
+                col.update_card_inner(card, existing, col.usn()?)
+            })
+        } else {
+            self.transact_no_undo(|col| {
+                col.update_card_inner(card, existing, col.usn()?)?;
+                Ok(OpOutput {
+                    output: (),
+                    changes: OpChanges {
+                        op: Op::UpdateCard,
+                        changes: StateChanges {
+                            card: true,
+                            ..Default::default()
+                        },
+                    },
+                })
+            })
+        }
     }
 
     #[cfg(test)]
@@ -203,7 +231,7 @@ impl Collection {
         Ok(())
     }
 
-    pub fn set_deck(&mut self, cards: &[CardID], deck_id: DeckID) -> Result<()> {
+    pub fn set_deck(&mut self, cards: &[CardID], deck_id: DeckID) -> Result<OpOutput<()>> {
         let deck = self.get_deck(deck_id)?.ok_or(AnkiError::NotFound)?;
         if deck.is_filtered() {
             return Err(AnkiError::DeckIsFiltered);
@@ -211,7 +239,7 @@ impl Collection {
         self.storage.set_search_table_to_card_ids(cards, false)?;
         let sched = self.scheduler_version();
         let usn = self.usn()?;
-        self.transact(Some(UndoableOpKind::SetDeck), |col| {
+        self.transact(Op::SetDeck, |col| {
             for mut card in col.storage.all_searched_cards()? {
                 if card.deck_id == deck_id {
                     continue;
@@ -224,6 +252,24 @@ impl Collection {
         })
     }
 
+    pub fn set_card_flag(&mut self, cards: &[CardID], flag: u32) -> Result<OpOutput<()>> {
+        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(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/collection.rs

@@ -1,7 +1,6 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
-use crate::i18n::I18n;
 use crate::log::Logger;
 use crate::types::Usn;
 use crate::{
@@ -12,6 +11,7 @@ use crate::{
     undo::UndoManager,
 };
 use crate::{err::Result, scheduler::queue::CardQueues};
+use crate::{i18n::I18n, ops::StateChanges};
 use std::{collections::HashMap, path::PathBuf, sync::Arc};
 
 pub fn open_collection<P: Into<PathBuf>>(
@@ -65,6 +65,9 @@ pub struct CollectionState {
     pub(crate) notetype_cache: HashMap<NoteTypeID, Arc<NoteType>>,
     pub(crate) deck_cache: HashMap<DeckID, Arc<Deck>>,
     pub(crate) card_queues: Option<CardQueues>,
+    /// True if legacy Python code has executed SQL that has modified the
+    /// database, requiring modification time to be bumped.
+    pub(crate) modified_by_dbproxy: bool,
 }
 
 pub struct Collection {
@@ -80,9 +83,7 @@ pub struct Collection {
 }
 
 impl Collection {
-    /// Execute the provided closure in a transaction, rolling back if
+    fn transact_inner<F, R>(&mut self, op: Option<Op>, func: F) -> Result<OpOutput<R>>
-    /// an error is returned.
-    pub(crate) fn transact<F, R>(&mut self, op: Option<UndoableOpKind>, func: F) -> Result<R>
     where
         F: FnOnce(&mut Collection) -> Result<R>,
     {
@@ -92,21 +93,56 @@ impl Collection {
         let mut res = func(self);
 
         if res.is_ok() {
-            if let Err(e) = self.storage.mark_modified() {
+            if let Err(e) = self.storage.set_modified() {
                 res = Err(e);
             } else if let Err(e) = self.storage.commit_rust_trx() {
                 res = Err(e);
             }
         }
 
-        if res.is_err() {
+        match res {
-            self.discard_undo_and_study_queues();
+            Ok(output) => {
-            self.storage.rollback_rust_trx()?;
+                let changes = if op.is_some() {
-        } else {
+                    let changes = self.op_changes()?;
-            self.end_undoable_operation();
+                    self.maybe_clear_study_queues_after_op(changes);
+                    self.maybe_coalesce_note_undo_entry(changes);
+                    changes
+                } else {
+                    self.clear_study_queues();
+                    // dummy value, not used by transact_no_undo(). only required
+                    // until we can migrate all the code to undoable ops
+                    OpChanges {
+                        op: Op::SetFlag,
+                        changes: StateChanges::default(),
+                    }
+                };
+                self.end_undoable_operation();
+                Ok(OpOutput { output, changes })
+            }
+            Err(err) => {
+                self.discard_undo_and_study_queues();
+                self.storage.rollback_rust_trx()?;
+                Err(err)
+            }
         }
+    }
 
-        res
+    /// Execute the provided closure in a transaction, rolling back if
+    /// an error is returned. Records undo state, and returns changes.
+    pub(crate) fn transact<F, R>(&mut self, op: Op, func: F) -> Result<OpOutput<R>>
+    where
+        F: FnOnce(&mut Collection) -> Result<R>,
+    {
+        self.transact_inner(Some(op), func)
+    }
+
+    /// Execute the provided closure in a transaction, rolling back if
+    /// an error is returned.
+    pub(crate) fn transact_no_undo<F, R>(&mut self, func: F) -> Result<R>
+    where
+        F: FnOnce(&mut Collection) -> Result<R>,
+    {
+        self.transact_inner(None, func).map(|out| out.output)
     }
 
     pub(crate) fn close(self, downgrade: bool) -> Result<()> {
@@ -120,7 +156,7 @@ impl Collection {
 
     /// Prepare for upload. Caller should not create transaction.
     pub(crate) fn before_upload(&mut self) -> Result<()> {
-        self.transact(None, |col| {
+        self.transact_no_undo(|col| {
             col.storage.clear_all_graves()?;
             col.storage.clear_pending_note_usns()?;
             col.storage.clear_pending_card_usns()?;

rslib/src/config/undo.rs

@@ -71,7 +71,7 @@ mod test {
     fn undo() -> Result<()> {
         let mut col = open_test_collection();
         // the op kind doesn't matter, we just need undo enabled
-        let op = Some(UndoableOpKind::Bury);
+        let op = Op::Bury;
         // test key
         let key = BoolKey::NormalizeNoteText;
 

rslib/src/dbcheck.rs

@@ -129,7 +129,7 @@ impl Collection {
         debug!(self.log, "optimize");
         self.storage.optimize()?;
 
-        self.transact(None, |col| col.check_database_inner(progress_fn))
+        self.transact_no_undo(|col| col.check_database_inner(progress_fn))
     }
 
     fn check_database_inner<F>(&mut self, mut progress_fn: F) -> Result<CheckDatabaseOutput>

rslib/src/decks/mod.rs

@@ -10,16 +10,14 @@ pub use crate::backend_proto::{
     deck_kind::Kind as DeckKind, filtered_search_term::FilteredSearchOrder, Deck as DeckProto,
     DeckCommon, DeckKind as DeckKindProto, FilteredDeck, FilteredSearchTerm, NormalDeck,
 };
-use crate::{
+use crate::{backend_proto as pb, markdown::render_markdown, text::sanitize_html_no_images};
-    backend_proto as pb, markdown::render_markdown, text::sanitize_html_no_images,
-    undo::UndoableOpKind,
-};
 use crate::{
     collection::Collection,
     deckconf::DeckConfID,
     define_newtype,
     err::{AnkiError, Result},
     i18n::TR,
+    prelude::*,
     text::normalize_to_nfc,
     timestamp::TimestampSecs,
     types::Usn,
@@ -269,7 +267,7 @@ impl Collection {
     /// or rename children as required. Prefer add_deck() or update_deck() to
     /// be explicit about your intentions; this function mainly exists so we
     /// can integrate with older Python code that behaved this way.
-    pub(crate) fn add_or_update_deck(&mut self, deck: &mut Deck) -> Result<()> {
+    pub(crate) fn add_or_update_deck(&mut self, deck: &mut Deck) -> Result<OpOutput<()>> {
         if deck.id.0 == 0 {
             self.add_deck(deck)
         } else {
@@ -278,12 +276,12 @@ impl Collection {
     }
 
     /// Add a new deck. The id must be 0, as it will be automatically assigned.
-    pub fn add_deck(&mut self, deck: &mut Deck) -> Result<()> {
+    pub fn add_deck(&mut self, deck: &mut Deck) -> Result<OpOutput<()>> {
         if deck.id.0 != 0 {
             return Err(AnkiError::invalid_input("deck to add must have id 0"));
         }
 
-        self.transact(Some(UndoableOpKind::AddDeck), |col| {
+        self.transact(Op::AddDeck, |col| {
             let usn = col.usn()?;
             col.prepare_deck_for_update(deck, usn)?;
             deck.set_modified(usn);
@@ -292,15 +290,15 @@ impl Collection {
         })
     }
 
-    pub fn update_deck(&mut self, deck: &mut Deck) -> Result<()> {
+    pub fn update_deck(&mut self, deck: &mut Deck) -> Result<OpOutput<()>> {
-        self.transact(Some(UndoableOpKind::UpdateDeck), |col| {
+        self.transact(Op::UpdateDeck, |col| {
             let existing_deck = col.storage.get_deck(deck.id)?.ok_or(AnkiError::NotFound)?;
             col.update_deck_inner(deck, existing_deck, col.usn()?)
         })
     }
 
-    pub fn rename_deck(&mut self, did: DeckID, new_human_name: &str) -> Result<()> {
+    pub fn rename_deck(&mut self, did: DeckID, new_human_name: &str) -> Result<OpOutput<()>> {
-        self.transact(Some(UndoableOpKind::RenameDeck), |col| {
+        self.transact(Op::RenameDeck, |col| {
             let existing_deck = col.storage.get_deck(did)?.ok_or(AnkiError::NotFound)?;
             let mut deck = existing_deck.clone();
             deck.name = human_deck_name_to_native(new_human_name);
@@ -466,9 +464,9 @@ impl Collection {
         self.storage.get_deck_id(&machine_name)
     }
 
-    pub fn remove_decks_and_child_decks(&mut self, dids: &[DeckID]) -> Result<usize> {
+    pub fn remove_decks_and_child_decks(&mut self, dids: &[DeckID]) -> Result<OpOutput<usize>> {
-        let mut card_count = 0;
+        self.transact(Op::RemoveDeck, |col| {
-        self.transact(Some(UndoableOpKind::RemoveDeck), |col| {
+            let mut card_count = 0;
             let usn = col.usn()?;
             for did in dids {
                 if let Some(deck) = col.storage.get_deck(*did)? {
@@ -483,9 +481,8 @@ impl Collection {
                     }
                 }
             }
-            Ok(())
+            Ok(card_count)
-        })?;
+        })
-        Ok(card_count)
     }
 
     pub(crate) fn remove_single_deck(&mut self, deck: &Deck, usn: Usn) -> Result<usize> {
@@ -625,9 +622,9 @@ impl Collection {
         &mut self,
         source_decks: &[DeckID],
         target: Option<DeckID>,
-    ) -> Result<()> {
+    ) -> Result<OpOutput<()>> {
         let usn = self.usn()?;
-        self.transact(Some(UndoableOpKind::RenameDeck), |col| {
+        self.transact(Op::RenameDeck, |col| {
             let target_deck;
             let mut target_name = None;
             if let Some(target) = target {

rslib/src/filtered.rs

@@ -169,7 +169,7 @@ pub(crate) struct DeckFilterContext<'a> {
 
 impl Collection {
     pub fn empty_filtered_deck(&mut self, did: DeckID) -> Result<()> {
-        self.transact(None, |col| col.return_all_cards_in_filtered_deck(did))
+        self.transact_no_undo(|col| col.return_all_cards_in_filtered_deck(did))
     }
     pub(super) fn return_all_cards_in_filtered_deck(&mut self, did: DeckID) -> Result<()> {
         let cids = self.storage.all_cards_in_single_deck(did)?;
@@ -206,7 +206,7 @@ impl Collection {
             today: self.timing_today()?.days_elapsed,
         };
 
-        self.transact(None, |col| {
+        self.transact_no_undo(|col| {
             col.return_all_cards_in_filtered_deck(did)?;
             col.build_filtered_deck(ctx)
         })

rslib/src/findreplace.rs

@@ -45,8 +45,8 @@ impl Collection {
         search_re: &str,
         repl: &str,
         field_name: Option<String>,
-    ) -> Result<usize> {
+    ) -> Result<OpOutput<usize>> {
-        self.transact(None, |col| {
+        self.transact(Op::FindAndReplace, |col| {
             let norm = col.get_bool(BoolKey::NormalizeNoteText);
             let search = if norm {
                 normalize_to_nfc(search_re)
@@ -119,8 +119,8 @@ mod test {
         col.add_note(&mut note2, DeckID(1))?;
 
         let nids = col.search_notes("")?;
-        let cnt = col.find_and_replace(nids.clone(), "(?i)AAA", "BBB", None)?;
+        let out = col.find_and_replace(nids.clone(), "(?i)AAA", "BBB", None)?;
-        assert_eq!(cnt, 2);
+        assert_eq!(out.output, 2);
 
         let note = col.storage.get_note(note.id)?.unwrap();
         // but the update should be limited to the specified field when it was available
@@ -138,10 +138,10 @@ mod test {
                 "Text".into()
             ]
         );
-        let cnt = col.find_and_replace(nids, "BBB", "ccc", Some("Front".into()))?;
+        let out = col.find_and_replace(nids, "BBB", "ccc", Some("Front".into()))?;
         // still 2, as the caller is expected to provide only note ids that have
         // that field, and if we can't find the field we fall back on all fields
-        assert_eq!(cnt, 2);
+        assert_eq!(out.output, 2);
 
         let note = col.storage.get_note(note.id)?.unwrap();
         // but the update should be limited to the specified field when it was available

rslib/src/lib.rs

@@ -24,6 +24,7 @@ mod markdown;
 pub mod media;
 pub mod notes;
 pub mod notetype;
+pub mod ops;
 mod preferences;
 pub mod prelude;
 pub mod revlog;

rslib/src/media/check.rs

@@ -572,7 +572,7 @@ pub(crate) mod test {
 
         let progress = |_n| true;
 
-        let (output, report) = col.transact(None, |ctx| {
+        let (output, report) = col.transact_no_undo(|ctx| {
             let mut checker = MediaChecker::new(ctx, &mgr, progress);
             let output = checker.check()?;
             let summary = checker.summarize_output(&mut output.clone());
@@ -642,7 +642,7 @@ Unused: unused.jpg
 
         let progress = |_n| true;
 
-        col.transact(None, |ctx| {
+        col.transact_no_undo(|ctx| {
             let mut checker = MediaChecker::new(ctx, &mgr, progress);
             checker.restore_trash()
         })?;
@@ -656,7 +656,7 @@ Unused: unused.jpg
 
         // if we repeat the process, restoring should do the same thing if the contents are equal
         fs::write(trash_folder.join("test.jpg"), "test")?;
-        col.transact(None, |ctx| {
+        col.transact_no_undo(|ctx| {
             let mut checker = MediaChecker::new(ctx, &mgr, progress);
             checker.restore_trash()
         })?;
@@ -668,7 +668,7 @@ Unused: unused.jpg
 
         // but rename if required
         fs::write(trash_folder.join("test.jpg"), "test2")?;
-        col.transact(None, |ctx| {
+        col.transact_no_undo(|ctx| {
             let mut checker = MediaChecker::new(ctx, &mgr, progress);
             checker.restore_trash()
         })?;
@@ -692,7 +692,7 @@ Unused: unused.jpg
 
         let progress = |_n| true;
 
-        let mut output = col.transact(None, |ctx| {
+        let mut output = col.transact_no_undo(|ctx| {
             let mut checker = MediaChecker::new(ctx, &mgr, progress);
             checker.check()
         })?;

rslib/src/notes/mod.rs

@@ -3,7 +3,6 @@
 
 pub(crate) mod undo;
 
-use crate::backend_proto::note_is_duplicate_or_empty_out::State as DuplicateState;
 use crate::{
     backend_proto as pb,
     decks::DeckID,
@@ -16,9 +15,11 @@ use crate::{
     timestamp::TimestampSecs,
     types::Usn,
 };
+use crate::{
+    backend_proto::note_is_duplicate_or_empty_out::State as DuplicateState, ops::StateChanges,
+};
 use itertools::Itertools;
 use num_integer::Integer;
-use regex::{Regex, Replacer};
 use std::{
     borrow::Cow,
     collections::{HashMap, HashSet},
@@ -47,6 +48,23 @@ pub struct Note {
     pub(crate) checksum: Option<u32>,
 }
 
+/// Information required for updating tags while leaving note content alone.
+/// Tags are stored in their DB form, separated by spaces.
+#[derive(Debug, PartialEq, Clone)]
+pub(crate) struct NoteTags {
+    pub id: NoteID,
+    pub mtime: TimestampSecs,
+    pub usn: Usn,
+    pub tags: String,
+}
+
+impl NoteTags {
+    pub(crate) fn set_modified(&mut self, usn: Usn) {
+        self.mtime = TimestampSecs::now();
+        self.usn = usn;
+    }
+}
+
 impl Note {
     pub(crate) fn new(notetype: &NoteType) -> Self {
         Note {
@@ -191,38 +209,6 @@ impl Note {
             .collect()
     }
 
-    pub(crate) fn remove_tags(&mut self, re: &Regex) -> bool {
-        let old_len = self.tags.len();
-        self.tags.retain(|tag| !re.is_match(tag));
-        old_len > self.tags.len()
-    }
-
-    pub(crate) fn replace_tags<T: Replacer>(&mut self, re: &Regex, mut repl: T) -> bool {
-        let mut changed = false;
-        for tag in &mut self.tags {
-            if let Cow::Owned(rep) = re.replace_all(tag, |caps: &regex::Captures| {
-                if let Some(expanded) = repl.by_ref().no_expansion() {
-                    if expanded.trim().is_empty() {
-                        "".to_string()
-                    } else {
-                        // include "::" if it was matched
-                        format!(
-                            "{}{}",
-                            expanded,
-                            caps.get(caps.len() - 1).map_or("", |m| m.as_str())
-                        )
-                    }
-                } else {
-                    tag.to_string()
-                }
-            }) {
-                *tag = rep;
-                changed = true;
-            }
-        }
-        changed
-    }
-
     /// Pad or merge fields to match note type.
     pub(crate) fn fix_field_count(&mut self, nt: &NoteType) {
         while self.fields.len() < nt.fields.len() {
@@ -305,8 +291,8 @@ impl Collection {
         Ok(())
     }
 
-    pub fn add_note(&mut self, note: &mut Note, did: DeckID) -> Result<()> {
+    pub fn add_note(&mut self, note: &mut Note, did: DeckID) -> Result<OpOutput<()>> {
-        self.transact(Some(UndoableOpKind::AddNote), |col| {
+        self.transact(Op::AddNote, |col| {
             let nt = col
                 .get_notetype(note.notetype_id)?
                 .ok_or_else(|| AnkiError::invalid_input("missing note type"))?;
@@ -334,29 +320,49 @@ impl Collection {
     }
 
     #[cfg(test)]
-    pub(crate) fn update_note(&mut self, note: &mut Note) -> Result<()> {
+    pub(crate) fn update_note(&mut self, note: &mut Note) -> Result<OpOutput<()>> {
-        self.update_note_with_op(note, Some(UndoableOpKind::UpdateNote))
+        self.update_note_maybe_undoable(note, true)
     }
 
-    pub(crate) fn update_note_with_op(
+    pub(crate) fn update_note_maybe_undoable(
         &mut self,
         note: &mut Note,
-        op: Option<UndoableOpKind>,
+        undoable: bool,
-    ) -> Result<()> {
+    ) -> Result<OpOutput<()>> {
+        if undoable {
+            self.transact(Op::UpdateNote, |col| col.update_note_inner(note))
+        } else {
+            self.transact_no_undo(|col| {
+                col.update_note_inner(note)?;
+                Ok(OpOutput {
+                    output: (),
+                    changes: OpChanges {
+                        op: Op::UpdateNote,
+                        changes: StateChanges {
+                            note: true,
+                            tag: true,
+                            card: true,
+                            ..Default::default()
+                        },
+                    },
+                })
+            })
+        }
+    }
+
+    pub(crate) fn update_note_inner(&mut self, note: &mut Note) -> Result<()> {
         let mut existing_note = self.storage.get_note(note.id)?.ok_or(AnkiError::NotFound)?;
         if !note_differs_from_db(&mut existing_note, note) {
             // nothing to do
             return Ok(());
         }
-
+        let nt = self
-        self.transact(op, |col| {
+            .get_notetype(note.notetype_id)?
-            let nt = col
+            .ok_or_else(|| AnkiError::invalid_input("missing note type"))?;
-                .get_notetype(note.notetype_id)?
+        let ctx = CardGenContext::new(&nt, self.usn()?);
-                .ok_or_else(|| AnkiError::invalid_input("missing note type"))?;
+        let norm = self.get_bool(BoolKey::NormalizeNoteText);
-            let ctx = CardGenContext::new(&nt, col.usn()?);
+        self.update_note_inner_generating_cards(&ctx, note, &existing_note, true, norm)?;
-            let norm = col.get_bool(BoolKey::NormalizeNoteText);
+        Ok(())
-            col.update_note_inner_generating_cards(&ctx, note, &existing_note, true, norm)
-        })
     }
 
     pub(crate) fn update_note_inner_generating_cards(
@@ -392,13 +398,13 @@ impl Collection {
         if mark_note_modified {
             note.set_modified(usn);
         }
-        self.update_note_undoable(note, original, true)
+        self.update_note_undoable(note, original)
     }
 
     /// Remove provided notes, and any cards that use them.
-    pub(crate) fn remove_notes(&mut self, nids: &[NoteID]) -> Result<()> {
+    pub(crate) fn remove_notes(&mut self, nids: &[NoteID]) -> Result<OpOutput<()>> {
         let usn = self.usn()?;
-        self.transact(Some(UndoableOpKind::RemoveNote), |col| {
+        self.transact(Op::RemoveNote, |col| {
             for nid in nids {
                 let nid = *nid;
                 if let Some(_existing_note) = col.storage.get_note(nid)? {
@@ -408,27 +414,28 @@ impl Collection {
                     col.remove_note_only_undoable(nid, usn)?;
                 }
             }
-
             Ok(())
         })
     }
 
     /// Update cards and field cache after notes modified externally.
     /// If gencards is false, skip card generation.
-    pub(crate) fn after_note_updates(
+    pub fn after_note_updates(
         &mut self,
         nids: &[NoteID],
         generate_cards: bool,
         mark_notes_modified: bool,
-    ) -> Result<()> {
+    ) -> Result<OpOutput<()>> {
-        self.transform_notes(nids, |_note, _nt| {
+        self.transact(Op::UpdateNote, |col| {
-            Ok(TransformNoteOutput {
+            col.transform_notes(nids, |_note, _nt| {
-                changed: true,
+                Ok(TransformNoteOutput {
-                generate_cards,
+                    changed: true,
-                mark_modified: mark_notes_modified,
+                    generate_cards,
+                    mark_modified: mark_notes_modified,
+                })
             })
+            .map(|_| ())
         })
-        .map(|_| ())
     }
 
     pub(crate) fn transform_notes<F>(

rslib/src/notes/undo.rs

@@ -3,6 +3,8 @@
 
 use crate::{prelude::*, undo::UndoableChange};
 
+use super::NoteTags;
+
 #[derive(Debug)]
 pub(crate) enum UndoableNoteChange {
     Added(Box<Note>),
@@ -10,6 +12,7 @@ pub(crate) enum UndoableNoteChange {
     Removed(Box<Note>),
     GraveAdded(Box<(NoteID, Usn)>),
     GraveRemoved(Box<(NoteID, Usn)>),
+    TagsUpdated(Box<NoteTags>),
 }
 
 impl Collection {
@@ -21,27 +24,25 @@ impl Collection {
                     .storage
                     .get_note(note.id)?
                     .ok_or_else(|| AnkiError::invalid_input("note disappeared"))?;
-                self.update_note_undoable(&note, &current, false)
+                self.update_note_undoable(&note, &current)
             }
             UndoableNoteChange::Removed(note) => self.restore_deleted_note(*note),
             UndoableNoteChange::GraveAdded(e) => self.remove_note_grave(e.0, e.1),
             UndoableNoteChange::GraveRemoved(e) => self.add_note_grave(e.0, e.1),
+            UndoableNoteChange::TagsUpdated(note_tags) => {
+                let current = self
+                    .storage
+                    .get_note_tags_by_id(note_tags.id)?
+                    .ok_or_else(|| AnkiError::invalid_input("note disappeared"))?;
+                self.update_note_tags_undoable(&note_tags, current)
+            }
         }
     }
 
     /// Saves in the undo queue, and commits to DB.
     /// No validation, card generation or normalization is done.
-    /// If `coalesce_updates` is true, successive updates within a 1 minute
+    pub(super) fn update_note_undoable(&mut self, note: &Note, original: &Note) -> Result<()> {
-    /// period will not result in further undo entries.
+        self.save_undo(UndoableNoteChange::Updated(Box::new(original.clone())));
-    pub(super) fn update_note_undoable(
-        &mut self,
-        note: &Note,
-        original: &Note,
-        coalesce_updates: bool,
-    ) -> Result<()> {
-        if !coalesce_updates || !self.note_was_just_updated(note) {
-            self.save_undo(UndoableNoteChange::Updated(Box::new(original.clone())));
-        }
         self.storage.update_note(note)?;
 
         Ok(())
@@ -57,6 +58,31 @@ impl Collection {
         Ok(())
     }
 
+    /// If note is edited multiple times in quick succession, avoid creating extra undo entries.
+    pub(crate) fn maybe_coalesce_note_undo_entry(&mut self, changes: OpChanges) {
+        if changes.op != Op::UpdateNote {
+            return;
+        }
+
+        if let Some(previous_op) = self.previous_undo_op() {
+            if previous_op.kind != Op::UpdateNote {
+                return;
+            }
+
+            if let (
+                Some(UndoableChange::Note(UndoableNoteChange::Updated(previous))),
+                Some(UndoableChange::Note(UndoableNoteChange::Updated(current))),
+            ) = (
+                previous_op.changes.last(),
+                self.current_undo_op().and_then(|op| op.changes.last()),
+            ) {
+                if previous.id == current.id && previous_op.timestamp.elapsed_secs() < 60 {
+                    self.pop_last_change();
+                }
+            }
+        }
+    }
+
     /// Add a note, not adding any cards.
     pub(super) fn add_note_only_undoable(&mut self, note: &mut Note) -> Result<(), AnkiError> {
         self.storage.add_note(note)?;
@@ -65,6 +91,15 @@ impl Collection {
         Ok(())
     }
 
+    pub(crate) fn update_note_tags_undoable(
+        &mut self,
+        tags: &NoteTags,
+        original: NoteTags,
+    ) -> Result<()> {
+        self.save_undo(UndoableNoteChange::TagsUpdated(Box::new(original)));
+        self.storage.update_note_tags(tags)
+    }
+
     fn remove_note_without_grave(&mut self, note: Note) -> Result<()> {
         self.storage.remove_note(note.id)?;
         self.save_undo(UndoableNoteChange::Removed(Box::new(note)));
@@ -86,22 +121,4 @@ impl Collection {
         self.save_undo(UndoableNoteChange::GraveRemoved(Box::new((nid, usn))));
         self.storage.remove_note_grave(nid)
     }
-
-    /// True only if the last operation was UpdateNote, and the same note was just updated less than
-    /// a minute ago.
-    fn note_was_just_updated(&self, before_change: &Note) -> bool {
-        self.previous_undo_op()
-            .map(|op| {
-                if let Some(UndoableChange::Note(UndoableNoteChange::Updated(note))) =
-                    op.changes.last()
-                {
-                    note.id == before_change.id
-                        && op.kind == UndoableOpKind::UpdateNote
-                        && op.timestamp.elapsed_secs() < 60
-                } else {
-                    false
-                }
-            })
-            .unwrap_or(false)
-    }
 }

rslib/src/notetype/mod.rs

@@ -376,7 +376,7 @@ impl From<NoteType> for NoteTypeProto {
 impl Collection {
     /// Add a new notetype, and allocate it an ID.
     pub fn add_notetype(&mut self, nt: &mut NoteType) -> Result<()> {
-        self.transact(None, |col| {
+        self.transact_no_undo(|col| {
             let usn = col.usn()?;
             nt.set_modified(usn);
             col.add_notetype_inner(nt, usn)
@@ -415,7 +415,7 @@ impl Collection {
         let existing = self.get_notetype(nt.id)?;
         let norm = self.get_bool(BoolKey::NormalizeNoteText);
         nt.prepare_for_update(existing.as_ref().map(AsRef::as_ref))?;
-        self.transact(None, |col| {
+        self.transact_no_undo(|col| {
             if let Some(existing_notetype) = existing {
                 if existing_notetype.mtime_secs > nt.mtime_secs {
                     return Err(AnkiError::invalid_input("attempt to save stale notetype"));
@@ -484,7 +484,7 @@ impl Collection {
     pub fn remove_notetype(&mut self, ntid: NoteTypeID) -> Result<()> {
         // fixme: currently the storage layer is taking care of removing the notes and cards,
         // but we need to do it in this layer in the future for undo handling
-        self.transact(None, |col| {
+        self.transact_no_undo(|col| {
             col.storage.set_schema_modified()?;
             col.state.notetype_cache.remove(&ntid);
             col.clear_aux_config_for_notetype(ntid)?;

rslib/src/ops.rs

@@ -0,0 +1,86 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+use crate::prelude::*;
+
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub enum Op {
+    AddDeck,
+    AddNote,
+    AnswerCard,
+    Bury,
+    ClearUnusedTags,
+    FindAndReplace,
+    RemoveDeck,
+    RemoveNote,
+    RemoveTag,
+    RenameDeck,
+    RenameTag,
+    ReparentTag,
+    ScheduleAsNew,
+    SetDeck,
+    SetDueDate,
+    SetFlag,
+    SortCards,
+    Suspend,
+    UnburyUnsuspend,
+    UpdateCard,
+    UpdateDeck,
+    UpdateNote,
+    UpdatePreferences,
+    UpdateTag,
+}
+
+impl Op {
+    pub fn describe(self, i18n: &I18n) -> String {
+        let key = match self {
+            Op::AddDeck => TR::UndoAddDeck,
+            Op::AddNote => TR::UndoAddNote,
+            Op::AnswerCard => TR::UndoAnswerCard,
+            Op::Bury => TR::StudyingBury,
+            Op::RemoveDeck => TR::DecksDeleteDeck,
+            Op::RemoveNote => TR::StudyingDeleteNote,
+            Op::RenameDeck => TR::ActionsRenameDeck,
+            Op::ScheduleAsNew => TR::UndoForgetCard,
+            Op::SetDueDate => TR::ActionsSetDueDate,
+            Op::Suspend => TR::StudyingSuspend,
+            Op::UnburyUnsuspend => TR::UndoUnburyUnsuspend,
+            Op::UpdateCard => TR::UndoUpdateCard,
+            Op::UpdateDeck => TR::UndoUpdateDeck,
+            Op::UpdateNote => TR::UndoUpdateNote,
+            Op::UpdatePreferences => TR::PreferencesPreferences,
+            Op::UpdateTag => TR::UndoUpdateTag,
+            Op::SetDeck => TR::BrowsingChangeDeck,
+            Op::SetFlag => TR::UndoSetFlag,
+            Op::FindAndReplace => TR::BrowsingFindAndReplace,
+            Op::ClearUnusedTags => TR::BrowsingClearUnusedTags,
+            Op::SortCards => TR::BrowsingReschedule,
+            Op::RenameTag => TR::ActionsRenameTag,
+            Op::RemoveTag => TR::ActionsRemoveTag,
+            Op::ReparentTag => TR::UndoReparent,
+        };
+
+        i18n.tr(key).to_string()
+    }
+}
+
+#[derive(Debug, Default, Clone, Copy)]
+pub struct StateChanges {
+    pub card: bool,
+    pub note: bool,
+    pub deck: bool,
+    pub tag: bool,
+    pub notetype: bool,
+    pub preference: bool,
+}
+
+#[derive(Debug, Clone, Copy)]
+pub struct OpChanges {
+    pub op: Op,
+    pub changes: StateChanges,
+}
+
+pub struct OpOutput<T> {
+    pub output: T,
+    pub changes: OpChanges,
+}

rslib/src/preferences.rs

@@ -10,6 +10,7 @@ use crate::{
     collection::Collection,
     config::BoolKey,
     err::Result,
+    prelude::*,
     scheduler::timing::local_minutes_west_for_stamp,
 };
 
@@ -22,17 +23,13 @@ impl Collection {
         })
     }
 
-    pub fn set_preferences(&mut self, prefs: Preferences) -> Result<()> {
+    pub fn set_preferences(&mut self, prefs: Preferences) -> Result<OpOutput<()>> {
-        self.transact(
+        self.transact(Op::UpdatePreferences, |col| {
-            Some(crate::undo::UndoableOpKind::UpdatePreferences),
+            col.set_preferences_inner(prefs)
-            |col| col.set_preferences_inner(prefs),
+        })
-        )
     }
 
-    fn set_preferences_inner(
+    fn set_preferences_inner(&mut self, prefs: Preferences) -> Result<()> {
-        &mut self,
-        prefs: Preferences,
-    ) -> Result<(), crate::prelude::AnkiError> {
         if let Some(sched) = prefs.scheduling {
             self.set_scheduling_preferences(sched)?;
         }

rslib/src/prelude.rs

@@ -11,9 +11,9 @@ pub use crate::{
     i18n::{tr_args, tr_strs, I18n, TR},
     notes::{Note, NoteID},
     notetype::{NoteType, NoteTypeID},
+    ops::{Op, OpChanges, OpOutput},
     revlog::RevlogID,
     timestamp::{TimestampMillis, TimestampSecs},
     types::Usn,
-    undo::UndoableOpKind,
 };
 pub use slog::{debug, Logger};

rslib/src/scheduler/answering/mod.rs

@@ -240,10 +240,8 @@ impl Collection {
     }
 
     /// Answer card, writing its new state to the database.
-    pub fn answer_card(&mut self, answer: &CardAnswer) -> Result<()> {
+    pub fn answer_card(&mut self, answer: &CardAnswer) -> Result<OpOutput<()>> {
-        self.transact(Some(UndoableOpKind::AnswerCard), |col| {
+        self.transact(Op::AnswerCard, |col| col.answer_card_inner(answer))
-            col.answer_card_inner(answer)
-        })
     }
 
     fn answer_card_inner(&mut self, answer: &CardAnswer) -> Result<()> {
@@ -274,9 +272,7 @@ impl Collection {
             self.add_leech_tag(card.note_id)?;
         }
 
-        self.update_queues_after_answering_card(&card, timing)?;
+        self.update_queues_after_answering_card(&card, timing)
-
-        Ok(())
     }
 
     fn maybe_bury_siblings(&mut self, card: &Card, config: &DeckConf) -> Result<()> {

rslib/src/scheduler/bury_and_suspend.rs

@@ -68,8 +68,8 @@ impl Collection {
         self.storage.clear_searched_cards_table()
     }
 
-    pub fn unbury_or_unsuspend_cards(&mut self, cids: &[CardID]) -> Result<()> {
+    pub fn unbury_or_unsuspend_cards(&mut self, cids: &[CardID]) -> Result<OpOutput<()>> {
-        self.transact(Some(UndoableOpKind::UnburyUnsuspend), |col| {
+        self.transact(Op::UnburyUnsuspend, |col| {
             col.storage.set_search_table_to_card_ids(cids, false)?;
             col.unsuspend_or_unbury_searched_cards()
         })
@@ -81,7 +81,7 @@ impl Collection {
             UnburyDeckMode::UserOnly => "is:buried-manually",
             UnburyDeckMode::SchedOnly => "is:buried-sibling",
         };
-        self.transact(None, |col| {
+        self.transact_no_undo(|col| {
             col.search_cards_into_table(&format!("deck:current {}", search), SortMode::NoOrder)?;
             col.unsuspend_or_unbury_searched_cards()
         })
@@ -124,12 +124,12 @@ impl Collection {
         &mut self,
         cids: &[CardID],
         mode: BuryOrSuspendMode,
-    ) -> Result<()> {
+    ) -> Result<OpOutput<()>> {
         let op = match mode {
-            BuryOrSuspendMode::Suspend => UndoableOpKind::Suspend,
+            BuryOrSuspendMode::Suspend => Op::Suspend,
-            BuryOrSuspendMode::BurySched | BuryOrSuspendMode::BuryUser => UndoableOpKind::Bury,
+            BuryOrSuspendMode::BurySched | BuryOrSuspendMode::BuryUser => Op::Bury,
         };
-        self.transact(Some(op), |col| {
+        self.transact(op, |col| {
             col.storage.set_search_table_to_card_ids(cids, false)?;
             col.bury_or_suspend_searched_cards(mode)
         })

rslib/src/scheduler/new.rs

@@ -7,9 +7,9 @@ use crate::{
     decks::DeckID,
     err::Result,
     notes::NoteID,
+    prelude::*,
     search::SortMode,
     types::Usn,
-    undo::UndoableOpKind,
 };
 use rand::seq::SliceRandom;
 use std::collections::{HashMap, HashSet};
@@ -24,12 +24,14 @@ impl Card {
         self.ease_factor = 0;
     }
 
-    /// If the card is new, change its position.
+    /// If the card is new, change its position, and return true.
-    fn set_new_position(&mut self, position: u32) {
+    fn set_new_position(&mut self, position: u32) -> bool {
         if self.queue != CardQueue::New || self.ctype != CardType::New {
-            return;
+            false
+        } else {
+            self.due = position as i32;
+            true
         }
-        self.due = position as i32;
     }
 }
 pub(crate) struct NewCardSorter {
@@ -103,10 +105,10 @@ fn nids_in_preserved_order(cards: &[Card]) -> Vec<NoteID> {
 }
 
 impl Collection {
-    pub fn reschedule_cards_as_new(&mut self, cids: &[CardID], log: bool) -> Result<()> {
+    pub fn reschedule_cards_as_new(&mut self, cids: &[CardID], log: bool) -> Result<OpOutput<()>> {
         let usn = self.usn()?;
         let mut position = self.get_next_card_position();
-        self.transact(Some(UndoableOpKind::ScheduleAsNew), |col| {
+        self.transact(Op::ScheduleAsNew, |col| {
             col.storage.set_search_table_to_card_ids(cids, true)?;
             let cards = col.storage.all_searched_cards_in_search_order()?;
             for mut card in cards {
@@ -119,8 +121,7 @@ impl Collection {
                 position += 1;
             }
             col.set_next_card_position(position)?;
-            col.storage.clear_searched_cards_table()?;
+            col.storage.clear_searched_cards_table()
-            Ok(())
         })
     }
 
@@ -131,9 +132,9 @@ impl Collection {
         step: u32,
         order: NewCardSortOrder,
         shift: bool,
-    ) -> Result<()> {
+    ) -> Result<OpOutput<usize>> {
         let usn = self.usn()?;
-        self.transact(None, |col| {
+        self.transact(Op::SortCards, |col| {
             col.sort_cards_inner(cids, starting_from, step, order, shift, usn)
         })
     }
@@ -146,24 +147,28 @@ impl Collection {
         order: NewCardSortOrder,
         shift: bool,
         usn: Usn,
-    ) -> Result<()> {
+    ) -> Result<usize> {
         if shift {
             self.shift_existing_cards(starting_from, step * cids.len() as u32, usn)?;
         }
         self.storage.set_search_table_to_card_ids(cids, true)?;
         let cards = self.storage.all_searched_cards_in_search_order()?;
         let sorter = NewCardSorter::new(&cards, starting_from, step, order);
+        let mut count = 0;
         for mut card in cards {
             let original = card.clone();
-            card.set_new_position(sorter.position(&card));
+            if card.set_new_position(sorter.position(&card)) {
-            self.update_card_inner(&mut card, original, usn)?;
+                count += 1;
+                self.update_card_inner(&mut card, original, usn)?;
+            }
         }
-        self.storage.clear_searched_cards_table()
+        self.storage.clear_searched_cards_table()?;
+        Ok(count)
     }
 
     /// This creates a transaction - we probably want to split it out
     /// in the future if calling it as part of a deck options update.
-    pub fn sort_deck(&mut self, deck: DeckID, random: bool) -> Result<()> {
+    pub fn sort_deck(&mut self, deck: DeckID, random: bool) -> Result<OpOutput<usize>> {
         let cids = self.search_cards(&format!("did:{} is:new", deck), SortMode::NoOrder)?;
         let order = if random {
             NewCardSortOrder::Random

rslib/src/scheduler/queue/mod.rs

@@ -139,6 +139,13 @@ impl Collection {
         self.state.card_queues = None;
     }
 
+    pub(crate) fn maybe_clear_study_queues_after_op(&mut self, op: OpChanges) {
+        if op.op != Op::AnswerCard && (op.changes.card || op.changes.deck || op.changes.preference)
+        {
+            self.state.card_queues = None;
+        }
+    }
+
     pub(crate) fn update_queues_after_answering_card(
         &mut self,
         card: &Card,

rslib/src/scheduler/reviews.rs

@@ -7,8 +7,7 @@ use crate::{
     config::StringKey,
     deckconf::INITIAL_EASE_FACTOR_THOUSANDS,
     err::Result,
-    prelude::AnkiError,
+    prelude::*,
-    undo::UndoableOpKind,
 };
 use lazy_static::lazy_static;
 use rand::distributions::{Distribution, Uniform};
@@ -94,13 +93,13 @@ impl Collection {
         cids: &[CardID],
         days: &str,
         context: Option<StringKey>,
-    ) -> Result<()> {
+    ) -> Result<OpOutput<()>> {
         let spec = parse_due_date_str(days)?;
         let usn = self.usn()?;
         let today = self.timing_today()?.days_elapsed;
         let mut rng = rand::thread_rng();
         let distribution = Uniform::from(spec.min..=spec.max);
-        self.transact(Some(UndoableOpKind::SetDueDate), |col| {
+        self.transact(Op::SetDueDate, |col| {
             col.storage.set_search_table_to_card_ids(cids, false)?;
             for mut card in col.storage.all_searched_cards()? {
                 let original = card.clone();

rslib/src/storage/card/mod.rs

@@ -445,7 +445,7 @@ impl super::SqliteStorage {
 
     /// Injects the provided card IDs into the search_cids table, for
     /// when ids have arrived outside of a search.
-    /// Clear with clear_searched_cards().
+    /// Clear with clear_searched_cards_table().
     pub(crate) fn set_search_table_to_card_ids(
         &mut self,
         cards: &[CardID],

rslib/src/storage/note/get_tags.sql

@@ -0,0 +1,5 @@
+SELECT id,
+  mod,
+  usn,
+  tags
+FROM notes

rslib/src/storage/note/mod.rs

@@ -5,7 +5,7 @@ use std::collections::HashSet;
 
 use crate::{
     err::Result,
-    notes::{Note, NoteID},
+    notes::{Note, NoteID, NoteTags},
     notetype::NoteTypeID,
     tags::{join_tags, split_tags},
     timestamp::TimestampMillis,
@@ -20,22 +20,6 @@ pub(crate) fn join_fields(fields: &[String]) -> String {
     fields.join("\x1f")
 }
 
-fn row_to_note(row: &Row) -> Result<Note> {
-    Ok(Note::new_from_storage(
-        row.get(0)?,
-        row.get(1)?,
-        row.get(2)?,
-        row.get(3)?,
-        row.get(4)?,
-        split_tags(row.get_raw(5).as_str()?)
-            .map(Into::into)
-            .collect(),
-        split_fields(row.get_raw(6).as_str()?),
-        Some(row.get(7)?),
-        Some(row.get(8).unwrap_or_default()),
-    ))
-}
-
 impl super::SqliteStorage {
     pub fn get_note(&self, nid: NoteID) -> Result<Option<Note>> {
         self.db
@@ -175,18 +159,103 @@ impl super::SqliteStorage {
         Ok(seen)
     }
 
-    pub(crate) fn for_each_note_tags<F>(&self, mut func: F) -> Result<()>
+    pub(crate) fn get_note_tags_by_id(&mut self, note_id: NoteID) -> Result<Option<NoteTags>> {
+        self.db
+            .prepare_cached(&format!("{} where id = ?", include_str!("get_tags.sql")))?
+            .query_and_then(&[note_id], row_to_note_tags)?
+            .next()
+            .transpose()
+    }
+
+    pub(crate) fn get_note_tags_by_id_list(
+        &mut self,
+        note_ids: &[NoteID],
+    ) -> Result<Vec<NoteTags>> {
+        self.set_search_table_to_note_ids(note_ids)?;
+        let out = self
+            .db
+            .prepare_cached(&format!(
+                "{} where id in (select nid from search_nids)",
+                include_str!("get_tags.sql")
+            ))?
+            .query_and_then(NO_PARAMS, row_to_note_tags)?
+            .collect::<Result<Vec<_>>>()?;
+        self.clear_searched_notes_table()?;
+        Ok(out)
+    }
+
+    pub(crate) fn get_note_tags_by_predicate<F>(&mut self, want: F) -> Result<Vec<NoteTags>>
     where
-        F: FnMut(NoteID, String) -> Result<()>,
+        F: Fn(&str) -> bool,
     {
-        let mut stmt = self.db.prepare_cached("select id, tags from notes")?;
+        let mut query_stmt = self.db.prepare_cached(include_str!("get_tags.sql"))?;
-        let mut rows = stmt.query(NO_PARAMS)?;
+        let mut rows = query_stmt.query(NO_PARAMS)?;
+        let mut output = vec![];
         while let Some(row) = rows.next()? {
-            let id: NoteID = row.get(0)?;
+            let tags = row.get_raw(3).as_str()?;
-            let tags: String = row.get(1)?;
+            if want(tags) {
-            func(id, tags)?
+                output.push(row_to_note_tags(row)?)
+            }
+        }
+        Ok(output)
+    }
+
+    pub(crate) fn update_note_tags(&mut self, note: &NoteTags) -> Result<()> {
+        self.db
+            .prepare_cached(include_str!("update_tags.sql"))?
+            .execute(params![note.mtime, note.usn, note.tags, note.id])?;
+        Ok(())
+    }
+
+    fn setup_searched_notes_table(&self) -> Result<()> {
+        self.db
+            .execute_batch(include_str!("search_nids_setup.sql"))?;
+        Ok(())
+    }
+
+    fn clear_searched_notes_table(&self) -> Result<()> {
+        self.db
+            .execute("drop table if exists search_nids", NO_PARAMS)?;
+        Ok(())
+    }
+
+    /// Injects the provided card IDs into the search_nids table, for
+    /// when ids have arrived outside of a search.
+    /// Clear with clear_searched_notes_table().
+    fn set_search_table_to_note_ids(&mut self, notes: &[NoteID]) -> Result<()> {
+        self.setup_searched_notes_table()?;
+        let mut stmt = self
+            .db
+            .prepare_cached("insert into search_nids values (?)")?;
+        for nid in notes {
+            stmt.execute(&[nid])?;
         }
 
         Ok(())
     }
 }
+
+fn row_to_note(row: &Row) -> Result<Note> {
+    Ok(Note::new_from_storage(
+        row.get(0)?,
+        row.get(1)?,
+        row.get(2)?,
+        row.get(3)?,
+        row.get(4)?,
+        split_tags(row.get_raw(5).as_str()?)
+            .map(Into::into)
+            .collect(),
+        split_fields(row.get_raw(6).as_str()?),
+        Some(row.get(7)?),
+        Some(row.get(8).unwrap_or_default()),
+    ))
+}
+
+fn row_to_note_tags(row: &Row) -> Result<NoteTags> {
+    Ok(NoteTags {
+        id: row.get(0)?,
+        mtime: row.get(1)?,
+        usn: row.get(2)?,
+        tags: row.get(3)?,
+    })
+}

rslib/src/storage/note/search_nids_setup.sql

@@ -0,0 +1,2 @@
+DROP TABLE IF EXISTS search_nids;
+CREATE TEMPORARY TABLE search_nids (nid integer PRIMARY KEY NOT NULL);

rslib/src/storage/note/update_tags.sql

@@ -0,0 +1,5 @@
+UPDATE notes
+SET mod = ?,
+  usn = ?,
+  tags = ?
+WHERE id = ?

rslib/src/storage/sqlite.rs

@@ -257,7 +257,7 @@ impl SqliteStorage {
 
     //////////////////////////////////////////
 
-    pub(crate) fn mark_modified(&self) -> Result<()> {
+    pub(crate) fn set_modified(&self) -> Result<()> {
         self.set_modified_time(TimestampMillis::now())
     }
 

rslib/src/storage/tag/get.sql

@@ -0,0 +1,4 @@
+SELECT tag,
+  usn,
+  collapsed
+FROM tags

rslib/src/storage/tag/mod.rs

@@ -19,7 +19,7 @@ impl SqliteStorage {
     /// All tags in the collection, in alphabetical order.
     pub(crate) fn all_tags(&self) -> Result<Vec<Tag>> {
         self.db
-            .prepare_cached("select tag, usn, collapsed from tags")?
+            .prepare_cached(include_str!("get.sql"))?
             .query_and_then(NO_PARAMS, row_to_tag)?
             .collect()
     }
@@ -43,7 +43,7 @@ impl SqliteStorage {
 
     pub(crate) fn get_tag(&self, name: &str) -> Result<Option<Tag>> {
         self.db
-            .prepare_cached("select tag, usn, collapsed from tags where tag = ?")?
+            .prepare_cached(&format!("{} where tag = ?", include_str!("get.sql")))?
             .query_and_then(&[name], row_to_tag)?
             .next()
             .transpose()
@@ -65,13 +65,24 @@ impl SqliteStorage {
             .map_err(Into::into)
     }
 
-    // for undo in the future
+    pub(crate) fn get_tags_by_predicate<F>(&self, want: F) -> Result<Vec<Tag>>
-    #[allow(dead_code)]
+    where
-    pub(crate) fn get_tag_and_children(&self, name: &str) -> Result<Vec<Tag>> {
+        F: Fn(&str) -> bool,
-        self.db
+    {
-            .prepare_cached("select tag, usn, collapsed from tags where tag regexp ?")?
+        let mut query_stmt = self.db.prepare_cached(include_str!("get.sql"))?;
-            .query_and_then(&[format!("(?i)^{}($|::)", regex::escape(name))], row_to_tag)?
+        let mut rows = query_stmt.query(NO_PARAMS)?;
-            .collect()
+        let mut output = vec![];
+        while let Some(row) = rows.next()? {
+            let tag = row.get_raw(0).as_str()?;
+            if want(tag) {
+                output.push(Tag {
+                    name: tag.to_owned(),
+                    usn: row.get(1)?,
+                    expanded: !row.get(2)?,
+                })
+            }
+        }
+        Ok(output)
     }
 
     pub(crate) fn remove_single_tag(&self, tag: &str) -> Result<()> {
@@ -82,23 +93,6 @@ impl SqliteStorage {
         Ok(())
     }
 
-    pub(crate) fn clear_tag_and_children(&self, tag: &str) -> Result<()> {
-        self.db
-            .prepare_cached("delete from tags where tag regexp ?")?
-            .execute(&[format!("(?i)^{}($|::)", regex::escape(tag))])?;
-
-        Ok(())
-    }
-
-    /// Clear all matching tags where tag_group is a regexp group that should not match whitespace.
-    pub(crate) fn clear_tag_group(&self, tag_group: &str) -> Result<()> {
-        self.db
-            .prepare_cached("delete from tags where tag regexp ?")?
-            .execute(&[format!("(?i)^{}($|::)", tag_group)])?;
-
-        Ok(())
-    }
-
     pub(crate) fn set_tag_collapsed(&self, tag: &str, collapsed: bool) -> Result<()> {
         self.db
             .prepare_cached("update tags set collapsed = ? where tag = ?")?

rslib/src/sync/server.rs

@@ -210,7 +210,8 @@ impl SyncServer for LocalServer {
         _col_folder: Option<&Path>,
     ) -> Result<NamedTempFile> {
         // bump usn/mod & close
-        self.col.transact(None, |col| col.storage.increment_usn())?;
+        self.col
+            .transact_no_undo(|col| col.storage.increment_usn())?;
         let col_path = self.col.col_path.clone();
         self.col.close(true)?;
 

rslib/src/tags/bulkadd.rs

@@ -0,0 +1,88 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+//! Adding tags to selected notes in the browse screen.
+
+use std::collections::HashSet;
+use unicase::UniCase;
+
+use super::{join_tags, split_tags};
+use crate::{notes::NoteTags, prelude::*};
+
+impl Collection {
+    pub fn add_tags_to_notes(&mut self, nids: &[NoteID], tags: &str) -> Result<OpOutput<usize>> {
+        self.transact(Op::UpdateTag, |col| col.add_tags_to_notes_inner(nids, tags))
+    }
+}
+
+impl Collection {
+    fn add_tags_to_notes_inner(&mut self, nids: &[NoteID], tags: &str) -> Result<usize> {
+        let usn = self.usn()?;
+
+        // will update tag list for any new tags, and match case
+        let tags_to_add = self.canonified_tags_as_vec(tags, usn)?;
+
+        // modify notes
+        let mut match_count = 0;
+        let notes = self.storage.get_note_tags_by_id_list(nids)?;
+        for original in notes {
+            if let Some(updated_tags) = add_missing_tags(&original.tags, &tags_to_add) {
+                match_count += 1;
+                let mut note = NoteTags {
+                    tags: updated_tags,
+                    ..original
+                };
+                note.set_modified(usn);
+                self.update_note_tags_undoable(&note, original)?;
+            }
+        }
+
+        Ok(match_count)
+    }
+}
+
+/// Returns the sorted new tag string if any tags were added.
+fn add_missing_tags(note_tags: &str, desired: &[UniCase<String>]) -> Option<String> {
+    let mut note_tags: HashSet<_> = split_tags(note_tags)
+        .map(ToOwned::to_owned)
+        .map(UniCase::new)
+        .collect();
+
+    let mut modified = false;
+    for tag in desired {
+        if !note_tags.contains(tag) {
+            note_tags.insert(tag.clone());
+            modified = true;
+        }
+    }
+    if !modified {
+        return None;
+    }
+
+    // sort
+    let mut tags: Vec<_> = note_tags.into_iter().collect::<Vec<_>>();
+    tags.sort_unstable();
+
+    // turn back into a string
+    let tags: Vec<_> = tags.into_iter().map(|s| s.into_inner()).collect();
+    Some(join_tags(&tags))
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn add_missing() {
+        let desired: Vec<_> = ["xyz", "abc", "DEF"]
+            .iter()
+            .map(|s| UniCase::new(s.to_string()))
+            .collect();
+
+        let add_to = |text| add_missing_tags(text, &desired).unwrap();
+
+        assert_eq!(&add_to(""), " abc DEF xyz ");
+        assert_eq!(&add_to("XYZ deF aaa"), " aaa abc deF XYZ ");
+        assert_eq!(add_missing_tags("def xyz abc", &desired).is_none(), true);
+    }
+}

rslib/src/tags/findreplace.rs

@@ -0,0 +1,142 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+use std::borrow::Cow;
+
+use regex::{NoExpand, Regex, Replacer};
+
+use super::{is_tag_separator, join_tags, split_tags};
+use crate::{notes::NoteTags, prelude::*};
+
+impl Collection {
+    /// Replace occurences of a search with a new value in tags.
+    pub fn find_and_replace_tag(
+        &mut self,
+        nids: &[NoteID],
+        search: &str,
+        replacement: &str,
+        regex: bool,
+        match_case: bool,
+    ) -> Result<OpOutput<usize>> {
+        if replacement.contains(is_tag_separator) {
+            return Err(AnkiError::invalid_input(
+                "replacement name can not contain a space",
+            ));
+        }
+
+        let mut search = if regex {
+            Cow::from(search)
+        } else {
+            Cow::from(regex::escape(search))
+        };
+
+        if !match_case {
+            search = format!("(?i){}", search).into();
+        }
+
+        self.transact(Op::UpdateTag, |col| {
+            if regex {
+                col.replace_tags_for_notes_inner(nids, Regex::new(&search)?, replacement)
+            } else {
+                col.replace_tags_for_notes_inner(nids, Regex::new(&search)?, NoExpand(replacement))
+            }
+        })
+    }
+}
+
+impl Collection {
+    fn replace_tags_for_notes_inner<R: Replacer>(
+        &mut self,
+        nids: &[NoteID],
+        regex: Regex,
+        mut repl: R,
+    ) -> Result<usize> {
+        let usn = self.usn()?;
+        let mut match_count = 0;
+        let notes = self.storage.get_note_tags_by_id_list(nids)?;
+
+        for original in notes {
+            if let Some(updated_tags) = replace_tags(&original.tags, &regex, repl.by_ref()) {
+                let (tags, _) = self.canonify_tags(updated_tags, usn)?;
+
+                match_count += 1;
+                let mut note = NoteTags {
+                    tags: join_tags(&tags),
+                    ..original
+                };
+                note.set_modified(usn);
+                self.update_note_tags_undoable(&note, original)?;
+            }
+        }
+
+        Ok(match_count)
+    }
+}
+
+/// If any tags are changed, return the new tags list.
+/// The returned tags will need to be canonified.
+fn replace_tags<R>(tags: &str, regex: &Regex, mut repl: R) -> Option<Vec<String>>
+where
+    R: Replacer,
+{
+    let maybe_replaced: Vec<_> = split_tags(tags)
+        .map(|tag| regex.replace_all(tag, repl.by_ref()))
+        .collect();
+
+    if maybe_replaced
+        .iter()
+        .any(|cow| matches!(cow, Cow::Owned(_)))
+    {
+        Some(maybe_replaced.into_iter().map(|s| s.to_string()).collect())
+    } else {
+        // nothing matched
+        None
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::{collection::open_test_collection, decks::DeckID};
+
+    #[test]
+    fn find_replace() -> Result<()> {
+        let mut col = open_test_collection();
+        let nt = col.get_notetype_by_name("Basic")?.unwrap();
+        let mut note = nt.new_note();
+        note.tags.push("test".into());
+        col.add_note(&mut note, DeckID(1))?;
+
+        col.find_and_replace_tag(&[note.id], "foo|test", "bar", true, false)?;
+        let note = col.storage.get_note(note.id)?.unwrap();
+        assert_eq!(note.tags[0], "bar");
+
+        col.find_and_replace_tag(&[note.id], "BAR", "baz", false, true)?;
+        let note = col.storage.get_note(note.id)?.unwrap();
+        assert_eq!(note.tags[0], "bar");
+
+        col.find_and_replace_tag(&[note.id], "b.r", "baz", false, false)?;
+        let note = col.storage.get_note(note.id)?.unwrap();
+        assert_eq!(note.tags[0], "bar");
+
+        col.find_and_replace_tag(&[note.id], "b.r", "baz", true, false)?;
+        let note = col.storage.get_note(note.id)?.unwrap();
+        assert_eq!(note.tags[0], "baz");
+
+        let out = col.add_tags_to_notes(&[note.id], "cee aye")?;
+        assert_eq!(out.output, 1);
+        let note = col.storage.get_note(note.id)?.unwrap();
+        assert_eq!(&note.tags, &["aye", "baz", "cee"]);
+
+        // if all tags already on note, it doesn't get updated
+        let out = col.add_tags_to_notes(&[note.id], "cee aye")?;
+        assert_eq!(out.output, 0);
+
+        // empty replacement deletes tag
+        col.find_and_replace_tag(&[note.id], "b.*|.*ye", "", true, false)?;
+        let note = col.storage.get_note(note.id)?.unwrap();
+        assert_eq!(&note.tags, &["cee"]);
+
+        Ok(())
+    }
+}

rslib/src/tags/matcher.rs

@@ -0,0 +1,160 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+use regex::{Captures, Regex};
+use std::{borrow::Cow, collections::HashSet};
+
+use super::{join_tags, split_tags};
+use crate::prelude::*;
+pub(crate) struct TagMatcher {
+    regex: Regex,
+    new_tags: HashSet<String>,
+}
+
+/// Helper to match any of the provided space-separated tags in a space-
+/// separated list of tags, and replace the prefix.
+///
+/// Tracks seen tags during replacement, so the tag list can be updated as well.
+impl TagMatcher {
+    pub fn new(space_separated_tags: &str) -> Result<Self> {
+        // convert "fo*o bar" into "fo\*o|bar"
+        let tags: Vec<_> = split_tags(space_separated_tags)
+            .map(regex::escape)
+            .collect();
+        let tags = tags.join("|");
+
+        let regex = Regex::new(&format!(
+            r#"(?ix)
+            # start of string, or a space
+            (?:^|\ )
+            # 1: the tag prefix
+            (
+                {}
+            )
+            (?:
+                # 2: an optional child separator
+                (::)
+                # or a space/end of string the end of the string
+                |\ |$
+            )
+        "#,
+            tags
+        ))?;
+
+        Ok(Self {
+            regex,
+            new_tags: HashSet::new(),
+        })
+    }
+
+    pub fn is_match(&self, space_separated_tags: &str) -> bool {
+        self.regex.is_match(space_separated_tags)
+    }
+
+    pub fn replace(&mut self, space_separated_tags: &str, replacement: &str) -> String {
+        let tags: Vec<_> = split_tags(space_separated_tags)
+            .map(|tag| {
+                let out = self.regex.replace(tag, |caps: &Captures| {
+                    // if we captured the child separator, add it to the replacement
+                    if caps.get(2).is_some() {
+                        Cow::Owned(format!("{}::", replacement))
+                    } else {
+                        Cow::Borrowed(replacement)
+                    }
+                });
+                if let Cow::Owned(out) = out {
+                    if !self.new_tags.contains(&out) {
+                        self.new_tags.insert(out.clone());
+                    }
+                    out
+                } else {
+                    out.to_string()
+                }
+            })
+            .collect();
+
+        join_tags(tags.as_slice())
+    }
+
+    /// The `replacement` function should return the text to use as a replacement.
+    pub fn replace_with_fn<F>(&mut self, space_separated_tags: &str, replacer: F) -> String
+    where
+        F: Fn(&str) -> String,
+    {
+        let tags: Vec<_> = split_tags(space_separated_tags)
+            .map(|tag| {
+                let out = self.regex.replace(tag, |caps: &Captures| {
+                    let replacement = replacer(caps.get(1).unwrap().as_str());
+                    // if we captured the child separator, add it to the replacement
+                    if caps.get(2).is_some() {
+                        format!("{}::", replacement)
+                    } else {
+                        replacement
+                    }
+                });
+                if let Cow::Owned(out) = out {
+                    if !self.new_tags.contains(&out) {
+                        self.new_tags.insert(out.clone());
+                    }
+                    out
+                } else {
+                    out.to_string()
+                }
+            })
+            .collect();
+
+        join_tags(tags.as_slice())
+    }
+
+    /// Remove any matching tags. Does not update seen_tags.
+    pub fn remove(&mut self, space_separated_tags: &str) -> String {
+        let tags: Vec<_> = split_tags(space_separated_tags)
+            .filter(|&tag| !self.is_match(tag))
+            .map(ToString::to_string)
+            .collect();
+
+        join_tags(tags.as_slice())
+    }
+
+    /// Returns all replaced values that were used, so they can be registered
+    /// into the tag list.
+    pub fn into_new_tags(self) -> HashSet<String> {
+        self.new_tags
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn regex() -> Result<()> {
+        let re = TagMatcher::new("one two")?;
+        assert_eq!(re.is_match(" foo "), false);
+        assert_eq!(re.is_match(" foo one "), true);
+        assert_eq!(re.is_match(" two foo "), true);
+
+        let mut re = TagMatcher::new("foo")?;
+        assert_eq!(re.is_match("foo"), true);
+        assert_eq!(re.is_match(" foo "), true);
+        assert_eq!(re.is_match(" bar foo baz "), true);
+        assert_eq!(re.is_match(" bar FOO baz "), true);
+        assert_eq!(re.is_match(" bar foof baz "), false);
+        assert_eq!(re.is_match(" barfoo "), false);
+
+        let mut as_xxx = |text| re.replace(text, "xxx");
+
+        assert_eq!(&as_xxx(" baz FOO "), " baz xxx ");
+        assert_eq!(&as_xxx(" x foo::bar x "), " x xxx::bar x ");
+        assert_eq!(
+            &as_xxx(" x foo::bar bar::foo x "),
+            " x xxx::bar bar::foo x "
+        );
+        assert_eq!(
+            &as_xxx(" x foo::bar foo::bar::baz x "),
+            " x xxx::bar xxx::bar::baz x "
+        );
+
+        Ok(())
+    }
+}

rslib/src/tags/mod.rs

@@ -1,22 +1,20 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
+mod bulkadd;
+mod findreplace;
+mod matcher;
+mod register;
+mod remove;
+mod rename;
+mod reparent;
+mod tree;
 pub(crate) mod undo;
 
-use crate::{
-    backend_proto::TagTreeNode,
-    collection::Collection,
-    err::{AnkiError, Result},
-    notes::{NoteID, TransformNoteOutput},
-    prelude::*,
-    text::{normalize_to_nfc, to_re},
-    types::Usn,
-};
-
-use regex::{NoExpand, Regex, Replacer};
-use std::{borrow::Cow, collections::HashSet, iter::Peekable};
 use unicase::UniCase;
 
+use crate::prelude::*;
+
 #[derive(Debug, Clone, PartialEq)]
 pub struct Tag {
     pub name: String,
@@ -50,41 +48,6 @@ fn is_tag_separator(c: char) -> bool {
     c == ' ' || c == '\u{3000}'
 }
 
-fn invalid_char_for_tag(c: char) -> bool {
-    c.is_ascii_control() || is_tag_separator(c) || c == '"'
-}
-
-fn normalized_tag_name_component(comp: &str) -> Cow<str> {
-    let mut out = normalize_to_nfc(comp);
-    if out.contains(invalid_char_for_tag) {
-        out = out.replace(invalid_char_for_tag, "").into();
-    }
-    let trimmed = out.trim();
-    if trimmed.is_empty() {
-        "blank".to_string().into()
-    } else if trimmed.len() != out.len() {
-        trimmed.to_string().into()
-    } else {
-        out
-    }
-}
-
-fn normalize_tag_name(name: &str) -> Cow<str> {
-    if name
-        .split("::")
-        .any(|comp| matches!(normalized_tag_name_component(comp), Cow::Owned(_)))
-    {
-        let comps: Vec<_> = name
-            .split("::")
-            .map(normalized_tag_name_component)
-            .collect::<Vec<_>>();
-        comps.join("::").into()
-    } else {
-        // no changes required
-        name.into()
-    }
-}
-
 fn immediate_parent_name_unicase(tag_name: UniCase<&str>) -> Option<UniCase<&str>> {
     tag_name.rsplitn(2, '\x1f').nth(1).map(UniCase::new)
 }
@@ -93,236 +56,7 @@ fn immediate_parent_name_str(tag_name: &str) -> Option<&str> {
     tag_name.rsplitn(2, "::").nth(1)
 }
 
-/// Arguments are expected in 'human' form with an :: separator.
-pub(crate) fn drag_drop_tag_name(dragged: &str, dropped: Option<&str>) -> Option<String> {
-    let dragged_base = dragged.rsplit("::").next().unwrap();
-    if let Some(dropped) = dropped {
-        if dropped.starts_with(dragged) {
-            // foo onto foo::bar, or foo onto itself -> no-op
-            None
-        } else {
-            // foo::bar onto baz -> baz::bar
-            Some(format!("{}::{}", dropped, dragged_base))
-        }
-    } else {
-        // foo::bar onto top level -> bar
-        Some(dragged_base.into())
-    }
-}
-
-/// For the given tag, check if immediate parent exists. If so, add
-/// tag and return.
-/// If the immediate parent is missing, check and add any missing parents.
-/// This should ensure that if an immediate parent is found, all ancestors
-/// are guaranteed to already exist.
-fn add_tag_and_missing_parents<'a, 'b>(
-    all: &'a mut HashSet<UniCase<&'b str>>,
-    missing: &'a mut Vec<UniCase<&'b str>>,
-    tag_name: UniCase<&'b str>,
-) {
-    if let Some(parent) = immediate_parent_name_unicase(tag_name) {
-        if !all.contains(&parent) {
-            missing.push(parent);
-            add_tag_and_missing_parents(all, missing, parent);
-        }
-    }
-    // finally, add provided tag
-    all.insert(tag_name);
-}
-
-/// Append any missing parents. Caller must sort afterwards.
-fn add_missing_parents(tags: &mut Vec<Tag>) {
-    let mut all_names: HashSet<UniCase<&str>> = HashSet::new();
-    let mut missing = vec![];
-    for tag in &*tags {
-        add_tag_and_missing_parents(&mut all_names, &mut missing, UniCase::new(&tag.name))
-    }
-    let mut missing: Vec<_> = missing
-        .into_iter()
-        .map(|n| Tag::new(n.to_string(), Usn(0)))
-        .collect();
-    tags.append(&mut missing);
-}
-
-fn tags_to_tree(mut tags: Vec<Tag>) -> TagTreeNode {
-    for tag in &mut tags {
-        tag.name = tag.name.replace("::", "\x1f");
-    }
-    add_missing_parents(&mut tags);
-    tags.sort_unstable_by(|a, b| UniCase::new(&a.name).cmp(&UniCase::new(&b.name)));
-    let mut top = TagTreeNode::default();
-    let mut it = tags.into_iter().peekable();
-    add_child_nodes(&mut it, &mut top);
-
-    top
-}
-
-fn add_child_nodes(tags: &mut Peekable<impl Iterator<Item = Tag>>, parent: &mut TagTreeNode) {
-    while let Some(tag) = tags.peek() {
-        let split_name: Vec<_> = tag.name.split('\x1f').collect();
-        match split_name.len() as u32 {
-            l if l <= parent.level => {
-                // next item is at a higher level
-                return;
-            }
-            l if l == parent.level + 1 => {
-                // next item is an immediate descendent of parent
-                parent.children.push(TagTreeNode {
-                    name: (*split_name.last().unwrap()).into(),
-                    children: vec![],
-                    level: parent.level + 1,
-                    expanded: tag.expanded,
-                });
-                tags.next();
-            }
-            _ => {
-                // next item is at a lower level
-                if let Some(last_child) = parent.children.last_mut() {
-                    add_child_nodes(tags, last_child)
-                } else {
-                    // immediate parent is missing
-                    tags.next();
-                }
-            }
-        }
-    }
-}
-
 impl Collection {
-    pub fn tag_tree(&mut self) -> Result<TagTreeNode> {
-        let tags = self.storage.all_tags()?;
-        let tree = tags_to_tree(tags);
-
-        Ok(tree)
-    }
-
-    /// Given a list of tags, fix case, ordering and duplicates.
-    /// Returns true if any new tags were added.
-    pub(crate) fn canonify_tags(
-        &mut self,
-        tags: Vec<String>,
-        usn: Usn,
-    ) -> Result<(Vec<String>, bool)> {
-        let mut seen = HashSet::new();
-        let mut added = false;
-
-        let tags: Vec<_> = tags.iter().flat_map(|t| split_tags(t)).collect();
-        for tag in tags {
-            let mut tag = Tag::new(tag.to_string(), usn);
-            added |= self.register_tag(&mut tag)?;
-            seen.insert(UniCase::new(tag.name));
-        }
-
-        // exit early if no non-empty tags
-        if seen.is_empty() {
-            return Ok((vec![], added));
-        }
-
-        // return the sorted, canonified tags
-        let mut tags = seen.into_iter().collect::<Vec<_>>();
-        tags.sort_unstable();
-        let tags: Vec<_> = tags.into_iter().map(|s| s.into_inner()).collect();
-
-        Ok((tags, added))
-    }
-
-    /// Adjust tag casing to match any existing parents, and register it if it's not already
-    /// in the tags list. True if the tag was added and not already in tag list.
-    /// In the case the tag is already registered, tag will be mutated to match the existing
-    /// name.
-    pub(crate) fn register_tag(&mut self, tag: &mut Tag) -> Result<bool> {
-        let normalized_name = normalize_tag_name(&tag.name);
-        if normalized_name.is_empty() {
-            // this should not be possible
-            return Err(AnkiError::invalid_input("blank tag"));
-        }
-        if let Some(existing_tag) = self.storage.get_tag(&normalized_name)? {
-            tag.name = existing_tag.name;
-            Ok(false)
-        } else {
-            if let Some(new_name) = self.adjusted_case_for_parents(&normalized_name)? {
-                tag.name = new_name;
-            } else if let Cow::Owned(new_name) = normalized_name {
-                tag.name = new_name;
-            }
-            self.register_tag_undoable(&tag)?;
-            Ok(true)
-        }
-    }
-
-    /// If parent tag(s) exist and differ in case, return a rewritten tag.
-    fn adjusted_case_for_parents(&self, tag: &str) -> Result<Option<String>> {
-        if let Some(parent_tag) = self.first_existing_parent_tag(&tag)? {
-            let child_split: Vec<_> = tag.split("::").collect();
-            let parent_count = parent_tag.matches("::").count() + 1;
-            Ok(Some(format!(
-                "{}::{}",
-                parent_tag,
-                &child_split[parent_count..].join("::")
-            )))
-        } else {
-            Ok(None)
-        }
-    }
-
-    fn first_existing_parent_tag(&self, mut tag: &str) -> Result<Option<String>> {
-        while let Some(parent_name) = immediate_parent_name_str(tag) {
-            if let Some(parent_tag) = self.storage.preferred_tag_case(parent_name)? {
-                return Ok(Some(parent_tag));
-            }
-            tag = parent_name;
-        }
-
-        Ok(None)
-    }
-
-    pub fn clear_unused_tags(&self) -> Result<()> {
-        let expanded: HashSet<_> = self.storage.expanded_tags()?.into_iter().collect();
-        self.storage.clear_all_tags()?;
-        let usn = self.usn()?;
-        for name in self.storage.all_tags_in_notes()? {
-            let name = normalize_tag_name(&name).into();
-            self.storage.register_tag(&Tag {
-                expanded: expanded.contains(&name),
-                name,
-                usn,
-            })?;
-        }
-
-        Ok(())
-    }
-
-    /// Take tags as a whitespace-separated string and remove them from all notes and the storage.
-    pub fn expunge_tags(&mut self, tags: &str) -> Result<usize> {
-        let tag_group = format!("({})", regex::escape(tags.trim()).replace(' ', "|"));
-        let nids = self.nids_for_tags(&tag_group)?;
-        let re = Regex::new(&format!("(?i)^{}(::.*)?$", tag_group))?;
-        self.transact(None, |col| {
-            col.storage.clear_tag_group(&tag_group)?;
-            col.transform_notes(&nids, |note, _nt| {
-                Ok(TransformNoteOutput {
-                    changed: note.remove_tags(&re),
-                    generate_cards: false,
-                    mark_modified: true,
-                })
-            })
-        })
-    }
-
-    /// Take tags as a regexp group, i.e. separated with pipes and wrapped in brackets, and return
-    /// the ids of all notes with one of them.
-    fn nids_for_tags(&mut self, tag_group: &str) -> Result<Vec<NoteID>> {
-        let mut stmt = self
-            .storage
-            .db
-            .prepare("select id from notes where tags regexp ?")?;
-        let args = format!("(?i).* {}(::| ).*", tag_group);
-        let nids = stmt
-            .query_map(&[args], |row| row.get(0))?
-            .collect::<std::result::Result<_, _>>()?;
-        Ok(nids)
-    }
-
     pub(crate) fn set_tag_expanded(&self, name: &str, expanded: bool) -> Result<()> {
         let mut name = name;
         let tag;
@@ -334,488 +68,4 @@ impl Collection {
         }
         self.storage.set_tag_collapsed(name, !expanded)
     }
-
-    fn replace_tags_for_notes_inner<R: Replacer>(
-        &mut self,
-        nids: &[NoteID],
-        tags: &[Regex],
-        mut repl: R,
-    ) -> Result<usize> {
-        self.transact(Some(UndoableOpKind::UpdateTag), |col| {
-            col.transform_notes(nids, |note, _nt| {
-                let mut changed = false;
-                for re in tags {
-                    if note.replace_tags(re, repl.by_ref()) {
-                        changed = true;
-                    }
-                }
-
-                Ok(TransformNoteOutput {
-                    changed,
-                    generate_cards: false,
-                    mark_modified: true,
-                })
-            })
-        })
-    }
-
-    /// Apply the provided list of regular expressions to note tags,
-    /// saving any modified notes.
-    pub fn replace_tags_for_notes(
-        &mut self,
-        nids: &[NoteID],
-        tags: &str,
-        repl: &str,
-        regex: bool,
-    ) -> Result<usize> {
-        // generate regexps
-        let tags = split_tags(tags)
-            .map(|tag| {
-                let tag = if regex { tag.into() } else { to_re(tag) };
-                Regex::new(&format!("(?i)^{}(::.*)?$", tag))
-                    .map_err(|_| AnkiError::invalid_input("invalid regex"))
-            })
-            .collect::<Result<Vec<Regex>>>()?;
-        if !regex {
-            self.replace_tags_for_notes_inner(nids, &tags, NoExpand(repl))
-        } else {
-            self.replace_tags_for_notes_inner(nids, &tags, repl)
-        }
-    }
-
-    pub fn add_tags_to_notes(&mut self, nids: &[NoteID], tags: &str) -> Result<usize> {
-        let tags: Vec<_> = split_tags(tags).collect();
-        let matcher = regex::RegexSet::new(
-            tags.iter()
-                .map(|s| regex::escape(s))
-                .map(|s| format!("(?i)^{}$", s)),
-        )
-        .map_err(|_| AnkiError::invalid_input("invalid regex"))?;
-
-        self.transact(Some(UndoableOpKind::UpdateTag), |col| {
-            col.transform_notes(nids, |note, _nt| {
-                let mut need_to_add = true;
-                let mut match_count = 0;
-                for tag in &note.tags {
-                    if matcher.is_match(tag) {
-                        match_count += 1;
-                    }
-                    if match_count == tags.len() {
-                        need_to_add = false;
-                        break;
-                    }
-                }
-
-                if need_to_add {
-                    note.tags.extend(tags.iter().map(|&s| s.to_string()))
-                }
-
-                Ok(TransformNoteOutput {
-                    changed: need_to_add,
-                    generate_cards: false,
-                    mark_modified: true,
-                })
-            })
-        })
-    }
-
-    pub fn drag_drop_tags(
-        &mut self,
-        source_tags: &[String],
-        target_tag: Option<String>,
-    ) -> Result<()> {
-        let source_tags_and_outputs: Vec<_> = source_tags
-            .iter()
-            // generate resulting names and filter out invalid ones
-            .flat_map(|source_tag| {
-                if let Some(output_name) = drag_drop_tag_name(source_tag, target_tag.as_deref()) {
-                    Some((source_tag, output_name))
-                } else {
-                    // invalid rename, ignore this tag
-                    None
-                }
-            })
-            .collect();
-
-        let regexps_and_replacements = source_tags_and_outputs
-            .iter()
-            // convert the names into regexps/replacements
-            .map(|(tag, output)| {
-                Regex::new(&format!(
-                    r#"(?ix)
-                    ^
-                    {}
-                    # optional children
-                    (::.+)?
-                    $
-                    "#,
-                    regex::escape(tag)
-                ))
-                .map_err(|_| AnkiError::invalid_input("invalid regex"))
-                .map(|regex| (regex, output))
-            })
-            .collect::<Result<Vec<_>>>()?;
-
-        // locate notes that match them
-        let mut nids = vec![];
-        self.storage.for_each_note_tags(|nid, tags| {
-            for tag in split_tags(&tags) {
-                for (regex, _) in &regexps_and_replacements {
-                    if regex.is_match(&tag) {
-                        nids.push(nid);
-                        break;
-                    }
-                }
-            }
-
-            Ok(())
-        })?;
-
-        if nids.is_empty() {
-            return Ok(());
-        }
-
-        // update notes
-        self.transact(None, |col| {
-            // clear the existing original tags
-            for (source_tag, _) in &source_tags_and_outputs {
-                col.storage.clear_tag_and_children(source_tag)?;
-            }
-
-            col.transform_notes(&nids, |note, _nt| {
-                let mut changed = false;
-                for (re, repl) in &regexps_and_replacements {
-                    if note.replace_tags(re, NoExpand(&repl).by_ref()) {
-                        changed = true;
-                    }
-                }
-
-                Ok(TransformNoteOutput {
-                    changed,
-                    generate_cards: false,
-                    mark_modified: true,
-                })
-            })
-        })?;
-
-        Ok(())
-    }
-}
-
-#[cfg(test)]
-mod test {
-    use super::*;
-    use crate::{collection::open_test_collection, decks::DeckID};
-
-    #[test]
-    fn tags() -> Result<()> {
-        let mut col = open_test_collection();
-        let nt = col.get_notetype_by_name("Basic")?.unwrap();
-        let mut note = nt.new_note();
-        col.add_note(&mut note, DeckID(1))?;
-
-        let tags: String = col.storage.db_scalar("select tags from notes")?;
-        assert_eq!(tags, "");
-
-        // first instance wins in case of duplicates
-        note.tags = vec!["foo".into(), "FOO".into()];
-        col.update_note(&mut note)?;
-        assert_eq!(&note.tags, &["foo"]);
-        let tags: String = col.storage.db_scalar("select tags from notes")?;
-        assert_eq!(tags, " foo ");
-
-        // existing case is used if in DB
-        note.tags = vec!["FOO".into()];
-        col.update_note(&mut note)?;
-        assert_eq!(&note.tags, &["foo"]);
-        assert_eq!(tags, " foo ");
-
-        // tags are normalized to nfc
-        note.tags = vec!["\u{fa47}".into()];
-        col.update_note(&mut note)?;
-        assert_eq!(&note.tags, &["\u{6f22}"]);
-
-        // if code incorrectly adds a space to a tag, it gets split
-        note.tags = vec!["one two".into()];
-        col.update_note(&mut note)?;
-        assert_eq!(&note.tags, &["one", "two"]);
-
-        // blanks should be handled
-        note.tags = vec![
-            "".into(),
-            "foo".into(),
-            " ".into(),
-            "::".into(),
-            "foo::".into(),
-        ];
-        col.update_note(&mut note)?;
-        assert_eq!(&note.tags, &["blank::blank", "foo", "foo::blank"]);
-
-        Ok(())
-    }
-
-    #[test]
-    fn bulk() -> Result<()> {
-        let mut col = open_test_collection();
-        let nt = col.get_notetype_by_name("Basic")?.unwrap();
-        let mut note = nt.new_note();
-        note.tags.push("test".into());
-        col.add_note(&mut note, DeckID(1))?;
-
-        col.replace_tags_for_notes(&[note.id], "foo test", "bar", false)?;
-        let note = col.storage.get_note(note.id)?.unwrap();
-        assert_eq!(note.tags[0], "bar");
-
-        col.replace_tags_for_notes(&[note.id], "b.r", "baz", false)?;
-        let note = col.storage.get_note(note.id)?.unwrap();
-        assert_eq!(note.tags[0], "bar");
-
-        col.replace_tags_for_notes(&[note.id], "b*r", "baz", false)?;
-        let note = col.storage.get_note(note.id)?.unwrap();
-        assert_eq!(note.tags[0], "baz");
-
-        col.replace_tags_for_notes(&[note.id], "b.r", "baz", true)?;
-        let note = col.storage.get_note(note.id)?.unwrap();
-        assert_eq!(note.tags[0], "baz");
-
-        let cnt = col.add_tags_to_notes(&[note.id], "cee aye")?;
-        assert_eq!(cnt, 1);
-        let note = col.storage.get_note(note.id)?.unwrap();
-        assert_eq!(&note.tags, &["aye", "baz", "cee"]);
-
-        // if all tags already on note, it doesn't get updated
-        let cnt = col.add_tags_to_notes(&[note.id], "cee aye")?;
-        assert_eq!(cnt, 0);
-
-        // empty replacement deletes tag
-        col.replace_tags_for_notes(&[note.id], "b.* .*ye", "", true)?;
-        let note = col.storage.get_note(note.id)?.unwrap();
-        assert_eq!(&note.tags, &["cee"]);
-
-        let mut note = col.storage.get_note(note.id)?.unwrap();
-        note.tags = vec![
-            "foo::bar".into(),
-            "foo::bar::foo".into(),
-            "bar::foo".into(),
-            "bar::foo::bar".into(),
-        ];
-        col.update_note(&mut note)?;
-        col.replace_tags_for_notes(&[note.id], "bar::foo", "foo::bar", false)?;
-        let note = col.storage.get_note(note.id)?.unwrap();
-        assert_eq!(&note.tags, &["foo::bar", "foo::bar::bar", "foo::bar::foo",]);
-
-        // ensure replacements fully match
-        let mut note = col.storage.get_note(note.id)?.unwrap();
-        note.tags = vec!["foobar".into(), "barfoo".into(), "foo".into()];
-        col.update_note(&mut note)?;
-        col.replace_tags_for_notes(&[note.id], "foo", "", false)?;
-        let note = col.storage.get_note(note.id)?.unwrap();
-        assert_eq!(&note.tags, &["barfoo", "foobar"]);
-
-        // tag children are also cleared when clearing their parent
-        col.storage.clear_all_tags()?;
-        for name in vec!["a", "a::b", "A::b::c"] {
-            col.register_tag(&mut Tag::new(name.to_string(), Usn(0)))?;
-        }
-        col.storage.clear_tag_and_children("a")?;
-        assert_eq!(col.storage.all_tags()?, vec![]);
-
-        Ok(())
-    }
-
-    fn node(name: &str, level: u32, children: Vec<TagTreeNode>) -> TagTreeNode {
-        TagTreeNode {
-            name: name.into(),
-            level,
-            children,
-
-            ..Default::default()
-        }
-    }
-
-    fn leaf(name: &str, level: u32) -> TagTreeNode {
-        node(name, level, vec![])
-    }
-
-    #[test]
-    fn tree() -> Result<()> {
-        let mut col = open_test_collection();
-        let nt = col.get_notetype_by_name("Basic")?.unwrap();
-        let mut note = nt.new_note();
-        note.tags.push("foo::bar::a".into());
-        note.tags.push("foo::bar::b".into());
-        col.add_note(&mut note, DeckID(1))?;
-
-        // missing parents are added
-        assert_eq!(
-            col.tag_tree()?,
-            node(
-                "",
-                0,
-                vec![node(
-                    "foo",
-                    1,
-                    vec![node("bar", 2, vec![leaf("a", 3), leaf("b", 3)])]
-                )]
-            )
-        );
-
-        // differing case should result in only one parent case being added -
-        // the first one
-        col.storage.clear_all_tags()?;
-        note.tags[0] = "foo::BAR::a".into();
-        note.tags[1] = "FOO::bar::b".into();
-        col.update_note(&mut note)?;
-        assert_eq!(
-            col.tag_tree()?,
-            node(
-                "",
-                0,
-                vec![node(
-                    "foo",
-                    1,
-                    vec![node("BAR", 2, vec![leaf("a", 3), leaf("b", 3)])]
-                )]
-            )
-        );
-
-        // things should work even if the immediate parent is not missing
-        col.storage.clear_all_tags()?;
-        note.tags[0] = "foo::bar::baz".into();
-        note.tags[1] = "foo::bar::baz::quux".into();
-        col.update_note(&mut note)?;
-        assert_eq!(
-            col.tag_tree()?,
-            node(
-                "",
-                0,
-                vec![node(
-                    "foo",
-                    1,
-                    vec![node("bar", 2, vec![node("baz", 3, vec![leaf("quux", 4)])])]
-                )]
-            )
-        );
-
-        // numbers have a smaller ascii number than ':', so a naive sort on
-        // '::' would result in one::two being nested under one1.
-        col.storage.clear_all_tags()?;
-        note.tags[0] = "one".into();
-        note.tags[1] = "one1".into();
-        note.tags.push("one::two".into());
-        col.update_note(&mut note)?;
-        assert_eq!(
-            col.tag_tree()?,
-            node(
-                "",
-                0,
-                vec![node("one", 1, vec![leaf("two", 2)]), leaf("one1", 1)]
-            )
-        );
-
-        // children should match the case of their parents
-        col.storage.clear_all_tags()?;
-        note.tags[0] = "FOO".into();
-        note.tags[1] = "foo::BAR".into();
-        note.tags[2] = "foo::bar::baz".into();
-        col.update_note(&mut note)?;
-        assert_eq!(note.tags, vec!["FOO", "FOO::BAR", "FOO::BAR::baz"]);
-
-        Ok(())
-    }
-
-    #[test]
-    fn clearing() -> Result<()> {
-        let mut col = open_test_collection();
-        let nt = col.get_notetype_by_name("Basic")?.unwrap();
-        let mut note = nt.new_note();
-        note.tags.push("one".into());
-        note.tags.push("two".into());
-        col.add_note(&mut note, DeckID(1))?;
-
-        col.set_tag_expanded("one", true)?;
-        col.clear_unused_tags()?;
-        assert_eq!(col.storage.get_tag("one")?.unwrap().expanded, true);
-        assert_eq!(col.storage.get_tag("two")?.unwrap().expanded, false);
-
-        Ok(())
-    }
-
-    fn alltags(col: &Collection) -> Vec<String> {
-        col.storage
-            .all_tags()
-            .unwrap()
-            .into_iter()
-            .map(|t| t.name)
-            .collect()
-    }
-
-    #[test]
-    fn dragdrop() -> Result<()> {
-        let mut col = open_test_collection();
-        let nt = col.get_notetype_by_name("Basic")?.unwrap();
-        for tag in &[
-            "another",
-            "parent1::child1::grandchild1",
-            "parent1::child1",
-            "parent1",
-            "parent2",
-            "yet::another",
-        ] {
-            let mut note = nt.new_note();
-            note.tags.push(tag.to_string());
-            col.add_note(&mut note, DeckID(1))?;
-        }
-
-        // two decks with the same base name; they both get mapped
-        // to parent1::another
-        col.drag_drop_tags(
-            &["another".to_string(), "yet::another".to_string()],
-            Some("parent1".to_string()),
-        )?;
-
-        assert_eq!(
-            alltags(&col),
-            &[
-                "parent1",
-                "parent1::another",
-                "parent1::child1",
-                "parent1::child1::grandchild1",
-                "parent2",
-            ]
-        );
-
-        // child and children moved to parent2
-        col.drag_drop_tags(
-            &["parent1::child1".to_string()],
-            Some("parent2".to_string()),
-        )?;
-
-        assert_eq!(
-            alltags(&col),
-            &[
-                "parent1",
-                "parent1::another",
-                "parent2",
-                "parent2::child1",
-                "parent2::child1::grandchild1",
-            ]
-        );
-
-        // empty target reparents to root
-        col.drag_drop_tags(&["parent1::another".to_string()], None)?;
-
-        assert_eq!(
-            alltags(&col),
-            &[
-                "another",
-                "parent1",
-                "parent2",
-                "parent2::child1",
-                "parent2::child1::grandchild1",
-            ]
-        );
-
-        Ok(())
-    }
 }

rslib/src/tags/register.rs

@@ -0,0 +1,213 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+use super::{immediate_parent_name_str, is_tag_separator, split_tags, Tag};
+use crate::{prelude::*, text::normalize_to_nfc, types::Usn};
+
+use std::{borrow::Cow, collections::HashSet};
+use unicase::UniCase;
+
+impl Collection {
+    /// Given a list of tags, fix case, ordering and duplicates.
+    /// Returns true if any new tags were added.
+    /// Each tag is split on spaces, so if you have a &str, you
+    /// can pass that in as a one-element vec.
+    pub(crate) fn canonify_tags(
+        &mut self,
+        tags: Vec<String>,
+        usn: Usn,
+    ) -> Result<(Vec<String>, bool)> {
+        let mut seen = HashSet::new();
+        let mut added = false;
+
+        let tags: Vec<_> = tags.iter().flat_map(|t| split_tags(t)).collect();
+        for tag in tags {
+            let mut tag = Tag::new(tag.to_string(), usn);
+            added |= self.register_tag(&mut tag)?;
+            seen.insert(UniCase::new(tag.name));
+        }
+
+        // exit early if no non-empty tags
+        if seen.is_empty() {
+            return Ok((vec![], added));
+        }
+
+        // return the sorted, canonified tags
+        let mut tags = seen.into_iter().collect::<Vec<_>>();
+        tags.sort_unstable();
+        let tags: Vec<_> = tags.into_iter().map(|s| s.into_inner()).collect();
+
+        Ok((tags, added))
+    }
+
+    /// Returns true if any cards were added to the tag list.
+    pub(crate) fn canonified_tags_as_vec(
+        &mut self,
+        tags: &str,
+        usn: Usn,
+    ) -> Result<Vec<UniCase<String>>> {
+        let mut out_tags = vec![];
+
+        for tag in split_tags(tags) {
+            let mut tag = Tag::new(tag.to_string(), usn);
+            self.register_tag(&mut tag)?;
+            out_tags.push(UniCase::new(tag.name));
+        }
+
+        Ok(out_tags)
+    }
+
+    /// Adjust tag casing to match any existing parents, and register it if it's not already
+    /// in the tags list. True if the tag was added and not already in tag list.
+    /// In the case the tag is already registered, tag will be mutated to match the existing
+    /// name.
+    pub(crate) fn register_tag(&mut self, tag: &mut Tag) -> Result<bool> {
+        let is_new = self.prepare_tag_for_registering(tag)?;
+        if is_new {
+            self.register_tag_undoable(&tag)?;
+        }
+        Ok(is_new)
+    }
+
+    /// Create a tag object, normalize text, and match parents/existing case if available.
+    /// True if tag is new.
+    pub(super) fn prepare_tag_for_registering(&self, tag: &mut Tag) -> Result<bool> {
+        let normalized_name = normalize_tag_name(&tag.name);
+        if normalized_name.is_empty() {
+            // this should not be possible
+            return Err(AnkiError::invalid_input("blank tag"));
+        }
+        if let Some(existing_tag) = self.storage.get_tag(&normalized_name)? {
+            tag.name = existing_tag.name;
+            Ok(false)
+        } else {
+            if let Some(new_name) = self.adjusted_case_for_parents(&normalized_name)? {
+                tag.name = new_name;
+            } else if let Cow::Owned(new_name) = normalized_name {
+                tag.name = new_name;
+            }
+            Ok(true)
+        }
+    }
+
+    pub(super) fn register_tag_string(&mut self, tag: String, usn: Usn) -> Result<bool> {
+        let mut tag = Tag::new(tag, usn);
+        self.register_tag(&mut tag)
+    }
+}
+
+impl Collection {
+    /// If parent tag(s) exist and differ in case, return a rewritten tag.
+    fn adjusted_case_for_parents(&self, tag: &str) -> Result<Option<String>> {
+        if let Some(parent_tag) = self.first_existing_parent_tag(&tag)? {
+            let child_split: Vec<_> = tag.split("::").collect();
+            let parent_count = parent_tag.matches("::").count() + 1;
+            Ok(Some(format!(
+                "{}::{}",
+                parent_tag,
+                &child_split[parent_count..].join("::")
+            )))
+        } else {
+            Ok(None)
+        }
+    }
+
+    fn first_existing_parent_tag(&self, mut tag: &str) -> Result<Option<String>> {
+        while let Some(parent_name) = immediate_parent_name_str(tag) {
+            if let Some(parent_tag) = self.storage.preferred_tag_case(parent_name)? {
+                return Ok(Some(parent_tag));
+            }
+            tag = parent_name;
+        }
+
+        Ok(None)
+    }
+}
+
+fn invalid_char_for_tag(c: char) -> bool {
+    c.is_ascii_control() || is_tag_separator(c) || c == '"'
+}
+
+fn normalized_tag_name_component(comp: &str) -> Cow<str> {
+    let mut out = normalize_to_nfc(comp);
+    if out.contains(invalid_char_for_tag) {
+        out = out.replace(invalid_char_for_tag, "").into();
+    }
+    let trimmed = out.trim();
+    if trimmed.is_empty() {
+        "blank".to_string().into()
+    } else if trimmed.len() != out.len() {
+        trimmed.to_string().into()
+    } else {
+        out
+    }
+}
+
+fn normalize_tag_name(name: &str) -> Cow<str> {
+    if name
+        .split("::")
+        .any(|comp| matches!(normalized_tag_name_component(comp), Cow::Owned(_)))
+    {
+        let comps: Vec<_> = name
+            .split("::")
+            .map(normalized_tag_name_component)
+            .collect::<Vec<_>>();
+        comps.join("::").into()
+    } else {
+        // no changes required
+        name.into()
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::{collection::open_test_collection, decks::DeckID};
+
+    #[test]
+    fn tags() -> Result<()> {
+        let mut col = open_test_collection();
+        let nt = col.get_notetype_by_name("Basic")?.unwrap();
+        let mut note = nt.new_note();
+        col.add_note(&mut note, DeckID(1))?;
+
+        let tags: String = col.storage.db_scalar("select tags from notes")?;
+        assert_eq!(tags, "");
+
+        // first instance wins in case of duplicates
+        note.tags = vec!["foo".into(), "FOO".into()];
+        col.update_note(&mut note)?;
+        assert_eq!(&note.tags, &["foo"]);
+        let tags: String = col.storage.db_scalar("select tags from notes")?;
+        assert_eq!(tags, " foo ");
+
+        // existing case is used if in DB
+        note.tags = vec!["FOO".into()];
+        col.update_note(&mut note)?;
+        assert_eq!(&note.tags, &["foo"]);
+        assert_eq!(tags, " foo ");
+
+        // tags are normalized to nfc
+        note.tags = vec!["\u{fa47}".into()];
+        col.update_note(&mut note)?;
+        assert_eq!(&note.tags, &["\u{6f22}"]);
+
+        // if code incorrectly adds a space to a tag, it gets split
+        note.tags = vec!["one two".into()];
+        col.update_note(&mut note)?;
+        assert_eq!(&note.tags, &["one", "two"]);
+
+        // blanks should be handled
+        note.tags = vec![
+            "".into(),
+            "foo".into(),
+            " ".into(),
+            "::".into(),
+            "foo::".into(),
+        ];
+        col.update_note(&mut note)?;
+        assert_eq!(&note.tags, &["blank::blank", "foo", "foo::blank"]);
+
+        Ok(())
+    }
+}

rslib/src/tags/remove.rs

@@ -0,0 +1,126 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+use super::matcher::TagMatcher;
+use crate::prelude::*;
+
+impl Collection {
+    /// Take tags as a whitespace-separated string and remove them from all notes and the tag list.
+    pub fn remove_tags(&mut self, tags: &str) -> Result<OpOutput<usize>> {
+        self.transact(Op::RemoveTag, |col| col.remove_tags_inner(tags))
+    }
+
+    /// Remove whitespace-separated tags from provided notes.
+    pub fn remove_tags_from_notes(
+        &mut self,
+        nids: &[NoteID],
+        tags: &str,
+    ) -> Result<OpOutput<usize>> {
+        self.transact(Op::RemoveTag, |col| {
+            col.remove_tags_from_notes_inner(nids, tags)
+        })
+    }
+
+    /// Remove tags not referenced by notes, returning removed count.
+    pub fn clear_unused_tags(&mut self) -> Result<OpOutput<usize>> {
+        self.transact(Op::ClearUnusedTags, |col| col.clear_unused_tags_inner())
+    }
+}
+
+impl Collection {
+    fn remove_tags_inner(&mut self, tags: &str) -> Result<usize> {
+        let usn = self.usn()?;
+
+        // gather tags that need removing
+        let mut re = TagMatcher::new(tags)?;
+        let matched_notes = self
+            .storage
+            .get_note_tags_by_predicate(|tags| re.is_match(tags))?;
+        let match_count = matched_notes.len();
+
+        // remove from the tag list
+        for tag in self.storage.get_tags_by_predicate(|tag| re.is_match(tag))? {
+            self.remove_single_tag_undoable(tag)?;
+        }
+
+        // replace tags
+        for mut note in matched_notes {
+            let original = note.clone();
+            note.tags = re.remove(&note.tags);
+            note.set_modified(usn);
+            self.update_note_tags_undoable(&note, original)?;
+        }
+
+        Ok(match_count)
+    }
+
+    fn remove_tags_from_notes_inner(&mut self, nids: &[NoteID], tags: &str) -> Result<usize> {
+        let usn = self.usn()?;
+
+        let mut re = TagMatcher::new(tags)?;
+        let mut match_count = 0;
+        let notes = self.storage.get_note_tags_by_id_list(nids)?;
+
+        for mut note in notes {
+            if !re.is_match(&note.tags) {
+                continue;
+            }
+
+            match_count += 1;
+            let original = note.clone();
+            note.tags = re.remove(&note.tags);
+            note.set_modified(usn);
+            self.update_note_tags_undoable(&note, original)?;
+        }
+
+        Ok(match_count)
+    }
+
+    fn clear_unused_tags_inner(&mut self) -> Result<usize> {
+        let mut count = 0;
+        let in_notes = self.storage.all_tags_in_notes()?;
+        let need_remove = self
+            .storage
+            .all_tags()?
+            .into_iter()
+            .filter(|tag| !in_notes.contains(&tag.name));
+        for tag in need_remove {
+            self.remove_single_tag_undoable(tag)?;
+            count += 1;
+        }
+
+        Ok(count)
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::collection::open_test_collection;
+    use crate::tags::Tag;
+
+    #[test]
+    fn clearing() -> Result<()> {
+        let mut col = open_test_collection();
+        let nt = col.get_notetype_by_name("Basic")?.unwrap();
+        let mut note = nt.new_note();
+        note.tags.push("one".into());
+        note.tags.push("two".into());
+        col.add_note(&mut note, DeckID(1))?;
+
+        col.set_tag_expanded("one", true)?;
+        col.clear_unused_tags()?;
+        assert_eq!(col.storage.get_tag("one")?.unwrap().expanded, true);
+        assert_eq!(col.storage.get_tag("two")?.unwrap().expanded, false);
+
+        // tag children are also cleared when clearing their parent
+        col.storage.clear_all_tags()?;
+        for name in vec!["a", "a::b", "A::b::c"] {
+            col.register_tag(&mut Tag::new(name.to_string(), Usn(0)))?;
+        }
+        col.remove_tags("a")?;
+        assert_eq!(col.storage.all_tags()?, vec![]);
+
+        Ok(())
+    }
+}

rslib/src/tags/rename.rs

@@ -0,0 +1,68 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+use super::{is_tag_separator, matcher::TagMatcher, Tag};
+use crate::prelude::*;
+
+impl Collection {
+    /// Rename a given tag and its children on all notes that reference it, returning changed
+    /// note count.
+    pub fn rename_tag(&mut self, old_prefix: &str, new_prefix: &str) -> Result<OpOutput<usize>> {
+        self.transact(Op::RenameTag, |col| {
+            col.rename_tag_inner(old_prefix, new_prefix)
+        })
+    }
+}
+
+impl Collection {
+    fn rename_tag_inner(&mut self, old_prefix: &str, new_prefix: &str) -> Result<usize> {
+        if new_prefix.contains(is_tag_separator) {
+            return Err(AnkiError::invalid_input(
+                "replacement name can not contain a space",
+            ));
+        }
+        if new_prefix.trim().is_empty() {
+            return Err(AnkiError::invalid_input(
+                "replacement name must not be empty",
+            ));
+        }
+
+        let usn = self.usn()?;
+
+        // match existing case if available, and ensure normalized.
+        let mut tag = Tag::new(new_prefix.to_string(), usn);
+        self.prepare_tag_for_registering(&mut tag)?;
+        let new_prefix = &tag.name;
+
+        // gather tags that need replacing
+        let mut re = TagMatcher::new(old_prefix)?;
+        let matched_notes = self
+            .storage
+            .get_note_tags_by_predicate(|tags| re.is_match(tags))?;
+        let match_count = matched_notes.len();
+        if match_count == 0 {
+            // no matches; exit early so we don't clobber the empty tag entries
+            return Ok(0);
+        }
+
+        // remove old prefix from the tag list
+        for tag in self.storage.get_tags_by_predicate(|tag| re.is_match(tag))? {
+            self.remove_single_tag_undoable(tag)?;
+        }
+
+        // replace tags
+        for mut note in matched_notes {
+            let original = note.clone();
+            note.tags = re.replace(&note.tags, new_prefix);
+            note.set_modified(usn);
+            self.update_note_tags_undoable(&note, original)?;
+        }
+
+        // update tag list
+        for tag in re.into_new_tags() {
+            self.register_tag_string(tag, usn)?;
+        }
+
+        Ok(match_count)
+    }
+}