decorator for saveNow(), mkII

Mostly @RumovZ's work from https://github.com/ankitects/anki/pull/1066,
with a workaround for the issue encountered on
6e0e17b2b9

Fix is to use pyqtSlot() to specify the slot signature, as described
on https://stackoverflow.com/questions/44371451/python-pyqt-qt-qmenu-qaction-syntax

Also renamed saveNow() for PEP8, but have not updated all the existing
calls to use the decorator yet - might be easiest to do at the same time
as perform_op() calls are added.

qt/aqt/addcards.py

@@ -107,7 +107,7 @@ class AddCards(QDialog):
         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
@@ -259,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/browser.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
+
 from __future__ import annotations
 
 import html
@@ -42,6 +43,8 @@ from aqt.utils import (
     askUser,
     current_top_level_widget,
     disable_help_button,
+    ensure_editor_saved,
+    ensure_editor_saved_on_trigger,
     getTag,
     openHelp,
     qtMenuShortcutWorkaround,
@@ -226,7 +229,7 @@ class DataModel(QAbstractTableModel):
         self.browser.mw.progress.finish()
 
     def reverse(self) -> None:
-        self.browser.editor.saveNow(self._reverse)
+        self.browser.editor.call_after_note_saved(self._reverse)
 
     def _reverse(self) -> None:
         self.beginReset()
@@ -612,7 +615,7 @@ class Browser(QMainWindow):
         if self._closeEventHasCleanedUp:
             evt.accept()
             return
-        self.editor.saveNow(self._closeWindow)
+        self.editor.call_after_note_saved(self._closeWindow)
         evt.ignore()
 
     def _closeWindow(self) -> None:
@@ -629,13 +632,11 @@ class Browser(QMainWindow):
         self.mw.deferred_delete_and_garbage_collect(self)
         self.close()
 
+    @ensure_editor_saved
     def closeWithCallback(self, onsuccess: Callable) -> None:
-        def callback() -> None:
         self._closeWindow()
         onsuccess()
 
-        self.editor.saveNow(callback)
-
     def keyPressEvent(self, evt: QKeyEvent) -> None:
         if evt.key() == Qt.Key_Escape:
             self.close()
@@ -700,10 +701,8 @@ class Browser(QMainWindow):
         self.form.searchEdit.setFocus()
 
     # search triggered by user
+    @ensure_editor_saved
     def onSearchActivated(self) -> None:
-        self.editor.saveNow(self._onSearchActivated)
-
-    def _onSearchActivated(self) -> None:
         text = self.form.searchEdit.lineEdit().text()
         try:
             normed = self.col.build_search_string(text)
@@ -773,7 +772,7 @@ class Browser(QMainWindow):
                 self.search_for(search, "")
                 self.focusCid(card.id)
 
-            self.editor.saveNow(on_show_single_card)
+            self.editor.call_after_note_saved(on_show_single_card)
 
     def onReset(self) -> None:
         self.sidebar.refresh()
@@ -832,11 +831,9 @@ QTableView {{ gridline-color: {grid} }}
         self.editor = aqt.editor.Editor(self.mw, self.form.fieldsArea, self)
         gui_hooks.editor_did_init_left_buttons.remove(add_preview_button)
 
+    @ensure_editor_saved
     def onRowChanged(self, current: QItemSelection, previous: QItemSelection) -> None:
-        "Update current note and hide/show editor."
+        """Update current note and hide/show editor."""
-        self.editor.saveNow(lambda: self._onRowChanged(current, previous))
-
-    def _onRowChanged(self, current: QItemSelection, previous: QItemSelection) -> None:
         if self._closeEventHasCleanedUp:
             return
         update = self.updateTitle()
@@ -883,11 +880,9 @@ QTableView {{ gridline-color: {grid} }}
         qconnect(hh.sortIndicatorChanged, self.onSortChanged)
         qconnect(hh.sectionMoved, self.onColumnMoved)
 
+    @ensure_editor_saved
     def onSortChanged(self, idx: int, ord: int) -> None:
-        ord_bool = bool(ord)
+        ord = bool(ord)
-        self.editor.saveNow(lambda: self._onSortChanged(idx, ord_bool))
-
-    def _onSortChanged(self, idx: int, ord: bool) -> None:
         type = self.model.activeCols[idx]
         noSort = ("question", "answer")
         if type in noSort:
@@ -935,10 +930,8 @@ QTableView {{ gridline-color: {grid} }}
         gui_hooks.browser_header_will_show_context_menu(self, m)
         m.exec_(gpos)
 
+    @ensure_editor_saved_on_trigger
     def toggleField(self, type: str) -> None:
-        self.editor.saveNow(lambda: self._toggleField(type))
-
-    def _toggleField(self, type: str) -> None:
         self.model.beginReset()
         if type in self.model.activeCols:
             if len(self.model.activeCols) < 2:
@@ -1115,10 +1108,8 @@ where id in %s"""
     # Misc menu options
     ######################################################################
 
+    @ensure_editor_saved_on_trigger
     def onChangeModel(self) -> None:
-        self.editor.saveNow(self._onChangeModel)
-
-    def _onChangeModel(self) -> None:
         nids = self.oneModelNotes()
         if nids:
             ChangeModel(self, nids)
@@ -1192,6 +1183,7 @@ where id in %s"""
     # Deck change
     ######################################################################
 
+    @ensure_editor_saved_on_trigger
     def set_deck_of_selected_cards(self) -> None:
         from aqt.studydeck import StudyDeck
 
@@ -1222,38 +1214,30 @@ where id in %s"""
     # Tags
     ######################################################################
 
+    @ensure_editor_saved_on_trigger
     def add_tags_to_selected_notes(
         self,
         tags: Optional[str] = None,
     ) -> None:
         "Shows prompt if tags not provided."
-
-        def op() -> None:
         if not (
-                tags2 := self.maybe_prompt_for_tags(
+            tags := self.maybe_prompt_for_tags(tags, tr(TR.BROWSING_ENTER_TAGS_TO_ADD))
-                    tags, tr(TR.BROWSING_ENTER_TAGS_TO_ADD)
-                )
         ):
             return
-            nids = self.selectedNotes()
+        add_tags(mw=self.mw, note_ids=self.selectedNotes(), space_separated_tags=tags)
-            add_tags(mw=self.mw, note_ids=nids, space_separated_tags=tags2)
-
-        self.editor.saveNow(op)
 
+    @ensure_editor_saved_on_trigger
     def remove_tags_from_selected_notes(self, tags: Optional[str] = None) -> None:
         "Shows prompt if tags not provided."
-
-        def op() -> None:
         if not (
-                tags2 := self.maybe_prompt_for_tags(
+            tags := self.maybe_prompt_for_tags(
                 tags, tr(TR.BROWSING_ENTER_TAGS_TO_DELETE)
             )
         ):
             return
-            nids = self.selectedNotes()
+        remove_tags(
-            remove_tags(mw=self.mw, note_ids=nids, space_separated_tags=tags2)
+            mw=self.mw, note_ids=self.selectedNotes(), space_separated_tags=tags
-
+        )
-        self.editor.saveNow(op)
 
     def _maybe_prompt_for_tags(self, tags: Optional[str], prompt: str) -> Optional[str]:
         if tags is not None:
@@ -1265,10 +1249,8 @@ where id in %s"""
         else:
             return tags
 
+    @ensure_editor_saved_on_trigger
     def clearUnusedTags(self) -> None:
-        self.editor.saveNow(self._clearUnusedTags)
-
-    def _clearUnusedTags(self) -> None:
         def on_done(fut: Future) -> None:
             fut.result()
             self.on_tag_list_update()
@@ -1284,10 +1266,8 @@ where id in %s"""
     def current_card_is_suspended(self) -> bool:
         return bool(self.card and self.card.queue == QUEUE_TYPE_SUSPENDED)
 
+    @ensure_editor_saved_on_trigger
     def suspend_selected_cards(self) -> None:
-        self.editor.saveNow(self._suspend_selected_cards)
-
-    def _suspend_selected_cards(self) -> None:
         want_suspend = not self.current_card_is_suspended()
         cids = self.selectedCards()
 
@@ -1310,7 +1290,7 @@ where id in %s"""
     def onSetFlag(self, n: int) -> None:
         if not self.card:
             return
-        self.editor.saveNow(lambda: self._on_set_flag(n))
+        self.editor.call_after_note_saved(lambda: self._on_set_flag(n))
 
     def _on_set_flag(self, flag: int) -> None:
         # flag needs toggling off?
@@ -1351,10 +1331,8 @@ where id in %s"""
     # Repositioning
     ######################################################################
 
+    @ensure_editor_saved_on_trigger
     def reposition(self) -> None:
-        self.editor.saveNow(self._reposition)
-
-    def _reposition(self) -> None:
         cids = self.selectedCards()
         cids2 = self.col.db.list(
             f"select id from cards where type = {CARD_TYPE_NEW} and id in "
@@ -1395,32 +1373,28 @@ where id in %s"""
     # Scheduling
     ######################################################################
 
+    @ensure_editor_saved_on_trigger
     def set_due_date(self) -> None:
-        self.editor.saveNow(
+        set_due_date_dialog(
-            lambda: set_due_date_dialog(
             mw=self.mw,
             parent=self,
             card_ids=self.selectedCards(),
             config_key=Config.String.SET_DUE_BROWSER,
         )
-        )
 
+    @ensure_editor_saved_on_trigger
     def forget_cards(self) -> None:
-        self.editor.saveNow(
+        forget_cards(
-            lambda: forget_cards(
             mw=self.mw,
             parent=self,
             card_ids=self.selectedCards(),
         )
-        )
 
     # Edit: selection
     ######################################################################
 
+    @ensure_editor_saved_on_trigger
     def selectNotes(self) -> None:
-        self.editor.saveNow(self._selectNotes)
-
-    def _selectNotes(self) -> None:
         nids = self.selectedNotes()
         # clear the selection so we don't waste energy preserving it
         tv = self.form.tableView
@@ -1484,10 +1458,8 @@ where id in %s"""
     # Edit: replacing
     ######################################################################
 
+    @ensure_editor_saved_on_trigger
     def onFindReplace(self) -> None:
-        self.editor.saveNow(self._onFindReplace)
-
-    def _onFindReplace(self) -> None:
         nids = self.selectedNotes()
         if not nids:
             return
@@ -1560,10 +1532,8 @@ where id in %s"""
     # Edit: finding dupes
     ######################################################################
 
+    @ensure_editor_saved
     def onFindDupes(self) -> None:
-        self.editor.saveNow(self._onFindDupes)
-
-    def _onFindDupes(self) -> None:
         d = QDialog(self)
         self.mw.garbage_collect_on_dialog_finish(d)
         frm = aqt.forms.finddupes.Ui_Dialog()
@@ -1682,14 +1652,14 @@ where id in %s"""
 
     def onPreviousCard(self) -> None:
         self.focusTo = self.editor.currentField
-        self.editor.saveNow(self._onPreviousCard)
+        self.editor.call_after_note_saved(self._onPreviousCard)
 
     def _onPreviousCard(self) -> None:
         self._moveCur(QAbstractItemView.MoveUp)
 
     def onNextCard(self) -> None:
         self.focusTo = self.editor.currentField
-        self.editor.saveNow(self._onNextCard)
+        self.editor.call_after_note_saved(self._onNextCard)
 
     def _onNextCard(self) -> None:
         self._moveCur(QAbstractItemView.MoveDown)

qt/aqt/editcurrent.py

@@ -62,7 +62,7 @@ class EditCurrent(QDialog):
         self.saveAndClose()
 
     def saveAndClose(self) -> None:
-        self.editor.saveNow(self._saveAndClose)
+        self.editor.call_after_note_saved(self._saveAndClose)
 
     def _saveAndClose(self) -> None:
         self.cleanup_and_close()
@@ -72,6 +72,6 @@ class EditCurrent(QDialog):
             self._saveAndClose()
             onsuccess()
 
-        self.editor.saveNow(callback)
+        self.editor.call_after_note_saved(callback)
 
     onReset = on_operation_did_execute

qt/aqt/editor.py

@@ -409,7 +409,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
@@ -417,7 +417,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
@@ -568,7 +568,9 @@ 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
@@ -577,6 +579,8 @@ class Editor:
         self.saveTags()
         self.web.evalWithCallback("saveNow(%d)" % keepFocus, lambda res: callback())
 
+    saveNow = call_after_note_saved
+
     def checkValid(self) -> None:
         cols = [""] * len(self.note.fields)
         err = self.note.duplicate_or_empty()
@@ -626,7 +630,7 @@ class Editor:
 
     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)
@@ -732,7 +736,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

qt/aqt/previewer.py

@@ -317,12 +317,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/sidebar.py

@@ -618,7 +618,7 @@ 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(
@@ -650,7 +650,7 @@ class SidebarTreeView(QTreeView):
                 lambda: self.col.tags.drag_drop(source_ids, target_name), on_done
             )
 
-        self.browser.editor.saveNow(on_save)
+        self.browser.editor.call_after_note_saved(on_save)
         return True
 
     def _on_search(self, index: QModelIndex) -> None:
@@ -1187,10 +1187,7 @@ class SidebarTreeView(QTreeView):
     # Tags
     ###########################
 
-    def remove_tags(self, item: SidebarItem) -> None:
+    def remove_tags(self, _item: SidebarItem) -> None:
-        self.browser.editor.saveNow(lambda: self._remove_tags(item))
-
-    def _remove_tags(self, _item: SidebarItem) -> None:
         tags = self._selected_tags()
 
         def do_remove() -> int:
@@ -1211,7 +1208,9 @@ class SidebarTreeView(QTreeView):
         if new_name and new_name != item.name:
             # block repainting until collection is updated
             self.setUpdatesEnabled(False)
-            self.browser.editor.saveNow(lambda: self._rename_tag(item, new_name))
+            self.browser.editor.call_after_note_saved(
+                lambda: self._rename_tag(item, new_name)
+            )
 
     def _rename_tag(self, item: SidebarItem, new_name: str) -> None:
         old_name = item.full_name

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,
@@ -988,3 +989,29 @@ 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