Merge pull request #968 from abdnh/sidebar-expand-matches

Expand sidebar match trees one level

pylib/anki/tags.py

@@ -13,7 +13,7 @@ from __future__ import annotations
 
 import pprint
 import re
-from typing import Collection, List, Match, Optional, Sequence, Tuple
+from typing import Collection, List, Match, Optional, Sequence
 
 import anki  # pylint: disable=unused-import
 import anki._backend.backend_pb2 as _pb
@@ -28,19 +28,15 @@ class TagManager:
     def __init__(self, col: anki.collection.Collection) -> None:
         self.col = col.weakref()
 
-    # all tags
+    # legacy add-on code expects a List return type
     def all(self) -> List[str]:
-        return [t.name for t in self.col._backend.all_tags()]
+        return list(self.col._backend.all_tags())
 
     def __repr__(self) -> str:
         d = dict(self.__dict__)
         del d["col"]
         return f"{super().__repr__()} {pprint.pformat(d, width=300)}"
 
-    # # List of (tag, usn)
-    def allItems(self) -> List[Tuple[str, int]]:
-        return [(t.name, t.usn) for t in self.col._backend.all_tags()]
-
     def tree(self) -> TagTreeNode:
         return self.col._backend.tag_tree()
 
@@ -72,9 +68,9 @@ class TagManager:
         res = self.col.db.list(query)
         return list(set(self.split(" ".join(res))))
 
-    def set_collapsed(self, tag: str, collapsed: bool) -> None:
-        "Set browser collapse state for tag, registering the tag if missing."
-        self.col._backend.set_tag_collapsed(name=tag, collapsed=collapsed)
+    def set_expanded(self, tag: str, expanded: bool) -> None:
+        "Set browser expansion state for tag, registering the tag if missing."
+        self.col._backend.set_tag_expanded(name=tag, expanded=expanded)
 
     # Bulk addition/removal from notes
     #############################################################

qt/aqt/browser.py

@@ -949,6 +949,8 @@ QTableView {{ gridline-color: {grid} }}
         l = QVBoxLayout()
         l.addWidget(searchBar)
         l.addWidget(self.sidebar)
+        l.setContentsMargins(0, 0, 0, 0)
+        l.setSpacing(0)
         w = QWidget()
         w.setLayout(l)
         dw.setWidget(w)

qt/aqt/sidebar.py

@@ -55,6 +55,8 @@ class SidebarItemType(Enum):
     TEMPLATE = 8
     SAVED_SEARCH_ROOT = 9
     DECK_ROOT = 10
+    NOTETYPE_ROOT = 11
+    TAG_ROOT = 12
 
 
 #  used by an add-on hook
@@ -93,6 +95,8 @@ class SidebarItem:
         self.parentItem: Optional["SidebarItem"] = None
         self.tooltip: Optional[str] = None
         self.row_in_parent: Optional[int] = None
+        self._search_matches_self = False
+        self._search_matches_child = False
 
     def addChild(self, cb: "SidebarItem") -> None:
         self.children.append(cb)
@@ -104,6 +108,31 @@ class SidebarItem:
         except ValueError:
             return None
 
+    def is_expanded(self, searching: bool) -> bool:
+        if not searching:
+            return self.expanded
+        else:
+            if self._search_matches_child:
+                return True
+            # if search matches top level, expand children one level
+            return self._search_matches_self and self.item_type in (
+                SidebarItemType.SAVED_SEARCH_ROOT,
+                SidebarItemType.DECK_ROOT,
+                SidebarItemType.NOTETYPE_ROOT,
+                SidebarItemType.TAG_ROOT,
+            )
+
+    def is_highlighted(self) -> bool:
+        return self._search_matches_self
+
+    def search(self, lowered_text: str) -> bool:
+        "True if we or child matched."
+        self._search_matches_self = lowered_text in self.name.lower()
+        self._search_matches_child = any(
+            [child.search(lowered_text) for child in self.children]
+        )
+        return self._search_matches_self or self._search_matches_child
+
 
 class SidebarModel(QAbstractItemModel):
     def __init__(self, root: SidebarItem) -> None:
