From 1e849316beedc3406a27fbedb3f7abdf8c4a5733 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sun, 14 Mar 2021 19:54:15 +1000 Subject: [PATCH] more reset refactoring 'card modified' covers the common case where we need to rebuild the study queue, but is also set when changing the card flags. We want to avoid a queue rebuild in that case, as it causes UI flicker, and may result in a different card being shown. Note marking doesn't trigger a queue build, but still causes flicker, and may return the user back to the front side when they were looking at the answer. I still think entity-based change tracking is the simplest in the common case, but to solve the above, I've introduced an enum describing the last operation that was taken. This currently is not trying to list out all possible operations, and just describes the ones we want to special-case. Other changes: - Fire the old 'state_did_reset' hook after an operation is performed, so legacy code can refresh itself after an operation is performed. - Fire the new `operation_did_execute` hook when mw.reset() is called, so that as the UI is updated to the use the new hook, it will still be able to refresh after legacy code calls mw.reset() - Update the deck browser, overview and review screens to listen to the new hook, instead of relying on the main window to call moveToState() - Add a 'set flag' backend action, so we can distinguish it from a normal card update. - Drop the separate added/modified entries in the change list in favour of a single entry per entity. - Add typing to mw.state - Tweak perform_op() - Convert a few more actions to use perform_op() --- ftl/core/undo.ftl | 1 + pylib/anki/cards.py | 3 +- pylib/anki/collection.py | 22 ++-- qt/.pylintrc | 1 + qt/aqt/browser.py | 53 +++------- qt/aqt/deckbrowser.py | 9 ++ qt/aqt/main.py | 82 +++++++++++---- qt/aqt/overview.py | 10 ++ qt/aqt/reviewer.py | 97 ++++++++++------- qt/aqt/scheduling.py | 53 +++------- qt/tools/genhooks_gui.py | 13 ++- rslib/backend.proto | 38 ++++--- rslib/src/backend/card.rs | 11 ++ rslib/src/backend/collection.rs | 6 +- rslib/src/backend/notes.rs | 8 +- rslib/src/backend/ops.rs | 52 +++++++--- rslib/src/card/mod.rs | 27 +++++ rslib/src/ops.rs | 179 ++++++++++++++++---------------- rslib/src/undo/mod.rs | 28 ++--- 19 files changed, 397 insertions(+), 296 deletions(-) diff --git a/ftl/core/undo.ftl b/ftl/core/undo.ftl index c2e95f1e9..61d60d945 100644 --- a/ftl/core/undo.ftl +++ b/ftl/core/undo.ftl @@ -18,3 +18,4 @@ undo-update-note = Update Note undo-update-card = Update Card undo-update-deck = Update Deck undo-forget-card = Forget Card +undo-set-flag = Set Flag diff --git a/pylib/anki/cards.py b/pylib/anki/cards.py index e08bb46f3..7d9a4a20a 100644 --- a/pylib/anki/cards.py +++ b/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 diff --git a/pylib/anki/collection.py b/pylib/anki/collection.py index dcd4d2fd6..e0d1b3ebe 100644 --- a/pylib/anki/collection.py +++ b/pylib/anki/collection.py @@ -54,7 +54,7 @@ GraphPreferences = _pb.GraphPreferences BuiltinSort = _pb.SortOrder.Builtin Preferences = _pb.Preferences UndoStatus = _pb.UndoStatus -StateChanges = _pb.StateChanges +OperationInfo = _pb.OperationInfo DefaultsForAdding = _pb.DeckAndNotetype @@ -783,8 +783,6 @@ table.review-log {{ {revlog_style} }} assert_exhaustive(self._undo) assert False - return status - def clear_python_undo(self) -> None: """Clear the Python undo state. The backend will automatically clear backend undo state when @@ -812,6 +810,11 @@ table.review-log {{ {revlog_style} }} assert_exhaustive(self._undo) assert False + def op_affects_study_queue(self, op: OperationInfo) -> bool: + if op.kind == op.SET_CARD_FLAG: + return False + return op.changes.card or op.changes.deck or op.changes.preference + def _check_backend_undo_status(self) -> Optional[UndoStatus]: """Return undo status if undo available on backend. If backend has undo available, clear the Python undo state.""" @@ -981,21 +984,10 @@ table.review-log {{ {revlog_style} }} self._logHnd.close() self._logHnd = None - # Card Flags ########################################################################## def set_user_flag_for_cards(self, flag: int, cids: List[int]) -> None: - assert 0 <= flag <= 7 - self.db.execute( - "update cards set flags = (flags & ~?) | ?, usn=?, mod=? where id in %s" - % ids2str(cids), - 0b111, - flag, - self.usn(), - intTime(), - ) - - ########################################################################## + self._backend.set_flag(card_ids=cids, flag=flag) def set_wants_abort(self) -> None: self._backend.set_wants_abort() diff --git a/qt/.pylintrc b/qt/.pylintrc index 3507ed84c..844ae633a 100644 --- a/qt/.pylintrc +++ b/qt/.pylintrc @@ -8,6 +8,7 @@ ignored-modules=win32file,pywintypes,socket,win32pipe,winrt,pyaudio ignored-classes= SearchNode, Config, + OperationInfo [REPORTS] output-format=colorized diff --git a/qt/aqt/browser.py b/qt/aqt/browser.py index 9c7332474..4d21fddbe 100644 --- a/qt/aqt/browser.py +++ b/qt/aqt/browser.py @@ -12,7 +12,7 @@ from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple, Union, import aqt import aqt.forms from anki.cards import Card -from anki.collection import Collection, Config, SearchNode, StateChanges +from anki.collection import Collection, Config, OperationInfo, SearchNode from anki.consts import * from anki.errors import InvalidInput, NotFoundError from anki.lang import without_unicode_isolation @@ -281,13 +281,8 @@ class DataModel(QAbstractTableModel): else: tv.selectRow(0) - def maybe_redraw_after_operation(self, changes: StateChanges) -> None: - if ( - changes.card_modified - or changes.note_modified - or changes.deck_modified - or changes.notetype_modified - ): + def maybe_redraw_after_operation(self, op: OperationInfo) -> None: + if op.changes.card or op.changes.note or op.changes.deck or op.changes.notetype: self.reset() # Column data @@ -493,9 +488,9 @@ class Browser(QMainWindow): # as that will block the UI self.setUpdatesEnabled(False) - def on_operation_did_execute(self, changes: StateChanges) -> None: + def on_operation_did_execute(self, op: OperationInfo) -> None: self.setUpdatesEnabled(True) - self.model.maybe_redraw_after_operation(changes) + self.model.maybe_redraw_after_operation(op) def setupMenus(self) -> None: # pylint: disable=unnecessary-lambda @@ -1159,14 +1154,10 @@ where id in %s""" # select the next card if there is one self._onNextCard() - def do_remove() -> None: - self.col.remove_notes(nids) - - def on_done(fut: Future) -> None: - fut.result() - tooltip(tr(TR.BROWSING_NOTE_DELETED, count=len(nids))) - - self.perform_op(do_remove, on_done, reset_model=True) + self.mw.perform_op( + lambda: self.col.remove_notes(nids), + success=lambda _: tooltip(tr(TR.BROWSING_NOTE_DELETED, count=len(nids))), + ) # legacy @@ -1196,14 +1187,7 @@ where id in %s""" return did = self.col.decks.id(ret.name) - def do_move() -> None: - self.col.set_deck(cids, did) - - def on_done(fut: Future) -> None: - fut.result() - self.mw.requireReset(reason=ResetReason.BrowserSetDeck, context=self) - - self.perform_op(do_move, on_done) + self.mw.perform_op(lambda: self.col.set_deck(cids, did)) # legacy @@ -1247,9 +1231,8 @@ where id in %s""" if not ok: return - self.model.beginReset() - func(self.selectedNotes(), tags) - self.model.endReset() + nids = self.selectedNotes() + self.mw.perform_op(lambda: func(nids, tags)) self.mw.requireReset(reason=ResetReason.BrowserAddTags, context=self) def clearUnusedTags(self) -> None: @@ -1304,8 +1287,9 @@ where id in %s""" # flag needs toggling off? if n == self.card.user_flag(): n = 0 - self.col.set_user_flag_for_cards(n, self.selectedCards()) - self.model.reset() + + cids = self.selectedCards() + self.mw.perform_op(lambda: self.col.set_user_flag_for_cards(n, cids)) def _updateFlagsMenu(self) -> None: flag = self.card and self.card.user_flag() @@ -1382,11 +1366,6 @@ where id in %s""" # Scheduling ###################################################################### - def _after_schedule(self) -> None: - self.model.reset() - # updates undo status - self.mw.requireReset(reason=ResetReason.BrowserReschedule, context=self) - def set_due_date(self) -> None: self.editor.saveNow( lambda: set_due_date_dialog( @@ -1394,7 +1373,6 @@ where id in %s""" parent=self, card_ids=self.selectedCards(), config_key=Config.String.SET_DUE_BROWSER, - on_done=self._after_schedule, ) ) @@ -1404,7 +1382,6 @@ where id in %s""" mw=self.mw, parent=self, card_ids=self.selectedCards(), - on_done=self._after_schedule, ) ) diff --git a/qt/aqt/deckbrowser.py b/qt/aqt/deckbrowser.py index d74b4b822..87aaaf6b7 100644 --- a/qt/aqt/deckbrowser.py +++ b/qt/aqt/deckbrowser.py @@ -8,6 +8,7 @@ from dataclasses import dataclass from typing import Any import aqt +from anki.collection import OperationInfo from anki.decks import DeckTreeNode from anki.errors import DeckIsFilteredError from anki.utils import intTime @@ -61,6 +62,7 @@ class DeckBrowser: self.bottom = BottomBar(mw, mw.bottomWeb) self.scrollPos = QPoint(0, 0) self._v1_message_dismissed_at = 0 + gui_hooks.operation_did_execute.append(self.on_operation_did_execute) def show(self) -> None: av_player.stop_and_clear_queue() @@ -72,6 +74,13 @@ class DeckBrowser: def refresh(self) -> None: self._renderPage() + def on_operation_did_execute(self, op: OperationInfo) -> None: + if self.mw.state != "deckBrowser": + return + + if self.mw.col.op_affects_study_queue(op): + self.refresh() + # Event handlers ########################################################################## diff --git a/qt/aqt/main.py b/qt/aqt/main.py index 392802407..8c86f6de6 100644 --- a/qt/aqt/main.py +++ b/qt/aqt/main.py @@ -19,6 +19,7 @@ from typing import ( Callable, Dict, List, + Literal, Optional, Sequence, TextIO, @@ -43,6 +44,7 @@ from anki.collection import ( Checkpoint, Collection, Config, + OperationInfo, ReviewUndo, UndoResult, UndoStatus, @@ -112,6 +114,11 @@ class ResetRequired: self.mw = mw +MainWindowState = Literal[ + "startup", "deckBrowser", "overview", "review", "resetRequired", "profileManager" +] + + class AnkiQt(QMainWindow): col: Collection pm: ProfileManagerType @@ -128,7 +135,7 @@ class AnkiQt(QMainWindow): ) -> None: QMainWindow.__init__(self) self.backend = backend - self.state = "startup" + self.state: MainWindowState = "startup" self.opts = opts self.col: Optional[Collection] = None self.taskman = TaskManager(self) @@ -664,12 +671,12 @@ class AnkiQt(QMainWindow): self.pm.save() self.progress.finish() - # State machine + # Tracking main window state (deck browser, reviewer, etc) ########################################################################## - def moveToState(self, state: str, *args: Any) -> None: + def moveToState(self, state: MainWindowState, *args: Any) -> None: # print("-> move from", self.state, "to", state) - oldState = self.state or "dummy" + oldState = self.state cleanup = getattr(self, f"_{oldState}Cleanup", None) if cleanup: # pylint: disable=not-callable @@ -711,20 +718,27 @@ class AnkiQt(QMainWindow): def perform_op( self, op: Callable[[], T], - on_success: Optional[Callable[[T], None]] = None, - on_exception: Optional[Callable[[BaseException], None]] = None, + *, + success: Optional[Callable[[T], None]] = None, + failure: Optional[Callable[[BaseException], None]] = None, ) -> None: """Run the provided operation on a background thread. - - Ensures any changes in the editor have been saved. + - Shows progress popup for the duration of the op. - Ensures the browser doesn't try to redraw during the operation, which can lead to a frozen UI - Updates undo state at the end of the operation + - Commits changes + - Fires the `operation_(will|did)_reset` hooks + - Fires the legacy `state_did_reset` hook - on_success() will be called with the return value of op() - if op() threw an exception, on_exception() will be called with it, - if it was provided + Be careful not to call any UI routines in `op`, as that may crash Qt. + This includes things select .selectedCards() in the browse screen. + on_success() will be called with the return value of op(). + + If op() throws an exception, it will be shown in a popup, or + passed to on_exception() if it is provided. """ gui_hooks.operation_will_execute() @@ -732,28 +746,48 @@ class AnkiQt(QMainWindow): def wrapped_done(future: Future) -> None: try: if exception := future.exception(): - if on_exception: - on_exception(exception) + if failure: + failure(exception) else: showWarning(str(exception)) else: - if on_success: - on_success(future.result()) + if success: + success(future.result()) finally: status = self.col.undo_status() - self._update_undo_actions_for_status(status) - gui_hooks.operation_did_execute(status.changes) + self._update_undo_actions_for_status_and_save(status) + print("last op", status.last_op) + gui_hooks.operation_did_execute(status.last_op) + # fire legacy hook so old code notices changes + gui_hooks.state_did_reset() self.taskman.with_progress(op, wrapped_done) - def reset(self, guiOnly: bool = False) -> None: - "Called for non-trivial edits. Rebuilds queue and updates UI." + 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 = OperationInfo() + for field in op.changes.DESCRIPTOR.fields: + setattr(op.changes, field.name, True) + gui_hooks.operation_did_execute(op) + + def reset(self, unused_arg: bool = False) -> None: + """Legacy method of telling UI to refresh after changes made to DB. + + New code should use mw.perform_op() instead.""" + if self.col: - if not guiOnly: - self.col.reset() + # fire new `operation_did_execute` hook first. If the overview + # or review screen are currently open, they will rebuild the study + # queues (via mw.col.reset()) + self._synthesize_op_did_execute_from_reset() + # fire the old reset hook gui_hooks.state_did_reset() + self.update_undo_actions() - self.moveToState(self.state) + + # fixme: double-check + # self.moveToState(self.state) def requireReset( self, @@ -784,7 +818,7 @@ class AnkiQt(QMainWindow): # windows self.progress.timer(100, self.maybeReset, False) - def _resetRequiredState(self, oldState: str) -> None: + def _resetRequiredState(self, oldState: MainWindowState) -> None: if oldState != "resetRequired": self.returnState = oldState if self.resetModal: @@ -1160,7 +1194,7 @@ title="%s" %s>%s""" % ( self.form.actionUndo.setEnabled(False) gui_hooks.undo_state_did_change(False) - def _update_undo_actions_for_status(self, status: UndoStatus) -> None: + def _update_undo_actions_for_status_and_save(self, status: UndoStatus) -> None: """Update menu text and enable/disable menu item as appropriate. Plural as this may handle redo in the future too.""" undo_action = status.undo @@ -1175,6 +1209,8 @@ title="%s" %s>%s""" % ( 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() diff --git a/qt/aqt/overview.py b/qt/aqt/overview.py index 723528602..5e2fc979c 100644 --- a/qt/aqt/overview.py +++ b/qt/aqt/overview.py @@ -6,6 +6,7 @@ from dataclasses import dataclass from typing import Any, Callable, Dict, List, Optional, Tuple import aqt +from anki.collection import OperationInfo from aqt import gui_hooks from aqt.sound import av_player from aqt.toolbar import BottomBar @@ -42,6 +43,7 @@ class Overview: self.mw = mw self.web = mw.web self.bottom = BottomBar(mw, mw.bottomWeb) + gui_hooks.operation_did_execute.append(self.on_operation_did_execute) def show(self) -> None: av_player.stop_and_clear_queue() @@ -56,6 +58,14 @@ class Overview: self.mw.web.setFocus() gui_hooks.overview_did_refresh(self) + def on_operation_did_execute(self, op: OperationInfo) -> None: + if self.mw.state != "overview": + return + + if self.mw.col.op_affects_study_queue(op): + # will also cover the deck description modified case + self.refresh() + # Handlers ############################################################ diff --git a/qt/aqt/reviewer.py b/qt/aqt/reviewer.py index 21f009de1..5109b6ec4 100644 --- a/qt/aqt/reviewer.py +++ b/qt/aqt/reviewer.py @@ -13,7 +13,7 @@ from PyQt5.QtCore import Qt from anki import hooks from anki.cards import Card -from anki.collection import Config, StateChanges +from anki.collection import Config, OperationInfo from anki.utils import stripHTML from aqt import AnkiQt, gui_hooks from aqt.profiles import VideoDriver @@ -87,17 +87,26 @@ class Reviewer: gui_hooks.reviewer_will_end() self.card = None - def on_operation_did_execute(self, changes: StateChanges) -> None: - need_queue_rebuild = ( - changes.card_added - or changes.card_modified - or changes.deck_modified - or changes.preference_modified - ) + def on_operation_did_execute(self, op: OperationInfo) -> None: + if self.mw.state != "review": + return - if need_queue_rebuild: + if op.kind == OperationInfo.UPDATE_NOTE_TAGS: + self.card.load() + self._update_mark_icon() + elif op.kind == OperationInfo.SET_CARD_FLAG: + # fixme: v3 mtime check + self.card.load() + self._update_flag_icon() + elif self.mw.col.op_affects_study_queue(op): + # need queue rebuild self.mw.col.reset() self.nextCard() + return + elif op.changes.note or op.changes.notetype or op.changes.tag: + # need redraw of current card + self.card.load() + self._showQuestion() # Fetching a card ########################################################################## @@ -795,24 +804,27 @@ time = %(time)d; def onOptions(self) -> None: self.mw.onDeckConf(self.mw.col.decks.get(self.card.odid or self.card.did)) - def set_flag_on_current_card(self, flag: int) -> None: - # need to toggle off? - if self.card.user_flag() == flag: - flag = 0 - self.card.set_user_flag(flag) - self.mw.col.update_card(self.card) - self.mw.update_undo_actions() - self._update_flag_icon() + def set_flag_on_current_card(self, desired_flag: int) -> None: + def op() -> None: + # need to toggle off? + if self.card.user_flag() == desired_flag: + flag = 0 + else: + flag = desired_flag + self.mw.col.set_user_flag_for_cards(flag, [self.card.id]) + + self.mw.perform_op(op) def toggle_mark_on_current_note(self) -> None: - note = self.card.note() - if note.has_tag("marked"): - note.remove_tag("marked") - else: - note.add_tag("marked") - self.mw.col.update_note(note) - self.mw.update_undo_actions() - self._update_mark_icon() + def op() -> None: + tag = "marked" + note = self.card.note() + if note.has_tag(tag): + self.mw.col.tags.bulk_remove([note.id], tag) + else: + self.mw.col.tags.bulk_add([note.id], tag) + + self.mw.perform_op(op) def on_set_due(self) -> None: if self.mw.state != "review" or not self.card: @@ -823,28 +835,33 @@ time = %(time)d; parent=self.mw, card_ids=[self.card.id], config_key=Config.String.SET_DUE_REVIEWER, - on_done=self.mw.reset, ) def suspend_current_note(self) -> None: - self.mw.col.sched.suspend_cards([c.id for c in self.card.note().cards()]) - self.mw.reset() - tooltip(tr(TR.STUDYING_NOTE_SUSPENDED)) + self.mw.perform_op( + lambda: self.mw.col.sched.suspend_cards( + [c.id for c in self.card.note().cards()] + ), + success=lambda _: tooltip(tr(TR.STUDYING_NOTE_SUSPENDED)), + ) def suspend_current_card(self) -> None: - self.mw.col.sched.suspend_cards([self.card.id]) - self.mw.reset() - tooltip(tr(TR.STUDYING_CARD_SUSPENDED)) + self.mw.perform_op( + lambda: self.mw.col.sched.suspend_cards([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)) + self.mw.perform_op( + lambda: self.mw.col.sched.bury_cards([self.card.id]), + success=lambda _: tooltip(tr(TR.STUDYING_CARD_BURIED)), + ) def bury_current_note(self) -> None: - self.mw.col.sched.bury_note(self.card.note()) - self.mw.reset() - tooltip(tr(TR.STUDYING_NOTE_BURIED)) + self.mw.perform_op( + lambda: self.mw.col.sched.bury_note(self.card.note()), + success=lambda _: tooltip(tr(TR.STUDYING_NOTE_BURIED)), + ) def delete_current_note(self) -> None: # need to check state because the shortcut is global to the main @@ -855,7 +872,9 @@ time = %(time)d; self.mw.perform_op( lambda: self.mw.col.remove_notes([self.card.note().id]), - lambda _: tooltip(tr(TR.STUDYING_NOTE_AND_ITS_CARD_DELETED, count=cnt)), + success=lambda _: tooltip( + tr(TR.STUDYING_NOTE_AND_ITS_CARD_DELETED, count=cnt) + ), ) def onRecordVoice(self) -> None: diff --git a/qt/aqt/scheduling.py b/qt/aqt/scheduling.py index f16d49b1a..c6302b194 100644 --- a/qt/aqt/scheduling.py +++ b/qt/aqt/scheduling.py @@ -3,14 +3,13 @@ from __future__ import annotations -from concurrent.futures import Future from typing import List, Optional import aqt from anki.collection import Config from anki.lang import TR from aqt.qt import * -from aqt.utils import getText, showWarning, tooltip, tr +from aqt.utils import getText, tooltip, tr def set_due_date_dialog( @@ -19,12 +18,13 @@ def set_due_date_dialog( parent: QDialog, card_ids: List[int], config_key: Optional[Config.String.Key.V], - on_done: Callable[[], None], ) -> None: if not card_ids: return - default = mw.col.get_config_string(config_key) if config_key is not None else "" + default_text = ( + mw.col.get_config_string(config_key) if config_key is not None else "" + ) prompt = "\n".join( [ tr(TR.SCHEDULING_SET_DUE_DATE_PROMPT, cards=len(card_ids)), @@ -34,49 +34,28 @@ def set_due_date_dialog( (days, success) = getText( prompt=prompt, parent=parent, - default=default, + default=default_text, title=tr(TR.ACTIONS_SET_DUE_DATE), ) if not success or not days.strip(): return - def set_due() -> None: - mw.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( + 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, - ) - - 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: +def forget_cards(*, mw: aqt.AnkiQt, parent: QDialog, card_ids: List[int]) -> 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 + 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 + ), ) diff --git a/qt/tools/genhooks_gui.py b/qt/tools/genhooks_gui.py index d527e674c..e04b78c1d 100644 --- a/qt/tools/genhooks_gui.py +++ b/qt/tools/genhooks_gui.py @@ -394,7 +394,10 @@ hooks = [ Hook( name="state_did_reset", legacy_hook="reset", - doc="Called when the interface needs to be redisplayed after non-trivial changes have been made.", + doc="""Legacy 'reset' hook. Called by mw.reset() and mw.perform_op() to redraw the UI. + + New code should use `operation_did_execute` instead. + """, ), Hook( name="operation_will_execute", @@ -405,10 +408,14 @@ hooks = [ Hook( name="operation_did_execute", args=[ - "changes: anki.collection.StateChanges", + "op: anki.collection.OperationInfo", ], doc="""Called after an operation completes. - Changes can be inspected to determine whether the UI needs updating.""", + Changes can be inspected to determine whether the UI needs updating. + + This will also be called when the legacy mw.reset() is used. When called via + mw.reset(), `operation_will_execute` will not be called. + """, ), # Webview ################### diff --git a/rslib/backend.proto b/rslib/backend.proto index 494cd10f2..759f3eb65 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -267,6 +267,7 @@ service CardsService { rpc UpdateCard(UpdateCardIn) returns (Empty); rpc RemoveCards(RemoveCardsIn) returns (Empty); rpc SetDeck(SetDeckIn) returns (Empty); + rpc SetFlag(SetFlagIn) returns (Empty); } // Protobuf stored in .anki2 files @@ -1442,22 +1443,30 @@ message GetQueuedCardsOut { } } -message StateChanges { - bool card_added = 1; - bool card_modified = 2; - bool note_added = 3; - bool note_modified = 4; - bool deck_added = 5; - bool deck_modified = 6; - bool tag_modified = 7; - bool notetype_modified = 8; - bool preference_modified = 9; +message OperationInfo { + message Changes { + bool card = 1; + bool note = 2; + bool deck = 3; + bool tag = 4; + bool notetype = 5; + bool preference = 6; + } + // this is not an exhaustive list; we can add more cases as we need them + enum Kind { + OTHER = 0; + UPDATE_NOTE_TAGS = 1; + SET_CARD_FLAG = 2; + } + + Kind kind = 1; + Changes changes = 2; } message UndoStatus { string undo = 1; string redo = 2; - StateChanges changes = 3; + OperationInfo last_op = 3; } message DefaultsForAddingIn { @@ -1472,4 +1481,9 @@ message DeckAndNotetype { message RenameDeckIn { int64 deck_id = 1; string new_name = 2; -} \ No newline at end of file +} + +message SetFlagIn { + repeated int64 card_ids = 1; + uint32 flag = 2; +} diff --git a/rslib/src/backend/card.rs b/rslib/src/backend/card.rs index 83c2d6eec..4125cec33 100644 --- a/rslib/src/backend/card.rs +++ b/rslib/src/backend/card.rs @@ -54,6 +54,13 @@ impl CardsService for Backend { let deck_id = input.deck_id.into(); self.with_col(|col| col.set_deck(&cids, deck_id).map(Into::into)) } + + fn set_flag(&self, input: pb::SetFlagIn) -> Result { + self.with_col(|col| { + col.set_card_flag(&to_card_ids(input.card_ids), input.flag) + .map(Into::into) + }) + } } impl TryFrom for Card { @@ -111,3 +118,7 @@ impl From for pb::Card { } } } + +fn to_card_ids(v: Vec) -> Vec { + v.into_iter().map(CardID).collect() +} diff --git a/rslib/src/backend/collection.rs b/rslib/src/backend/collection.rs index 6153d1f9d..83df57dda 100644 --- a/rslib/src/backend/collection.rs +++ b/rslib/src/backend/collection.rs @@ -85,20 +85,20 @@ impl CollectionService for Backend { } fn get_undo_status(&self, _input: pb::Empty) -> Result { - 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 { 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 { self.with_col(|col| { col.redo()?; - Ok(col.undo_status()) + Ok(col.undo_status().into_protobuf(&col.i18n)) }) } } diff --git a/rslib/src/backend/notes.rs b/rslib/src/backend/notes.rs index bb5331a69..335a858b2 100644 --- a/rslib/src/backend/notes.rs +++ b/rslib/src/backend/notes.rs @@ -95,7 +95,7 @@ impl NotesService for Backend { fn add_note_tags(&self, input: pb::AddNoteTagsIn) -> Result { self.with_col(|col| { - col.add_tags_to_notes(&to_nids(input.nids), &input.tags) + col.add_tags_to_notes(&to_note_ids(input.nids), &input.tags) .map(|n| n as u32) }) .map(Into::into) @@ -104,7 +104,7 @@ impl NotesService for Backend { fn update_note_tags(&self, input: pb::UpdateNoteTagsIn) -> Result { self.with_col(|col| { col.replace_tags_for_notes( - &to_nids(input.nids), + &to_note_ids(input.nids), &input.tags, &input.replacement, input.regex, @@ -127,7 +127,7 @@ impl NotesService for Backend { self.with_col(|col| { col.transact(None, |col| { col.after_note_updates( - &to_nids(input.nids), + &to_note_ids(input.nids), input.generate_cards, input.mark_notes_modified, )?; @@ -167,6 +167,6 @@ impl NotesService for Backend { } } -fn to_nids(ids: Vec) -> Vec { +fn to_note_ids(ids: Vec) -> Vec { ids.into_iter().map(NoteID).collect() } diff --git a/rslib/src/backend/ops.rs b/rslib/src/backend/ops.rs index d1d99eaa1..4d1447215 100644 --- a/rslib/src/backend/ops.rs +++ b/rslib/src/backend/ops.rs @@ -1,20 +1,48 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html -use crate::{backend_proto as pb, ops::StateChanges}; +use pb::operation_info::{Changes, Kind}; -impl From for pb::StateChanges { +use crate::{backend_proto as pb, ops::StateChanges, prelude::*, undo::UndoStatus}; + +impl From for Changes { fn from(c: StateChanges) -> Self { - pb::StateChanges { - card_added: c.card_added, - card_modified: c.card_modified, - note_added: c.note_added, - note_modified: c.note_modified, - deck_added: c.deck_added, - deck_modified: c.deck_modified, - tag_modified: c.tag_modified, - notetype_modified: c.notetype_modified, - preference_modified: c.preference_modified, + Changes { + card: c.card, + note: c.note, + deck: c.deck, + tag: c.tag, + notetype: c.notetype, + preference: c.preference, + } + } +} + +impl From for Kind { + fn from(o: Op) -> Self { + match o { + Op::SetFlag => Kind::SetCardFlag, + Op::UpdateTag => Kind::UpdateNoteTags, + _ => Kind::Other, + } + } +} + +impl From for pb::OperationInfo { + fn from(op: Op) -> Self { + pb::OperationInfo { + changes: Some(op.state_changes().into()), + kind: Kind::from(op) as i32, + } + } +} + +impl UndoStatus { + pub(crate) fn into_protobuf(self, i18n: &I18n) -> pb::UndoStatus { + pb::UndoStatus { + undo: self.undo.map(|op| op.describe(i18n)).unwrap_or_default(), + redo: self.redo.map(|op| op.describe(i18n)).unwrap_or_default(), + last_op: self.undo.map(Into::into), } } } diff --git a/rslib/src/card/mod.rs b/rslib/src/card/mod.rs index 85b8ffc63..490933290 100644 --- a/rslib/src/card/mod.rs +++ b/rslib/src/card/mod.rs @@ -111,6 +111,15 @@ impl Card { self.deck_id = deck; } + fn set_flag(&mut self, flag: u8) { + // we currently only allow 4 flags + assert!(flag < 5); + + // but reserve space for 7, preserving the rest of + // the flags (up to a byte) + self.flags = (self.flags & !0b111) | flag + } + /// Return the total number of steps left to do, ignoring the /// "steps today" number packed into the DB representation. pub fn remaining_steps(&self) -> u32 { @@ -221,6 +230,24 @@ impl Collection { }) } + pub fn set_card_flag(&mut self, cards: &[CardID], flag: u32) -> Result<()> { + if flag > 4 { + return Err(AnkiError::invalid_input("invalid flag")); + } + let flag = flag as u8; + + self.storage.set_search_table_to_card_ids(cards, false)?; + let usn = self.usn()?; + self.transact(Some(Op::SetFlag), |col| { + for mut card in col.storage.all_searched_cards()? { + let original = card.clone(); + card.set_flag(flag); + col.update_card_inner(&mut card, original, usn)?; + } + Ok(()) + }) + } + /// Get deck config for the given card. If missing, return default values. #[allow(dead_code)] pub(crate) fn deck_config_for_card(&mut self, card: &Card) -> Result { diff --git a/rslib/src/ops.rs b/rslib/src/ops.rs index 3326f402e..3c8ff1c78 100644 --- a/rslib/src/ops.rs +++ b/rslib/src/ops.rs @@ -13,7 +13,9 @@ pub enum Op { RemoveNote, RenameDeck, ScheduleAsNew, + SetDeck, SetDueDate, + SetFlag, Suspend, UnburyUnsuspend, UpdateCard, @@ -21,91 +23,11 @@ pub enum Op { UpdateNote, UpdatePreferences, UpdateTag, - SetDeck, } impl Op { - /// Used internally to decide whether the study queues need to be invalidated. - pub(crate) fn needs_study_queue_reset(self) -> bool { - let changes = self.state_changes(); - self != Op::AnswerCard - && (changes.card_added - || changes.card_modified - || changes.deck_modified - || changes.preference_modified) - } - - pub fn state_changes(self) -> StateChanges { - let default = Default::default; - match self { - Op::ScheduleAsNew - | Op::SetDueDate - | Op::Suspend - | Op::UnburyUnsuspend - | Op::UpdateCard - | Op::SetDeck - | Op::Bury => StateChanges { - card_modified: true, - ..default() - }, - Op::AnswerCard => StateChanges { - card_modified: true, - // this also modifies the daily counts stored in the - // deck, but the UI does not care about that - ..default() - }, - Op::AddDeck => StateChanges { - deck_added: true, - ..default() - }, - Op::AddNote => StateChanges { - card_added: true, - note_added: true, - tag_modified: true, - ..default() - }, - Op::RemoveDeck => StateChanges { - card_modified: true, - note_modified: true, - deck_modified: true, - ..default() - }, - Op::RemoveNote => StateChanges { - card_modified: true, - note_modified: true, - ..default() - }, - Op::RenameDeck => StateChanges { - deck_modified: true, - ..default() - }, - Op::UpdateDeck => StateChanges { - deck_modified: true, - ..default() - }, - Op::UpdateNote => StateChanges { - note_modified: true, - // edits may result in new cards being generated - card_added: true, - // and may result in new tags being added - tag_modified: true, - ..default() - }, - Op::UpdatePreferences => StateChanges { - preference_modified: true, - ..default() - }, - Op::UpdateTag => StateChanges { - tag_modified: true, - ..default() - }, - } - } -} - -impl Collection { - pub fn describe_op_kind(&self, op: Op) -> String { - let key = match op { + pub fn describe(self, i18n: &I18n) -> String { + let key = match self { Op::AddDeck => TR::UndoAddDeck, Op::AddNote => TR::UndoAddNote, Op::AnswerCard => TR::UndoAnswerCard, @@ -123,21 +45,94 @@ impl Collection { Op::UpdatePreferences => TR::PreferencesPreferences, Op::UpdateTag => TR::UndoUpdateTag, Op::SetDeck => TR::BrowsingChangeDeck, + Op::SetFlag => TR::UndoSetFlag, }; - self.i18n.tr(key).to_string() + i18n.tr(key).to_string() + } + + /// Used internally to decide whether the study queues need to be invalidated. + pub(crate) fn needs_study_queue_reset(self) -> bool { + let changes = self.state_changes(); + self != Op::AnswerCard && (changes.card || changes.deck || changes.preference) + } + + pub fn state_changes(self) -> StateChanges { + let default = Default::default; + match self { + Op::ScheduleAsNew + | Op::SetDueDate + | Op::Suspend + | Op::UnburyUnsuspend + | Op::UpdateCard + | Op::SetDeck + | Op::Bury + | Op::SetFlag => StateChanges { + card: true, + ..default() + }, + Op::AnswerCard => StateChanges { + card: true, + // this also modifies the daily counts stored in the + // deck, but the UI does not care about that + ..default() + }, + Op::AddDeck => StateChanges { + deck: true, + ..default() + }, + Op::AddNote => StateChanges { + card: true, + note: true, + tag: true, + ..default() + }, + Op::RemoveDeck => StateChanges { + card: true, + note: true, + deck: true, + ..default() + }, + Op::RemoveNote => StateChanges { + card: true, + note: true, + ..default() + }, + Op::RenameDeck => StateChanges { + deck: true, + ..default() + }, + Op::UpdateDeck => StateChanges { + deck: true, + ..default() + }, + Op::UpdateNote => StateChanges { + note: true, + // edits may result in new cards being generated + card: true, + // and may result in new tags being added + tag: true, + ..default() + }, + Op::UpdatePreferences => StateChanges { + preference: true, + ..default() + }, + Op::UpdateTag => StateChanges { + note: true, + tag: true, + ..default() + }, + } } } #[derive(Debug, Default, Clone, Copy)] pub struct StateChanges { - pub card_added: bool, - pub card_modified: bool, - pub note_added: bool, - pub note_modified: bool, - pub deck_added: bool, - pub deck_modified: bool, - pub tag_modified: bool, - pub notetype_modified: bool, - pub preference_modified: bool, + pub card: bool, + pub note: bool, + pub deck: bool, + pub tag: bool, + pub notetype: bool, + pub preference: bool, } diff --git a/rslib/src/undo/mod.rs b/rslib/src/undo/mod.rs index 75fd90208..73986776d 100644 --- a/rslib/src/undo/mod.rs +++ b/rslib/src/undo/mod.rs @@ -6,7 +6,6 @@ mod changes; pub use crate::ops::Op; pub(crate) use changes::UndoableChange; -use crate::backend_proto as pb; use crate::prelude::*; use std::collections::VecDeque; @@ -32,6 +31,11 @@ impl Default for UndoMode { } } +pub struct UndoStatus { + pub undo: Option, + pub redo: Option, +} + #[derive(Debug, Default)] pub(crate) struct UndoManager { // undo steps are added to the front of a double-ended queue, so we can @@ -75,6 +79,8 @@ impl UndoManager { self.undo_steps.truncate(UNDO_LIMIT - 1); self.undo_steps.push_front(step); } + } else { + println!("no undo changes, discarding step"); } } println!("ended, undo steps count now {}", self.undo_steps.len()); @@ -141,22 +147,10 @@ impl Collection { Ok(()) } - pub fn undo_status(&self) -> pb::UndoStatus { - pb::UndoStatus { - undo: self - .can_undo() - .map(|op| self.describe_op_kind(op)) - .unwrap_or_default(), - redo: self - .can_redo() - .map(|op| self.describe_op_kind(op)) - .unwrap_or_default(), - changes: Some( - self.can_undo() - .map(|op| op.state_changes()) - .unwrap_or_default() - .into(), - ), + pub fn undo_status(&self) -> UndoStatus { + UndoStatus { + undo: self.can_undo(), + redo: self.can_redo(), } }