From 964a69e54e7f41c34143a92cb6bdc1a7360ab625 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Fri, 15 May 2020 21:21:10 +1000 Subject: [PATCH] handle default deck and filtered deck suppression in the backend --- proto/backend.proto | 8 ++- pylib/anki/decks.py | 107 +++++++--------------------------- pylib/anki/rsbackend.py | 17 ++++-- pylib/anki/sched.py | 4 +- pylib/tests/test_decks.py | 25 +++----- pylib/tests/test_schedv2.py | 10 ++-- qt/aqt/browser.py | 5 -- qt/aqt/exporting.py | 3 +- qt/aqt/main.py | 3 +- qt/aqt/studydeck.py | 11 ++-- qt/aqt/tagedit.py | 4 +- rslib/src/backend/mod.rs | 10 +++- rslib/src/decks/mod.rs | 30 ++++++++++ rslib/src/decks/tree.rs | 23 +++++++- rslib/src/storage/deck/mod.rs | 9 +++ 15 files changed, 131 insertions(+), 138 deletions(-) diff --git a/proto/backend.proto b/proto/backend.proto index 649fbea93..402fa1b54 100644 --- a/proto/backend.proto +++ b/proto/backend.proto @@ -84,7 +84,7 @@ message BackendInput { Empty get_empty_cards = 70; int64 get_deck_legacy = 71; string get_deck_id_by_name = 72; - Empty get_deck_names = 73; + GetDeckNamesIn get_deck_names = 73; AddOrUpdateDeckLegacyIn add_or_update_deck_legacy = 74; bool new_deck_legacy = 75; int64 remove_deck = 76; @@ -784,3 +784,9 @@ message Preferences { message ClozeNumbersInNoteOut { repeated uint32 numbers = 1; } + +message GetDeckNamesIn { + bool skip_empty_default = 1; + // if unset, implies skip_empty_default + bool include_filtered = 2; +} diff --git a/pylib/anki/decks.py b/pylib/anki/decks.py index 6fafae871..4220e6e46 100644 --- a/pylib/anki/decks.py +++ b/pylib/anki/decks.py @@ -4,8 +4,7 @@ from __future__ import annotations import copy -import unicodedata -from typing import Any, Dict, List, Optional, Tuple, Union +from typing import Any, Dict, List, Optional, Sequence, Tuple, Union import anki # pylint: disable=unused-import import anki.backend_pb2 as pb @@ -186,17 +185,13 @@ class DeckManager: if did in self.active(): self.select(self.all_names_and_ids()[0].id) - def allNames(self, dyn: bool = True, force_default: bool = True) -> List: - "An unsorted list of all deck names." - if dyn: - return [x["name"] for x in self.all(force_default=force_default)] - else: - return [ - x["name"] for x in self.all(force_default=force_default) if not x["dyn"] - ] - - def all_names_and_ids(self) -> List[pb.DeckNameID]: - return self.col.backend.get_deck_names_and_ids() + def all_names_and_ids( + self, skip_empty_default=False, include_filtered=True + ) -> Sequence[pb.DeckNameID]: + "A sorted sequence of deck names and IDs." + return self.col.backend.get_deck_names_and_ids( + skip_empty_default, include_filtered + ) def id_for_name(self, name: str) -> Optional[int]: return self.col.backend.get_deck_id_by_name(name) @@ -221,29 +216,28 @@ class DeckManager: def deck_tree(self) -> pb.DeckTreeNode: return self.col.backend.deck_tree(include_counts=False) - def all(self, force_default: bool = True) -> List: - """A list of all decks. - - list contains default deck if either: - * force_default is True - * there are no other deck - * default deck contains a card - * default deck has a child (assumed not to be the case if assume_no_child) - """ - decks = self.get_all_legacy() - if not force_default and not self.should_default_be_displayed(force_default): - decks = [deck for deck in decks if deck["id"] != 1] - return decks + def all(self) -> List: + "All decks. Expensive; prefer all_names_and_ids()" + return self.get_all_legacy() def allIds(self) -> List[str]: + print("decks.allIds() is deprecated, use .all_names_and_ids()") return [str(x.id) for x in self.all_names_and_ids()] + def allNames(self, dyn: bool = True, force_default: bool = True) -> List: + print("decks.allNames() is deprecated, use .all_names_and_ids()") + return [ + x.name + for x in self.all_names_and_ids( + skip_empty_default=not force_default, include_filtered=dyn + ) + ] + def collapse(self, did) -> None: deck = self.get(did) deck["collapsed"] = not deck["collapsed"] self.save(deck) - # fixme def collapseBrowser(self, did) -> None: deck = self.get(did) collapsed = deck.get("browserCollapsed", False) @@ -504,57 +498,6 @@ class DeckManager: def for_card_ids(self, cids: List[int]) -> List[int]: return self.col.db.list(f"select did from cards where id in {ids2str(cids)}") - # fixme - def _recoverOrphans(self) -> None: - pass - # dids = list(self.decks.keys()) - # mod = self.col.db.mod - # self.col.db.execute( - # "update cards set did = 1 where did not in " + ids2str(dids) - # ) - # self.col.db.mod = mod - - def checkIntegrity(self) -> None: - self._recoverOrphans() - - def should_deck_be_displayed( - self, deck, force_default: bool = True, assume_no_child: bool = False - ) -> bool: - """Whether the deck should appear in main window, browser side list, filter, deck selection... - - True, except for empty default deck without children""" - if deck["id"] != "1": - return True - return self.should_default_be_displayed(force_default, assume_no_child) - - def should_default_be_displayed( - self, - force_default: bool = True, - assume_no_child: bool = False, - default_deck: Optional[Dict[str, Any]] = None, - ) -> bool: - """Whether the default deck should appear in main window, browser side list, filter, deck selection... - - True, except for empty default deck (without children)""" - if force_default: - return True - if self.col.db.scalar("select 1 from cards where did = 1 limit 1"): - return True - # fixme - return False - # if len(self.all_names_and_ids()) == 1: - # return True - # # looking for children - # if assume_no_child: - # return False - # if default_deck is None: - # default_deck = self.get(1) - # defaultName = default_deck["name"] - # for name in self.allNames(): - # if name.startswith(f"{defaultName}::"): - # return True - # return False - # Deck selection ############################################################# @@ -673,11 +616,3 @@ class DeckManager: def isDyn(self, did: Union[int, str]) -> Any: return self.get(did)["dyn"] - - @staticmethod - def normalizeName(name: str) -> str: - return unicodedata.normalize("NFC", name.lower()) - - @staticmethod - def equalName(name1: str, name2: str) -> bool: - return DeckManager.normalizeName(name1) == DeckManager.normalizeName(name2) diff --git a/pylib/anki/rsbackend.py b/pylib/anki/rsbackend.py index 1ef8b2ee3..07f5f0b9b 100644 --- a/pylib/anki/rsbackend.py +++ b/pylib/anki/rsbackend.py @@ -686,12 +686,17 @@ class RustBackend: except NotFoundError: return None - def get_deck_names_and_ids(self) -> List[pb.DeckNameID]: - return list( - self._run_command( - pb.BackendInput(get_deck_names=pb.Empty()) - ).get_deck_names.entries - ) + def get_deck_names_and_ids( + self, skip_empty_default: bool, include_filtered: bool = True + ) -> Sequence[pb.DeckNameID]: + return self._run_command( + pb.BackendInput( + get_deck_names=pb.GetDeckNamesIn( + skip_empty_default=skip_empty_default, + include_filtered=include_filtered, + ) + ) + ).get_deck_names.entries def add_or_update_deck_legacy( self, deck: Dict[str, Any], preserve_usn: bool diff --git a/pylib/anki/sched.py b/pylib/anki/sched.py index 87a172e46..6cdf02d19 100644 --- a/pylib/anki/sched.py +++ b/pylib/anki/sched.py @@ -395,7 +395,9 @@ limit %d""" else: # benchmarks indicate it's about 10x faster to search all decks # with the index than scan the table - extra = " and did in " + ids2str(self.col.decks.allIds()) + extra = " and did in " + ids2str( + d.id for d in self.col.decks.all_names_and_ids() + ) # review cards in relearning self.col.db.execute( f""" diff --git a/pylib/tests/test_decks.py b/pylib/tests/test_decks.py index 0bd8a35ae..1cd1494dd 100644 --- a/pylib/tests/test_decks.py +++ b/pylib/tests/test_decks.py @@ -65,9 +65,10 @@ def test_rename(): # should be able to rename into a completely different branch, creating # parents as necessary d.decks.rename(d.decks.get(id), "foo::bar") - assert "foo" in d.decks.allNames() - assert "foo::bar" in d.decks.allNames() - assert "hello::world" not in d.decks.allNames() + names = [n.name for n in d.decks.all_names_and_ids()] + assert "foo" in names + assert "foo::bar" in names + assert "hello::world" not in names # create another deck id = d.decks.id("tmp") # we can't rename it if it conflicts @@ -76,8 +77,9 @@ def test_rename(): d.decks.id("one::two::three") id = d.decks.id("one") d.decks.rename(d.decks.get(id), "yo") + names = [n.name for n in d.decks.all_names_and_ids()] for n in "yo", "yo::two", "yo::two::three": - assert n in d.decks.allNames() + assert n in names # over filtered filteredId = d.decks.newDyn("filtered") filtered = d.decks.get(filteredId) @@ -96,7 +98,7 @@ def test_renameForDragAndDrop(): d = getEmptyCol() def deckNames(): - return [name for name in sorted(d.decks.allNames()) if name != "Default"] + return [n.name for n in d.decks.all_names_and_ids(skip_empty_default=True)] languages_did = d.decks.id("Languages") chinese_did = d.decks.id("Chinese") @@ -151,16 +153,3 @@ def test_renameForDragAndDrop(): # '' is a convenient alias for the top level DID d.decks.renameForDragAndDrop(hsk_did, "") assert deckNames() == ["Chinese", "HSK", "Languages"] - - -def test_check(): - d = getEmptyCol() - - # currently disabled - see 5418af00f733ca62b0c087d1422feae01d6571b0 - # foo_did = d.decks.id("foo") - # FOO_did = d.decks.id("bar") - # FOO = d.decks.byName("bar") - # FOO["name"] = "FOO" - # d.decks.save(FOO) - # d.decks._checkDeckTree() - # assert "foo" not in d.decks.allNames() or "FOO" not in d.decks.allNames() diff --git a/pylib/tests/test_schedv2.py b/pylib/tests/test_schedv2.py index 364f15636..a25dfbd08 100644 --- a/pylib/tests/test_schedv2.py +++ b/pylib/tests/test_schedv2.py @@ -441,9 +441,9 @@ def test_review_limits(): c.flush() tree = d.sched.deckDueTree() - # (('Default', 1, 0, 0, 0, ()), ('parent', 1514457677462, 5, 0, 0, (('child', 1514457677463, 5, 0, 0, ()),))) - assert tree[1][2] == 5 # parent - assert tree[1][5][0][2] == 5 # child + # (('parent', 1514457677462, 5, 0, 0, (('child', 1514457677463, 5, 0, 0, ()),))) + assert tree[0][2] == 5 # parent + assert tree[0][5][0][2] == 5 # child # .counts() should match d.decks.select(child["id"]) @@ -456,8 +456,8 @@ def test_review_limits(): assert d.sched.counts() == (0, 0, 4) tree = d.sched.deckDueTree() - assert tree[1][2] == 4 # parent - assert tree[1][5][0][2] == 4 # child + assert tree[0][2] == 4 # parent + assert tree[0][5][0][2] == 4 # child def test_button_spacing(): diff --git a/qt/aqt/browser.py b/qt/aqt/browser.py index e22f04092..b7ee5e4b0 100644 --- a/qt/aqt/browser.py +++ b/qt/aqt/browser.py @@ -1148,11 +1148,6 @@ QTableView {{ gridline-color: {grid} }} def fillGroups(root, nodes: Sequence[DeckTreeNode], head=""): for node in nodes: - if node.deck_id == 1 and not node.children: - if not self.mw.col.decks.should_default_be_displayed( - force_default=False, assume_no_child=True - ): - continue def set_filter(): full_name = head + node.name # pylint: disable=cell-var-from-loop diff --git a/qt/aqt/exporting.py b/qt/aqt/exporting.py index ced41aa95..782e4d383 100644 --- a/qt/aqt/exporting.py +++ b/qt/aqt/exporting.py @@ -49,7 +49,8 @@ class ExportDialog(QDialog): self.exporterChanged(idx) # deck list if self.cids is None: - self.decks = [_("All Decks")] + sorted(self.col.decks.allNames()) + self.decks = [_("All Decks")] + self.decks.extend(d.name for d in self.col.decks.all_names_and_ids()) else: self.decks = [_("Selected Notes")] self.frm.deck.addItems(self.decks) diff --git a/qt/aqt/main.py b/qt/aqt/main.py index fe355ff34..a5e472c4f 100644 --- a/qt/aqt/main.py +++ b/qt/aqt/main.py @@ -1115,8 +1115,7 @@ title="%s" %s>%s""" % ( if not search: if not deck["dyn"]: search = 'deck:"%s" ' % deck["name"] - decks = self.col.decks.allNames() - while _("Filtered Deck %d") % n in decks: + while self.col.decks.id_for_name(_("Filtered Deck %d") % n): n += 1 name = _("Filtered Deck %d") % n did = self.col.decks.newDyn(name) diff --git a/qt/aqt/studydeck.py b/qt/aqt/studydeck.py index 9e0f02f8d..afa321cb1 100644 --- a/qt/aqt/studydeck.py +++ b/qt/aqt/studydeck.py @@ -3,7 +3,6 @@ # License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html import aqt -from anki.decks import DeckManager from anki.lang import _ from aqt import gui_hooks from aqt.qt import * @@ -52,10 +51,12 @@ class StudyDeck(QDialog): if title: self.setWindowTitle(title) if not names: - names = sorted( - self.mw.col.decks.allNames(dyn=dyn, force_default=False), - key=DeckManager._path, - ) + names = [ + d.name + for d in self.mw.col.decks.all_names_and_ids( + include_filtered=dyn, skip_empty_default=True + ) + ] self.nameFunc = None self.origNames = names else: diff --git a/qt/aqt/tagedit.py b/qt/aqt/tagedit.py index 5a673eb47..1ad91c5c8 100644 --- a/qt/aqt/tagedit.py +++ b/qt/aqt/tagedit.py @@ -32,9 +32,9 @@ class TagEdit(QLineEdit): "Set the current col, updating list of available tags." self.col = col if self.type == 0: - l = sorted(self.col.tags.all()) + l = self.col.tags.all() else: - l = sorted(self.col.decks.allNames()) + l = (d.name for d in self.col.decks.all_names_and_ids()) self.model.setStringList(l) def focusInEvent(self, evt): diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index e53e8cd31..1da602bbc 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -341,7 +341,7 @@ impl Backend { Value::GetDeckIdByName(name) => { OValue::GetDeckIdByName(self.get_deck_id_by_name(&name)?) } - Value::GetDeckNames(_) => OValue::GetDeckNames(self.get_deck_names()?), + Value::GetDeckNames(input) => OValue::GetDeckNames(self.get_deck_names(input)?), Value::AddOrUpdateDeckLegacy(input) => { OValue::AddOrUpdateDeckLegacy(self.add_or_update_deck_legacy(input)?) } @@ -1011,9 +1011,13 @@ impl Backend { }) } - fn get_deck_names(&self) -> Result { + fn get_deck_names(&self, input: pb::GetDeckNamesIn) -> Result { self.with_col(|col| { - let names = col.storage.get_all_deck_names()?; + let names = if input.include_filtered { + col.get_all_deck_names(input.skip_empty_default)? + } else { + col.get_all_normal_deck_names()? + }; Ok(pb::DeckNames { entries: names .into_iter() diff --git a/rslib/src/decks/mod.rs b/rslib/src/decks/mod.rs index aa58213cb..3832ad5e8 100644 --- a/rslib/src/decks/mod.rs +++ b/rslib/src/decks/mod.rs @@ -187,6 +187,10 @@ fn immediate_parent_name(machine_name: &str) -> Option<&str> { } impl Collection { + pub(crate) fn default_deck_is_empty(&self) -> Result { + self.storage.deck_is_empty(DeckID(1)) + } + pub(crate) fn add_or_update_deck(&mut self, deck: &mut Deck, preserve_usn: bool) -> Result<()> { // fixme: vet cache clearing self.state.deck_cache.clear(); @@ -416,6 +420,32 @@ impl Collection { } Ok(()) } + + pub fn get_all_deck_names(&self, skip_empty_default: bool) -> Result> { + if skip_empty_default && self.default_deck_is_empty()? { + Ok(self + .storage + .get_all_deck_names()? + .into_iter() + .filter(|(id, _name)| id.0 != 1) + .collect()) + } else { + self.storage.get_all_deck_names() + } + } + + pub fn get_all_normal_deck_names(&mut self) -> Result> { + Ok(self + .storage + .get_all_deck_names()? + .into_iter() + .filter(|(id, _name)| id.0 != 1) + .filter(|(id, _name)| match self.get_deck(*id) { + Ok(Some(deck)) => !deck.is_filtered(), + _ => true, + }) + .collect()) + } } #[cfg(test)] diff --git a/rslib/src/decks/tree.rs b/rslib/src/decks/tree.rs index 21cff4f60..ff7a0386d 100644 --- a/rslib/src/decks/tree.rs +++ b/rslib/src/decks/tree.rs @@ -151,6 +151,21 @@ fn remaining_counts_for_deck( } } +fn hide_default_deck(node: &mut DeckTreeNode) { + for (idx, child) in node.children.iter().enumerate() { + // we can hide the default if it has no children + if child.deck_id == 1 && child.children.is_empty() { + if child.level == 1 && node.children.len() == 1 { + // can't remove if there are no other decks + } else { + // safe to remove + node.children.remove(idx); + } + return; + } + } +} + #[derive(Serialize_tuple)] pub(crate) struct LegacyDueCounts { name: String, @@ -187,6 +202,9 @@ impl Collection { .collect(); add_collapsed(&mut tree, &decks_map, !counts); + if self.default_deck_is_empty()? { + hide_default_deck(&mut tree); + } if counts { let counts = self.due_counts()?; @@ -251,8 +269,7 @@ mod test { let tree = col.deck_tree(false)?; - // 4 including default - assert_eq!(tree.children.len(), 4); + assert_eq!(tree.children.len(), 3); assert_eq!(tree.children[1].name, "2"); assert_eq!(tree.children[1].children[0].name, "a"); @@ -274,7 +291,7 @@ mod test { col.storage.remove_deck(col.get_deck_id("2::3")?.unwrap())?; let tree = col.deck_tree(false)?; - assert_eq!(tree.children.len(), 2); + assert_eq!(tree.children.len(), 1); Ok(()) } diff --git a/rslib/src/storage/deck/mod.rs b/rslib/src/storage/deck/mod.rs index 93ece69c9..dd9530876 100644 --- a/rslib/src/storage/deck/mod.rs +++ b/rslib/src/storage/deck/mod.rs @@ -178,6 +178,15 @@ impl SqliteStorage { .collect() } + pub(crate) fn deck_is_empty(&self, did: DeckID) -> Result { + self.db + .prepare_cached("select null from cards where did=?")? + .query(&[did])? + .next() + .map(|o| o.is_none()) + .map_err(Into::into) + } + // Upgrading/downgrading/legacy pub(super) fn add_default_deck(&self, i18n: &I18n) -> Result<()> {