@@ -120,6 +149,9 @@ class SidebarModel(QAbstractItemModel):
     def item_for_index(self, idx: QModelIndex) -> SidebarItem:
         return idx.internalPointer()
 
+    def search(self, text: str) -> bool:
+        return self.root.search(text.lower())
+
     # Qt API
     ######################################################################
 
@@ -204,24 +236,20 @@ class SidebarModel(QAbstractItemModel):
 
 
 def expand_where_necessary(
-    model: SidebarModel, tree: QTreeView, parent: Optional[QModelIndex] = None
+    model: SidebarModel,
+    tree: QTreeView,
+    parent: Optional[QModelIndex] = None,
+    searching: bool = False,
 ) -> None:
     parent = parent or QModelIndex()
     for row in range(model.rowCount(parent)):
         idx = model.index(row, 0, parent)
         if not idx.isValid():
             continue
-        expand_where_necessary(model, tree, idx)
-        item = model.item_for_index(idx)
-        if item and item.expanded:
-            tree.setExpanded(idx, True)
-
-
-class FilterModel(QSortFilterProxyModel):
-    def item_for_index(self, idx: QModelIndex) -> Optional[SidebarItem]:
-        if not idx.isValid():
-            return None
-        return self.mapToSource(idx).internalPointer()
+        expand_where_necessary(model, tree, idx, searching)
+        if item := model.item_for_index(idx):
+            if item.is_expanded(searching):
+                tree.setExpanded(idx, True)
 
 
 class SidebarSearchBar(QLineEdit):
@@ -297,7 +325,7 @@ class SidebarTreeView(QTreeView):
         bgcolor = QPalette().window().color().name()
         self.setStyleSheet("QTreeView { background: '%s'; }" % bgcolor)
 
-    def model(self) -> Union[FilterModel, SidebarModel]:
+    def model(self) -> SidebarModel:
         return super().model()
 
     def refresh(self) -> None:
@@ -321,38 +349,27 @@ class SidebarTreeView(QTreeView):
         self.mw.taskman.run_in_background(self._root_tree, on_done)
 
     def search_for(self, text: str) -> None:
+        self.showColumn(0)
         if not text.strip():
             self.current_search = None
             self.refresh()
             return
-        if not isinstance(self.model(), FilterModel):
-            filter_model = FilterModel(self)
-            filter_model.setSourceModel(self.model())
-            filter_model.setFilterCaseSensitivity(False)  # type: ignore
-            filter_model.setRecursiveFilteringEnabled(True)
-            self.setModel(filter_model)
-        else:
-            filter_model = self.model()
 
         self.current_search = text
-        # Without collapsing first, can be very slow. Surely there's
-        # a better way than this?
+        # start from a collapsed state, as it's faster
         self.collapseAll()
-        filter_model.setFilterFixedString(text)
-        self.expandAll()
+        self.setColumnHidden(0, not self.model().search(text))
+        expand_where_necessary(self.model(), self, searching=True)
 
     def drawRow(
         self, painter: QPainter, options: QStyleOptionViewItem, idx: QModelIndex
     ) -> None:
-        if self.current_search is None:
-            return super().drawRow(painter, options, idx)
-        if not (item := self.model().item_for_index(idx)):
-            return super().drawRow(painter, options, idx)
-        if self.current_search.lower() in item.name.lower():
-            brush = QBrush(theme_manager.qcolor("suspended-bg"))
-            painter.save()
-            painter.fillRect(options.rect, brush)
-            painter.restore()
+        if self.current_search and (item := self.model().item_for_index(idx)):
+            if item.is_highlighted():
+                brush = QBrush(theme_manager.qcolor("suspended-bg"))
+                painter.save()
+                painter.fillRect(options.rect, brush)
+                painter.restore()
         return super().drawRow(painter, options, idx)
 
     def dropEvent(self, event: QDropEvent) -> None:
@@ -522,8 +539,8 @@ class SidebarTreeView(QTreeView):
 
                 def toggle_expand() -> Callable[[bool], None]:
                     full_name = head + node.name  # pylint: disable=cell-var-from-loop
