current deck change is now undoable

- make sure we set flag in changes when config var changed
- move current deck get/set into backend
- set_config() now returns a bool indicating whether a change was
made, so other operations can be gated off it
- active decks generation is deferred until sched.reset()

pylib/anki/decks.py

@@ -447,30 +447,31 @@ class DeckManager:
     # Deck selection
     #############################################################
 
-    def active(self) -> List[DeckId]:
+    def get_current(self) -> Deck:
-        "The currrently active dids."
+        return self.col._backend.get_current_deck()
-        return self.col.get_config("activeDecks", [1])
 
-    def selected(self) -> DeckId:
+    def set_current(self, deck: DeckId) -> OpChanges:
-        "The currently selected did."
+        return self.col._backend.set_current_deck(deck)
-        return DeckId(int(self.col.conf["curDeck"]))
+
+    def get_current_id(self) -> DeckId:
+        "The currently selected deck ID."
+        return DeckId(self.get_current().id)
+
+    # legacy
 
     def current(self) -> DeckDict:
         return self.get(self.selected())
 
     def select(self, did: DeckId) -> None:
-        "Select a new branch."
         # make sure arg is an int; legacy callers may be passing in a string
         did = DeckId(did)
-        current = self.selected()
+        self.set_current(did)
-        active = self.deck_and_child_ids(did)
+        self.col.reset()
-        if current != did or active != self.active():
-            self.col.conf["curDeck"] = did
-            self.col.conf["activeDecks"] = active
 
-    # don't use this, it will likely go away
+    def active(self) -> List[DeckId]:
-    def update_active(self) -> None:
+        return self.col.sched.active_decks
-        self.select(self.current()["id"])
+
+    selected = get_current_id
 
     # Parents/children
     #############################################################
@@ -518,7 +519,7 @@ class DeckManager:
         )
 
     def deck_and_child_ids(self, deck_id: DeckId) -> List[DeckId]:
-        parent_name = self.get_legacy(deck_id)["name"]
+        parent_name = self.col.get_deck(deck_id).name
         out = [deck_id]
         out.extend(self.child_ids(parent_name))
         return out

pylib/anki/scheduler/base.py

@@ -85,8 +85,9 @@ select id from cards where did in %s and queue = {QUEUE_TYPE_REV} and due <= ? l
     # fixme: only used by totalRevForCurrentDeck and old deck stats;
     # schedv2 defines separate version
     def _deckLimit(self) -> str:
-        self.col.decks.update_active()
+        return ids2str(
-        return ids2str(self.col.decks.active())
+            self.col.decks.deck_and_child_ids(self.col.decks.get_current_id())
+        )
 
     # Filtered deck handling
     ##########################################################################

pylib/anki/scheduler/v1.py

@@ -33,7 +33,7 @@ class Scheduler(V2):
     def __init__(  # pylint: disable=super-init-not-called
         self, col: anki.collection.Collection
     ) -> None:
-        self.col = col.weakref()
+        super().__init__(col)
         self.queueLimit = 50
         self.reportLimit = 1000
         self.dynReportLimit = 99999
@@ -42,7 +42,6 @@ class Scheduler(V2):
         self.revCount = 0
         self.newCount = 0
         self._haveQueues = False
-        self._updateCutoff()
 
     def answerCard(self, card: Card, ease: int) -> None:
         self.col.log()

pylib/anki/scheduler/v2.py

@@ -48,7 +48,16 @@ class Scheduler(SchedulerBaseWithLegacy):
         self.reps = 0
         self._haveQueues = False
         self._lrnCutoff = 0
-        self._updateCutoff()
+        self._active_decks: List[DeckId] = []
+        self._current_deck_id = DeckId(1)
+
+    @property
+    def active_decks(self) -> List[DeckId]:
+        "Caller must make sure to make a copy."
+        return self._active_decks
+
+    def _update_active_decks(self) -> None:
+        self._active_decks = self.col.decks.deck_and_child_ids(self._current_deck_id)
 
     # Daily cutoff
     ##########################################################################
@@ -65,8 +74,8 @@ class Scheduler(SchedulerBaseWithLegacy):
     ##########################################################################
 
     def reset(self) -> None:
