From 9baa8530d57933f42d887351328c228339d9f3b4 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Fri, 22 May 2020 10:47:14 +1000 Subject: [PATCH] move deck/notetype update hooks to gui We need to migrate away from firing hooks in libanki, since libanki methods may be running on a background thread, and hook consumers typically expect the code to run in the main thread. We could document it, but it would frequently be forgotten about, and could lead to crashes. https://anki.tenderapp.com/discussions/ankidesktop/41748-qobject-cannot-create-children-for-a-parent-that-is-in-a-different-thread-when-hitting-the-save-button-on-clayoutpy-window --- pylib/anki/decks.py | 3 --- pylib/anki/hooks.py | 8 +++---- pylib/anki/models.py | 4 ---- pylib/tools/genhooks.py | 21 ++++++++---------- qt/aqt/browser.py | 11 +++++---- qt/aqt/clayout.py | 1 + qt/aqt/deckbrowser.py | 3 +++ qt/aqt/gui_hooks.py | 48 ++++++++++++++++++++++++++++++++++++++++ qt/tools/genhooks_gui.py | 2 ++ 9 files changed, 72 insertions(+), 29 deletions(-) diff --git a/pylib/anki/decks.py b/pylib/anki/decks.py index e4112cc6e..846e0c333 100644 --- a/pylib/anki/decks.py +++ b/pylib/anki/decks.py @@ -8,7 +8,6 @@ from typing import Any, Dict, Iterable, List, Optional, Sequence, Tuple, Union import anki # pylint: disable=unused-import import anki.backend_pb2 as pb -from anki import hooks from anki.consts import * from anki.errors import DeckRenameError from anki.lang import _ @@ -96,8 +95,6 @@ class DeckManager: deck["name"] = name self.update(deck, preserve_usn=False) - hooks.deck_added(deck) - return deck["id"] def rem(self, did: int, cardsToo: bool = True, childrenToo: bool = True) -> None: diff --git a/pylib/anki/hooks.py b/pylib/anki/hooks.py index 9e78dbf71..90c8f0c4f 100644 --- a/pylib/anki/hooks.py +++ b/pylib/anki/hooks.py @@ -188,6 +188,8 @@ card_will_flush = _CardWillFlushHook() class _DeckAddedHook: + """Obsolete, do not use.""" + _hooks: List[Callable[[Dict[str, Any]], None]] = [] def append(self, cb: Callable[[Dict[str, Any]], None]) -> None: @@ -206,8 +208,6 @@ class _DeckAddedHook: # if the hook fails, remove it self._hooks.remove(hook) raise - # legacy support - runHook("newDeck") deck_added = _DeckAddedHook() @@ -306,6 +306,8 @@ media_files_did_export = _MediaFilesDidExportHook() class _NoteTypeAddedHook: + """Obsolete, do not use.""" + _hooks: List[Callable[[Dict[str, Any]], None]] = [] def append(self, cb: Callable[[Dict[str, Any]], None]) -> None: @@ -324,8 +326,6 @@ class _NoteTypeAddedHook: # if the hook fails, remove it self._hooks.remove(hook) raise - # legacy support - runHook("newModel") note_type_added = _NoteTypeAddedHook() diff --git a/pylib/anki/models.py b/pylib/anki/models.py index a23852552..56e015aa2 100644 --- a/pylib/anki/models.py +++ b/pylib/anki/models.py @@ -9,7 +9,6 @@ from typing import Any, Dict, List, Optional, Tuple, Union import anki # pylint: disable=unused-import import anki.backend_pb2 as pb -from anki import hooks from anki.consts import * from anki.lang import _ from anki.rsbackend import StockNoteType @@ -81,9 +80,6 @@ class ModelManager: self.update(m, preserve_usn=False) - # fixme: badly named; also fires on updates - hooks.note_type_added(m) - # legacy def flush(self) -> None: pass diff --git a/pylib/tools/genhooks.py b/pylib/tools/genhooks.py index e90d5d954..6714706aa 100644 --- a/pylib/tools/genhooks.py +++ b/pylib/tools/genhooks.py @@ -8,6 +8,8 @@ To add a new hook: - update the hooks list below - run 'make develop' - send a pull request that includes the changes to this file and hooks.py + +In most cases, hooks are better placed in genhooks_gui.py """ import os @@ -25,24 +27,12 @@ hooks = [ args=["col: anki.collection.Collection", "ids: List[int]"], legacy_hook="remNotes", ), - Hook( - name="deck_added", - args=["deck: Dict[str, Any]"], - legacy_hook="newDeck", - legacy_no_args=True, - ), Hook(name="media_files_did_export", args=["count: int"]), Hook( name="exporters_list_created", args=["exporters: List[Tuple[str, Any]]"], legacy_hook="exportersList", ), - Hook( - name="note_type_added", - args=["notetype: Dict[str, Any]"], - legacy_hook="newModel", - legacy_no_args=True, - ), Hook(name="sync_stage_did_change", args=["stage: str"], legacy_hook="sync"), Hook(name="sync_progress_did_change", args=["msg: str"], legacy_hook="syncMsg"), Hook( @@ -101,6 +91,13 @@ hooks = [ doc="""Allows changing the number of rev card for this deck (without considering descendants).""", ), + # obsolete + Hook(name="deck_added", args=["deck: Dict[str, Any]"], doc="Obsolete, do not use."), + Hook( + name="note_type_added", + args=["notetype: Dict[str, Any]"], + doc="Obsolete, do not use.", + ), ] if __name__ == "__main__": diff --git a/qt/aqt/browser.py b/qt/aqt/browser.py index 9d7422f9f..f10e59d7e 100644 --- a/qt/aqt/browser.py +++ b/qt/aqt/browser.py @@ -14,7 +14,6 @@ from typing import Callable, List, Optional, Sequence, Union import anki import aqt import aqt.forms -from anki import hooks from anki.cards import Card from anki.collection import Collection from anki.consts import * @@ -1879,8 +1878,8 @@ update cards set usn=?, mod=?, did=? where id in """ gui_hooks.editor_did_fire_typing_timer.append(self.refreshCurrentCard) gui_hooks.editor_did_load_note.append(self.onLoadNote) gui_hooks.editor_did_unfocus_field.append(self.on_unfocus_field) - hooks.note_type_added.append(self.on_item_added) - hooks.deck_added.append(self.on_item_added) + gui_hooks.sidebar_should_refresh_decks.append(self.on_item_added) + gui_hooks.sidebar_should_refresh_notetypes.append(self.on_item_added) def teardownHooks(self) -> None: gui_hooks.undo_state_did_change.remove(self.onUndoState) @@ -1888,14 +1887,14 @@ update cards set usn=?, mod=?, did=? where id in """ gui_hooks.editor_did_fire_typing_timer.remove(self.refreshCurrentCard) gui_hooks.editor_did_load_note.remove(self.onLoadNote) gui_hooks.editor_did_unfocus_field.remove(self.on_unfocus_field) - hooks.note_type_added.remove(self.on_item_added) - hooks.deck_added.remove(self.on_item_added) + gui_hooks.sidebar_should_refresh_decks.remove(self.on_item_added) + gui_hooks.sidebar_should_refresh_notetypes.remove(self.on_item_added) def on_unfocus_field(self, changed: bool, note: Note, field_idx: int) -> None: self.refreshCurrentCard(note) # covers the tag, note and deck case - def on_item_added(self, item: Any) -> None: + def on_item_added(self, item: Any = None) -> None: self.maybeRefreshSidebar() def on_tag_list_update(self): diff --git a/qt/aqt/clayout.py b/qt/aqt/clayout.py index dcfd39a7b..7eed73c2c 100644 --- a/qt/aqt/clayout.py +++ b/qt/aqt/clayout.py @@ -749,6 +749,7 @@ Enter deck to place new %s cards in, or leave blank:""" self.mw.reset() tooltip("Changes saved.", parent=self.parent()) self.cleanup() + gui_hooks.sidebar_should_refresh_notetypes() return QDialog.accept(self) self.mw.taskman.with_progress(save, on_done) diff --git a/qt/aqt/deckbrowser.py b/qt/aqt/deckbrowser.py index 73c9e05d6..b67463ff1 100644 --- a/qt/aqt/deckbrowser.py +++ b/qt/aqt/deckbrowser.py @@ -82,6 +82,7 @@ class DeckBrowser: deck = getOnlyText(_("Name for deck:")) if deck: self.mw.col.decks.id(deck) + gui_hooks.sidebar_should_refresh_decks() self.refresh() elif cmd == "drag": draggedDeckDid, ontoDeckDid = arg.split(",") @@ -256,6 +257,7 @@ where id > ?""", return try: self.mw.col.decks.rename(deck, newName) + gui_hooks.sidebar_should_refresh_decks() except DeckRenameError as e: return showWarning(e.description) self.show() @@ -276,6 +278,7 @@ where id > ?""", def _dragDeckOnto(self, draggedDeckDid, ontoDeckDid): try: self.mw.col.decks.renameForDragAndDrop(draggedDeckDid, ontoDeckDid) + gui_hooks.sidebar_should_refresh_decks() except DeckRenameError as e: return showWarning(e.description) diff --git a/qt/aqt/gui_hooks.py b/qt/aqt/gui_hooks.py index b6f1bd093..6fb3b7125 100644 --- a/qt/aqt/gui_hooks.py +++ b/qt/aqt/gui_hooks.py @@ -1774,6 +1774,54 @@ class _ReviewerWillShowContextMenuHook: reviewer_will_show_context_menu = _ReviewerWillShowContextMenuHook() +class _SidebarShouldRefreshDecksHook: + _hooks: List[Callable[[], None]] = [] + + def append(self, cb: Callable[[], None]) -> None: + """()""" + self._hooks.append(cb) + + def remove(self, cb: Callable[[], None]) -> None: + if cb in self._hooks: + self._hooks.remove(cb) + + def __call__(self) -> None: + for hook in self._hooks: + try: + hook() + except: + # if the hook fails, remove it + self._hooks.remove(hook) + raise + + +sidebar_should_refresh_decks = _SidebarShouldRefreshDecksHook() + + +class _SidebarShouldRefreshNotetypesHook: + _hooks: List[Callable[[], None]] = [] + + def append(self, cb: Callable[[], None]) -> None: + """()""" + self._hooks.append(cb) + + def remove(self, cb: Callable[[], None]) -> None: + if cb in self._hooks: + self._hooks.remove(cb) + + def __call__(self) -> None: + for hook in self._hooks: + try: + hook() + except: + # if the hook fails, remove it + self._hooks.remove(hook) + raise + + +sidebar_should_refresh_notetypes = _SidebarShouldRefreshNotetypesHook() + + class _StateDidChangeHook: _hooks: List[Callable[[str, str], None]] = [] diff --git a/qt/tools/genhooks_gui.py b/qt/tools/genhooks_gui.py index 0498c915a..806442eaa 100644 --- a/qt/tools/genhooks_gui.py +++ b/qt/tools/genhooks_gui.py @@ -544,6 +544,8 @@ hooks = [ legacy_hook="currentModelChanged", legacy_no_args=True, ), + Hook(name="sidebar_should_refresh_decks"), + Hook(name="sidebar_should_refresh_notetypes"), Hook( name="deck_browser_will_show_options_menu", args=["menu: QMenu", "deck_id: int"],