-                    return lambda expanded: self.mw.col.tags.set_collapsed(
-                        full_name, not expanded
+                    return lambda expanded: self.mw.col.tags.set_expanded(
+                        full_name, expanded
                     )
 
                 item = SidebarItem(
@@ -531,7 +548,7 @@ class SidebarTreeView(QTreeView):
                     icon,
                     self._filter_func(SearchTerm(tag=head + node.name)),
                     toggle_expand(),
-                    not node.collapsed,
+                    node.expanded,
                     item_type=SidebarItemType.TAG,
                     full_name=head + node.name,
                 )
@@ -545,6 +562,7 @@ class SidebarTreeView(QTreeView):
             name=TR.BROWSING_SIDEBAR_TAGS,
             icon=icon,
             collapse_key=ConfigBoolKey.COLLAPSE_TAGS,
+            type=SidebarItemType.TAG_ROOT,
         )
         render(root, tree.children)
 
@@ -591,6 +609,7 @@ class SidebarTreeView(QTreeView):
             name=TR.BROWSING_SIDEBAR_NOTETYPES,
             icon=icon,
             collapse_key=ConfigBoolKey.COLLAPSE_NOTETYPES,
+            type=SidebarItemType.NOTETYPE_ROOT,
         )
 
         for nt in sorted(self.col.models.all(), key=lambda nt: nt["name"].lower()):

rslib/backend.proto

@@ -41,6 +41,10 @@ message Bool {
   bool val = 1;
 }
 
+message StringList {
+  repeated string vals = 1;
+}
+
 // IDs used in RPC calls
 ///////////////////////////////////////////////////////////
 