-        self.col.decks.update_active()
+        self._current_deck_id = self.col.decks.selected()
-        self._updateCutoff()
+        self._update_active_decks()
         self._reset_counts()
         self._resetLrn()
         self._resetRev()
@@ -74,10 +83,8 @@ class Scheduler(SchedulerBaseWithLegacy):
         self._haveQueues = True
 
     def _reset_counts(self) -> None:
-        tree = self.deck_due_tree(self.col.decks.selected())
+        tree = self.deck_due_tree(self._current_deck_id)
-        node = self.col.decks.find_deck_in_tree(
+        node = self.col.decks.find_deck_in_tree(tree, self._current_deck_id)
-            tree, DeckId(int(self.col.conf["curDeck"]))
-        )
         if not node:
             # current deck points to a missing deck
             self.newCount = 0

pylib/tests/test_decks.py

@@ -18,6 +18,7 @@ def test_basic():
     assert col.decks.id("new deck") == parentId
     # we start with the default col selected
     assert col.decks.selected() == 1
+    col.reset()
     assert col.decks.active() == [1]
     # we can select a different col
     col.decks.select(parentId)

qt/aqt/deckbrowser.py

@@ -17,6 +17,7 @@ from aqt.operations.deck import (
     remove_decks,
     rename_deck,
     reparent_decks,
+    set_current_deck,
     set_deck_collapsed,
 )
 from aqt.qt import *
@@ -79,7 +80,7 @@ class DeckBrowser:
     def op_executed(
         self, changes: OpChanges, handler: Optional[object], focused: bool
     ) -> bool:
-        if changes.study_queues:
+        if changes.study_queues and handler is not self:
             self._refresh_needed = True
 
         if focused:
@@ -96,7 +97,7 @@ class DeckBrowser:
         else:
             cmd = url
         if cmd == "open":
-            self._selDeck(arg)
+            self.set_current_deck(DeckId(int(arg)))
         elif cmd == "opts":
             self._showOptions(arg)
         elif cmd == "shared":
@@ -119,9 +120,10 @@ class DeckBrowser:
             self.refresh()
         return False
 
-    def _selDeck(self, did: str) -> None:
+    def set_current_deck(self, deck_id: DeckId) -> None:
-        self.mw.col.decks.select(DeckId(int(did)))
+        set_current_deck(parent=self.mw, deck_id=deck_id).success(
-        self.mw.onOverview()
+            lambda _: self.mw.onOverview()
+        ).run_in_background(initiator=self)
 
     # HTML generation
     ##########################################################################

qt/aqt/main.py

@@ -53,6 +53,7 @@ from aqt.legacy import install_pylib_legacy
 from aqt.mediacheck import check_media_db
 from aqt.mediasync import MediaSyncer
 from aqt.operations.collection import undo
+from aqt.operations.deck import set_current_deck
 from aqt.profiles import ProfileManager as ProfileManagerType
 from aqt.qt import *
 from aqt.qt import sip
