Make flag changes undoable again

The previous change in 1871b57663 failed
to consider the browser refreshing case, as reported here:
https://forums.ankiweb.net/t/anki-2-1-50-beta-3-4/17501/30

I previously attempted to solve this by having SetFlag skip the queue
rebuild, then mutating the captured mtimes in the queues. That didn't
work correctly when undoing, as the queue mutations weren't recorded.
This approach combines that attempt and the previous change: flag
setting is an undoable operation again, but does not change the card's
modification time, so it can be applied/undone without a queue build
being required. Instead of special-casing flag changes in the review
screen, we now just redraw the flag on changes.card, as any other card
op will have triggered a queue rebuild.
This commit is contained in:
Damien Elmes 2022-02-25 13:39:23 +10:00
parent 776631bbf5
commit d52c36e920
3 changed files with 22 additions and 9 deletions

View File

@ -22,6 +22,7 @@ from anki.collection import Config, OpChanges, OpChangesWithCount
from anki.scheduler.v3 import CardAnswer, NextStates, QueuedCards
from anki.scheduler.v3 import Scheduler as V3Scheduler
from anki.tags import MARKED_TAG
from anki.types import assert_exhaustive
from anki.utils import strip_html
from aqt import AnkiQt, gui_hooks
from aqt.browser.card_info import PreviousReviewerCardInfo, ReviewerCardInfo
@ -49,6 +50,7 @@ from aqt.utils import askUserDialog, downArrow, qtMenuShortcutWorkaround, toolti
class RefreshNeeded(Enum):
NOTE_TEXT = auto()
QUEUES = auto()
FLAG = auto()
class ReviewerBottomBar:
@ -171,6 +173,14 @@ class Reviewer:
self._redraw_current_card()
self.mw.fade_in_webview()
self._refresh_needed = None
elif self._refresh_needed is RefreshNeeded.FLAG:
self.card.load()
self._update_flag_icon()
# for when modified in browser
self.mw.fade_in_webview()
self._refresh_needed = None
elif self._refresh_needed:
assert_exhaustive(self._refresh_needed)
def op_executed(
self, changes: OpChanges, handler: object | None, focused: bool
@ -180,6 +190,8 @@ class Reviewer:
self._refresh_needed = RefreshNeeded.QUEUES
elif changes.note_text:
self._refresh_needed = RefreshNeeded.NOTE_TEXT
elif changes.card:
self._refresh_needed = RefreshNeeded.FLAG
if focused and self._refresh_needed:
self.refresh_if_needed()
@ -991,10 +1003,6 @@ time = %(time)d;
self._card_info.toggle()
def set_flag_on_current_card(self, desired_flag: int) -> None:
def redraw_flag(out: OpChangesWithCount) -> None:
self.card.load()
self._update_flag_icon()
# need to toggle off?
if self.card.user_flag() == desired_flag:
flag = 0
@ -1002,8 +1010,8 @@ time = %(time)d;
flag = desired_flag
set_card_flag(parent=self.mw, card_ids=[self.card.id], flag=flag).success(
redraw_flag
).run_in_background(initiator=self)
lambda _: None
).run_in_background()
def set_flag_func(self, desired_flag: int) -> Callable:
return lambda: self.set_flag_on_current_card(desired_flag)

View File

@ -279,12 +279,15 @@ impl Collection {
self.storage.set_search_table_to_card_ids(cards, false)?;
let usn = self.usn()?;
self.transact(Op::SkipUndo, |col| {
self.transact(Op::SetFlag, |col| {
let mut count = 0;
for mut card in col.storage.all_searched_cards()? {
let original = card.clone();
if card.set_flag(flag) {
// To avoid having to rebuild the study queues, we mark the card as requiring
// a sync, but do not change its modification time.
card.usn = usn;
col.storage.update_card(&card)?;
col.update_card_undoable(&mut card, original)?;
count += 1;
}
}

View File

@ -29,6 +29,7 @@ pub enum Op {
ScheduleAsNew,
SetCardDeck,
SetDueDate,
SetFlag,
SortCards,
Suspend,
UnburyUnsuspend,
@ -67,6 +68,7 @@ impl Op {
Op::UpdatePreferences => tr.preferences_preferences(),
Op::UpdateTag => tr.actions_update_tag(),
Op::SetCardDeck => tr.browsing_change_deck(),
Op::SetFlag => tr.actions_set_flag(),
Op::FindAndReplace => tr.browsing_find_and_replace(),
Op::ClearUnusedTags => tr.browsing_clear_unused_tags(),
Op::SortCards => tr.browsing_reschedule(),
@ -154,7 +156,7 @@ impl OpChanges {
pub fn requires_study_queue_rebuild(&self) -> bool {
let c = &self.changes;
c.card
(c.card && self.op != Op::SetFlag)
|| c.deck
|| (c.config && matches!(self.op, Op::SetCurrentDeck | Op::UpdatePreferences))
|| c.deck_config