@@ -212,8 +216,8 @@ service BackendService {
   // tags
 
   rpc ClearUnusedTags(Empty) returns (Empty);
-  rpc AllTags(Empty) returns (AllTagsOut);
-  rpc SetTagCollapsed(SetTagCollapsedIn) returns (Empty);
+  rpc AllTags(Empty) returns (StringList);
+  rpc SetTagExpanded(SetTagExpandedIn) returns (Empty);
   rpc ClearTag(String) returns (Empty);
   rpc TagTree(Empty) returns (TagTreeNode);
 
@@ -842,19 +846,9 @@ message AddOrUpdateDeckConfigLegacyIn {
   bool preserve_usn_and_mtime = 2;
 }
 
-message AllTagsOut {
-  repeated Tag tags = 1;
-}
-
-message SetTagCollapsedIn {
+message SetTagExpandedIn {
   string name = 1;
-  bool collapsed = 2;
-}
-
-message Tag {
-  string name = 1;
-  sint32 usn = 2;
-  bool collapsed = 3;
+  bool expanded = 2;
 }
 
 message GetChangedTagsOut {
@@ -865,7 +859,7 @@ message TagTreeNode {
   string name = 1;
   repeated TagTreeNode children = 2;
   uint32 level = 3;
-  bool collapsed = 4;
+  bool expanded = 4;
 }
 
 message SetConfigJsonIn {

rslib/src/backend/mod.rs

@@ -1399,22 +1399,23 @@ impl BackendService for Backend {
     // tags
     //-------------------------------------------------------------------
 
-    fn all_tags(&self, _input: Empty) -> BackendResult<pb::AllTagsOut> {
-        let tags: Vec<pb::Tag> = self.with_col(|col| {
-            Ok(col
-                .storage
-                .all_tags()?
-                .into_iter()
-                .map(|t| t.into())
-                .collect())
-        })?;
-        Ok(pb::AllTagsOut { tags })
+    fn all_tags(&self, _input: Empty) -> BackendResult<pb::StringList> {
+        Ok(pb::StringList {
+            vals: self.with_col(|col| {
+                Ok(col
+                    .storage
+                    .all_tags()?
+                    .into_iter()
+                    .map(|t| t.name)
+                    .collect())
+            })?,
+        })
     }
 
-    fn set_tag_collapsed(&self, input: pb::SetTagCollapsedIn) -> BackendResult<pb::Empty> {
+    fn set_tag_expanded(&self, input: pb::SetTagExpandedIn) -> BackendResult<pb::Empty> {
         self.with_col(|col| {
             col.transact(None, |col| {
-                col.set_tag_collapsed(&input.name, input.collapsed)?;
+                col.set_tag_expanded(&input.name, input.expanded)?;
                 Ok(().into())
             })
         })

rslib/src/dbcheck.rs

@@ -242,7 +242,7 @@ impl Collection {
         let usn = self.usn()?;
         let stamp = TimestampMillis::now();
 
-        let collapsed_tags = self.storage.collapsed_tags()?;
+        let expanded_tags = self.storage.expanded_tags()?;
         self.storage.clear_tags()?;
 
         let total_notes = self.storage.total_notes()?;
@@ -296,7 +296,7 @@ impl Collection {
 
         // the note rebuilding process took care of adding tags back, so we just need
         // to ensure to restore the collapse state
-        self.storage.restore_collapsed_tags(&collapsed_tags)?;
+        self.storage.restore_expanded_tags(&expanded_tags)?;
 
         // if the collection is empty and the user has deleted all note types, ensure at least
         // one note type exists
@@ -646,12 +646,12 @@ mod test {
         note.tags.push("two".into());
         col.add_note(&mut note, DeckID(1))?;
 
-        col.set_tag_collapsed("two", true)?;
+        col.set_tag_expanded("one", true)?;
 
         col.check_database(progress_fn)?;
 
-        assert_eq!(col.storage.get_tag("one")?.unwrap().collapsed, false);
-        assert_eq!(col.storage.get_tag("two")?.unwrap().collapsed, true);
+        assert_eq!(col.storage.get_tag("one")?.unwrap().expanded, true);
+        assert_eq!(col.storage.get_tag("two")?.unwrap().expanded, false);
 
         Ok(())
     }

rslib/src/decks/mod.rs

@@ -45,7 +45,11 @@ impl Deck {
             name: "".into(),
             mtime_secs: TimestampSecs(0),
             usn: Usn(0),
-            common: DeckCommon::default(),
+            common: DeckCommon {
+                study_collapsed: true,
+                browser_collapsed: true,
+                ..Default::default()
+            },
             kind: DeckKind::Normal(norm),
         }
     }

rslib/src/filtered.rs

@@ -133,7 +133,11 @@ impl Deck {
             name: "".into(),
             mtime_secs: TimestampSecs(0),
             usn: Usn(0),
-            common: DeckCommon::default(),
+            common: DeckCommon {
+                study_collapsed: true,
+                browser_collapsed: true,
+                ..Default::default()
+            },
             kind: DeckKind::Filtered(filt),
         }
     }

rslib/src/storage/tag/mod.rs

@@ -11,7 +11,7 @@ fn row_to_tag(row: &Row) -> Result<Tag> {
     Ok(Tag {
         name: row.get(0)?,
         usn: row.get(1)?,
-        collapsed: row.get(2)?,
+        expanded: !row.get(2)?,
     })
 }
 
@@ -24,17 +24,17 @@ impl SqliteStorage {
             .collect()
     }
 
-    pub(crate) fn collapsed_tags(&self) -> Result<Vec<String>> {
+    pub(crate) fn expanded_tags(&self) -> Result<Vec<String>> {
         self.db
-            .prepare_cached("select tag from tags where collapsed = true")?
+            .prepare_cached("select tag from tags where collapsed = false")?
             .query_and_then(NO_PARAMS, |r| r.get::<_, String>(0).map_err(Into::into))?
             .collect::<Result<Vec<_>>>()
     }
 
-    pub(crate) fn restore_collapsed_tags(&self, tags: &[String]) -> Result<()> {
+    pub(crate) fn restore_expanded_tags(&self, tags: &[String]) -> Result<()> {
         let mut stmt = self
             .db
-            .prepare_cached("update tags set collapsed = true where tag = ?")?;
+            .prepare_cached("update tags set collapsed = false where tag = ?")?;
         for tag in tags {
             stmt.execute(&[tag])?;
         }
@@ -52,7 +52,7 @@ impl SqliteStorage {
     pub(crate) fn register_tag(&self, tag: &Tag) -> Result<()> {
         self.db
             .prepare_cached(include_str!("add.sql"))?
-            .execute(params![tag.name, tag.usn, tag.collapsed])?;
+            .execute(params![tag.name, tag.usn, !tag.expanded])?;
         Ok(())
     }
 

rslib/src/storage/upgrades/mod.rs

@@ -35,6 +35,10 @@ impl SqliteStorage {
             self.upgrade_tags_to_schema17()?;
             self.db.execute_batch("update col set ver = 17")?;
         }
+        // fixme: on the next schema upgrade, change _collapsed to _expanded
+        // in DeckCommon and invert existing values, so that we can avoid
+        // serializing the values in the default case, and use
+        // DeckCommon::default() in new_normal() and new_filtered()
 
         Ok(())
     }

rslib/src/tags.rs

@@ -2,7 +2,7 @@
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 use crate::{
-    backend_proto::{Tag as TagProto, TagTreeNode},
+    backend_proto::TagTreeNode,
     collection::Collection,
     err::{AnkiError, Result},
     notes::{NoteID, TransformNoteOutput},
@@ -18,27 +18,7 @@ use unicase::UniCase;
 pub struct Tag {
     pub name: String,
     pub usn: Usn,
-    pub collapsed: bool,
-}
-
-impl From<Tag> for TagProto {
-    fn from(t: Tag) -> Self {
-        TagProto {
-            name: t.name,
-            usn: t.usn.0,
-            collapsed: t.collapsed,
-        }
-    }
-}
-
-impl From<TagProto> for Tag {
-    fn from(t: TagProto) -> Self {
-        Tag {
-            name: t.name,
-            usn: Usn(t.usn),
-            collapsed: t.collapsed,
-        }
-    }
+    pub expanded: bool,
 }
 
 impl Tag {
@@ -46,7 +26,7 @@ impl Tag {
         Tag {
             name,
             usn,
-            collapsed: false,
+            expanded: false,
         }
     }
 }
@@ -171,7 +151,7 @@ fn add_child_nodes(tags: &mut Peekable<impl Iterator<Item = Tag>>, parent: &mut
                     name: (*split_name.last().unwrap()).into(),
                     children: vec![],
                     level: parent.level + 1,
-                    collapsed: tag.collapsed,
+                    expanded: tag.expanded,
                 });
                 tags.next();
             }
@@ -273,13 +253,13 @@ impl Collection {
     }
 
     pub fn clear_unused_tags(&self) -> Result<()> {
-        let collapsed: HashSet<_> = self.storage.collapsed_tags()?.into_iter().collect();
+        let expanded: HashSet<_> = self.storage.expanded_tags()?.into_iter().collect();
         self.storage.clear_tags()?;
         let usn = self.usn()?;
         for name in self.storage.all_tags_in_notes()? {
             let name = normalize_tag_name(&name).into();
             self.storage.register_tag(&Tag {
-                collapsed: collapsed.contains(&name),
+                expanded: expanded.contains(&name),
                 name,
                 usn,
             })?;
@@ -288,7 +268,7 @@ impl Collection {
         Ok(())
     }
 
-    pub(crate) fn set_tag_collapsed(&self, name: &str, collapsed: bool) -> Result<()> {
+    pub(crate) fn set_tag_expanded(&self, name: &str, expanded: bool) -> Result<()> {
         let mut name = name;
         let tag;
         if self.storage.get_tag(name)?.is_none() {
@@ -297,7 +277,7 @@ impl Collection {
             self.storage.register_tag(&tag)?;
             name = &tag.name;
         }
-        self.storage.set_tag_collapsed(name, collapsed)
+        self.storage.set_tag_collapsed(name, !expanded)
     }
 
     fn replace_tags_for_notes_inner<R: Replacer>(
@@ -503,6 +483,7 @@ mod test {
             name: name.into(),
             level,
             children,
+
             ..Default::default()
         }
     }
@@ -607,10 +588,10 @@ mod test {
         note.tags.push("two".into());
         col.add_note(&mut note, DeckID(1))?;
 
-        col.set_tag_collapsed("two", true)?;
+        col.set_tag_expanded("one", true)?;
         col.clear_unused_tags()?;
-        assert_eq!(col.storage.get_tag("one")?.unwrap().collapsed, false);
-        assert_eq!(col.storage.get_tag("two")?.unwrap().collapsed, true);
+        assert_eq!(col.storage.get_tag("one")?.unwrap().expanded, true);
+        assert_eq!(col.storage.get_tag("two")?.unwrap().expanded, false);
 
         Ok(())
     }