@@ -1425,8 +1426,11 @@ title="%s" %s>%s</button>""" % (
 
         ret = StudyDeck(self, dyn=True, current=self.col.decks.current()["name"])
         if ret.name:
-            self.col.decks.select(self.col.decks.id(ret.name))
+            # fixme: this is silly, it should be returning an ID
-            self.moveToState("overview")
+            deck_id = self.col.decks.id(ret.name)
+            set_current_deck(parent=self, deck_id=deck_id).success(
+                lambda out: self.moveToState("overview")
+            ).run_in_background()
 
     def onEmptyCards(self) -> None:
         show_empty_cards(self)

qt/aqt/operations/deck.py

@@ -76,3 +76,7 @@ def set_deck_collapsed(
             deck_id=deck_id, collapsed=collapsed, scope=scope
         ),
     )
+
+
+def set_current_deck(*, parent: QWidget, deck_id: DeckId) -> CollectionOp[OpChanges]:
+    return CollectionOp(parent, lambda col: col.decks.set_current(deck_id))

rslib/backend.proto

@@ -154,6 +154,8 @@ service DecksService {
   rpc GetOrCreateFilteredDeck(DeckId) returns (FilteredDeckForUpdate);
   rpc AddOrUpdateFilteredDeck(FilteredDeckForUpdate) returns (OpChangesWithId);
   rpc FilteredDeckOrderLabels(Empty) returns (StringList);
+  rpc SetCurrentDeck(DeckId) returns (OpChanges);
+  rpc GetCurrentDeck(Empty) returns (Deck);
 }
 
 service NotesService {
@@ -1504,7 +1506,7 @@ message OpChanges {
   bool deck = 3;
   bool tag = 4;
   bool notetype = 5;
-  bool preference = 6;
+  bool config = 6;
 
   bool browser_table = 7;
   bool browser_sidebar = 8;

rslib/src/backend/config.rs

@@ -67,7 +67,7 @@ impl ConfigService for Backend {
             col.transact_no_undo(|col| {
                 // ensure it's a well-formed object
                 let val: Value = serde_json::from_slice(&input.value_json)?;
-                col.set_config(input.key.as_str(), &val)
+                col.set_config(input.key.as_str(), &val).map(|_| ())
             })
         })
         .map(Into::into)
@@ -98,7 +98,7 @@ impl ConfigService for Backend {
         self.with_col(|col| {
             col.transact_no_undo(|col| col.set_bool(input.key().into(), input.value))
         })
-        .map(Into::into)
+        .map(|_| ().into())
     }
 
     fn get_config_string(&self, input: pb::config::String) -> Result<pb::String> {
@@ -113,7 +113,7 @@ impl ConfigService for Backend {
         self.with_col(|col| {
             col.transact_no_undo(|col| col.set_string(input.key().into(), &input.value))
         })
-        .map(Into::into)
+        .map(|_| ().into())
     }
 
     fn get_preferences(&self, _input: pb::Empty) -> Result<pb::Preferences> {

rslib/src/backend/decks.rs

@@ -187,6 +187,16 @@ impl DecksService for Backend {
         })
         .map(Into::into)
     }
+
+    fn set_current_deck(&self, input: pb::DeckId) -> Result<pb::OpChanges> {
+        self.with_col(|col| col.set_current_deck(input.did.into()))
+            .map(Into::into)
+    }
+
+    fn get_current_deck(&self, _input: pb::Empty) -> Result<pb::Deck> {
+        self.with_col(|col| col.get_current_deck())
+            .map(|deck| (*deck).clone().into())
+    }
 }
 
 impl From<pb::DeckId> for DeckId {

rslib/src/backend/ops.rs

@@ -16,7 +16,7 @@ impl From<OpChanges> for pb::OpChanges {
             deck: c.changes.deck,
             tag: c.changes.tag,
             notetype: c.changes.notetype,
-            preference: c.changes.preference,
+            config: c.changes.config,
             browser_table: c.requires_browser_table_redraw(),
             browser_sidebar: c.requires_browser_sidebar_redraw(),
             editor: c.requires_editor_redraw(),

rslib/src/card/mod.rs

@@ -239,7 +239,7 @@ impl Collection {
         self.storage.set_search_table_to_card_ids(cards, false)?;
         let sched = self.scheduler_version();
         let usn = self.usn()?;
-        self.transact(Op::SetDeck, |col| {
+        self.transact(Op::SetCardDeck, |col| {
             for mut card in col.storage.all_searched_cards()? {
                 if card.deck_id == deck_id {
                     continue;

rslib/src/config/bool.rs

@@ -69,7 +69,7 @@ impl Collection {
         }
     }
 
-    pub(crate) fn set_bool(&mut self, key: BoolKey, value: bool) -> Result<()> {
+    pub(crate) fn set_bool(&mut self, key: BoolKey, value: bool) -> Result<bool> {
         self.set_config(key, &value)
     }
 }

rslib/src/config/deck.rs

@@ -1,7 +1,6 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
-use super::ConfigKey;
 use crate::prelude::*;
 
 use strum::IntoStaticStr;
@@ -20,11 +19,6 @@ impl DeckConfigKey {
 }
 
 impl Collection {
-    pub(crate) fn get_current_deck_id(&self) -> DeckId {
-        self.get_config_optional(ConfigKey::CurrentDeckId)
-            .unwrap_or(DeckId(1))
-    }
-
     pub(crate) fn clear_aux_config_for_deck(&self, ntid: DeckId) -> Result<()> {
         self.remove_config_prefix(&build_aux_deck_key(ntid, ""))
     }
@@ -38,7 +32,7 @@ impl Collection {
         &mut self,
         did: DeckId,
         ntid: NotetypeId,
-    ) -> Result<()> {
+    ) -> Result<bool> {
         let key = DeckConfigKey::LastNotetype.for_deck(did);
         self.set_config(key.as_str(), &ntid)
     }

rslib/src/config/mod.rs

@@ -103,7 +103,8 @@ impl Collection {
         self.get_config_optional(key).unwrap_or_default()
     }
 
-    pub(crate) fn set_config<'a, T: Serialize, K>(&mut self, key: K, val: &T) -> Result<()>
+    /// True if added, or new value is different.
+    pub(crate) fn set_config<'a, T: Serialize, K>(&mut self, key: K, val: &T) -> Result<bool>
     where
         K: Into<&'a str>,
     {
@@ -148,6 +149,7 @@ impl Collection {
         columns: Vec<browser_table::Column>,
     ) -> Result<()> {
         self.set_config(ConfigKey::DesktopBrowserCardColumns, &columns)
+            .map(|_| ())
     }
 
     pub(crate) fn get_desktop_browser_note_columns(&self) -> Option<Vec<browser_table::Column>> {
@@ -159,6 +161,7 @@ impl Collection {
         columns: Vec<browser_table::Column>,
     ) -> Result<()> {
         self.set_config(ConfigKey::DesktopBrowserNoteColumns, &columns)
+            .map(|_| ())
     }
 
     pub(crate) fn get_creation_utc_offset(&self) -> Option<i32> {
@@ -169,6 +172,7 @@ impl Collection {
         self.state.scheduler_info = None;
         if let Some(mins) = mins {
             self.set_config(ConfigKey::CreationOffset, &mins)
+                .map(|_| ())
         } else {
             self.remove_config(ConfigKey::CreationOffset)
         }
@@ -180,7 +184,7 @@ impl Collection {
 
     pub(crate) fn set_configured_utc_offset(&mut self, mins: i32) -> Result<()> {
         self.state.scheduler_info = None;
-        self.set_config(ConfigKey::LocalOffset, &mins)
+        self.set_config(ConfigKey::LocalOffset, &mins).map(|_| ())
     }
 
     pub(crate) fn get_v2_rollover(&self) -> Option<u8> {
@@ -190,7 +194,7 @@ impl Collection {
 
     pub(crate) fn set_v2_rollover(&mut self, hour: u32) -> Result<()> {
         self.state.scheduler_info = None;
-        self.set_config(ConfigKey::Rollover, &hour)
+        self.set_config(ConfigKey::Rollover, &hour).map(|_| ())
     }
 
     pub(crate) fn get_next_card_position(&self) -> u32 {
@@ -207,6 +211,7 @@ impl Collection {
 
     pub(crate) fn set_next_card_position(&mut self, pos: u32) -> Result<()> {
         self.set_config(ConfigKey::NextNewCardPosition, &pos)
+            .map(|_| ())
     }
 
     pub(crate) fn scheduler_version(&self) -> SchedulerVersion {
@@ -218,6 +223,7 @@ impl Collection {
     pub(crate) fn set_scheduler_version_config_key(&mut self, ver: SchedulerVersion) -> Result<()> {
         self.state.scheduler_info = None;
         self.set_config(ConfigKey::SchedulerVersion, &ver)
+            .map(|_| ())
     }
 
     pub(crate) fn learn_ahead_secs(&self) -> u32 {
@@ -227,6 +233,7 @@ impl Collection {
 
     pub(crate) fn set_learn_ahead_secs(&mut self, secs: u32) -> Result<()> {
         self.set_config(ConfigKey::LearnAheadSecs, &secs)
+            .map(|_| ())
     }
 
     pub(crate) fn get_new_review_mix(&self) -> NewReviewMix {
@@ -239,6 +246,7 @@ impl Collection {
 
     pub(crate) fn set_new_review_mix(&mut self, mix: NewReviewMix) -> Result<()> {
         self.set_config(ConfigKey::NewReviewMix, &(mix as u8))
+            .map(|_| ())
     }
 
     pub(crate) fn get_first_day_of_week(&self) -> Weekday {
@@ -248,6 +256,7 @@ impl Collection {
 
     pub(crate) fn set_first_day_of_week(&mut self, weekday: Weekday) -> Result<()> {
         self.set_config(ConfigKey::FirstDayOfWeek, &weekday)
+            .map(|_| ())
     }
 
     pub(crate) fn get_answer_time_limit_secs(&self) -> u32 {
@@ -257,6 +266,7 @@ impl Collection {
 
     pub(crate) fn set_answer_time_limit_secs(&mut self, secs: u32) -> Result<()> {
         self.set_config(ConfigKey::AnswerTimeLimitSecs, &secs)
+            .map(|_| ())
     }
 
     pub(crate) fn get_last_unburied_day(&self) -> u32 {
@@ -266,6 +276,7 @@ impl Collection {
 
     pub(crate) fn set_last_unburied_day(&mut self, day: u32) -> Result<()> {
         self.set_config(ConfigKey::LastUnburiedDay, &day)
+            .map(|_| ())
     }
 }
 

rslib/src/config/notetype.rs

@@ -30,6 +30,7 @@ impl Collection {
 
     pub(crate) fn set_current_notetype_id(&mut self, ntid: NotetypeId) -> Result<()> {
         self.set_config(ConfigKey::CurrentNotetypeId, &ntid)
+            .map(|_| ())
     }
 
     pub(crate) fn clear_aux_config_for_notetype(&self, ntid: NotetypeId) -> Result<()> {
@@ -43,7 +44,7 @@ impl Collection {
 
     pub(crate) fn set_last_deck_for_notetype(&mut self, id: NotetypeId, did: DeckId) -> Result<()> {
         let key = NotetypeConfigKey::LastDeckAddedTo.for_notetype(id);
-        self.set_config(key.as_str(), &did)
+        self.set_config(key.as_str(), &did).map(|_| ())
     }
 }
 

rslib/src/config/string.rs

@@ -22,7 +22,7 @@ impl Collection {
             .unwrap_or_else(|| default.to_string())
     }
 
-    pub(crate) fn set_string(&mut self, key: StringKey, val: &str) -> Result<()> {
+    pub(crate) fn set_string(&mut self, key: StringKey, val: &str) -> Result<bool> {
         self.set_config(key, &val)
     }
 }

rslib/src/config/undo.rs

@@ -21,16 +21,19 @@ impl Collection {
                     .get_config_entry(&entry.key)?
                     .ok_or_else(|| AnkiError::invalid_input("config disappeared"))?;
                 self.update_config_entry_undoable(entry, current)
+                    .map(|_| ())
             }
             UndoableConfigChange::Removed(entry) => self.add_config_entry_undoable(entry),
         }
     }
 
-    pub(super) fn set_config_undoable(&mut self, entry: Box<ConfigEntry>) -> Result<()> {
+    /// True if added, or value changed.
+    pub(super) fn set_config_undoable(&mut self, entry: Box<ConfigEntry>) -> Result<bool> {
         if let Some(original) = self.storage.get_config_entry(&entry.key)? {
             self.update_config_entry_undoable(entry, original)
         } else {
-            self.add_config_entry_undoable(entry)
+            self.add_config_entry_undoable(entry)?;
+            Ok(true)
         }
     }
 
@@ -49,16 +52,19 @@ impl Collection {
         Ok(())
     }
 
+    /// True if new value differed.
     fn update_config_entry_undoable(
         &mut self,
         entry: Box<ConfigEntry>,
         original: Box<ConfigEntry>,
-    ) -> Result<()> {
+    ) -> Result<bool> {
         if entry.value != original.value {
             self.save_undo(UndoableConfigChange::Updated(original));
             self.storage.set_config_entry(&entry)?;
+            Ok(true)
+        } else {
+            Ok(false)
         }
-        Ok(())
     }
 }
 

rslib/src/decks/current.rs

@@ -0,0 +1,41 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+use std::sync::Arc;
+
+use crate::{config::ConfigKey, prelude::*};
+
+impl Collection {
+    pub fn set_current_deck(&mut self, deck: DeckId) -> Result<OpOutput<()>> {
+        self.transact(Op::SetCurrentDeck, |col| col.set_current_deck_inner(deck))
+    }
+
+    /// Fetch the current deck, falling back to the default if the previously
+    /// selected deck is invalid.
+    pub fn get_current_deck(&mut self) -> Result<Arc<Deck>> {
+        if let Some(deck) = self.get_deck(self.get_current_deck_id())? {
+            return Ok(deck);
+        }
+        self.get_deck(DeckId(1))?.ok_or(AnkiError::NotFound)
+    }
+}
+
+impl Collection {
+    /// The returned id may reference a deck that does not exist;
+    /// prefer using get_current_deck() instead.
+    pub(crate) fn get_current_deck_id(&self) -> DeckId {
+        self.get_config_optional(ConfigKey::CurrentDeckId)
+            .unwrap_or(DeckId(1))
+    }
+
+    fn set_current_deck_inner(&mut self, deck: DeckId) -> Result<()> {
+        if self.set_current_deck_id(deck)? {
+            self.state.card_queues = None;
+        }
+        Ok(())
+    }
+
+    fn set_current_deck_id(&mut self, did: DeckId) -> Result<bool> {
+        self.set_config(ConfigKey::CurrentDeckId, &did)
+    }
+}

rslib/src/decks/mod.rs

@@ -2,6 +2,7 @@
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 mod counts;
+mod current;
 mod filtered;
 mod schema11;
 mod tree;

rslib/src/ops.rs

@@ -23,7 +23,7 @@ pub enum Op {
     RenameTag,
     ReparentTag,
     ScheduleAsNew,
-    SetDeck,
+    SetCardDeck,
     SetDueDate,
     SetFlag,
     SortCards,
@@ -34,6 +34,7 @@ pub enum Op {
     UpdateNote,
     UpdatePreferences,
     UpdateTag,
+    SetCurrentDeck,
 }
 
 impl Op {
@@ -55,7 +56,7 @@ impl Op {
             Op::UpdateNote => tr.undo_update_note(),
             Op::UpdatePreferences => tr.preferences_preferences(),
             Op::UpdateTag => tr.undo_update_tag(),
-            Op::SetDeck => tr.browsing_change_deck(),
+            Op::SetCardDeck => tr.browsing_change_deck(),
             Op::SetFlag => tr.undo_set_flag(),
             Op::FindAndReplace => tr.browsing_find_and_replace(),
             Op::ClearUnusedTags => tr.browsing_clear_unused_tags(),
@@ -68,6 +69,7 @@ impl Op {
             Op::RebuildFilteredDeck => tr.undo_build_filtered_deck(),
             Op::EmptyFilteredDeck => tr.studying_empty(),
             Op::ExpandCollapse => tr.undo_expand_collapse(),
+            Op::SetCurrentDeck => tr.browsing_change_deck(),
         }
         .into()
     }
@@ -80,7 +82,7 @@ pub struct StateChanges {
     pub deck: bool,
     pub tag: bool,
     pub notetype: bool,
-    pub preference: bool,
+    pub config: bool,
 }
 
 #[derive(Debug, Clone, Copy)]
@@ -134,7 +136,12 @@ impl OpChanges {
 
     pub fn requires_study_queue_rebuild(&self) -> bool {
         let c = &self.changes;
-        !matches!(self.op, Op::AnswerCard | Op::ExpandCollapse)
+        if self.op == Op::AnswerCard {
-            && (c.card || c.deck || c.preference)
+            return false;
+        }
+
+        c.card
+            || (c.deck && self.op != Op::ExpandCollapse)
+            || (c.config && matches!(self.op, Op::SetCurrentDeck))
     }
 }

rslib/src/undo/mod.rs

@@ -126,7 +126,7 @@ impl UndoManager {
                 UndoableChange::Tag(_) => changes.tag = true,
                 UndoableChange::Revlog(_) => {}
                 UndoableChange::Queue(_) => {}
-                UndoableChange::Config(_) => {} // fixme: preferences?
+                UndoableChange::Config(_) => changes.config = true,
             }
         }