diff --git a/ftl/core/actions.ftl b/ftl/core/actions.ftl index 11be42642..01039426e 100644 --- a/ftl/core/actions.ftl +++ b/ftl/core/actions.ftl @@ -25,6 +25,7 @@ actions-red-flag = Red Flag actions-rename = Rename actions-rename-deck = Rename Deck actions-rename-tag = Rename Tag +actions-remove-tag = Remove Tag actions-replay-audio = Replay Audio actions-reposition = Reposition actions-save = Save diff --git a/pylib/anki/rsbackend.py b/pylib/anki/rsbackend.py index 88316b23f..0a4c20b03 100644 --- a/pylib/anki/rsbackend.py +++ b/pylib/anki/rsbackend.py @@ -42,7 +42,8 @@ SchedTimingToday = pb.SchedTimingTodayOut BuiltinSortKind = pb.BuiltinSearchOrder.BuiltinSortKind BackendCard = pb.Card BackendNote = pb.Note -TagUsnTuple = pb.TagUsnTuple +Tag = pb.Tag +TagTreeNode = pb.TagTreeNode NoteType = pb.NoteType DeckTreeNode = pb.DeckTreeNode StockNoteType = pb.StockNoteType diff --git a/pylib/anki/tags.py b/pylib/anki/tags.py index f4f8008e3..5ced38aa2 100644 --- a/pylib/anki/tags.py +++ b/pylib/anki/tags.py @@ -16,6 +16,7 @@ import re from typing import Collection, List, Optional, Sequence, Tuple import anki # pylint: disable=unused-import +from anki.rsbackend import FilterToSearchIn from anki.utils import ids2str @@ -25,7 +26,7 @@ class TagManager: # all tags def all(self) -> List[str]: - return [t.tag for t in self.col.backend.all_tags()] + return [t.name for t in self.col.backend.all_tags()] def __repr__(self) -> str: d = dict(self.__dict__) @@ -34,7 +35,7 @@ class TagManager: # # List of (tag, usn) def allItems(self) -> List[Tuple[str, int]]: - return [(t.tag, t.usn) for t in self.col.backend.all_tags()] + return [(t.name, t.usn) for t in self.col.backend.all_tags()] # Registering and fetching tags ############################################################# @@ -42,34 +43,14 @@ class TagManager: def register( self, tags: Collection[str], usn: Optional[int] = None, clear=False ) -> None: - if usn is None: - preserve_usn = False - usn_ = 0 - else: - usn_ = usn - preserve_usn = True - - self.col.backend.register_tags( - tags=" ".join(tags), preserve_usn=preserve_usn, usn=usn_, clear_first=clear - ) + print("tags.register() is deprecated and no longer works") def registerNotes(self, nids: Optional[List[int]] = None) -> None: - "Add any missing tags from notes to the tags list." - # when called without an argument, the old list is cleared first. - if nids: - lim = " where id in " + ids2str(nids) - clear = False - else: - lim = "" - clear = True - self.register( - set( - self.split( - " ".join(self.col.db.list("select distinct tags from notes" + lim)) - ) - ), - clear=clear, - ) + "Clear unused tags and add any missing tags from notes to the tag list." + self.clear_unused_tags() + + def clear_unused_tags(self): + self.col.backend.clear_unused_tags() def byDeck(self, did, children=False) -> List[str]: basequery = "select n.tags from cards c, notes n WHERE c.nid = n.id" @@ -84,6 +65,10 @@ class TagManager: res = self.col.db.list(query) return list(set(self.split(" ".join(res)))) + def set_collapsed(self, tag: str, collapsed: bool): + "Set browser collapse state for tag, registering the tag if missing." + self.col.backend.set_tag_collapsed(name=tag, collapsed=collapsed) + # Bulk addition/removal from notes ############################################################# @@ -102,12 +87,12 @@ class TagManager: def rename_tag(self, old: str, new: str) -> int: "Rename provided tag, returning number of changed notes." - escaped_name = re.sub(r"[*_\\]", r"\\\g<0>", old) - escaped_name = '"{}"'.format(escaped_name.replace('"', '\\"')) - nids = self.col.find_notes("tag:" + escaped_name) + search = self.col.backend.filter_to_search(FilterToSearchIn(tag=old)) + nids = self.col.find_notes(search) if not nids: return 0 - return self.col.tags.bulk_update(nids, old, new, False) + escaped_name = re.sub(r"[*_\\]", r"\\\g<0>", old) + return self.bulk_update(nids, escaped_name, new, False) # legacy routines diff --git a/qt/aqt/browser.py b/qt/aqt/browser.py index cd6ab9dea..f0976b96c 100644 --- a/qt/aqt/browser.py +++ b/qt/aqt/browser.py @@ -30,6 +30,7 @@ from anki.rsbackend import ( FilterToSearchIn, InvalidInput, NamedFilter, + TagTreeNode, ) from anki.stats import CardStats from anki.utils import htmlToTextLine, ids2str, isMac, isWin @@ -468,8 +469,12 @@ class SidebarItem: expanded: bool = False, item_type: SidebarItemType = SidebarItemType.CUSTOM, id: int = 0, + full_name: str = None, ) -> None: self.name = name + if not full_name: + full_name = name + self.full_name = full_name self.icon = icon self.item_type = item_type self.id = id @@ -1166,15 +1171,31 @@ QTableView {{ gridline-color: {grid} }} root.addChild(item) def _userTagTree(self, root) -> None: - assert self.col - for t in self.col.tags.all(): - item = SidebarItem( - t, - ":/icons/tag.svg", - self._tag_filter(t), - item_type=SidebarItemType.TAG, - ) - root.addChild(item) + tree = self.col.backend.tag_tree() + + def fillGroups(root, nodes: Sequence[TagTreeNode], head=""): + for node in nodes: + + def toggle_expand(): + full_name = head + node.name # pylint: disable=cell-var-from-loop + return lambda expanded: self.mw.col.tags.set_collapsed( + full_name, not expanded + ) + + item = SidebarItem( + node.name, + ":/icons/tag.svg", + self._tag_filter(head + node.name), + toggle_expand(), + not node.collapsed, + item_type=SidebarItemType.TAG, + full_name=head + node.name, + ) + root.addChild(item) + newhead = head + node.name + "::" + fillGroups(item, node.children, newhead) + + fillGroups(root, tree.children) def _decksTree(self, root) -> None: tree = self.col.decks.deck_tree() diff --git a/qt/aqt/main.py b/qt/aqt/main.py index 1ca0f11f9..0f4aa2dbc 100644 --- a/qt/aqt/main.py +++ b/qt/aqt/main.py @@ -75,6 +75,7 @@ class ResetReason(enum.Enum): EditorBridgeCmd = "editorBridgeCmd" BrowserSetDeck = "browserSetDeck" BrowserAddTags = "browserAddTags" + BrowserRemoveTags = "browserRemoveTags" BrowserSuspend = "browserSuspend" BrowserReposition = "browserReposition" BrowserReschedule = "browserReschedule" diff --git a/qt/aqt/sidebar.py b/qt/aqt/sidebar.py index 4103a44df..d53b63563 100644 --- a/qt/aqt/sidebar.py +++ b/qt/aqt/sidebar.py @@ -76,7 +76,10 @@ class NewSidebarTreeView(SidebarTreeViewBase): (tr(TR.ACTIONS_RENAME), self.rename_deck), (tr(TR.ACTIONS_DELETE), self.delete_deck), ), - SidebarItemType.TAG: ((tr(TR.ACTIONS_RENAME), self.rename_tag),), + SidebarItemType.TAG: ( + (tr(TR.ACTIONS_RENAME), self.rename_tag), + (tr(TR.ACTIONS_DELETE), self.remove_tag), + ), } def onContextMenu(self, point: QPoint) -> None: @@ -111,16 +114,37 @@ class NewSidebarTreeView(SidebarTreeViewBase): self.browser.maybeRefreshSidebar() self.mw.deckBrowser.refresh() + def remove_tag(self, item: "aqt.browser.SidebarItem") -> None: + self.browser.editor.saveNow(lambda: self._remove_tag(item)) + + def _remove_tag(self, item: "aqt.browser.SidebarItem") -> None: + old_name = item.full_name + + def do_remove(): + self.mw.col.backend.clear_tag(old_name) + self.col.tags.rename_tag(old_name, "") + + def on_done(fut: Future): + self.mw.requireReset(reason=ResetReason.BrowserRemoveTags, context=self) + self.browser.model.endReset() + fut.result() + self.browser.maybeRefreshSidebar() + + self.mw.checkpoint(tr(TR.ACTIONS_REMOVE_TAG)) + self.browser.model.beginReset() + self.mw.taskman.run_in_background(do_remove, on_done) + def rename_tag(self, item: "aqt.browser.SidebarItem") -> None: self.browser.editor.saveNow(lambda: self._rename_tag(item)) def _rename_tag(self, item: "aqt.browser.SidebarItem") -> None: - old_name = item.name + old_name = item.full_name new_name = getOnlyText(tr(TR.ACTIONS_NEW_NAME), default=old_name) if new_name == old_name or not new_name: return def do_rename(): + self.mw.col.backend.clear_tag(old_name) return self.col.tags.rename_tag(old_name, new_name) def on_done(fut: Future): @@ -132,7 +156,7 @@ class NewSidebarTreeView(SidebarTreeViewBase): showInfo(tr(TR.BROWSING_TAG_RENAME_WARNING_EMPTY)) return - self.browser.clearUnusedTags() + self.browser.maybeRefreshSidebar() self.mw.checkpoint(tr(TR.ACTIONS_RENAME_TAG)) self.browser.model.beginReset() diff --git a/rslib/backend.proto b/rslib/backend.proto index 2a5e213b3..52bb6b24a 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -206,8 +206,11 @@ service BackendService { // tags - rpc RegisterTags(RegisterTagsIn) returns (Bool); + rpc ClearUnusedTags(Empty) returns (Empty); rpc AllTags(Empty) returns (AllTagsOut); + rpc SetTagCollapsed(SetTagCollapsedIn) returns (Empty); + rpc ClearTag(String) returns (Empty); + rpc TagTree(Empty) returns (TagTreeNode); // config/preferences @@ -812,26 +815,32 @@ message AddOrUpdateDeckConfigLegacyIn { bool preserve_usn_and_mtime = 2; } -message RegisterTagsIn { - string tags = 1; - bool preserve_usn = 2; - int32 usn = 3; - bool clear_first = 4; -} - message AllTagsOut { - repeated TagUsnTuple tags = 1; + repeated Tag tags = 1; } -message TagUsnTuple { - string tag = 1; +message SetTagCollapsedIn { + string name = 1; + bool collapsed = 2; +} + +message Tag { + string name = 1; sint32 usn = 2; + bool collapsed = 3; } message GetChangedTagsOut { repeated string tags = 1; } +message TagTreeNode { + string name = 1; + repeated TagTreeNode children = 2; + uint32 level = 3; + bool collapsed = 4; +} + message SetConfigJsonIn { string key = 1; bytes value_json = 2; diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index 10c7c5e00..92080e62c 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -1339,28 +1339,43 @@ impl BackendService for Backend { //------------------------------------------------------------------- fn all_tags(&self, _input: Empty) -> BackendResult { - let tags = self.with_col(|col| col.storage.all_tags())?; - let tags: Vec<_> = tags - .into_iter() - .map(|(tag, usn)| pb::TagUsnTuple { tag, usn: usn.0 }) - .collect(); + let tags: Vec = self.with_col(|col| { + Ok(col + .storage + .all_tags()? + .into_iter() + .map(|t| t.into()) + .collect()) + })?; Ok(pb::AllTagsOut { tags }) } - fn register_tags(&self, input: pb::RegisterTagsIn) -> BackendResult { + fn set_tag_collapsed(&self, input: pb::SetTagCollapsedIn) -> BackendResult { self.with_col(|col| { col.transact(None, |col| { - let usn = if input.preserve_usn { - Usn(input.usn) - } else { - col.usn()? - }; - col.register_tags(&input.tags, usn, input.clear_first) - .map(|val| pb::Bool { val }) + col.set_tag_collapsed(&input.name, input.collapsed)?; + Ok(().into()) }) }) } + fn clear_unused_tags(&self, _input: pb::Empty) -> BackendResult { + self.with_col(|col| col.transact(None, |col| col.clear_unused_tags().map(Into::into))) + } + + fn clear_tag(&self, tag: pb::String) -> BackendResult { + self.with_col(|col| { + col.transact(None, |col| { + col.storage.clear_tag(tag.val.as_str())?; + Ok(().into()) + }) + }) + } + + fn tag_tree(&self, _input: Empty) -> Result { + self.with_col(|col| col.tag_tree()) + } + // config/preferences //------------------------------------------------------------------- diff --git a/rslib/src/dbcheck.rs b/rslib/src/dbcheck.rs index dc78e2bc2..0ca55361b 100644 --- a/rslib/src/dbcheck.rs +++ b/rslib/src/dbcheck.rs @@ -242,7 +242,7 @@ impl Collection { let usn = self.usn()?; let stamp = TimestampMillis::now(); - // will rebuild tag list below + let collapsed_tags = self.storage.collapsed_tags()?; self.storage.clear_tags()?; let total_notes = self.storage.total_notes()?; @@ -294,6 +294,10 @@ 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)?; + // if the collection is empty and the user has deleted all note types, ensure at least // one note type exists if self.storage.get_all_notetype_names()?.is_empty() { @@ -632,4 +636,23 @@ mod test { Ok(()) } + + #[test] + fn tags() -> Result<()> { + let mut col = open_test_collection(); + let nt = col.get_notetype_by_name("Basic")?.unwrap(); + let mut note = nt.new_note(); + note.tags.push("one".into()); + note.tags.push("two".into()); + col.add_note(&mut note, DeckID(1))?; + + col.set_tag_collapsed("two", 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); + + Ok(()) + } } diff --git a/rslib/src/notes.rs b/rslib/src/notes.rs index 2abbf9f3f..a1606ac2e 100644 --- a/rslib/src/notes.rs +++ b/rslib/src/notes.rs @@ -155,7 +155,22 @@ impl Note { pub(crate) fn replace_tags(&mut self, re: &Regex, mut repl: T) -> bool { let mut changed = false; for tag in &mut self.tags { - if let Cow::Owned(rep) = re.replace_all(tag, repl.by_ref()) { + if let Cow::Owned(rep) = re.replace_all(tag, |caps: ®ex::Captures| { + if let Some(expanded) = repl.by_ref().no_expansion() { + if expanded.trim().is_empty() { + "".to_string() + } else { + // include "::" if it was matched + format!( + "{}{}", + expanded, + caps.get(caps.len() - 1).map_or("", |m| m.as_str()) + ) + } + } else { + tag.to_string() + } + }) { *tag = rep; changed = true; } diff --git a/rslib/src/search/sqlwriter.rs b/rslib/src/search/sqlwriter.rs index e64b7f6df..6921794c6 100644 --- a/rslib/src/search/sqlwriter.rs +++ b/rslib/src/search/sqlwriter.rs @@ -11,7 +11,7 @@ use crate::{ notetype::NoteTypeID, storage::ids_to_string, text::{ - escape_sql, is_glob, matches_glob, normalize_to_nfc, strip_html_preserving_media_filenames, + is_glob, matches_glob, normalize_to_nfc, strip_html_preserving_media_filenames, to_custom_re, to_re, to_sql, to_text, without_combining, }, timestamp::TimestampSecs, @@ -194,19 +194,16 @@ impl SqlWriter<'_> { write!(self.sql, "false").unwrap(); } else { match text { - "none" => write!(self.sql, "n.tags = ''").unwrap(), - "*" => write!(self.sql, "true").unwrap(), - s => { - if is_glob(s) { - write!(self.sql, "n.tags regexp ?").unwrap(); - let re = &to_custom_re(s, r"\S"); - self.args.push(format!("(?i).* {} .*", re)); - } else if let Some(tag) = self.col.storage.preferred_tag_case(&to_text(s))? { - write!(self.sql, "n.tags like ? escape '\\'").unwrap(); - self.args.push(format!("% {} %", escape_sql(&tag))); - } else { - write!(self.sql, "false").unwrap(); - } + "none" => { + write!(self.sql, "n.tags = ''").unwrap(); + } + "*" => { + write!(self.sql, "true").unwrap(); + } + text => { + write!(self.sql, "n.tags regexp ?").unwrap(); + let re = &to_custom_re(text, r"\S"); + self.args.push(format!("(?i).* {}(::| ).*", re)); } } } @@ -587,7 +584,6 @@ mod test { collection::{open_collection, Collection}, i18n::I18n, log, - types::Usn, }; use std::{fs, path::PathBuf}; use tempfile::tempdir; @@ -698,26 +694,27 @@ mod test { // dupes assert_eq!(s(ctx, "dupe:123,test"), ("(n.id in ())".into(), vec![])); - // if registered, searches with canonical - ctx.transact(None, |col| col.register_tag("One", Usn(-1))) - .unwrap(); + // tags assert_eq!( s(ctx, r"tag:one"), ( - "(n.tags like ? escape '\\')".into(), - vec![r"% One %".into()] + "(n.tags regexp ?)".into(), + vec!["(?i).* one(::| ).*".into()] + ) + ); + assert_eq!( + s(ctx, r"tag:foo::bar"), + ( + "(n.tags regexp ?)".into(), + vec!["(?i).* foo::bar(::| ).*".into()] ) ); - // unregistered tags without wildcards won't match - assert_eq!(s(ctx, "tag:unknown"), ("(false)".into(), vec![])); - - // wildcards force a regexp search assert_eq!( s(ctx, r"tag:o*n\*et%w%oth_re\_e"), ( "(n.tags regexp ?)".into(), - vec![r"(?i).* o\S*n\*et%w%oth\Sre_e .*".into()] + vec![r"(?i).* o\S*n\*et%w%oth\Sre_e(::| ).*".into()] ) ); assert_eq!(s(ctx, "tag:none"), ("(n.tags = '')".into(), vec![])); diff --git a/rslib/src/storage/note/mod.rs b/rslib/src/storage/note/mod.rs index eb921b1c6..f4bca28c6 100644 --- a/rslib/src/storage/note/mod.rs +++ b/rslib/src/storage/note/mod.rs @@ -1,6 +1,8 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html +use std::collections::HashSet; + use crate::{ err::Result, notes::{Note, NoteID}, @@ -156,4 +158,20 @@ impl super::SqliteStorage { .query_row(NO_PARAMS, |r| r.get(0)) .map_err(Into::into) } + + pub(crate) fn all_tags_in_notes(&self) -> Result> { + let mut stmt = self + .db + .prepare_cached("select tags from notes where tags != ''")?; + let mut query = stmt.query(NO_PARAMS)?; + let mut seen: HashSet = HashSet::new(); + while let Some(rows) = query.next()? { + for tag in split_tags(rows.get_raw(0).as_str()?) { + if !seen.contains(tag) { + seen.insert(tag.to_string()); + } + } + } + Ok(seen) + } } diff --git a/rslib/src/storage/tag/add.sql b/rslib/src/storage/tag/add.sql index 211807a5f..3f1f63656 100644 --- a/rslib/src/storage/tag/add.sql +++ b/rslib/src/storage/tag/add.sql @@ -1,3 +1,3 @@ INSERT - OR IGNORE INTO tags (tag, usn) -VALUES (?, ?) \ No newline at end of file + OR REPLACE INTO tags (tag, usn, collapsed) +VALUES (?, ?, ?) \ No newline at end of file diff --git a/rslib/src/storage/tag/alloc_id.sql b/rslib/src/storage/tag/alloc_id.sql new file mode 100644 index 000000000..9a5c80562 --- /dev/null +++ b/rslib/src/storage/tag/alloc_id.sql @@ -0,0 +1,10 @@ +SELECT CASE + WHEN ?1 IN ( + SELECT id + FROM tags + ) THEN ( + SELECT max(id) + 1 + FROM tags + ) + ELSE ?1 + END; \ No newline at end of file diff --git a/rslib/src/storage/tag/mod.rs b/rslib/src/storage/tag/mod.rs index 4d8da1984..e0f1b0bd4 100644 --- a/rslib/src/storage/tag/mod.rs +++ b/rslib/src/storage/tag/mod.rs @@ -2,24 +2,57 @@ // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html use super::SqliteStorage; -use crate::{err::Result, types::Usn}; -use rusqlite::{params, NO_PARAMS}; +use crate::{err::Result, tags::Tag, types::Usn}; + +use rusqlite::{params, Row, NO_PARAMS}; use std::collections::HashMap; +fn row_to_tag(row: &Row) -> Result { + Ok(Tag { + name: row.get(0)?, + usn: row.get(1)?, + collapsed: row.get(2)?, + }) +} + impl SqliteStorage { - pub(crate) fn all_tags(&self) -> Result> { + /// All tags in the collection, in alphabetical order. + pub(crate) fn all_tags(&self) -> Result> { self.db - .prepare_cached("select tag, usn from tags")? - .query_and_then(NO_PARAMS, |row| -> Result<_> { - Ok((row.get(0)?, row.get(1)?)) - })? + .prepare_cached("select tag, usn, collapsed from tags")? + .query_and_then(NO_PARAMS, row_to_tag)? .collect() } - pub(crate) fn register_tag(&self, tag: &str, usn: Usn) -> Result<()> { + pub(crate) fn collapsed_tags(&self) -> Result> { + self.db + .prepare_cached("select tag from tags where collapsed = true")? + .query_and_then(NO_PARAMS, |r| r.get::<_, String>(0).map_err(Into::into))? + .collect::>>() + } + + pub(crate) fn restore_collapsed_tags(&self, tags: &[String]) -> Result<()> { + let mut stmt = self + .db + .prepare_cached("update tags set collapsed = true where tag = ?")?; + for tag in tags { + stmt.execute(&[tag])?; + } + Ok(()) + } + + pub(crate) fn get_tag(&self, name: &str) -> Result> { + self.db + .prepare_cached("select tag, usn, collapsed from tags where tag = ?")? + .query_and_then(&[name], row_to_tag)? + .next() + .transpose() + } + + pub(crate) fn register_tag(&self, tag: &Tag) -> Result<()> { self.db .prepare_cached(include_str!("add.sql"))? - .execute(params![tag, usn])?; + .execute(params![tag.name, tag.usn, tag.collapsed])?; Ok(()) } @@ -32,6 +65,22 @@ impl SqliteStorage { .map_err(Into::into) } + pub(crate) fn clear_tag(&self, tag: &str) -> Result<()> { + self.db + .prepare_cached("delete from tags where tag regexp ?")? + .execute(&[format!("(?i)^{}($|::)", regex::escape(tag))])?; + + Ok(()) + } + + pub(crate) fn set_tag_collapsed(&self, tag: &str, collapsed: bool) -> Result<()> { + self.db + .prepare_cached("update tags set collapsed = ? where tag = ?")? + .execute(params![collapsed, tag])?; + + Ok(()) + } + pub(crate) fn clear_tags(&self) -> Result<()> { self.db.execute("delete from tags", NO_PARAMS)?; Ok(()) @@ -75,8 +124,11 @@ impl SqliteStorage { serde_json::from_str(row.get_raw(0).as_str()?).map_err(Into::into); tags })?; + let mut stmt = self + .db + .prepare_cached("insert or ignore into tags (tag, usn) values (?, ?)")?; for (tag, usn) in tags.into_iter() { - self.register_tag(&tag, usn)?; + stmt.execute(params![tag, usn])?; } self.db.execute_batch("update col set tags=''")?; @@ -85,11 +137,23 @@ impl SqliteStorage { pub(super) fn downgrade_tags_from_schema14(&self) -> Result<()> { let alltags = self.all_tags()?; - let tagsmap: HashMap = alltags.into_iter().collect(); + let tagsmap: HashMap = alltags.into_iter().map(|t| (t.name, t.usn)).collect(); self.db.execute( "update col set tags=?", params![serde_json::to_string(&tagsmap)?], )?; Ok(()) } + + pub(super) fn upgrade_tags_to_schema17(&self) -> Result<()> { + let tags = self + .db + .prepare_cached("select tag, usn from tags")? + .query_and_then(NO_PARAMS, |r| Ok(Tag::new(r.get(0)?, r.get(1)?)))? + .collect::>>()?; + self.db + .execute_batch(include_str!["../upgrades/schema17_upgrade.sql"])?; + tags.into_iter() + .try_for_each(|tag| -> Result<()> { self.register_tag(&tag) }) + } } diff --git a/rslib/src/storage/upgrades/mod.rs b/rslib/src/storage/upgrades/mod.rs index 2d1896945..acccf7328 100644 --- a/rslib/src/storage/upgrades/mod.rs +++ b/rslib/src/storage/upgrades/mod.rs @@ -6,7 +6,7 @@ pub(super) const SCHEMA_MIN_VERSION: u8 = 11; /// The version new files are initially created with. pub(super) const SCHEMA_STARTING_VERSION: u8 = 11; /// The maximum schema version we can open. -pub(super) const SCHEMA_MAX_VERSION: u8 = 16; +pub(super) const SCHEMA_MAX_VERSION: u8 = 17; use super::SqliteStorage; use crate::err::Result; @@ -31,6 +31,10 @@ impl SqliteStorage { self.upgrade_deck_conf_to_schema16(server)?; self.db.execute_batch("update col set ver = 16")?; } + if ver < 17 { + self.upgrade_tags_to_schema17()?; + self.db.execute_batch("update col set ver = 17")?; + } Ok(()) } diff --git a/rslib/src/storage/upgrades/schema17_upgrade.sql b/rslib/src/storage/upgrades/schema17_upgrade.sql new file mode 100644 index 000000000..c8abf52ee --- /dev/null +++ b/rslib/src/storage/upgrades/schema17_upgrade.sql @@ -0,0 +1,7 @@ +DROP TABLE tags; +CREATE TABLE tags ( + tag text NOT NULL PRIMARY KEY COLLATE unicase, + usn integer NOT NULL, + collapsed boolean NOT NULL, + config blob NULL +) without rowid; \ No newline at end of file diff --git a/rslib/src/sync/mod.rs b/rslib/src/sync/mod.rs index 28e73f9b9..254d34ba6 100644 --- a/rslib/src/sync/mod.rs +++ b/rslib/src/sync/mod.rs @@ -17,7 +17,7 @@ use crate::{ revlog::RevlogEntry, serde::{default_on_invalid, deserialize_int_from_number}, storage::open_and_check_sqlite_file, - tags::{join_tags, split_tags}, + tags::{join_tags, split_tags, Tag}, }; pub use http_client::FullSyncProgressFn; use http_client::HTTPSyncClient; @@ -898,7 +898,7 @@ impl Collection { fn merge_tags(&self, tags: Vec, latest_usn: Usn) -> Result<()> { for tag in tags { - self.register_tag(&tag, latest_usn)?; + self.register_tag(Tag::new(tag, latest_usn))?; } Ok(()) } @@ -1461,12 +1461,12 @@ mod test { col1.storage .all_tags()? .into_iter() - .map(|t| t.0) + .map(|t| t.name) .collect::>(), col2.storage .all_tags()? .into_iter() - .map(|t| t.0) + .map(|t| t.name) .collect::>() ); diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index e0443c7b0..0e70d77bb 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -2,16 +2,55 @@ // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html use crate::{ + backend_proto::{Tag as TagProto, TagTreeNode}, collection::Collection, err::{AnkiError, Result}, notes::{NoteID, TransformNoteOutput}, - text::to_re, - {text::normalize_to_nfc, types::Usn}, + text::{normalize_to_nfc, to_re}, + types::Usn, }; + use regex::{NoExpand, Regex, Replacer}; -use std::{borrow::Cow, collections::HashSet}; +use std::{borrow::Cow, collections::HashSet, iter::Peekable}; use unicase::UniCase; +#[derive(Debug, Clone, PartialEq)] +pub struct Tag { + pub name: String, + pub usn: Usn, + pub collapsed: bool, +} + +impl From for TagProto { + fn from(t: Tag) -> Self { + TagProto { + name: t.name, + usn: t.usn.0, + collapsed: t.collapsed, + } + } +} + +impl From for Tag { + fn from(t: TagProto) -> Self { + Tag { + name: t.name, + usn: Usn(t.usn), + collapsed: t.collapsed, + } + } +} + +impl Tag { + pub fn new(name: String, usn: Usn) -> Self { + Tag { + name, + usn, + collapsed: false, + } + } +} + pub(crate) fn split_tags(tags: &str) -> impl Iterator { tags.split(is_tag_separator).filter(|tag| !tag.is_empty()) } @@ -32,31 +71,134 @@ fn invalid_char_for_tag(c: char) -> bool { c.is_ascii_control() || is_tag_separator(c) || c == '"' } +fn normalized_tag_name_component(comp: &str) -> Cow { + let mut out = normalize_to_nfc(comp); + if out.contains(invalid_char_for_tag) { + out = out.replace(invalid_char_for_tag, "").into(); + } + let trimmed = out.trim(); + if trimmed.is_empty() { + "blank".to_string().into() + } else if trimmed.len() != out.len() { + trimmed.to_string().into() + } else { + out + } +} + +fn normalize_tag_name(name: &str) -> String { + let mut out = String::with_capacity(name.len()); + for comp in name.split("::") { + out.push_str(&normalized_tag_name_component(comp)); + out.push_str("::"); + } + out.trim_end_matches("::").into() +} + +fn immediate_parent_name(tag_name: UniCase<&str>) -> Option> { + tag_name.rsplitn(2, '\x1f').nth(1).map(UniCase::new) +} + +/// For the given tag, check if immediate parent exists. If so, add +/// tag and return. +/// If the immediate parent is missing, check and add any missing parents. +/// This should ensure that if an immediate parent is found, all ancestors +/// are guaranteed to already exist. +fn add_tag_and_missing_parents<'a, 'b>( + all: &'a mut HashSet>, + missing: &'a mut Vec>, + tag_name: UniCase<&'b str>, +) { + if let Some(parent) = immediate_parent_name(tag_name) { + if !all.contains(&parent) { + missing.push(parent); + add_tag_and_missing_parents(all, missing, parent); + } + } + // finally, add provided tag + all.insert(tag_name); +} + +/// Append any missing parents. Caller must sort afterwards. +fn add_missing_parents(tags: &mut Vec) { + let mut all_names: HashSet> = HashSet::new(); + let mut missing = vec![]; + for tag in &*tags { + add_tag_and_missing_parents(&mut all_names, &mut missing, UniCase::new(&tag.name)) + } + let mut missing: Vec<_> = missing + .into_iter() + .map(|n| Tag::new(n.to_string(), Usn(0))) + .collect(); + tags.append(&mut missing); +} + +fn tags_to_tree(mut tags: Vec) -> TagTreeNode { + for tag in &mut tags { + tag.name = tag.name.replace("::", "\x1f"); + } + add_missing_parents(&mut tags); + tags.sort_unstable_by(|a, b| UniCase::new(&a.name).cmp(&UniCase::new(&b.name))); + let mut top = TagTreeNode::default(); + let mut it = tags.into_iter().peekable(); + add_child_nodes(&mut it, &mut top); + + top +} + +fn add_child_nodes(tags: &mut Peekable>, parent: &mut TagTreeNode) { + while let Some(tag) = tags.peek() { + let split_name: Vec<_> = tag.name.split('\x1f').collect(); + match split_name.len() as u32 { + l if l <= parent.level => { + // next item is at a higher level + return; + } + l if l == parent.level + 1 => { + // next item is an immediate descendent of parent + parent.children.push(TagTreeNode { + name: (*split_name.last().unwrap()).into(), + children: vec![], + level: parent.level + 1, + collapsed: tag.collapsed, + }); + tags.next(); + } + _ => { + // next item is at a lower level + if let Some(last_child) = parent.children.last_mut() { + add_child_nodes(tags, last_child) + } else { + // immediate parent is missing + tags.next(); + } + } + } + } +} + impl Collection { + pub fn tag_tree(&mut self) -> Result { + let tags = self.storage.all_tags()?; + let tree = tags_to_tree(tags); + + Ok(tree) + } + /// Given a list of tags, fix case, ordering and duplicates. /// Returns true if any new tags were added. pub(crate) fn canonify_tags(&self, tags: Vec, usn: Usn) -> Result<(Vec, bool)> { let mut seen = HashSet::new(); let mut added = false; - let mut tags: Vec<_> = tags - .iter() - .flat_map(|t| split_tags(t)) - .map(|s| normalize_to_nfc(&s)) - .collect(); - - for tag in &mut tags { - if tag.contains(invalid_char_for_tag) { - *tag = tag.replace(invalid_char_for_tag, "").into(); - } - if tag.trim().is_empty() { + let tags: Vec<_> = tags.iter().flat_map(|t| split_tags(t)).collect(); + for tag in tags { + let t = self.register_tag(Tag::new(tag.to_string(), usn))?; + if t.0.name.is_empty() { continue; } - let tag = self.register_tag(tag, usn)?; - if matches!(tag, Cow::Borrowed(_)) { - added = true; - } - seen.insert(UniCase::new(tag)); + added |= t.1; + seen.insert(UniCase::new(t.0.name)); } // exit early if no non-empty tags @@ -67,35 +209,52 @@ impl Collection { // return the sorted, canonified tags let mut tags = seen.into_iter().collect::>(); tags.sort_unstable(); - let tags: Vec<_> = tags - .into_iter() - .map(|s| s.into_inner().to_string()) - .collect(); + let tags: Vec<_> = tags.into_iter().map(|s| s.into_inner()).collect(); Ok((tags, added)) } - pub(crate) fn register_tag<'a>(&self, tag: &'a str, usn: Usn) -> Result> { - if let Some(preferred) = self.storage.preferred_tag_case(tag)? { - Ok(preferred.into()) + /// Register tag if it doesn't exist. + /// Returns a tuple of the tag with its name normalized and a boolean indicating if it was added. + pub(crate) fn register_tag(&self, tag: Tag) -> Result<(Tag, bool)> { + let normalized_name = normalize_tag_name(&tag.name); + let mut t = Tag { + name: normalized_name.clone(), + ..tag + }; + if normalized_name.is_empty() { + return Ok((t, false)); + } + if let Some(preferred) = self.storage.preferred_tag_case(&normalized_name)? { + t.name = preferred; + Ok((t, false)) } else { - self.storage.register_tag(tag, usn)?; - Ok(tag.into()) + self.storage.register_tag(&t)?; + Ok((t, true)) } } - pub(crate) fn register_tags(&self, tags: &str, usn: Usn, clear_first: bool) -> Result { - let mut changed = false; - if clear_first { - self.storage.clear_tags()?; + pub fn clear_unused_tags(&self) -> Result<()> { + let collapsed: HashSet<_> = self.storage.collapsed_tags()?.into_iter().collect(); + self.storage.clear_tags()?; + let usn = self.usn()?; + for name in self.storage.all_tags_in_notes()? { + self.register_tag(Tag { + collapsed: collapsed.contains(&name), + name, + usn, + })?; } - for tag in split_tags(tags) { - let tag = self.register_tag(tag, usn)?; - if matches!(tag, Cow::Borrowed(_)) { - changed = true; - } + + Ok(()) + } + + pub(crate) fn set_tag_collapsed(&self, name: &str, collapsed: bool) -> Result<()> { + if self.storage.get_tag(name)?.is_none() { + // tag is missing, register it + self.register_tag(Tag::new(name.to_string(), self.usn()?))?; } - Ok(changed) + self.storage.set_tag_collapsed(name, collapsed) } fn replace_tags_for_notes_inner( @@ -135,11 +294,10 @@ impl Collection { let tags = split_tags(tags) .map(|tag| { let tag = if regex { tag.into() } else { to_re(tag) }; - Regex::new(&format!("(?i)^{}$", tag)) + Regex::new(&format!("(?i)^{}(::.*)?", tag)) .map_err(|_| AnkiError::invalid_input("invalid regex")) }) .collect::>>()?; - if !regex { self.replace_tags_for_notes_inner(nids, &tags, NoExpand(repl)) } else { @@ -263,6 +421,135 @@ mod test { let note = col.storage.get_note(note.id)?.unwrap(); assert_eq!(¬e.tags, &["cee"]); + let mut note = col.storage.get_note(note.id)?.unwrap(); + note.tags = vec![ + "foo::bar".into(), + "foo::bar::foo".into(), + "bar::foo".into(), + "bar::foo::bar".into(), + ]; + col.update_note(&mut note)?; + col.replace_tags_for_notes(&[note.id], "bar::foo", "foo::bar", false)?; + let note = col.storage.get_note(note.id)?.unwrap(); + assert_eq!(¬e.tags, &["foo::bar", "foo::bar::bar", "foo::bar::foo",]); + + // tag children are also cleared when clearing their parent + col.storage.clear_tags()?; + for name in vec!["a", "a::b", "A::b::c"] { + col.register_tag(Tag::new(name.to_string(), Usn(0)))?; + } + col.storage.clear_tag("a")?; + assert_eq!(col.storage.all_tags()?, vec![]); + + Ok(()) + } + + fn node(name: &str, level: u32, children: Vec) -> TagTreeNode { + TagTreeNode { + name: name.into(), + level, + children, + ..Default::default() + } + } + + fn leaf(name: &str, level: u32) -> TagTreeNode { + node(name, level, vec![]) + } + + #[test] + fn tree() -> Result<()> { + let mut col = open_test_collection(); + let nt = col.get_notetype_by_name("Basic")?.unwrap(); + let mut note = nt.new_note(); + note.tags.push("foo::bar::a".into()); + note.tags.push("foo::bar::b".into()); + col.add_note(&mut note, DeckID(1))?; + + // missing parents are added + assert_eq!( + col.tag_tree()?, + node( + "", + 0, + vec![node( + "foo", + 1, + vec![node("bar", 2, vec![leaf("a", 3), leaf("b", 3)])] + )] + ) + ); + + // differing case should result in only one parent case being added - + // the first one + col.storage.clear_tags()?; + *(&mut note.tags[0]) = "foo::BAR::a".into(); + *(&mut note.tags[1]) = "FOO::bar::b".into(); + col.update_note(&mut note)?; + assert_eq!( + col.tag_tree()?, + node( + "", + 0, + vec![node( + "foo", + 1, + vec![node("BAR", 2, vec![leaf("a", 3), leaf("b", 3)])] + )] + ) + ); + + // things should work even if the immediate parent is not missing + col.storage.clear_tags()?; + *(&mut note.tags[0]) = "foo::bar::baz".into(); + *(&mut note.tags[1]) = "foo::bar::baz::quux".into(); + col.update_note(&mut note)?; + assert_eq!( + col.tag_tree()?, + node( + "", + 0, + vec![node( + "foo", + 1, + vec![node("bar", 2, vec![node("baz", 3, vec![leaf("quux", 4)])])] + )] + ) + ); + + // numbers have a smaller ascii number than ':', so a naive sort on + // '::' would result in one::two being nested under one1. + col.storage.clear_tags()?; + *(&mut note.tags[0]) = "one".into(); + *(&mut note.tags[1]) = "one1".into(); + note.tags.push("one::two".into()); + col.update_note(&mut note)?; + assert_eq!( + col.tag_tree()?, + node( + "", + 0, + vec![node("one", 1, vec![leaf("two", 2)]), leaf("one1", 1)] + ) + ); + + Ok(()) + } + + #[test] + fn clearing() -> Result<()> { + let mut col = open_test_collection(); + let nt = col.get_notetype_by_name("Basic")?.unwrap(); + let mut note = nt.new_note(); + note.tags.push("one".into()); + note.tags.push("two".into()); + col.add_note(&mut note, DeckID(1))?; + + col.set_tag_collapsed("two", 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); + Ok(()) } } diff --git a/rslib/src/text.rs b/rslib/src/text.rs index 10391334e..40be9342b 100644 --- a/rslib/src/text.rs +++ b/rslib/src/text.rs @@ -323,14 +323,6 @@ pub(crate) fn to_text(txt: &str) -> Cow { RE.replace_all(&txt, "$1") } -/// Escape characters special to SQL: \%_ -pub(crate) fn escape_sql(txt: &str) -> Cow { - lazy_static! { - static ref RE: Regex = Regex::new(r"[\\%_]").unwrap(); - } - RE.replace_all(&txt, r"\$0") -} - /// Escape Anki wildcards and the backslash for escaping them: \*_ pub(crate) fn escape_anki_wildcards(txt: &str) -> Cow { lazy_static! { @@ -407,7 +399,6 @@ mod test { assert_eq!(&to_custom_re("f_o*", r"\d"), r"f\do\d*"); assert_eq!(&to_sql("%f_o*"), r"\%f_o%"); assert_eq!(&to_text(r"\*\_*_"), "*_*_"); - assert_eq!(&escape_sql(r"1\2%3_"), r"1\\2\%3\_"); assert!(is_glob(r"\\\\_")); assert!(!is_glob(r"\\\_")); assert!(matches_glob("foo*bar123", r"foo\*bar*"));