From 0331d8b588e2173af33aea3807538f17daf042bb Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Thu, 18 Mar 2021 11:46:11 +1000 Subject: [PATCH] make reposition undoable --- ftl/core/browsing.ftl | 5 ++ pylib/anki/scheduler/base.py | 39 +++++++++------ pylib/tests/test_schedv1.py | 56 --------------------- pylib/tests/test_schedv2.py | 8 ++- qt/aqt/browser.py | 80 ++++++++++-------------------- qt/aqt/scheduling_ops.py | 70 +++++++++++++++++++++++++- rslib/backend.proto | 4 +- rslib/src/backend/scheduler/mod.rs | 4 +- rslib/src/ops.rs | 2 + rslib/src/scheduler/new.rs | 28 +++++++---- 10 files changed, 155 insertions(+), 141 deletions(-) diff --git a/ftl/core/browsing.ftl b/ftl/core/browsing.ftl index 46b317627..f84bb3890 100644 --- a/ftl/core/browsing.ftl +++ b/ftl/core/browsing.ftl @@ -146,3 +146,8 @@ browsing-removed-unused-tags-count = [one] Removed { $count } unused tag. *[other] Removed { $count } unused tags. } +browsing-changed-new-position = + { $count -> + [one] Changed position of { $count } new card. + *[other] Changed position of { $count } new cards. + } diff --git a/pylib/anki/scheduler/base.py b/pylib/anki/scheduler/base.py index 06f2dd61b..4b4af388f 100644 --- a/pylib/anki/scheduler/base.py +++ b/pylib/anki/scheduler/base.py @@ -5,7 +5,7 @@ from __future__ import annotations import anki import anki._backend.backend_pb2 as _pb -from anki.collection import OpChanges +from anki.collection import OpChanges, OpChangesWithCount from anki.config import Config SchedTimingToday = _pb.SchedTimingTodayOut @@ -167,20 +167,20 @@ select id from cards where did in %s and queue = {QUEUE_TYPE_REV} and due <= ? l # Repositioning new cards ########################################################################## - def sortCards( + def reposition_new_cards( self, - cids: List[int], - start: int = 1, - step: int = 1, - shuffle: bool = False, - shift: bool = False, - ) -> None: - self.col._backend.sort_cards( - card_ids=cids, - starting_from=start, - step_size=step, - randomize=shuffle, - shift_existing=shift, + card_ids: Sequence[int], + starting_from: int, + step_size: int, + randomize: bool, + shift_existing: bool, + ) -> OpChangesWithCount: + return self.col._backend.sort_cards( + card_ids=card_ids, + starting_from=starting_from, + step_size=step_size, + randomize=randomize, + shift_existing=shift_existing, ) def randomizeCards(self, did: int) -> None: @@ -204,3 +204,14 @@ select id from cards where did in %s and queue = {QUEUE_TYPE_REV} and due <= ? l # in order due? if conf["new"]["order"] == NEW_CARDS_RANDOM: self.randomizeCards(did) + + # legacy + def sortCards( + self, + cids: List[int], + start: int = 1, + step: int = 1, + shuffle: bool = False, + shift: bool = False, + ) -> None: + self.reposition_new_cards(cids, start, step, shuffle, shift) diff --git a/pylib/tests/test_schedv1.py b/pylib/tests/test_schedv1.py index 6504147b9..4a02066da 100644 --- a/pylib/tests/test_schedv1.py +++ b/pylib/tests/test_schedv1.py @@ -1023,62 +1023,6 @@ def test_deckFlow(): col.sched.answerCard(c, 2) -def test_reorder(): - col = getEmptyCol() - # add a note with default deck - note = col.newNote() - note["Front"] = "one" - col.addNote(note) - note2 = col.newNote() - note2["Front"] = "two" - col.addNote(note2) - assert note2.cards()[0].due == 2 - found = False - # 50/50 chance of being reordered - for i in range(20): - col.sched.randomizeCards(1) - if note.cards()[0].due != note.id: - found = True - break - assert found - col.sched.orderCards(1) - assert note.cards()[0].due == 1 - # shifting - note3 = col.newNote() - note3["Front"] = "three" - col.addNote(note3) - note4 = col.newNote() - note4["Front"] = "four" - col.addNote(note4) - assert note.cards()[0].due == 1 - assert note2.cards()[0].due == 2 - assert note3.cards()[0].due == 3 - assert note4.cards()[0].due == 4 - col.sched.sortCards([note3.cards()[0].id, note4.cards()[0].id], start=1, shift=True) - assert note.cards()[0].due == 3 - assert note2.cards()[0].due == 4 - assert note3.cards()[0].due == 1 - assert note4.cards()[0].due == 2 - - -def test_forget(): - col = getEmptyCol() - note = col.newNote() - note["Front"] = "one" - col.addNote(note) - c = note.cards()[0] - c.queue = QUEUE_TYPE_REV - c.type = CARD_TYPE_REV - c.ivl = 100 - c.due = 0 - c.flush() - col.reset() - assert col.sched.counts() == (0, 0, 1) - col.sched.forgetCards([c.id]) - col.reset() - assert col.sched.counts() == (1, 0, 0) - - def test_norelearn(): col = getEmptyCol() # add a note diff --git a/pylib/tests/test_schedv2.py b/pylib/tests/test_schedv2.py index d9876c562..5f9a46aae 100644 --- a/pylib/tests/test_schedv2.py +++ b/pylib/tests/test_schedv2.py @@ -1211,7 +1211,13 @@ def test_reorder(): assert note2.cards()[0].due == 2 assert note3.cards()[0].due == 3 assert note4.cards()[0].due == 4 - col.sched.sortCards([note3.cards()[0].id, note4.cards()[0].id], start=1, shift=True) + col.sched.reposition_new_cards( + [note3.cards()[0].id, note4.cards()[0].id], + starting_from=1, + shift_existing=True, + step_size=1, + randomize=False, + ) assert note.cards()[0].due == 3 assert note2.cards()[0].due == 4 assert note3.cards()[0].due == 1 diff --git a/qt/aqt/browser.py b/qt/aqt/browser.py index 0770cd1a4..fd8ff34d4 100644 --- a/qt/aqt/browser.py +++ b/qt/aqt/browser.py @@ -37,6 +37,7 @@ from aqt.previewer import Previewer from aqt.qt import * from aqt.scheduling_ops import ( forget_cards, + reposition_new_cards_dialog, set_due_date_dialog, suspend_cards, unsuspend_cards, @@ -247,7 +248,7 @@ class DataModel(QAbstractTableModel): self.endReset() def saveSelection(self) -> None: - cards = self.browser.selectedCards() + cards = self.browser.selected_cards() self.selectedCards = {id: True for id in cards} if getattr(self.browser, "card", None): self.focusedCard = self.browser.card.id @@ -1076,13 +1077,13 @@ QTableView {{ gridline-color: {grid} }} # Menu helpers ###################################################################### - def selectedCards(self) -> List[int]: + def selected_cards(self) -> List[int]: return [ self.model.cards[idx.row()] for idx in self.form.tableView.selectionModel().selectedRows() ] - def selectedNotes(self) -> List[int]: + def selected_notes(self) -> List[int]: return self.col.db.list( """ select distinct nid from cards @@ -1098,11 +1099,11 @@ where id in %s""" def selectedNotesAsCards(self) -> List[int]: return self.col.db.list( "select id from cards where nid in (%s)" - % ",".join([str(s) for s in self.selectedNotes()]) + % ",".join([str(s) for s in self.selected_notes()]) ) def oneModelNotes(self) -> List[int]: - sf = self.selectedNotes() + sf = self.selected_notes() if not sf: return [] mods = self.col.db.scalar( @@ -1119,6 +1120,11 @@ where id in %s""" def onHelp(self) -> None: openHelp(HelpPage.BROWSING) + # legacy + + selectedCards = selected_cards + selectedNotes = selected_notes + # Misc menu options ###################################################################### @@ -1174,7 +1180,7 @@ where id in %s""" return # nothing selected? - nids = self.selectedNotes() + nids = self.selected_notes() if not nids: return @@ -1198,7 +1204,7 @@ where id in %s""" def set_deck_of_selected_cards(self) -> None: from aqt.studydeck import StudyDeck - cids = self.selectedCards() + cids = self.selected_cards() if not cids: return @@ -1235,7 +1241,7 @@ where id in %s""" tags := tags or self._prompt_for_tags(tr(TR.BROWSING_ENTER_TAGS_TO_ADD)) ): return - add_tags(mw=self.mw, note_ids=self.selectedNotes(), space_separated_tags=tags) + add_tags(mw=self.mw, note_ids=self.selected_notes(), space_separated_tags=tags) @ensure_editor_saved_on_trigger def remove_tags_from_selected_notes(self, tags: Optional[str] = None) -> None: @@ -1245,7 +1251,7 @@ where id in %s""" ): return remove_tags( - mw=self.mw, note_ids=self.selectedNotes(), space_separated_tags=tags + mw=self.mw, note_ids=self.selected_notes(), space_separated_tags=tags ) def _prompt_for_tags(self, prompt: str) -> Optional[str]: @@ -1272,7 +1278,7 @@ where id in %s""" @ensure_editor_saved_on_trigger def suspend_selected_cards(self) -> None: want_suspend = not self.current_card_is_suspended() - cids = self.selectedCards() + cids = self.selected_cards() if want_suspend: suspend_cards(mw=self.mw, card_ids=cids) @@ -1300,7 +1306,7 @@ where id in %s""" if flag == self.card.user_flag(): flag = 0 - cids = self.selectedCards() + cids = self.selected_cards() set_card_flag(mw=self.mw, card_ids=cids, flag=flag) def _updateFlagsMenu(self) -> None: @@ -1331,57 +1337,25 @@ where id in %s""" def isMarked(self) -> bool: return bool(self.card and self.card.note().has_tag("Marked")) - # Repositioning + # Scheduling ###################################################################### @ensure_editor_saved_on_trigger def reposition(self) -> None: - cids = self.selectedCards() - cids2 = self.col.db.list( - f"select id from cards where type = {CARD_TYPE_NEW} and id in " - + ids2str(cids) - ) - if not cids2: - showInfo(tr(TR.BROWSING_ONLY_NEW_CARDS_CAN_BE_REPOSITIONED)) + if self.card and self.card.queue != QUEUE_TYPE_NEW: + showInfo(tr(TR.BROWSING_ONLY_NEW_CARDS_CAN_BE_REPOSITIONED), parent=self) return - d = QDialog(self) - disable_help_button(d) - d.setWindowModality(Qt.WindowModal) - frm = aqt.forms.reposition.Ui_Dialog() - frm.setupUi(d) - (pmin, pmax) = self.col.db.first( - f"select min(due), max(due) from cards where type={CARD_TYPE_NEW} and odid=0" - ) - pmin = pmin or 0 - pmax = pmax or 0 - txt = tr(TR.BROWSING_QUEUE_TOP, val=pmin) - txt += "\n" + tr(TR.BROWSING_QUEUE_BOTTOM, val=pmax) - frm.label.setText(txt) - frm.start.selectAll() - if not d.exec_(): - return - self.model.beginReset() - self.mw.checkpoint(tr(TR.ACTIONS_REPOSITION)) - self.col.sched.sortCards( - cids, - start=frm.start.value(), - step=frm.step.value(), - shuffle=frm.randomize.isChecked(), - shift=frm.shift.isChecked(), - ) - self.search() - self.mw.requireReset(reason=ResetReason.BrowserReposition, context=self) - self.model.endReset() - # Scheduling - ###################################################################### + reposition_new_cards_dialog( + mw=self.mw, parent=self, card_ids=self.selected_cards() + ) @ensure_editor_saved_on_trigger def set_due_date(self) -> None: set_due_date_dialog( mw=self.mw, parent=self, - card_ids=self.selectedCards(), + card_ids=self.selected_cards(), config_key=Config.String.SET_DUE_BROWSER, ) @@ -1390,7 +1364,7 @@ where id in %s""" forget_cards( mw=self.mw, parent=self, - card_ids=self.selectedCards(), + card_ids=self.selected_cards(), ) # Edit: selection @@ -1398,7 +1372,7 @@ where id in %s""" @ensure_editor_saved_on_trigger def selectNotes(self) -> None: - nids = self.selectedNotes() + nids = self.selected_notes() # clear the selection so we don't waste energy preserving it tv = self.form.tableView tv.selectionModel().clear() @@ -1465,7 +1439,7 @@ where id in %s""" @ensure_editor_saved_on_trigger def onFindReplace(self) -> None: - nids = self.selectedNotes() + nids = self.selected_notes() if not nids: return import anki.find diff --git a/qt/aqt/scheduling_ops.py b/qt/aqt/scheduling_ops.py index 4d351405c..7468ed00a 100644 --- a/qt/aqt/scheduling_ops.py +++ b/qt/aqt/scheduling_ops.py @@ -6,12 +6,12 @@ from __future__ import annotations from typing import List, Optional, Sequence import aqt -from anki.collection import Config +from anki.collection import CARD_TYPE_NEW, Config from anki.lang import TR from aqt import AnkiQt from aqt.main import PerformOpOptionalSuccessCallback from aqt.qt import * -from aqt.utils import getText, tooltip, tr +from aqt.utils import disable_help_button, getText, tooltip, tr def set_due_date_dialog( @@ -63,6 +63,72 @@ def forget_cards(*, mw: aqt.AnkiQt, parent: QWidget, card_ids: List[int]) -> Non ) +def reposition_new_cards_dialog( + *, mw: AnkiQt, parent: QWidget, card_ids: Sequence[int] +) -> None: + assert mw.col.db + row = mw.col.db.first( + f"select min(due), max(due) from cards where type={CARD_TYPE_NEW} and odid=0" + ) + assert row + (min_position, max_position) = row + min_position = max(min_position or 0, 0) + max_position = max_position or 0 + + d = QDialog(parent) + disable_help_button(d) + d.setWindowModality(Qt.WindowModal) + frm = aqt.forms.reposition.Ui_Dialog() + frm.setupUi(d) + + txt = tr(TR.BROWSING_QUEUE_TOP, val=min_position) + txt += "\n" + tr(TR.BROWSING_QUEUE_BOTTOM, val=max_position) + frm.label.setText(txt) + + frm.start.selectAll() + if not d.exec_(): + return + + start = frm.start.value() + step = frm.step.value() + randomize = frm.randomize.isChecked() + shift = frm.shift.isChecked() + + reposition_new_cards( + mw=mw, + parent=parent, + card_ids=card_ids, + starting_from=start, + step_size=step, + randomize=randomize, + shift_existing=shift, + ) + + +def reposition_new_cards( + *, + mw: AnkiQt, + parent: QWidget, + card_ids: Sequence[int], + starting_from: int, + step_size: int, + randomize: bool, + shift_existing: bool, +) -> None: + mw.perform_op( + lambda: mw.col.sched.reposition_new_cards( + card_ids=card_ids, + starting_from=starting_from, + step_size=step_size, + randomize=randomize, + shift_existing=shift_existing, + ), + success=lambda out: tooltip( + tr(TR.BROWSING_CHANGED_NEW_POSITION, count=out.count), parent=parent + ), + ) + + def suspend_cards( *, mw: AnkiQt, diff --git a/rslib/backend.proto b/rslib/backend.proto index 1bc90634e..d8d1475f5 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -120,8 +120,8 @@ service SchedulingService { rpc RebuildFilteredDeck(DeckID) returns (UInt32); rpc ScheduleCardsAsNew(ScheduleCardsAsNewIn) returns (OpChanges); rpc SetDueDate(SetDueDateIn) returns (OpChanges); - rpc SortCards(SortCardsIn) returns (Empty); - rpc SortDeck(SortDeckIn) returns (Empty); + rpc SortCards(SortCardsIn) returns (OpChangesWithCount); + rpc SortDeck(SortDeckIn) returns (OpChangesWithCount); rpc GetNextCardStates(CardID) returns (NextCardStates); rpc DescribeNextStates(NextCardStates) returns (StringList); rpc StateIsLeech(SchedulingState) returns (Bool); diff --git a/rslib/src/backend/scheduler/mod.rs b/rslib/src/backend/scheduler/mod.rs index 521b8aed4..2dae46537 100644 --- a/rslib/src/backend/scheduler/mod.rs +++ b/rslib/src/backend/scheduler/mod.rs @@ -118,7 +118,7 @@ impl SchedulingService for Backend { self.with_col(|col| col.set_due_date(&cids, &days, config).map(Into::into)) } - fn sort_cards(&self, input: pb::SortCardsIn) -> Result { + fn sort_cards(&self, input: pb::SortCardsIn) -> Result { let cids: Vec<_> = input.card_ids.into_iter().map(CardID).collect(); let (start, step, random, shift) = ( input.starting_from, @@ -137,7 +137,7 @@ impl SchedulingService for Backend { }) } - fn sort_deck(&self, input: pb::SortDeckIn) -> Result { + fn sort_deck(&self, input: pb::SortDeckIn) -> Result { self.with_col(|col| { col.sort_deck(input.deck_id.into(), input.randomize) .map(Into::into) diff --git a/rslib/src/ops.rs b/rslib/src/ops.rs index f80bfdec1..91f9c785a 100644 --- a/rslib/src/ops.rs +++ b/rslib/src/ops.rs @@ -18,6 +18,7 @@ pub enum Op { SetDeck, SetDueDate, SetFlag, + SortCards, Suspend, UnburyUnsuspend, UpdateCard, @@ -50,6 +51,7 @@ impl Op { Op::SetFlag => TR::UndoSetFlag, Op::FindAndReplace => TR::BrowsingFindAndReplace, Op::ClearUnusedTags => TR::BrowsingClearUnusedTags, + Op::SortCards => TR::BrowsingReschedule, }; i18n.tr(key).to_string() diff --git a/rslib/src/scheduler/new.rs b/rslib/src/scheduler/new.rs index 2e7b3de47..c01913f40 100644 --- a/rslib/src/scheduler/new.rs +++ b/rslib/src/scheduler/new.rs @@ -24,12 +24,14 @@ impl Card { self.ease_factor = 0; } - /// If the card is new, change its position. - fn set_new_position(&mut self, position: u32) { + /// If the card is new, change its position, and return true. + fn set_new_position(&mut self, position: u32) -> bool { if self.queue != CardQueue::New || self.ctype != CardType::New { - return; + false + } else { + self.due = position as i32; + true } - self.due = position as i32; } } pub(crate) struct NewCardSorter { @@ -130,9 +132,9 @@ impl Collection { step: u32, order: NewCardSortOrder, shift: bool, - ) -> Result<()> { + ) -> Result> { let usn = self.usn()?; - self.transact_no_undo(|col| { + self.transact(Op::SortCards, |col| { col.sort_cards_inner(cids, starting_from, step, order, shift, usn) }) } @@ -145,24 +147,28 @@ impl Collection { order: NewCardSortOrder, shift: bool, usn: Usn, - ) -> Result<()> { + ) -> Result { if shift { self.shift_existing_cards(starting_from, step * cids.len() as u32, usn)?; } self.storage.set_search_table_to_card_ids(cids, true)?; let cards = self.storage.all_searched_cards_in_search_order()?; let sorter = NewCardSorter::new(&cards, starting_from, step, order); + let mut count = 0; for mut card in cards { let original = card.clone(); - card.set_new_position(sorter.position(&card)); - self.update_card_inner(&mut card, original, usn)?; + if card.set_new_position(sorter.position(&card)) { + count += 1; + self.update_card_inner(&mut card, original, usn)?; + } } - self.storage.clear_searched_cards_table() + self.storage.clear_searched_cards_table()?; + Ok(count) } /// This creates a transaction - we probably want to split it out /// in the future if calling it as part of a deck options update. - pub fn sort_deck(&mut self, deck: DeckID, random: bool) -> Result<()> { + pub fn sort_deck(&mut self, deck: DeckID, random: bool) -> Result> { let cids = self.search_cards(&format!("did:{} is:new", deck), SortMode::NoOrder)?; let order = if random { NewCardSortOrder::Random