From b466f0ce9070118dd5265daf50f4f672d47bed0e Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Thu, 4 Mar 2021 19:17:19 +1000 Subject: [PATCH] rework undo - use dataclasses for the review/checkpoint undo cases, instead of the nasty ad-hoc list structure - expose backend review undo to Python, and hook it into GUI - redo is not currently exposed on the GUI, and the backend can only cope with reviews done by the new scheduler at the moment - the initial undo prototype code was bumping mtime/usn on undo, but that was not ideal, as it was breaking the queue handling which expected the mtime to match. The original rationale for bumping mtime/usn was to avoid problems with syncing, but various operations like removing a revlog can't be synced anyway - so we just need to ensure we clear the undo queue prior to syncing --- pylib/anki/collection.py | 201 +++++++++++++++-------- pylib/anki/notes.py | 2 +- pylib/anki/sched.py | 2 +- pylib/anki/scheduler.py | 2 - pylib/anki/schedv2.py | 2 +- qt/aqt/addcards.py | 2 +- qt/aqt/main.py | 69 ++++++-- rslib/backend.proto | 9 +- rslib/src/backend/dbproxy.rs | 24 ++- rslib/src/backend/mod.rs | 23 ++- rslib/src/card.rs | 11 +- rslib/src/decks/mod.rs | 20 +-- rslib/src/notes.rs | 21 +-- rslib/src/revlog.rs | 4 +- rslib/src/scheduler/answering/undo.rs | 6 +- rslib/src/scheduler/queue/builder/mod.rs | 31 ++-- rslib/src/scheduler/queue/entry.rs | 81 +++++++++ rslib/src/scheduler/queue/learning.rs | 83 +++++++--- rslib/src/scheduler/queue/limits.rs | 14 +- rslib/src/scheduler/queue/main.rs | 35 ++-- rslib/src/scheduler/queue/mod.rs | 191 ++++++++------------- rslib/src/scheduler/queue/undo.rs | 83 ++++++++++ rslib/src/sync/mod.rs | 1 + rslib/src/sync/server.rs | 1 + rslib/src/tags.rs | 9 +- rslib/src/undo.rs | 45 ++++- 26 files changed, 654 insertions(+), 318 deletions(-) create mode 100644 rslib/src/scheduler/queue/entry.rs create mode 100644 rslib/src/scheduler/queue/undo.rs diff --git a/pylib/anki/collection.py b/pylib/anki/collection.py index ee195cfcd..cd592be93 100644 --- a/pylib/anki/collection.py +++ b/pylib/anki/collection.py @@ -11,6 +11,7 @@ import sys 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 @@ -34,6 +35,7 @@ from anki.scheduler import Scheduler as V2TestScheduler from anki.schedv2 import Scheduler as V2Scheduler from anki.sync import SyncAuth, SyncOutput, SyncStatus from anki.tags import TagManager +from anki.types import assert_exhaustive from anki.utils import ( devMode, from_json_bytes, @@ -52,11 +54,27 @@ EmptyCardsReport = _pb.EmptyCardsReport GraphPreferences = _pb.GraphPreferences BuiltinSort = _pb.SortOrder.Builtin Preferences = _pb.Preferences +UndoStatus = _pb.UndoStatus + + +@dataclass +class ReviewUndo: + card: Card + was_leech: bool + + +@dataclass +class Checkpoint: + name: str + + +@dataclass +class BackendUndo: + name: str class Collection: sched: Union[V1Scheduler, V2Scheduler] - _undo: List[Any] def __init__( self, @@ -74,7 +92,7 @@ class Collection: self.log(self.path, anki.version) self._lastSave = time.time() - self.clearUndo() + self._undo: _UndoInfo = None self.media = MediaManager(self, server) self.models = ModelManager(self) self.decks = DeckManager(self) @@ -146,7 +164,7 @@ class Collection: def upgrade_to_v2_scheduler(self) -> None: self._backend.upgrade_scheduler() - self.clearUndo() + self.clear_python_undo() self._loadScheduler() def is_2021_test_scheduler_enabled(self) -> bool: @@ -228,7 +246,7 @@ class Collection: # outside of a transaction, we need to roll back self.db.rollback() - self._markOp(name) + self._save_checkpoint(name) self._lastSave = time.time() def autosave(self) -> Optional[bool]: @@ -730,91 +748,136 @@ table.review-log {{ {revlog_style} }} # Undo ########################################################################## - # this data structure is a mess, and will be updated soon - # in the review case, [1, "Review", [firstReviewedCard, secondReviewedCard, ...], wasLeech] - # in the checkpoint case, [2, "action name"] - # wasLeech should have been recorded for each card, not globally - def clearUndo(self) -> None: + def undo_status(self) -> UndoStatus: + "Return the undo status. At the moment, redo is not supported." + # check backend first + status = self._backend.get_undo_status() + if status.undo or status.redo: + return status + + if not self._undo: + return status + + if isinstance(self._undo, _ReviewsUndo): + status.undo = self.tr(TR.SCHEDULING_REVIEW) + elif isinstance(self._undo, Checkpoint): + status.undo = self._undo.name + else: + 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 + any SQL DML is executed, or an operation that doesn't support undo + is run.""" self._undo = None - def undoName(self) -> Any: - "Undo menu item name, or None if undo unavailable." - if not self._undo: + def undo(self) -> Union[None, BackendUndo, Checkpoint, ReviewUndo]: + """Returns ReviewUndo if undoing a v1/v2 scheduler review. + Returns None if the undo queue was empty.""" + # backend? + status = self._backend.get_undo_status() + if status.undo: + self._backend.undo() + self.clear_python_undo() + return BackendUndo(name=status.undo) + + if isinstance(self._undo, _ReviewsUndo): + return self._undo_review() + elif isinstance(self._undo, Checkpoint): + return self._undo_checkpoint() + elif self._undo is None: return None - return self._undo[1] - - def undo(self) -> Any: - if self._undo[0] == 1: - return self._undoReview() else: - self._undoOp() + assert_exhaustive(self._undo) + assert False - def markReview(self, card: Card) -> None: - old: List[Any] = [] - if self._undo: - if self._undo[0] == 1: - old = self._undo[2] - self.clearUndo() - wasLeech = card.note().hasTag("leech") or False - self._undo = [ - 1, - self.tr(TR.SCHEDULING_REVIEW), - old + [copy.copy(card)], - wasLeech, - ] + def save_card_review_undo_info(self, card: Card) -> None: + "Used by V1 and V2 schedulers to record state prior to review." + if not isinstance(self._undo, _ReviewsUndo): + self._undo = _ReviewsUndo() + + was_leech = card.note().hasTag("leech") + entry = ReviewUndo(card=copy.copy(card), was_leech=was_leech) + self._undo.entries.append(entry) + + def _undo_checkpoint(self) -> Checkpoint: + assert isinstance(self._undo, Checkpoint) + self.rollback() + undo = self._undo + self.clear_python_undo() + return undo + + def _save_checkpoint(self, name: Optional[str]) -> None: + "Call via .save(). If name not provided, clear any existing checkpoint." + if name: + self._undo = Checkpoint(name=name) + else: + # saving disables old checkpoint, but not review undo + if not isinstance(self._undo, _ReviewsUndo): + self.clear_python_undo() + + def _undo_review(self) -> ReviewUndo: + "Undo a v1/v2 review." + assert isinstance(self._undo, _ReviewsUndo) + entry = self._undo.entries.pop() + if not self._undo.entries: + self.clear_python_undo() + + card = entry.card - def _undoReview(self) -> Any: - data = self._undo[2] - wasLeech = self._undo[3] - c = data.pop() # pytype: disable=attribute-error - if not data: - self.clearUndo() # remove leech tag if it didn't have it before - if not wasLeech and c.note().hasTag("leech"): - c.note().delTag("leech") - c.note().flush() + if not entry.was_leech and card.note().hasTag("leech"): + card.note().delTag("leech") + card.note().flush() + # write old data - c.flush() + card.flush() + # and delete revlog entry if not previewing - conf = self.sched._cardConf(c) + conf = self.sched._cardConf(card) previewing = conf["dyn"] and not conf["resched"] if not previewing: last = self.db.scalar( - "select id from revlog where cid = ? " "order by id desc limit 1", c.id + "select id from revlog where cid = ? " "order by id desc limit 1", + card.id, ) self.db.execute("delete from revlog where id = ?", last) + # restore any siblings self.db.execute( "update cards set queue=type,mod=?,usn=? where queue=-2 and nid=?", intTime(), self.usn(), - c.nid, + card.nid, ) - # and finally, update daily counts - if self.sched.is_2021: - self._backend.requeue_undone_card(c.id) - else: - n = c.queue - if c.queue in (QUEUE_TYPE_DAY_LEARN_RELEARN, QUEUE_TYPE_PREVIEW): - n = QUEUE_TYPE_LRN - type = ("new", "lrn", "rev")[n] - self.sched._updateStats(c, type, -1) + + # update daily counts + n = card.queue + if card.queue in (QUEUE_TYPE_DAY_LEARN_RELEARN, QUEUE_TYPE_PREVIEW): + n = QUEUE_TYPE_LRN + type = ("new", "lrn", "rev")[n] + self.sched._updateStats(card, type, -1) self.sched.reps -= 1 - return c.id - def _markOp(self, name: Optional[str]) -> None: - "Call via .save()" - if name: - self._undo = [2, name] - else: - # saving disables old checkpoint, but not review undo - if self._undo and self._undo[0] == 2: - self.clearUndo() + # and refresh the queues + self.sched.reset() - def _undoOp(self) -> None: - self.rollback() - self.clearUndo() + return entry + + # legacy + + clearUndo = clear_python_undo + markReview = save_card_review_undo_info + + def undoName(self) -> Optional[str]: + "Undo menu item name, or None if undo unavailable." + status = self.undo_status() + return status.undo or None # DB maintenance ########################################################################## @@ -946,3 +1009,11 @@ table.review-log {{ {revlog_style} }} # legacy name _Collection = Collection + + +@dataclass +class _ReviewsUndo: + entries: List[ReviewUndo] = field(default_factory=list) + + +_UndoInfo = Union[_ReviewsUndo, Checkpoint, None] diff --git a/pylib/anki/notes.py b/pylib/anki/notes.py index 19b8e5fb8..2b90e35f9 100644 --- a/pylib/anki/notes.py +++ b/pylib/anki/notes.py @@ -154,7 +154,7 @@ class Note: # Tags ################################################## - def hasTag(self, tag: str) -> Any: + def hasTag(self, tag: str) -> bool: return self.col.tags.inList(tag, self.tags) def stringTags(self) -> Any: diff --git a/pylib/anki/sched.py b/pylib/anki/sched.py index 844353551..143397a02 100644 --- a/pylib/anki/sched.py +++ b/pylib/anki/sched.py @@ -45,7 +45,7 @@ class Scheduler(V2): def answerCard(self, card: Card, ease: int) -> None: self.col.log() assert 1 <= ease <= 4 - self.col.markReview(card) + self.col.save_card_review_undo_info(card) if self._burySiblingsOnAnswer: self._burySiblings(card) card.reps += 1 diff --git a/pylib/anki/scheduler.py b/pylib/anki/scheduler.py index 8ac969d48..f677558ca 100644 --- a/pylib/anki/scheduler.py +++ b/pylib/anki/scheduler.py @@ -121,8 +121,6 @@ class Scheduler: assert 1 <= ease <= 4 assert 0 <= card.queue <= 4 - self.col.markReview(card) - new_state = self._answerCard(card, ease) self._handle_leech(card, new_state) diff --git a/pylib/anki/schedv2.py b/pylib/anki/schedv2.py index cae96be64..3dd435785 100644 --- a/pylib/anki/schedv2.py +++ b/pylib/anki/schedv2.py @@ -451,7 +451,7 @@ limit ?""" self.col.log() assert 1 <= ease <= 4 assert 0 <= card.queue <= 4 - self.col.markReview(card) + self.col.save_card_review_undo_info(card) if self._burySiblingsOnAnswer: self._burySiblings(card) diff --git a/qt/aqt/addcards.py b/qt/aqt/addcards.py index c719d4bbb..33ccdeaf2 100644 --- a/qt/aqt/addcards.py +++ b/qt/aqt/addcards.py @@ -178,7 +178,7 @@ class AddCards(QDialog): if not askUser(tr(TR.ADDING_YOU_HAVE_A_CLOZE_DELETION_NOTE)): return None self.mw.col.add_note(note, self.deckChooser.selectedId()) - self.mw.col.clearUndo() + self.mw.col.clear_python_undo() self.addHistory(note) self.previousNote = note self.mw.requireReset(reason=ResetReason.AddCardsAddNote, context=self) diff --git a/qt/aqt/main.py b/qt/aqt/main.py index bf3b5d49d..174e67bd6 100644 --- a/qt/aqt/main.py +++ b/qt/aqt/main.py @@ -27,10 +27,11 @@ import aqt.toolbar import aqt.webview from anki import hooks from anki._backend import RustBackend as _RustBackend -from anki.collection import Collection +from anki.collection import BackendUndo, Checkpoint, Collection, ReviewUndo from anki.decks import Deck from anki.hooks import runHook from anki.sound import AVTag, SoundOrVideoTag +from anki.types import assert_exhaustive from anki.utils import devMode, ids2str, intTime, isMac, isWin, splitFields from aqt import gui_hooks from aqt.addons import DownloadLogEntry, check_and_prompt_for_updates, show_log_to_user @@ -911,7 +912,7 @@ title="%s" %s>%s""" % ( self.media_syncer.start() def on_collection_sync_finished() -> None: - self.col.clearUndo() + self.col.clear_python_undo() self.col.models._clear_cache() gui_hooks.sync_did_finish() self.reset() @@ -1024,29 +1025,63 @@ title="%s" %s>%s""" % ( ########################################################################## def onUndo(self) -> None: - n = self.col.undoName() - if not n: + reviewing = self.state == "review" + result = self.col.undo() + + if result is None: + # should not happen + showInfo("nothing to undo") + self.maybeEnableUndo() return - cid = self.col.undo() - if cid and self.state == "review": - card = self.col.getCard(cid) - self.col.sched.reset() - self.reviewer.cardQueue.append(card) - self.reviewer.nextCard() - gui_hooks.review_did_undo(cid) + + elif isinstance(result, ReviewUndo): + name = tr(TR.SCHEDULING_REVIEW) + + # restore the undone card if reviewing + if reviewing: + cid = result.card.id + card = self.col.getCard(cid) + self.reviewer.cardQueue.append(card) + self.reviewer.nextCard() + gui_hooks.review_did_undo(cid) + self.maybeEnableUndo() + return + + elif isinstance(result, BackendUndo): + name = result.name + + # new scheduler takes care of rebuilding queue + if reviewing and self.col.sched.is_2021: + self.reviewer.nextCard() + self.maybeEnableUndo() + return + + elif isinstance(result, Checkpoint): + name = result.name + else: - self.reset() - tooltip(tr(TR.QT_MISC_REVERTED_TO_STATE_PRIOR_TO, val=n.lower())) - gui_hooks.state_did_revert(n) + assert_exhaustive(result) + assert False + + self.reset() + tooltip(tr(TR.QT_MISC_REVERTED_TO_STATE_PRIOR_TO, val=name)) + gui_hooks.state_did_revert(name) self.maybeEnableUndo() def maybeEnableUndo(self) -> None: - if self.col and self.col.undoName(): - self.form.actionUndo.setText(tr(TR.QT_MISC_UNDO2, val=self.col.undoName())) + if self.col: + status = self.col.undo_status() + undo_action = status.undo or None + else: + undo_action = None + + 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.QT_MISC_UNDO)) + self.form.actionUndo.setText(tr(TR.UNDO_UNDO)) self.form.actionUndo.setEnabled(False) gui_hooks.undo_state_did_change(False) diff --git a/rslib/backend.proto b/rslib/backend.proto index d1621c570..3210e0174 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -122,7 +122,6 @@ service BackendService { rpc UpgradeScheduler(Empty) returns (Empty); rpc GetQueuedCards(GetQueuedCardsIn) returns (GetQueuedCardsOut); rpc ClearCardQueues(Empty) returns (Empty); - rpc RequeueUndoneCard(CardID) returns (Empty); // stats @@ -199,6 +198,9 @@ service BackendService { rpc OpenCollection(OpenCollectionIn) returns (Empty); rpc CloseCollection(CloseCollectionIn) returns (Empty); rpc CheckDatabase(Empty) returns (CheckDatabaseOut); + rpc GetUndoStatus(Empty) returns (UndoStatus); + rpc Undo(Empty) returns (UndoStatus); + rpc Redo(Empty) returns (UndoStatus); // sync @@ -1391,3 +1393,8 @@ message GetQueuedCardsOut { CongratsInfoOut congrats_info = 2; } } + +message UndoStatus { + string undo = 1; + string redo = 2; +} diff --git a/rslib/src/backend/dbproxy.rs b/rslib/src/backend/dbproxy.rs index 68e8d95d1..98dad02a9 100644 --- a/rslib/src/backend/dbproxy.rs +++ b/rslib/src/backend/dbproxy.rs @@ -75,6 +75,7 @@ pub(super) fn db_command_bytes(col: &mut Collection, input: &[u8]) -> Result { + maybe_clear_undo(col, &sql); if first_row_only { db_query_row(&col.storage, &sql, &args)? } else { @@ -94,11 +95,32 @@ pub(super) fn db_command_bytes(col: &mut Collection, input: &[u8]) -> Result db_execute_many(&col.storage, &sql, &args)?, + DBRequest::ExecuteMany { sql, args } => { + maybe_clear_undo(col, &sql); + db_execute_many(&col.storage, &sql, &args)? + } }; Ok(serde_json::to_vec(&resp)?) } +fn maybe_clear_undo(col: &mut Collection, sql: &str) { + if !is_dql(sql) { + println!("clearing undo due to {}", sql); + col.state.undo.clear(); + } +} + +/// Anything other than a select statement is false. +fn is_dql(sql: &str) -> bool { + let head: String = sql + .trim_start() + .chars() + .take(10) + .map(|c| c.to_ascii_lowercase()) + .collect(); + head.starts_with("select ") +} + pub(super) fn db_query_row(ctx: &SqliteStorage, sql: &str, args: &[SqlValue]) -> Result { let mut stmt = ctx.db.prepare_cached(sql)?; let columns = stmt.column_count(); diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index 4b7a1a52c..e90dac1d1 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -712,11 +712,6 @@ impl BackendService for Backend { }) } - fn requeue_undone_card(&self, input: pb::CardId) -> BackendResult { - self.with_col(|col| col.requeue_undone_card(input.into())) - .map(Into::into) - } - // statistics //----------------------------------------------- @@ -1320,6 +1315,24 @@ impl BackendService for Backend { }) } + fn get_undo_status(&self, _input: pb::Empty) -> Result { + self.with_col(|col| Ok(col.undo_status())) + } + + fn undo(&self, _input: pb::Empty) -> Result { + self.with_col(|col| { + col.undo()?; + Ok(col.undo_status()) + }) + } + + fn redo(&self, _input: pb::Empty) -> Result { + self.with_col(|col| { + col.redo()?; + Ok(col.undo_status()) + }) + } + // sync //------------------------------------------------------------------- diff --git a/rslib/src/card.rs b/rslib/src/card.rs index 1cbf0d393..21756838c 100644 --- a/rslib/src/card.rs +++ b/rslib/src/card.rs @@ -127,12 +127,12 @@ impl Card { pub(crate) struct CardUpdated(Card); impl Undo for CardUpdated { - fn undo(self: Box, col: &mut crate::collection::Collection, usn: Usn) -> Result<()> { + fn undo(self: Box, col: &mut crate::collection::Collection) -> Result<()> { let current = col .storage .get_card(self.0.id)? .ok_or_else(|| AnkiError::invalid_input("card disappeared"))?; - col.update_card(&mut self.0.clone(), ¤t, usn) + col.update_card_inner(&mut self.0.clone(), ¤t) } } @@ -164,12 +164,17 @@ impl Collection { Ok(card) } + /// Marks the card as modified, then saves it. pub(crate) fn update_card(&mut self, card: &mut Card, original: &Card, usn: Usn) -> Result<()> { + card.set_modified(usn); + self.update_card_inner(card, original) + } + + pub(crate) fn update_card_inner(&mut self, card: &mut Card, original: &Card) -> Result<()> { if card.id.0 == 0 { return Err(AnkiError::invalid_input("card id not set")); } self.save_undo(Box::new(CardUpdated(original.clone()))); - card.set_modified(usn); self.storage.update_card(card) } diff --git a/rslib/src/decks/mod.rs b/rslib/src/decks/mod.rs index 8192ec8d8..773cdda6d 100644 --- a/rslib/src/decks/mod.rs +++ b/rslib/src/decks/mod.rs @@ -271,11 +271,11 @@ impl Collection { let usn = col.usn()?; col.prepare_deck_for_update(deck, usn)?; + deck.set_modified(usn); if deck.id.0 == 0 { // TODO: undo support col.match_or_create_parents(deck, usn)?; - deck.set_modified(usn); col.storage.add_deck(deck) } else if let Some(existing_deck) = col.storage.get_deck(deck.id)? { let name_changed = existing_deck.name != deck.name; @@ -285,7 +285,7 @@ impl Collection { // rename children col.rename_child_decks(&existing_deck, &deck.name, usn)?; } - col.update_single_deck_no_check(deck, &existing_deck, usn)?; + col.update_single_deck_inner_undo_only(deck, &existing_deck)?; if name_changed { // after updating, we need to ensure all grandparents exist, which may not be the case // in the parent->child case @@ -313,15 +313,13 @@ impl Collection { } /// Update an individual, existing deck. Caller is responsible for ensuring deck - /// is normalized, matches parents, and is not a duplicate name. Bumps mtime. - pub(crate) fn update_single_deck_no_check( + /// is normalized, matches parents, is not a duplicate name, and bumping mtime. + pub(crate) fn update_single_deck_inner_undo_only( &mut self, deck: &mut Deck, original: &Deck, - usn: Usn, ) -> Result<()> { self.state.deck_cache.clear(); - deck.set_modified(usn); self.save_undo(Box::new(DeckUpdated(original.clone()))); self.storage.update_deck(deck) } @@ -372,7 +370,8 @@ impl Collection { let child_only = &child_components[old_component_count..]; let new_name = format!("{}\x1f{}", new_name, child_only.join("\x1f")); child.name = new_name; - self.update_single_deck_no_check(&mut child, &original, usn)?; + child.set_modified(usn); + self.update_single_deck_inner_undo_only(&mut child, &original)?; } Ok(()) @@ -601,7 +600,8 @@ impl Collection { let original = deck.clone(); deck.reset_stats_if_day_changed(today); mutator(&mut deck.common); - self.update_single_deck_no_check(deck, &original, usn) + deck.set_modified(usn); + self.update_single_deck_inner_undo_only(deck, &original) } pub fn drag_drop_decks( @@ -651,12 +651,12 @@ impl Collection { pub(crate) struct DeckUpdated(Deck); impl Undo for DeckUpdated { - fn undo(mut self: Box, col: &mut crate::collection::Collection, usn: Usn) -> Result<()> { + fn undo(mut self: Box, col: &mut crate::collection::Collection) -> Result<()> { let current = col .storage .get_deck(self.0.id)? .ok_or_else(|| AnkiError::invalid_input("deck disappeared"))?; - col.update_single_deck_no_check(&mut self.0, ¤t, usn) + col.update_single_deck_inner_undo_only(&mut self.0, ¤t) } } diff --git a/rslib/src/notes.rs b/rslib/src/notes.rs index c82995c24..d8a316988 100644 --- a/rslib/src/notes.rs +++ b/rslib/src/notes.rs @@ -369,24 +369,19 @@ impl Collection { ) -> Result<()> { self.canonify_note_tags(note, usn)?; note.prepare_for_update(nt, normalize_text)?; - self.update_note_inner_undo_and_mtime_only( - note, - original, - if mark_note_modified { Some(usn) } else { None }, - ) + if mark_note_modified { + note.set_modified(usn); + } + self.update_note_inner_undo_only(note, original) } - /// Bumps modification time if usn provided, saves in the undo queue, and commits to DB. + /// Saves in the undo queue, and commits to DB. /// No validation, card generation or normalization is done. - pub(crate) fn update_note_inner_undo_and_mtime_only( + pub(crate) fn update_note_inner_undo_only( &mut self, note: &mut Note, original: &Note, - update_usn: Option, ) -> Result<()> { - if let Some(usn) = update_usn { - note.set_modified(usn); - } self.save_undo(Box::new(NoteUpdated(original.clone()))); self.storage.update_note(note)?; @@ -557,12 +552,12 @@ impl Collection { pub(crate) struct NoteUpdated(Note); impl Undo for NoteUpdated { - fn undo(self: Box, col: &mut crate::collection::Collection, usn: Usn) -> Result<()> { + fn undo(self: Box, col: &mut crate::collection::Collection) -> Result<()> { let current = col .storage .get_note(self.0.id)? .ok_or_else(|| AnkiError::invalid_input("note disappeared"))?; - col.update_note_inner_undo_and_mtime_only(&mut self.0.clone(), ¤t, Some(usn)) + col.update_note_inner_undo_only(&mut self.0.clone(), ¤t) } } diff --git a/rslib/src/revlog.rs b/rslib/src/revlog.rs index 63af124b1..5d5cb9727 100644 --- a/rslib/src/revlog.rs +++ b/rslib/src/revlog.rs @@ -118,7 +118,7 @@ pub(crate) struct RevlogAdded(RevlogEntry); pub(crate) struct RevlogRemoved(RevlogEntry); impl Undo for RevlogAdded { - fn undo(self: Box, col: &mut crate::collection::Collection, _usn: Usn) -> Result<()> { + fn undo(self: Box, col: &mut crate::collection::Collection) -> Result<()> { col.storage.remove_revlog_entry(self.0.id)?; col.save_undo(Box::new(RevlogRemoved(self.0))); Ok(()) @@ -126,7 +126,7 @@ impl Undo for RevlogAdded { } impl Undo for RevlogRemoved { - fn undo(self: Box, col: &mut crate::collection::Collection, _usn: Usn) -> Result<()> { + fn undo(self: Box, col: &mut crate::collection::Collection) -> Result<()> { col.storage.add_revlog_entry(&self.0, false)?; col.save_undo(Box::new(RevlogAdded(self.0))); Ok(()) diff --git a/rslib/src/scheduler/answering/undo.rs b/rslib/src/scheduler/answering/undo.rs index dd5f42257..dc9ef5824 100644 --- a/rslib/src/scheduler/answering/undo.rs +++ b/rslib/src/scheduler/answering/undo.rs @@ -95,6 +95,8 @@ mod test { let deck = col.get_deck(DeckID(1))?.unwrap(); assert_eq!(deck.common.review_studied, 1); + assert_eq!(col.next_card()?.is_some(), false); + Ok(()) }; @@ -119,6 +121,8 @@ mod test { let deck = col.get_deck(DeckID(1))?.unwrap(); assert_eq!(deck.common.review_studied, 0); + assert_eq!(col.next_card()?.is_some(), true); + Ok(()) }; @@ -135,8 +139,6 @@ mod test { col.undo()?; assert_initial_state(&mut col)?; - // fixme: make sure queue state updated, esp. on redo - Ok(()) } } diff --git a/rslib/src/scheduler/queue/builder/mod.rs b/rslib/src/scheduler/queue/builder/mod.rs index f943965f7..af7d3491e 100644 --- a/rslib/src/scheduler/queue/builder/mod.rs +++ b/rslib/src/scheduler/queue/builder/mod.rs @@ -13,7 +13,7 @@ use std::{ use super::{ limits::{remaining_limits_capped_to_parents, RemainingLimits}, - CardQueues, LearningQueueEntry, QueueEntry, QueueEntryKind, + CardQueues, Counts, LearningQueueEntry, MainQueueEntry, MainQueueEntryKind, }; use crate::deckconf::{NewCardOrder, ReviewCardOrder, ReviewMix}; use crate::prelude::*; @@ -41,22 +41,22 @@ pub(crate) struct NewCard { pub extra: u64, } -impl From for QueueEntry { +impl From for MainQueueEntry { fn from(c: DueCard) -> Self { - QueueEntry { + MainQueueEntry { id: c.id, mtime: c.mtime, - kind: QueueEntryKind::Review, + kind: MainQueueEntryKind::Review, } } } -impl From for QueueEntry { +impl From for MainQueueEntry { fn from(c: NewCard) -> Self { - QueueEntry { + MainQueueEntry { id: c.id, mtime: c.mtime, - kind: QueueEntryKind::New, + kind: MainQueueEntryKind::New, } } } @@ -113,9 +113,12 @@ impl QueueBuilder { let main_iter = merge_new(main_iter, self.new, self.new_review_mix); CardQueues { - new_count, - review_count, - learn_count, + counts: Counts { + new: new_count, + review: review_count, + learning: learn_count, + }, + undo: Vec::new(), main: main_iter.collect(), due_learning, later_learning, @@ -130,7 +133,7 @@ fn merge_day_learning( reviews: Vec, day_learning: Vec, mode: ReviewMix, -) -> Box> { +) -> Box> { let day_learning_iter = day_learning.into_iter().map(Into::into); let reviews_iter = reviews.into_iter().map(Into::into); @@ -142,10 +145,10 @@ fn merge_day_learning( } fn merge_new( - review_iter: impl ExactSizeIterator + 'static, + review_iter: impl ExactSizeIterator + 'static, new: Vec, mode: ReviewMix, -) -> Box> { +) -> Box> { let new_iter = new.into_iter().map(Into::into); match mode { @@ -191,7 +194,7 @@ impl Collection { let timing = self.timing_for_timestamp(now)?; let (decks, parent_count) = self.storage.deck_with_parents_and_children(deck_id)?; let config = self.storage.get_deck_config_map()?; - let limits = remaining_limits_capped_to_parents(&decks, &config); + let limits = remaining_limits_capped_to_parents(&decks, &config, timing.days_elapsed); let selected_deck_limits = limits[parent_count]; let mut queues = QueueBuilder::default(); diff --git a/rslib/src/scheduler/queue/entry.rs b/rslib/src/scheduler/queue/entry.rs new file mode 100644 index 000000000..ec80b083f --- /dev/null +++ b/rslib/src/scheduler/queue/entry.rs @@ -0,0 +1,81 @@ +// Copyright: Ankitects Pty Ltd and contributors +// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html + +use super::{LearningQueueEntry, MainQueueEntry, MainQueueEntryKind}; +use crate::card::CardQueue; +use crate::prelude::*; + +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) enum QueueEntry { + IntradayLearning(LearningQueueEntry), + Main(MainQueueEntry), +} + +impl QueueEntry { + pub fn card_id(&self) -> CardID { + match self { + QueueEntry::IntradayLearning(e) => e.id, + QueueEntry::Main(e) => e.id, + } + } + + pub fn mtime(&self) -> TimestampSecs { + match self { + QueueEntry::IntradayLearning(e) => e.mtime, + QueueEntry::Main(e) => e.mtime, + } + } + + pub fn kind(&self) -> QueueEntryKind { + match self { + QueueEntry::IntradayLearning(_e) => QueueEntryKind::Learning, + QueueEntry::Main(e) => match e.kind { + MainQueueEntryKind::New => QueueEntryKind::New, + MainQueueEntryKind::Review => QueueEntryKind::Review, + }, + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) enum QueueEntryKind { + New, + Review, + Learning, +} + +impl From<&Card> for QueueEntry { + fn from(card: &Card) -> Self { + let kind = match card.queue { + CardQueue::Learn | CardQueue::PreviewRepeat => { + return QueueEntry::IntradayLearning(LearningQueueEntry { + due: TimestampSecs(card.due as i64), + id: card.id, + mtime: card.mtime, + }); + } + CardQueue::New => MainQueueEntryKind::New, + CardQueue::Review | CardQueue::DayLearn => MainQueueEntryKind::Review, + CardQueue::Suspended | CardQueue::SchedBuried | CardQueue::UserBuried => { + unreachable!() + } + }; + QueueEntry::Main(MainQueueEntry { + id: card.id, + mtime: card.mtime, + kind, + }) + } +} + +impl From for QueueEntry { + fn from(e: LearningQueueEntry) -> Self { + Self::IntradayLearning(e) + } +} + +impl From for QueueEntry { + fn from(e: MainQueueEntry) -> Self { + Self::Main(e) + } +} diff --git a/rslib/src/scheduler/queue/learning.rs b/rslib/src/scheduler/queue/learning.rs index c2f9cd83d..e00764b8a 100644 --- a/rslib/src/scheduler/queue/learning.rs +++ b/rslib/src/scheduler/queue/learning.rs @@ -6,9 +6,17 @@ use std::{ collections::VecDeque, }; -use super::{CardQueues, LearningQueueEntry}; +use super::CardQueues; use crate::{prelude::*, scheduler::timing::SchedTimingToday}; +#[derive(Debug, Clone, Copy, PartialEq, PartialOrd, Eq, Ord)] +pub(crate) struct LearningQueueEntry { + // due comes first, so the derived ordering sorts by due + pub due: TimestampSecs, + pub id: CardID, + pub mtime: TimestampSecs, +} + impl CardQueues { /// Check for any newly due cards, and then return the first, if any, /// that is due before now. @@ -32,7 +40,7 @@ impl CardQueues { pub(super) fn pop_learning_entry(&mut self, id: CardID) -> Option { if let Some(top) = self.due_learning.front() { if top.id == id { - self.learn_count -= 1; + self.counts.learning -= 1; return self.due_learning.pop_front(); } } @@ -42,7 +50,7 @@ impl CardQueues { // so for now we also check the head of the later_due queue if let Some(top) = self.later_learning.peek() { if top.0.id == id { - // self.learn_count -= 1; + // self.counts.learning -= 1; return self.later_learning.pop().map(|c| c.0); } } @@ -52,20 +60,34 @@ impl CardQueues { /// Given the just-answered `card`, place it back in the learning queues if it's still /// due today. Avoid placing it in a position where it would be shown again immediately. - pub(super) fn maybe_requeue_learning_card(&mut self, card: &Card, timing: SchedTimingToday) { - if !card.is_intraday_learning() { - return; + pub(super) fn maybe_requeue_learning_card( + &mut self, + card: &Card, + timing: SchedTimingToday, + ) -> Option { + // not due today? + if !card.is_intraday_learning() || card.due >= timing.next_day_at as i32 { + return None; } + let entry = LearningQueueEntry { + due: TimestampSecs(card.due as i64), + id: card.id, + mtime: card.mtime, + }; + + Some(self.requeue_learning_entry(entry, timing)) + } + + /// Caller must have validated learning entry is due today. + pub(super) fn requeue_learning_entry( + &mut self, + mut entry: LearningQueueEntry, + timing: SchedTimingToday, + ) -> LearningQueueEntry { let learn_ahead_limit = timing.now.adding_secs(self.learn_ahead_secs); - if card.due < learn_ahead_limit.0 as i32 { - let mut entry = LearningQueueEntry { - due: TimestampSecs(card.due as i64), - id: card.id, - mtime: card.mtime, - }; - + if entry.due < learn_ahead_limit { if self.learning_collapsed() { if let Some(next) = self.due_learning.front() { if next.due >= entry.due { @@ -83,13 +105,12 @@ impl CardQueues { // not collapsed; can add normally self.push_due_learning_card(entry); } - } else if card.due < timing.next_day_at as i32 { - self.later_learning.push(Reverse(LearningQueueEntry { - due: TimestampSecs(card.due as i64), - id: card.id, - mtime: card.mtime, - })); - }; + } else { + // due outside current learn ahead limit, but later today + self.later_learning.push(Reverse(entry)); + } + + entry } fn learning_collapsed(&self) -> bool { @@ -98,7 +119,7 @@ impl CardQueues { /// Adds card, maintaining correct sort order, and increments learning count. pub(super) fn push_due_learning_card(&mut self, entry: LearningQueueEntry) { - self.learn_count += 1; + self.counts.learning += 1; let target_idx = binary_search_by(&self.due_learning, |e| e.due.cmp(&entry.due)).unwrap_or_else(|e| e); self.due_learning.insert(target_idx, entry); @@ -113,6 +134,26 @@ impl CardQueues { self.push_due_learning_card(entry); } } + + pub(super) fn remove_requeued_learning_card_after_undo(&mut self, id: CardID) { + let due_idx = self + .due_learning + .iter() + .enumerate() + .find_map(|(idx, entry)| if entry.id == id { Some(idx) } else { None }); + if let Some(idx) = due_idx { + self.counts.learning -= 1; + self.due_learning.remove(idx); + } else { + // card may be in the later_learning binary heap - we can't remove + // it in place, so we have to rebuild it + self.later_learning = self + .later_learning + .drain() + .filter(|e| e.0.id != id) + .collect(); + } + } } /// Adapted from the Rust stdlib VecDeque implementation; we can drop this when the following diff --git a/rslib/src/scheduler/queue/limits.rs b/rslib/src/scheduler/queue/limits.rs index 8559f44a0..744f72dec 100644 --- a/rslib/src/scheduler/queue/limits.rs +++ b/rslib/src/scheduler/queue/limits.rs @@ -12,12 +12,12 @@ pub(crate) struct RemainingLimits { } impl RemainingLimits { - pub(crate) fn new(deck: &Deck, config: Option<&DeckConf>) -> Self { + pub(crate) fn new(deck: &Deck, config: Option<&DeckConf>, today: u32) -> Self { if let Some(config) = config { + let (new_today, rev_today) = deck.new_rev_counts(today); RemainingLimits { - review: ((config.inner.reviews_per_day as i32) - deck.common.review_studied).max(0) - as u32, - new: ((config.inner.new_per_day as i32) - deck.common.new_studied).max(0) as u32, + review: ((config.inner.reviews_per_day as i32) - rev_today).max(0) as u32, + new: ((config.inner.new_per_day as i32) - new_today).max(0) as u32, } } else { RemainingLimits { @@ -36,8 +36,9 @@ impl RemainingLimits { pub(super) fn remaining_limits_capped_to_parents( decks: &[Deck], config: &HashMap, + today: u32, ) -> Vec { - let mut limits = get_remaining_limits(decks, config); + let mut limits = get_remaining_limits(decks, config, today); cap_limits_to_parents(decks.iter().map(|d| d.name.as_str()), &mut limits); limits } @@ -47,6 +48,7 @@ pub(super) fn remaining_limits_capped_to_parents( fn get_remaining_limits( decks: &[Deck], config: &HashMap, + today: u32, ) -> Vec { decks .iter() @@ -57,7 +59,7 @@ fn get_remaining_limits( } else { None }; - RemainingLimits::new(deck, config) + RemainingLimits::new(deck, config, today) }) .collect() } diff --git a/rslib/src/scheduler/queue/main.rs b/rslib/src/scheduler/queue/main.rs index 60626d29f..718ba9e5a 100644 --- a/rslib/src/scheduler/queue/main.rs +++ b/rslib/src/scheduler/queue/main.rs @@ -1,21 +1,33 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html -use super::{CardQueues, QueueEntry, QueueEntryKind}; +use super::CardQueues; use crate::prelude::*; +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) struct MainQueueEntry { + pub id: CardID, + pub mtime: TimestampSecs, + pub kind: MainQueueEntryKind, +} + +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) enum MainQueueEntryKind { + New, + Review, +} + impl CardQueues { - pub(super) fn next_main_entry(&self) -> Option { + pub(super) fn next_main_entry(&self) -> Option { self.main.front().copied() } - pub(super) fn pop_main_entry(&mut self, id: CardID) -> Option { + pub(super) fn pop_main_entry(&mut self, id: CardID) -> Option { if let Some(last) = self.main.front() { if last.id == id { match last.kind { - QueueEntryKind::New => self.new_count -= 1, - QueueEntryKind::Review => self.review_count -= 1, - QueueEntryKind::Learning => unreachable!(), + MainQueueEntryKind::New => self.counts.new -= 1, + MainQueueEntryKind::Review => self.counts.review -= 1, } return self.main.pop_front(); } @@ -23,15 +35,4 @@ impl CardQueues { None } - - /// Add an undone card back to the 'front' of the list, and update - /// the counts. - pub(super) fn push_main_entry(&mut self, entry: QueueEntry) { - match entry.kind { - QueueEntryKind::New => self.new_count += 1, - QueueEntryKind::Review => self.review_count += 1, - QueueEntryKind::Learning => unreachable!(), - } - self.main.push_front(entry); - } } diff --git a/rslib/src/scheduler/queue/mod.rs b/rslib/src/scheduler/queue/mod.rs index 8948808fe..4967c1653 100644 --- a/rslib/src/scheduler/queue/mod.rs +++ b/rslib/src/scheduler/queue/mod.rs @@ -2,28 +2,40 @@ // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html mod builder; +mod entry; mod learning; mod limits; mod main; +mod undo; use std::{ cmp::Reverse, collections::{BinaryHeap, VecDeque}, }; -use crate::{backend_proto as pb, card::CardQueue, prelude::*, timestamp::TimestampSecs}; +use crate::{backend_proto as pb, prelude::*, timestamp::TimestampSecs}; pub(crate) use builder::{DueCard, NewCard}; +pub(crate) use { + entry::{QueueEntry, QueueEntryKind}, + learning::LearningQueueEntry, + main::{MainQueueEntry, MainQueueEntryKind}, +}; + +use self::undo::QueueUpdateAfterAnsweringCard; use super::{states::NextCardStates, timing::SchedTimingToday}; #[derive(Debug)] pub(crate) struct CardQueues { - new_count: usize, - review_count: usize, - learn_count: usize, + counts: Counts, + + /// Any undone items take precedence. + undo: Vec, + + main: VecDeque, - main: VecDeque, due_learning: VecDeque, + later_learning: BinaryHeap>, selected_deck: DeckID, @@ -31,118 +43,71 @@ pub(crate) struct CardQueues { learn_ahead_secs: i64, } -#[derive(Debug)] +#[derive(Debug, Copy, Clone)] pub(crate) struct Counts { - new: usize, - learning: usize, - review: usize, + pub new: usize, + pub learning: usize, + pub review: usize, +} + +#[derive(Debug)] +pub(crate) struct QueuedCard { + pub card: Card, + pub kind: QueueEntryKind, + pub next_states: NextCardStates, +} + +pub(crate) struct QueuedCards { + pub cards: Vec, + pub new_count: usize, + pub learning_count: usize, + pub review_count: usize, } impl CardQueues { /// Get the next due card, if there is one. fn next_entry(&mut self, now: TimestampSecs) -> Option { - self.next_learning_entry_due_before_now(now) + self.next_undo_entry() .map(Into::into) - .or_else(|| self.next_main_entry()) + .or_else(|| self.next_learning_entry_due_before_now(now).map(Into::into)) + .or_else(|| self.next_main_entry().map(Into::into)) .or_else(|| self.next_learning_entry_learning_ahead().map(Into::into)) } - /// Remove the provided card from the top of the learning or main queues. + /// Remove the provided card from the top of the queues. /// If it was not at the top, return an error. - fn pop_answered(&mut self, id: CardID) -> Result<()> { - if self.pop_main_entry(id).is_none() && self.pop_learning_entry(id).is_none() { - Err(AnkiError::invalid_input("not at top of queue")) + fn pop_answered(&mut self, id: CardID) -> Result { + if let Some(entry) = self.pop_undo_entry(id) { + Ok(entry) + } else if let Some(entry) = self.pop_main_entry(id) { + Ok(entry.into()) + } else if let Some(entry) = self.pop_learning_entry(id) { + Ok(entry.into()) } else { - Ok(()) + Err(AnkiError::invalid_input("not at top of queue")) } } - fn counts(&self) -> Counts { - Counts { - new: self.new_count, - learning: self.learn_count, - review: self.review_count, - } + pub(crate) fn counts(&self) -> Counts { + self.counts } fn is_stale(&self, deck: DeckID, current_day: u32) -> bool { self.selected_deck != deck || self.current_day != current_day } - fn update_after_answering_card(&mut self, card: &Card, timing: SchedTimingToday) -> Result<()> { - self.pop_answered(card.id)?; - self.maybe_requeue_learning_card(card, timing); - Ok(()) - } + fn update_after_answering_card( + &mut self, + card: &Card, + timing: SchedTimingToday, + ) -> Result { + let entry = self.pop_answered(card.id)?; + let requeued_learning = self.maybe_requeue_learning_card(card, timing); - /// Add a just-undone card back to the appropriate queue, updating counts. - pub(crate) fn push_undone_card(&mut self, card: &Card) { - if card.is_intraday_learning() { - self.push_due_learning_card(LearningQueueEntry { - due: TimestampSecs(card.due as i64), - id: card.id, - mtime: card.mtime, - }) - } else { - self.push_main_entry(card.into()) - } - } -} - -#[derive(Clone, Copy, Debug, PartialEq)] -pub(crate) struct QueueEntry { - id: CardID, - mtime: TimestampSecs, - kind: QueueEntryKind, -} - -#[derive(Clone, Copy, Debug, PartialEq)] -pub(crate) enum QueueEntryKind { - New, - /// Includes day-learning cards - Review, - Learning, -} - -impl PartialOrd for QueueEntry { - fn partial_cmp(&self, other: &Self) -> Option { - self.id.partial_cmp(&other.id) - } -} - -impl From<&Card> for QueueEntry { - fn from(card: &Card) -> Self { - let kind = match card.queue { - CardQueue::Learn | CardQueue::PreviewRepeat => QueueEntryKind::Learning, - CardQueue::New => QueueEntryKind::New, - CardQueue::Review | CardQueue::DayLearn => QueueEntryKind::Review, - CardQueue::Suspended | CardQueue::SchedBuried | CardQueue::UserBuried => { - unreachable!() - } - }; - QueueEntry { - id: card.id, - mtime: card.mtime, - kind, - } - } -} - -#[derive(Debug, Clone, Copy, PartialEq, PartialOrd, Eq, Ord)] -struct LearningQueueEntry { - // due comes first, so the derived ordering sorts by due - due: TimestampSecs, - id: CardID, - mtime: TimestampSecs, -} - -impl From for QueueEntry { - fn from(e: LearningQueueEntry) -> Self { - Self { - id: e.id, - mtime: e.mtime, - kind: QueueEntryKind::Learning, - } + Ok(QueueUpdateAfterAnsweringCard { + entry, + learning_requeue: requeued_learning, + }) } } @@ -168,30 +133,28 @@ impl Collection { } pub(crate) fn clear_queues(&mut self) { + // clearing the queue will remove any undone reviews from the undo queue, + // causing problems if we then try to redo them + self.state.undo.clear_redo(); self.state.card_queues = None; } - /// FIXME: remove this once undoing is moved into backend - pub(crate) fn requeue_undone_card(&mut self, card_id: CardID) -> Result<()> { - let card = self.storage.get_card(card_id)?.ok_or(AnkiError::NotFound)?; - self.get_queues()?.push_undone_card(&card); - Ok(()) - } - pub(crate) fn update_queues_after_answering_card( &mut self, card: &Card, timing: SchedTimingToday, ) -> Result<()> { if let Some(queues) = &mut self.state.card_queues { - queues.update_after_answering_card(card, timing) + let mutation = queues.update_after_answering_card(card, timing)?; + self.save_undo(Box::new(mutation)); + Ok(()) } else { // we currenly allow the queues to be empty for unit tests Ok(()) } } - fn get_queues(&mut self) -> Result<&mut CardQueues> { + pub(crate) fn get_queues(&mut self) -> Result<&mut CardQueues> { let timing = self.timing_today()?; let deck = self.get_current_deck_id(); let need_rebuild = self @@ -217,9 +180,9 @@ impl Collection { if let Some(entry) = queues.next_entry(TimestampSecs::now()) { let card = self .storage - .get_card(entry.id)? + .get_card(entry.card_id())? .ok_or(AnkiError::NotFound)?; - if card.mtime != entry.mtime { + if card.mtime != entry.mtime() { return Err(AnkiError::invalid_input( "bug: card modified without updating queue", )); @@ -231,7 +194,7 @@ impl Collection { cards.push(QueuedCard { card, next_states, - kind: entry.kind, + kind: entry.kind(), }); } @@ -255,17 +218,3 @@ impl Collection { .map(|mut resp| resp.cards.pop().unwrap())) } } - -#[derive(Debug)] -pub(crate) struct QueuedCard { - pub card: Card, - pub kind: QueueEntryKind, - pub next_states: NextCardStates, -} - -pub(crate) struct QueuedCards { - pub cards: Vec, - pub new_count: usize, - pub learning_count: usize, - pub review_count: usize, -} diff --git a/rslib/src/scheduler/queue/undo.rs b/rslib/src/scheduler/queue/undo.rs new file mode 100644 index 000000000..0d45e65c6 --- /dev/null +++ b/rslib/src/scheduler/queue/undo.rs @@ -0,0 +1,83 @@ +// Copyright: Ankitects Pty Ltd and contributors +// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html + +use super::{CardQueues, LearningQueueEntry, QueueEntry, QueueEntryKind}; +use crate::{prelude::*, undo::Undo}; + +#[derive(Debug)] +pub(super) struct QueueUpdateAfterAnsweringCard { + pub entry: QueueEntry, + pub learning_requeue: Option, +} + +impl Undo for QueueUpdateAfterAnsweringCard { + fn undo(self: Box, col: &mut Collection) -> Result<()> { + let queues = col.get_queues()?; + if let Some(learning) = self.learning_requeue { + queues.remove_requeued_learning_card_after_undo(learning.id); + } + queues.push_undo_entry(self.entry); + col.save_undo(Box::new(QueueUpdateAfterUndoingAnswer { + entry: self.entry, + learning_requeue: self.learning_requeue, + })); + + Ok(()) + } +} + +#[derive(Debug)] +pub(super) struct QueueUpdateAfterUndoingAnswer { + pub entry: QueueEntry, + pub learning_requeue: Option, +} + +impl Undo for QueueUpdateAfterUndoingAnswer { + fn undo(self: Box, col: &mut Collection) -> Result<()> { + let timing = col.timing_today()?; + let queues = col.get_queues()?; + let mut modified_learning = None; + if let Some(learning) = self.learning_requeue { + modified_learning = Some(queues.requeue_learning_entry(learning, timing)); + } + queues.pop_undo_entry(self.entry.card_id()); + col.save_undo(Box::new(QueueUpdateAfterAnsweringCard { + entry: self.entry, + learning_requeue: modified_learning, + })); + + Ok(()) + } +} + +impl CardQueues { + pub(super) fn next_undo_entry(&self) -> Option { + self.undo.last().copied() + } + + pub(super) fn pop_undo_entry(&mut self, id: CardID) -> Option { + if let Some(last) = self.undo.last() { + if last.card_id() == id { + match last.kind() { + QueueEntryKind::New => self.counts.new -= 1, + QueueEntryKind::Review => self.counts.review -= 1, + QueueEntryKind::Learning => self.counts.learning -= 1, + } + return self.undo.pop(); + } + } + + None + } + + /// Add an undone card back to the 'front' of the list, and update + /// the counts. + pub(super) fn push_undo_entry(&mut self, entry: QueueEntry) { + match entry.kind() { + QueueEntryKind::New => self.counts.new += 1, + QueueEntryKind::Review => self.counts.review += 1, + QueueEntryKind::Learning => self.counts.learning += 1, + } + self.undo.push(entry); + } +} diff --git a/rslib/src/sync/mod.rs b/rslib/src/sync/mod.rs index d5c34a612..0b3cc09d0 100644 --- a/rslib/src/sync/mod.rs +++ b/rslib/src/sync/mod.rs @@ -326,6 +326,7 @@ where SyncActionRequired::NoChanges => Ok(state.into()), SyncActionRequired::FullSyncRequired { .. } => Ok(state.into()), SyncActionRequired::NormalSyncRequired => { + self.col.state.undo.clear(); self.col.storage.begin_trx()?; self.col .unbury_if_day_rolled_over(self.col.timing_today()?)?; diff --git a/rslib/src/sync/server.rs b/rslib/src/sync/server.rs index cdb55af6f..9f02298cb 100644 --- a/rslib/src/sync/server.rs +++ b/rslib/src/sync/server.rs @@ -102,6 +102,7 @@ impl SyncServer for LocalServer { self.client_usn = client_usn; self.client_is_newer = client_is_newer; + self.col.state.undo.clear(); self.col.storage.begin_rust_trx()?; // make sure any pending cards have been unburied first if necessary diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index 3db2911e5..ba64d9fdc 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -255,7 +255,7 @@ impl Collection { self.storage.register_tag(&tag) } - pub(crate) fn remove_single_tag(&mut self, tag: &Tag, _usn: Usn) -> Result<()> { + pub(crate) fn remove_single_tag(&mut self, tag: &Tag) -> Result<()> { self.save_undo(Box::new(RemovedTag(tag.clone()))); self.storage.remove_single_tag(&tag.name) } @@ -488,14 +488,13 @@ struct AddedTag(Tag); struct RemovedTag(Tag); impl Undo for AddedTag { - fn undo(self: Box, col: &mut Collection, usn: Usn) -> Result<()> { - col.remove_single_tag(&self.0, usn) + fn undo(self: Box, col: &mut Collection) -> Result<()> { + col.remove_single_tag(&self.0) } } impl Undo for RemovedTag { - fn undo(mut self: Box, col: &mut Collection, usn: Usn) -> Result<()> { - self.0.usn = usn; + fn undo(self: Box, col: &mut Collection) -> Result<()> { col.register_tag_inner(&self.0) } } diff --git a/rslib/src/undo.rs b/rslib/src/undo.rs index 41a6608b1..ffc68298b 100644 --- a/rslib/src/undo.rs +++ b/rslib/src/undo.rs @@ -1,10 +1,11 @@ // 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; +use crate::i18n::TR; use crate::{ collection::{Collection, CollectionOp}, err::Result, - types::Usn, }; use std::{collections::VecDeque, fmt}; @@ -12,7 +13,7 @@ const UNDO_LIMIT: usize = 30; pub(crate) trait Undo: fmt::Debug + Send { /// Undo the recorded action. - fn undo(self: Box, col: &mut Collection, usn: Usn) -> Result<()>; + fn undo(self: Box, col: &mut Collection) -> Result<()>; } #[derive(Debug)] @@ -54,9 +55,7 @@ impl UndoManager { pub(crate) fn begin_step(&mut self, op: Option) { if op.is_none() { - // action doesn't support undoing; clear the queue - self.undo_steps.clear(); - self.redo_steps.clear(); + self.clear(); } else if self.mode == UndoMode::NormalOp { // a normal op clears the redo queue self.redo_steps.clear(); @@ -67,6 +66,15 @@ impl UndoManager { }); } + pub(crate) fn clear(&mut self) { + self.undo_steps.clear(); + self.redo_steps.clear(); + } + + pub(crate) fn clear_redo(&mut self) { + self.redo_steps.clear(); + } + pub(crate) fn end_step(&mut self) { if let Some(step) = self.current_step.take() { if self.mode == UndoMode::Undoing { @@ -105,9 +113,8 @@ impl Collection { let changes = step.changes; self.state.undo.mode = UndoMode::Undoing; let res = self.transact(Some(step.kind), |col| { - let usn = col.usn()?; for change in changes.into_iter().rev() { - change.undo(col, usn)?; + change.undo(col)?; } Ok(()) }); @@ -122,9 +129,8 @@ impl Collection { let changes = step.changes; self.state.undo.mode = UndoMode::Redoing; let res = self.transact(Some(step.kind), |col| { - let usn = col.usn()?; for change in changes.into_iter().rev() { - change.undo(col, usn)?; + change.undo(col)?; } Ok(()) }); @@ -138,6 +144,27 @@ impl Collection { pub(crate) fn save_undo(&mut self, item: Box) { self.state.undo.save(item) } + + pub fn describe_collection_op(&self, op: CollectionOp) -> String { + match op { + CollectionOp::UpdateCard => todo!(), + CollectionOp::AnswerCard => self.i18n.tr(TR::UndoAnswerCard), + } + .to_string() + } + + pub fn undo_status(&self) -> pb::UndoStatus { + pb::UndoStatus { + undo: self + .can_undo() + .map(|op| self.describe_collection_op(op)) + .unwrap_or_default(), + redo: self + .can_redo() + .map(|op| self.describe_collection_op(op)) + .unwrap_or_default(), + } + } } #[cfg(test)]