From b8fc195cdfd5cc5a396ca23eada3e548cbb869a8 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Tue, 6 Apr 2021 12:47:55 +1000 Subject: [PATCH] start migrating perform_op() into builder in separate file By passing back the builder to the calling code to run, we don't need to plumb extra arguments like success= and handler= through each operation, and the ability to override the default tooltip behaviour comes free on all operations --- qt/aqt/browser.py | 21 ++---- qt/aqt/main.py | 35 +++------ qt/aqt/operations/__init__.py | 131 ++++++++++++++++++++++++++++++++++ qt/aqt/operations/tag.py | 84 ++++++++++------------ qt/aqt/reviewer.py | 14 ++-- qt/aqt/sidebar.py | 13 ++-- qt/tools/genhooks_gui.py | 1 - 7 files changed, 196 insertions(+), 103 deletions(-) diff --git a/qt/aqt/browser.py b/qt/aqt/browser.py index 2fc5ba1e2..d4eadfaee 100644 --- a/qt/aqt/browser.py +++ b/qt/aqt/browser.py @@ -695,13 +695,8 @@ where id in %s""" if not (tags := tags or self._prompt_for_tags(tr.browsing_enter_tags_to_add())): return add_tags_to_notes( - mw=self.mw, - note_ids=self.selected_notes(), - space_separated_tags=tags, - success=lambda out: tooltip( - tr.browsing_notes_updated(count=out.count), parent=self - ), - ) + parent=self, note_ids=self.selected_notes(), space_separated_tags=tags + ).run(handler=self) @ensure_editor_saved_on_trigger def remove_tags_from_selected_notes(self, tags: Optional[str] = None) -> None: @@ -710,14 +705,10 @@ where id in %s""" tags := tags or self._prompt_for_tags(tr.browsing_enter_tags_to_delete()) ): return + remove_tags_from_notes( - mw=self.mw, - note_ids=self.selected_notes(), - space_separated_tags=tags, - success=lambda out: tooltip( - tr.browsing_notes_updated(count=out.count), parent=self - ), - ) + parent=self, note_ids=self.selected_notes(), space_separated_tags=tags + ).run(handler=self) def _prompt_for_tags(self, prompt: str) -> Optional[str]: (tags, ok) = getTag(self, self.col, prompt) @@ -728,7 +719,7 @@ where id in %s""" @ensure_editor_saved_on_trigger def clear_unused_tags(self) -> None: - clear_unused_tags(mw=self.mw, parent=self) + clear_unused_tags(parent=self).run() addTags = add_tags_to_selected_notes deleteTags = remove_tags_from_selected_notes diff --git a/qt/aqt/main.py b/qt/aqt/main.py index be5f4bf7a..7dedba7dd 100644 --- a/qt/aqt/main.py +++ b/qt/aqt/main.py @@ -61,6 +61,11 @@ from aqt.emptycards import show_empty_cards from aqt.legacy import install_pylib_legacy from aqt.mediacheck import check_media_db from aqt.mediasync import MediaSyncer +from aqt.operations import ( + CollectionOpFailureCallback, + CollectionOpSuccessCallback, + ResultWithChanges, +) from aqt.operations.collection import undo from aqt.profiles import ProfileManager as ProfileManagerType from aqt.qt import * @@ -91,30 +96,6 @@ from aqt.utils import ( 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, - OpChangesWithId, - OpChangesAfterUndo, - HasChangesProperty, - ], -) - -T = TypeVar("T") - -PerformOpOptionalSuccessCallback = Optional[Callable[[ResultWithChanges], Any]] -PerformOpOptionalFailureCallback = Optional[Callable[[Exception], Any]] - install_pylib_legacy() MainWindowState = Literal[ @@ -122,6 +103,12 @@ MainWindowState = Literal[ ] +T = TypeVar("T") + +PerformOpOptionalSuccessCallback = Optional[CollectionOpSuccessCallback] +PerformOpOptionalFailureCallback = Optional[CollectionOpFailureCallback] + + class AnkiQt(QMainWindow): col: Collection pm: ProfileManagerType diff --git a/qt/aqt/operations/__init__.py b/qt/aqt/operations/__init__.py index e3d8b5638..cb7b4b5d9 100644 --- a/qt/aqt/operations/__init__.py +++ b/qt/aqt/operations/__init__.py @@ -1,2 +1,133 @@ # 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._base import Future +from typing import Any, Callable, Generic, Optional, Protocol, TypeVar, Union + +import aqt +from anki.collection import ( + Collection, + OpChanges, + OpChangesAfterUndo, + OpChangesWithCount, + OpChangesWithId, +) +from aqt.qt import QWidget +from aqt.utils import showWarning + + +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, + OpChangesWithId, + OpChangesAfterUndo, + HasChangesProperty, + ], +) + +T = TypeVar("T") + +CollectionOpSuccessCallback = Callable[[ResultWithChanges], Any] +CollectionOpFailureCallback = Optional[Callable[[Exception], Any]] + + +class CollectionOp(Generic[ResultWithChanges]): + """Helper to perform a mutating DB operation on a background thread, and update UI. + + `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. + """ + + _success: Optional[CollectionOpSuccessCallback] = None + _failure: Optional[CollectionOpFailureCallback] = None + + def __init__(self, parent: QWidget, op: Callable[[Collection], ResultWithChanges]): + self._parent = parent + self._op = op + + def success(self, success: Optional[CollectionOpSuccessCallback]) -> CollectionOp: + self._success = success + return self + + def failure(self, failure: Optional[CollectionOpFailureCallback]) -> CollectionOp: + self._failure = failure + return self + + def run(self, *, handler: Optional[object] = None) -> None: + aqt.mw._increase_background_ops() + + def wrapped_op() -> ResultWithChanges: + return self._op(aqt.mw.col) + + def wrapped_done(future: Future) -> None: + aqt.mw._decrease_background_ops() + # did something go wrong? + if exception := future.exception(): + if isinstance(exception, Exception): + if self._failure: + self._failure(exception) + else: + showWarning(str(exception), self._parent) + return + else: + # BaseException like SystemExit; rethrow it + future.result() + + result = future.result() + try: + if self._success: + self._success(result) + finally: + # update undo status + status = aqt.mw.col.undo_status() + aqt.mw._update_undo_actions_for_status_and_save(status) + # fire change hooks + self._fire_change_hooks_after_op_performed(result, handler) + + aqt.mw.taskman.with_progress(wrapped_op, wrapped_done) + + def _fire_change_hooks_after_op_performed( + self, + result: ResultWithChanges, + handler: Optional[object], + ) -> None: + if isinstance(result, OpChanges): + changes = result + else: + changes = result.changes + + # fire new hook + print("op changes:") + print(changes) + aqt.gui_hooks.operation_did_execute(changes, handler) + # fire legacy hook so old code notices changes + if aqt.mw.col.op_made_changes(changes): + aqt.gui_hooks.state_did_reset() diff --git a/qt/aqt/operations/tag.py b/qt/aqt/operations/tag.py index 0f22ff4cb..6c2ea7611 100644 --- a/qt/aqt/operations/tag.py +++ b/qt/aqt/operations/tag.py @@ -3,94 +3,88 @@ from __future__ import annotations -from typing import Optional, Sequence +from typing import Sequence from anki.collection import OpChangesWithCount from anki.notes import NoteId from aqt import AnkiQt, QWidget -from aqt.main import PerformOpOptionalSuccessCallback +from aqt.operations import CollectionOp from aqt.utils import showInfo, tooltip, tr def add_tags_to_notes( *, - mw: AnkiQt, + parent: QWidget, note_ids: Sequence[NoteId], space_separated_tags: str, - success: PerformOpOptionalSuccessCallback = None, - handler: Optional[object] = None, -) -> None: - mw.perform_op( - lambda: mw.col.tags.bulk_add(note_ids, space_separated_tags), - success=success, - handler=handler, +) -> CollectionOp: + return CollectionOp( + parent, lambda col: col.tags.bulk_add(note_ids, space_separated_tags) + ).success( + lambda out: tooltip(tr.browsing_notes_updated(count=out.count), parent=parent) ) def remove_tags_from_notes( *, - mw: AnkiQt, + parent: QWidget, note_ids: Sequence[NoteId], space_separated_tags: str, - success: PerformOpOptionalSuccessCallback = None, - handler: Optional[object] = None, -) -> None: - mw.perform_op( - lambda: mw.col.tags.bulk_remove(note_ids, space_separated_tags), - success=success, - handler=handler, +) -> CollectionOp: + return CollectionOp( + parent, lambda col: col.tags.bulk_remove(note_ids, space_separated_tags) + ).success( + lambda out: tooltip(tr.browsing_notes_updated(count=out.count), parent=parent) ) -def clear_unused_tags(*, mw: AnkiQt, parent: QWidget) -> None: - mw.perform_op( - mw.col.tags.clear_unused_tags, - success=lambda out: tooltip( +def clear_unused_tags(*, parent: QWidget) -> CollectionOp: + return CollectionOp(parent, lambda col: col.tags.clear_unused_tags()).success( + lambda out: tooltip( tr.browsing_removed_unused_tags_count(count=out.count), parent=parent - ), + ) ) def rename_tag( *, - mw: AnkiQt, parent: QWidget, current_name: str, new_name: str, -) -> None: +) -> CollectionOp: def success(out: OpChangesWithCount) -> None: if out.count: tooltip(tr.browsing_notes_updated(count=out.count), parent=parent) else: showInfo(tr.browsing_tag_rename_warning_empty(), parent=parent) - mw.perform_op( - lambda: mw.col.tags.rename(old=current_name, new=new_name), - success=success, - ) + return CollectionOp( + parent, + lambda col: col.tags.rename(old=current_name, new=new_name), + ).success(success) def remove_tags_from_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.browsing_notes_updated(count=out.count), parent=parent - ), + *, parent: QWidget, space_separated_tags: str +) -> CollectionOp: + return CollectionOp( + parent, lambda col: col.tags.remove(space_separated_tags=space_separated_tags) + ).success( + lambda out: tooltip(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.browsing_notes_updated(count=out.count), parent=parent - ), + *, parent: QWidget, tags: Sequence[str], new_parent: str +) -> CollectionOp: + return CollectionOp( + parent, lambda col: col.tags.reparent(tags=tags, new_parent=new_parent) + ).success( + lambda out: tooltip(tr.browsing_notes_updated(count=out.count), parent=parent) ) -def set_tag_collapsed(*, mw: AnkiQt, tag: str, collapsed: bool) -> None: - mw.perform_op(lambda: mw.col.tags.set_collapsed(tag=tag, collapsed=collapsed)) +def set_tag_collapsed(*, parent: QWidget, tag: str, collapsed: bool) -> CollectionOp: + return CollectionOp( + parent, lambda col: col.tags.set_collapsed(tag=tag, collapsed=collapsed) + ) diff --git a/qt/aqt/reviewer.py b/qt/aqt/reviewer.py index 3f665d1a4..cebbf24a3 100644 --- a/qt/aqt/reviewer.py +++ b/qt/aqt/reviewer.py @@ -851,20 +851,14 @@ time = %(time)d; note = self.card.note() if note.has_tag(MARKED_TAG): remove_tags_from_notes( - mw=self.mw, - note_ids=[note.id], - space_separated_tags=MARKED_TAG, - handler=self, - success=redraw_mark, - ) + parent=self.mw, note_ids=[note.id], space_separated_tags=MARKED_TAG + ).success(redraw_mark).run(handler=self) else: add_tags_to_notes( - mw=self.mw, + parent=self.mw, note_ids=[note.id], space_separated_tags=MARKED_TAG, - handler=self, - success=redraw_mark, - ) + ).success(redraw_mark).run(handler=self) def on_set_due(self) -> None: if self.mw.state != "review" or not self.card: diff --git a/qt/aqt/sidebar.py b/qt/aqt/sidebar.py index 22bec6766..77cf27a75 100644 --- a/qt/aqt/sidebar.py +++ b/qt/aqt/sidebar.py @@ -652,7 +652,7 @@ class SidebarTreeView(QTreeView): else: new_parent = target.full_name - reparent_tags(mw=self.mw, parent=self.browser, tags=tags, new_parent=new_parent) + reparent_tags(parent=self.browser, tags=tags, new_parent=new_parent).run() return True @@ -947,8 +947,8 @@ class SidebarTreeView(QTreeView): def toggle_expand(node: TagTreeNode) -> Callable[[bool], None]: full_name = head + node.name return lambda expanded: set_tag_collapsed( - mw=self.mw, tag=full_name, collapsed=not expanded - ) + parent=self, tag=full_name, collapsed=not expanded + ).run() for node in nodes: item = SidebarItem( @@ -1209,9 +1209,7 @@ class SidebarTreeView(QTreeView): tags = self.mw.col.tags.join(self._selected_tags()) item.name = "..." - remove_tags_from_all_notes( - mw=self.mw, parent=self.browser, space_separated_tags=tags - ) + remove_tags_from_all_notes(parent=self.browser, space_separated_tags=tags).run() def rename_tag(self, item: SidebarItem, new_name: str) -> None: if not new_name or new_name == item.name: @@ -1226,11 +1224,10 @@ class SidebarTreeView(QTreeView): item.full_name = new_name rename_tag( - mw=self.mw, parent=self.browser, current_name=old_name, new_name=new_name, - ) + ).run() # Saved searches #################################### diff --git a/qt/tools/genhooks_gui.py b/qt/tools/genhooks_gui.py index a939112cf..f4b50570f 100644 --- a/qt/tools/genhooks_gui.py +++ b/qt/tools/genhooks_gui.py @@ -26,7 +26,6 @@ from anki.hooks import runFilter, runHook from anki.models import NotetypeDict from aqt.qt import QDialog, QEvent, QMenu, QWidget from aqt.tagedit import TagEdit -import aqt.operations """ # Hook list