From b276ce3dd508e859ca88bb3ae870548ccb936dd6 Mon Sep 17 00:00:00 2001 From: abdo Date: Wed, 6 Jan 2021 17:04:03 +0300 Subject: [PATCH 01/23] Hierarchical tags --- ftl/core/actions.ftl | 1 + pylib/anki/rsbackend.py | 3 +- pylib/anki/tags.py | 10 +- qt/aqt/browser.py | 44 +++-- qt/aqt/main.py | 1 + qt/aqt/sidebar.py | 30 +++- rslib/backend.proto | 274 ++++++++++++++++------------- rslib/src/backend/mod.rs | 46 ++++- rslib/src/notes.rs | 17 +- rslib/src/search/sqlwriter.rs | 49 +++--- rslib/src/storage/note/mod.rs | 81 ++++++++- rslib/src/storage/tag/add.sql | 7 +- rslib/src/storage/tag/alloc_id.sql | 13 ++ rslib/src/storage/tag/mod.rs | 129 ++++++++++++-- rslib/src/storage/upgrades/mod.rs | 9 +- rslib/src/sync/mod.rs | 12 +- rslib/src/tags.rs | 260 ++++++++++++++++++++++++--- rslib/src/text.rs | 9 - 18 files changed, 757 insertions(+), 238 deletions(-) create mode 100644 rslib/src/storage/tag/alloc_id.sql 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 1653bfe88..fb9e43329 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..ffd288e15 100644 --- a/pylib/anki/tags.py +++ b/pylib/anki/tags.py @@ -25,7 +25,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 +34,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 ############################################################# @@ -63,11 +63,7 @@ class TagManager: lim = "" clear = True self.register( - set( - self.split( - " ".join(self.col.db.list("select distinct tags from notes" + lim)) - ) - ), + self.col.backend.get_note_tags(nids), clear=clear, ) diff --git a/qt/aqt/browser.py b/qt/aqt/browser.py index af9c99ade..63ffdf002 100644 --- a/qt/aqt/browser.py +++ b/qt/aqt/browser.py @@ -21,7 +21,7 @@ from anki.consts import * from anki.lang import without_unicode_isolation from anki.models import NoteType from anki.notes import Note -from anki.rsbackend import ConcatSeparator, DeckTreeNode, InvalidInput +from anki.rsbackend import ConcatSeparator, DeckTreeNode, InvalidInput, TagTreeNode from anki.stats import CardStats from anki.utils import htmlToTextLine, ids2str, isMac, isWin from aqt import AnkiQt, gui_hooks @@ -1132,15 +1132,39 @@ 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", - lambda t=t: self.setFilter("tag", t), # type: ignore - 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 set_filter(): + full_name = head + node.name # pylint: disable=cell-var-from-loop + return lambda: self.setFilter("tag", full_name) + + def toggle_expand(): + tid = node.tag_id # pylint: disable=cell-var-from-loop + + def toggle(_): + tag = self.mw.col.backend.get_tag(tid) + tag.config.browser_collapsed = not tag.config.browser_collapsed + self.mw.col.backend.update_tag(tag) + + return toggle + + item = SidebarItem( + node.name, + ":/icons/tag.svg", + set_filter(), + toggle_expand(), + not node.collapsed, + item_type=SidebarItemType.TAG, + id=node.tag_id, + ) + 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..f616bf7f1 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 = self.mw.col.backend.get_tag(item.id).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 = self.mw.col.backend.get_tag(item.id).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 a99743240..10a3d0a95 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -66,158 +66,166 @@ message DeckConfigID { int64 dcid = 1; } +message TagID { + int64 tid = 1; +} + // New style RPC definitions /////////////////////////////////////////////////////////// service BackendService { - rpc LatestProgress(Empty) returns (Progress); - rpc SetWantsAbort(Empty) returns (Empty); + rpc LatestProgress (Empty) returns (Progress); + rpc SetWantsAbort (Empty) returns (Empty); - // card rendering + // card rendering - rpc ExtractAVTags(ExtractAVTagsIn) returns (ExtractAVTagsOut); - rpc ExtractLatex(ExtractLatexIn) returns (ExtractLatexOut); - rpc GetEmptyCards(Empty) returns (EmptyCardsReport); - rpc RenderExistingCard(RenderExistingCardIn) returns (RenderCardOut); - rpc RenderUncommittedCard(RenderUncommittedCardIn) returns (RenderCardOut); - rpc StripAVTags(String) returns (String); + rpc ExtractAVTags (ExtractAVTagsIn) returns (ExtractAVTagsOut); + rpc ExtractLatex (ExtractLatexIn) returns (ExtractLatexOut); + rpc GetEmptyCards (Empty) returns (EmptyCardsReport); + rpc RenderExistingCard (RenderExistingCardIn) returns (RenderCardOut); + rpc RenderUncommittedCard (RenderUncommittedCardIn) returns (RenderCardOut); + rpc StripAVTags (String) returns (String); - // searching + // searching - rpc NormalizeSearch(String) returns (String); - rpc SearchCards(SearchCardsIn) returns (SearchCardsOut); - rpc SearchNotes(SearchNotesIn) returns (SearchNotesOut); - rpc NegateSearch(String) returns (String); - rpc ConcatenateSearches(ConcatenateSearchesIn) returns (String); - rpc ReplaceSearchTerm(ReplaceSearchTermIn) returns (String); - rpc FindAndReplace(FindAndReplaceIn) returns (UInt32); + rpc NormalizeSearch (String) returns (String); + rpc SearchCards (SearchCardsIn) returns (SearchCardsOut); + rpc SearchNotes (SearchNotesIn) returns (SearchNotesOut); + rpc NegateSearch (String) returns (String); + rpc ConcatenateSearches (ConcatenateSearchesIn) returns (String); + rpc ReplaceSearchTerm (ReplaceSearchTermIn) returns (String); + rpc FindAndReplace (FindAndReplaceIn) returns (UInt32); - // scheduling + // scheduling - rpc LocalMinutesWest(Int64) returns (Int32); - rpc SetLocalMinutesWest(Int32) returns (Empty); - rpc SchedTimingToday(Empty) returns (SchedTimingTodayOut); - rpc StudiedToday(Empty) returns (String); - rpc StudiedTodayMessage(StudiedTodayMessageIn) returns (String); - rpc UpdateStats(UpdateStatsIn) returns (Empty); - rpc ExtendLimits(ExtendLimitsIn) returns (Empty); - rpc CountsForDeckToday(DeckID) returns (CountsForDeckTodayOut); - rpc CongratsInfo(Empty) returns (CongratsInfoOut); - rpc RestoreBuriedAndSuspendedCards(CardIDs) returns (Empty); - rpc UnburyCardsInCurrentDeck(UnburyCardsInCurrentDeckIn) returns (Empty); - rpc BuryOrSuspendCards(BuryOrSuspendCardsIn) returns (Empty); - rpc EmptyFilteredDeck(DeckID) returns (Empty); - rpc RebuildFilteredDeck(DeckID) returns (UInt32); - rpc ScheduleCardsAsReviews(ScheduleCardsAsReviewsIn) returns (Empty); - rpc ScheduleCardsAsNew(ScheduleCardsAsNewIn) returns (Empty); - rpc SortCards(SortCardsIn) returns (Empty); - rpc SortDeck(SortDeckIn) returns (Empty); + rpc LocalMinutesWest (Int64) returns (Int32); + rpc SetLocalMinutesWest (Int32) returns (Empty); + rpc SchedTimingToday (Empty) returns (SchedTimingTodayOut); + rpc StudiedToday (Empty) returns (String); + rpc StudiedTodayMessage (StudiedTodayMessageIn) returns (String); + rpc UpdateStats (UpdateStatsIn) returns (Empty); + rpc ExtendLimits (ExtendLimitsIn) returns (Empty); + rpc CountsForDeckToday (DeckID) returns (CountsForDeckTodayOut); + rpc CongratsInfo (Empty) returns (CongratsInfoOut); + rpc RestoreBuriedAndSuspendedCards (CardIDs) returns (Empty); + rpc UnburyCardsInCurrentDeck (UnburyCardsInCurrentDeckIn) returns (Empty); + rpc BuryOrSuspendCards (BuryOrSuspendCardsIn) returns (Empty); + rpc EmptyFilteredDeck (DeckID) returns (Empty); + rpc RebuildFilteredDeck (DeckID) returns (UInt32); + rpc ScheduleCardsAsReviews (ScheduleCardsAsReviewsIn) returns (Empty); + rpc ScheduleCardsAsNew (ScheduleCardsAsNewIn) returns (Empty); + rpc SortCards (SortCardsIn) returns (Empty); + rpc SortDeck (SortDeckIn) returns (Empty); - // stats + // stats - rpc CardStats(CardID) returns (String); - rpc Graphs(GraphsIn) returns (GraphsOut); + rpc CardStats (CardID) returns (String); + rpc Graphs(GraphsIn) returns (GraphsOut); - // media + // media - rpc CheckMedia(Empty) returns (CheckMediaOut); - rpc TrashMediaFiles(TrashMediaFilesIn) returns (Empty); - rpc AddMediaFile(AddMediaFileIn) returns (String); - rpc EmptyTrash(Empty) returns (Empty); - rpc RestoreTrash(Empty) returns (Empty); + rpc CheckMedia (Empty) returns (CheckMediaOut); + rpc TrashMediaFiles (TrashMediaFilesIn) returns (Empty); + rpc AddMediaFile (AddMediaFileIn) returns (String); + rpc EmptyTrash (Empty) returns (Empty); + rpc RestoreTrash (Empty) returns (Empty); - // decks + // decks - rpc AddOrUpdateDeckLegacy(AddOrUpdateDeckLegacyIn) returns (DeckID); - rpc DeckTree(DeckTreeIn) returns (DeckTreeNode); - rpc DeckTreeLegacy(Empty) returns (Json); - rpc GetAllDecksLegacy(Empty) returns (Json); - rpc GetDeckIDByName(String) returns (DeckID); - rpc GetDeckLegacy(DeckID) returns (Json); - rpc GetDeckNames(GetDeckNamesIn) returns (DeckNames); - rpc NewDeckLegacy(Bool) returns (Json); - rpc RemoveDeck(DeckID) returns (Empty); + rpc AddOrUpdateDeckLegacy (AddOrUpdateDeckLegacyIn) returns (DeckID); + rpc DeckTree (DeckTreeIn) returns (DeckTreeNode); + rpc DeckTreeLegacy (Empty) returns (Json); + rpc GetAllDecksLegacy (Empty) returns (Json); + rpc GetDeckIDByName (String) returns (DeckID); + rpc GetDeckLegacy (DeckID) returns (Json); + rpc GetDeckNames (GetDeckNamesIn) returns (DeckNames); + rpc NewDeckLegacy (Bool) returns (Json); + rpc RemoveDeck (DeckID) returns (Empty); - // deck config + // deck config - rpc AddOrUpdateDeckConfigLegacy(AddOrUpdateDeckConfigLegacyIn) - returns (DeckConfigID); - rpc AllDeckConfigLegacy(Empty) returns (Json); - rpc GetDeckConfigLegacy(DeckConfigID) returns (Json); - rpc NewDeckConfigLegacy(Empty) returns (Json); - rpc RemoveDeckConfig(DeckConfigID) returns (Empty); + rpc AddOrUpdateDeckConfigLegacy (AddOrUpdateDeckConfigLegacyIn) returns (DeckConfigID); + rpc AllDeckConfigLegacy (Empty) returns (Json); + rpc GetDeckConfigLegacy (DeckConfigID) returns (Json); + rpc NewDeckConfigLegacy (Empty) returns (Json); + rpc RemoveDeckConfig (DeckConfigID) returns (Empty); - // cards + // cards - rpc GetCard(CardID) returns (Card); - rpc UpdateCard(Card) returns (Empty); - rpc AddCard(Card) returns (CardID); - rpc RemoveCards(RemoveCardsIn) returns (Empty); - rpc SetDeck(SetDeckIn) returns (Empty); + rpc GetCard (CardID) returns (Card); + rpc UpdateCard (Card) returns (Empty); + rpc AddCard (Card) returns (CardID); + rpc RemoveCards (RemoveCardsIn) returns (Empty); + rpc SetDeck (SetDeckIn) returns (Empty); - // notes + // notes - rpc NewNote(NoteTypeID) returns (Note); - rpc AddNote(AddNoteIn) returns (NoteID); - rpc UpdateNote(Note) returns (Empty); - rpc GetNote(NoteID) returns (Note); - rpc RemoveNotes(RemoveNotesIn) returns (Empty); - rpc AddNoteTags(AddNoteTagsIn) returns (UInt32); - rpc UpdateNoteTags(UpdateNoteTagsIn) returns (UInt32); - rpc ClozeNumbersInNote(Note) returns (ClozeNumbersInNoteOut); - rpc AfterNoteUpdates(AfterNoteUpdatesIn) returns (Empty); - rpc FieldNamesForNotes(FieldNamesForNotesIn) returns (FieldNamesForNotesOut); - rpc NoteIsDuplicateOrEmpty(Note) returns (NoteIsDuplicateOrEmptyOut); - rpc CardsOfNote(NoteID) returns (CardIDs); + rpc NewNote (NoteTypeID) returns (Note); + rpc AddNote (AddNoteIn) returns (NoteID); + rpc UpdateNote (Note) returns (Empty); + rpc GetNote (NoteID) returns (Note); + rpc RemoveNotes (RemoveNotesIn) returns (Empty); + rpc AddNoteTags (AddNoteTagsIn) returns (UInt32); + rpc UpdateNoteTags (UpdateNoteTagsIn) returns (UInt32); + rpc GetNoteTags(GetNoteTagsIn) returns (GetNoteTagsOut); + rpc ClozeNumbersInNote (Note) returns (ClozeNumbersInNoteOut); + rpc AfterNoteUpdates (AfterNoteUpdatesIn) returns (Empty); + rpc FieldNamesForNotes (FieldNamesForNotesIn) returns (FieldNamesForNotesOut); + rpc NoteIsDuplicateOrEmpty (Note) returns (NoteIsDuplicateOrEmptyOut); + rpc CardsOfNote (NoteID) returns (CardIDs); - // note types + // note types - rpc AddOrUpdateNotetype(AddOrUpdateNotetypeIn) returns (NoteTypeID); - rpc GetStockNotetypeLegacy(GetStockNotetypeIn) returns (Json); - rpc GetNotetypeLegacy(NoteTypeID) returns (Json); - rpc GetNotetypeNames(Empty) returns (NoteTypeNames); - rpc GetNotetypeNamesAndCounts(Empty) returns (NoteTypeUseCounts); - rpc GetNotetypeIDByName(String) returns (NoteTypeID); - rpc RemoveNotetype(NoteTypeID) returns (Empty); + rpc AddOrUpdateNotetype (AddOrUpdateNotetypeIn) returns (NoteTypeID); + rpc GetStockNotetypeLegacy (GetStockNotetypeIn) returns (Json); + rpc GetNotetypeLegacy (NoteTypeID) returns (Json); + rpc GetNotetypeNames (Empty) returns (NoteTypeNames); + rpc GetNotetypeNamesAndCounts (Empty) returns (NoteTypeUseCounts); + rpc GetNotetypeIDByName (String) returns (NoteTypeID); + rpc RemoveNotetype (NoteTypeID) returns (Empty); - // collection + // collection - rpc OpenCollection(OpenCollectionIn) returns (Empty); - rpc CloseCollection(CloseCollectionIn) returns (Empty); - rpc CheckDatabase(Empty) returns (CheckDatabaseOut); + rpc OpenCollection (OpenCollectionIn) returns (Empty); + rpc CloseCollection (CloseCollectionIn) returns (Empty); + rpc CheckDatabase (Empty) returns (CheckDatabaseOut); - // sync + // sync - rpc SyncMedia(SyncAuth) returns (Empty); - rpc AbortSync(Empty) returns (Empty); - rpc AbortMediaSync(Empty) returns (Empty); - rpc BeforeUpload(Empty) returns (Empty); - rpc SyncLogin(SyncLoginIn) returns (SyncAuth); - rpc SyncStatus(SyncAuth) returns (SyncStatusOut); - rpc SyncCollection(SyncAuth) returns (SyncCollectionOut); - rpc FullUpload(SyncAuth) returns (Empty); - rpc FullDownload(SyncAuth) returns (Empty); + rpc SyncMedia (SyncAuth) returns (Empty); + rpc AbortSync (Empty) returns (Empty); + rpc AbortMediaSync (Empty) returns (Empty); + rpc BeforeUpload (Empty) returns (Empty); + rpc SyncLogin (SyncLoginIn) returns (SyncAuth); + rpc SyncStatus (SyncAuth) returns (SyncStatusOut); + rpc SyncCollection (SyncAuth) returns (SyncCollectionOut); + rpc FullUpload (SyncAuth) returns (Empty); + rpc FullDownload (SyncAuth) returns (Empty); - // translation/messages + // translation/messages - rpc TranslateString(TranslateStringIn) returns (String); - rpc FormatTimespan(FormatTimespanIn) returns (String); - rpc I18nResources(Empty) returns (Json); + rpc TranslateString (TranslateStringIn) returns (String); + rpc FormatTimespan (FormatTimespanIn) returns (String); + rpc I18nResources (Empty) returns (Json); - // tags + // tags - rpc RegisterTags(RegisterTagsIn) returns (Bool); - rpc AllTags(Empty) returns (AllTagsOut); + rpc RegisterTags (RegisterTagsIn) returns (Bool); + rpc AllTags (Empty) returns (AllTagsOut); + rpc GetTag (TagID) returns (Tag); + rpc UpdateTag (Tag) returns (Bool); + rpc ClearTag (String) returns (Bool); + rpc TagTree (Empty) returns (TagTreeNode); - // config/preferences + // config/preferences - rpc GetConfigJson(String) returns (Json); - rpc SetConfigJson(SetConfigJsonIn) returns (Empty); - rpc RemoveConfig(String) returns (Empty); - rpc SetAllConfig(Json) returns (Empty); - rpc GetAllConfig(Empty) returns (Json); - rpc GetPreferences(Empty) returns (Preferences); - rpc SetPreferences(Preferences) returns (Empty); + rpc GetConfigJson (String) returns (Json); + rpc SetConfigJson (SetConfigJsonIn) returns (Empty); + rpc RemoveConfig (String) returns (Empty); + rpc SetAllConfig (Json) returns (Empty); + rpc GetAllConfig (Empty) returns (Json); + rpc GetPreferences (Empty) returns (Preferences); + rpc SetPreferences (Preferences) returns (Empty); } // Protobuf stored in .anki2 files @@ -785,18 +793,32 @@ message RegisterTagsIn { } message AllTagsOut { - repeated TagUsnTuple tags = 1; + repeated Tag tags = 1; } -message TagUsnTuple { - string tag = 1; - sint32 usn = 2; +message TagConfig { + bool browser_collapsed = 1; +} + +message Tag { + int64 id = 1; + string name = 2; + sint32 usn = 3; + TagConfig config = 4; } message GetChangedTagsOut { repeated string tags = 1; } +message TagTreeNode { + int64 tag_id = 1; + string name = 2; + repeated TagTreeNode children = 3; + uint32 level = 5; + bool collapsed = 4; +} + message SetConfigJsonIn { string key = 1; bytes value_json = 2; @@ -903,6 +925,14 @@ message UpdateNoteTagsIn { bool regex = 4; } +message GetNoteTagsIn { + repeated int64 nids = 1; +} + +message GetNoteTagsOut { + repeated string tags = 1; +} + message CheckDatabaseOut { repeated string problems = 1; } diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index 22868773f..ab78d8f0d 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -44,6 +44,7 @@ use crate::{ get_remote_sync_meta, sync_abort, sync_login, FullSyncProgress, NormalSyncProgress, SyncActionRequired, SyncAuth, SyncMeta, SyncOutput, SyncStage, }, + tags::TagID, template::RenderedNode, text::{extract_av_tags, strip_av_tags, AVTag}, timestamp::TimestampSecs, @@ -919,6 +920,14 @@ impl BackendService for Backend { }) } + fn get_note_tags(&self, input: pb::GetNoteTagsIn) -> BackendResult { + self.with_col(|col| { + Ok(pb::GetNoteTagsOut { + tags: col.storage.get_note_tags(to_nids(input.nids))?, + }) + }) + } + fn cloze_numbers_in_note(&self, note: pb::Note) -> BackendResult { let mut set = HashSet::with_capacity(4); for field in ¬e.fields { @@ -1286,14 +1295,28 @@ 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.all_tags()?.into_iter().map(|t| t.into()).collect()))?; Ok(pb::AllTagsOut { tags }) } + fn get_tag(&self, input: pb::TagId) -> BackendResult { + self.with_col(|col| { + if let Some(tag) = col.storage.get_tag(TagID(input.tid))? { + Ok(tag.into()) + } else { + Err(AnkiError::NotFound) + } + }) + } + + fn update_tag(&self, tag: pb::Tag) -> BackendResult { + self.with_col(|col| { + col.update_tag(&tag.into())?; + Ok(pb::Bool { val: true }) + }) + } + fn register_tags(&self, input: pb::RegisterTagsIn) -> BackendResult { self.with_col(|col| { col.transact(None, |col| { @@ -1308,6 +1331,19 @@ impl BackendService for Backend { }) } + fn clear_tag(&self, tag: pb::String) -> BackendResult { + self.with_col(|col| { + col.transact(None, |col| { + col.clear_tag(tag.val.as_str())?; + Ok(pb::Bool { val: true }) + }) + }) + } + + fn tag_tree(&self, _input: Empty) -> Result { + self.with_col(|col| col.tag_tree()) + } + // config/preferences //------------------------------------------------------------------- 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 a29502d63..aba59b8dd 100644 --- a/rslib/src/search/sqlwriter.rs +++ b/rslib/src/search/sqlwriter.rs @@ -10,8 +10,9 @@ use crate::{ notes::field_checksum, notetype::NoteTypeID, storage::ids_to_string, + tags::human_tag_name_to_native, 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 +195,17 @@ 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"); + let native_name = human_tag_name_to_native(re); + self.args.push(format!("(?i).* {}(\x1f| ).*", native_name)); } } } @@ -568,7 +567,6 @@ mod test { collection::{open_collection, Collection}, i18n::I18n, log, - types::Usn, }; use std::{fs, path::PathBuf}; use tempfile::tempdir; @@ -678,26 +676,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(\x1f| ).*".into()] + ) + ); + assert_eq!( + s(ctx, r"tag:foo::bar"), + ( + "(n.tags regexp ?)".into(), + vec!["(?i).* foo\x1fbar(\x1f| ).*".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!["(?i).* o\\S*n\\*et%w%oth\\Sre_e(\u{1f}| ).*".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..2ddaa8655 100644 --- a/rslib/src/storage/note/mod.rs +++ b/rslib/src/storage/note/mod.rs @@ -5,7 +5,7 @@ use crate::{ err::Result, notes::{Note, NoteID}, notetype::NoteTypeID, - tags::{join_tags, split_tags}, + tags::{human_tag_name_to_native, join_tags, native_tag_name_to_human, split_tags}, timestamp::TimestampMillis, }; use rusqlite::{params, Row, NO_PARAMS}; @@ -18,6 +18,11 @@ pub(crate) fn join_fields(fields: &[String]) -> String { fields.join("\x1f") } +fn native_tags_str(tags: &[String]) -> String { + let s: Vec<_> = tags.iter().map(|t| human_tag_name_to_native(t)).collect(); + join_tags(&s) +} + fn row_to_note(row: &Row) -> Result { Ok(Note { id: row.get(0)?, @@ -26,7 +31,7 @@ fn row_to_note(row: &Row) -> Result { mtime: row.get(3)?, usn: row.get(4)?, tags: split_tags(row.get_raw(5).as_str()?) - .map(Into::into) + .map(|t| native_tag_name_to_human(t)) .collect(), fields: split_fields(row.get_raw(6).as_str()?), sort_field: None, @@ -52,7 +57,7 @@ impl super::SqliteStorage { note.notetype_id, note.mtime, note.usn, - join_tags(¬e.tags), + native_tags_str(¬e.tags), join_fields(¬e.fields()), note.sort_field.as_ref().unwrap(), note.checksum.unwrap(), @@ -70,7 +75,7 @@ impl super::SqliteStorage { note.notetype_id, note.mtime, note.usn, - join_tags(¬e.tags), + native_tags_str(¬e.tags), join_fields(¬e.fields()), note.sort_field.as_ref().unwrap(), note.checksum.unwrap(), @@ -88,7 +93,7 @@ impl super::SqliteStorage { note.notetype_id, note.mtime, note.usn, - join_tags(¬e.tags), + native_tags_str(¬e.tags), join_fields(¬e.fields()), note.sort_field.as_ref().unwrap(), note.checksum.unwrap(), @@ -156,4 +161,70 @@ impl super::SqliteStorage { .query_row(NO_PARAMS, |r| r.get(0)) .map_err(Into::into) } + + // get distinct note tags in human form + pub(crate) fn get_note_tags(&self, nids: Vec) -> Result> { + if nids.is_empty() { + self.db + .prepare_cached("select distinct tags from notes")? + .query_and_then(NO_PARAMS, |r| { + let t = r.get_raw(0).as_str()?; + Ok(native_tag_name_to_human(t)) + })? + .collect() + } else { + self.db + .prepare_cached("select distinct tags from notes where id in ?")? + .query_and_then(nids, |r| { + let t = r.get_raw(0).as_str()?; + Ok(native_tag_name_to_human(t)) + })? + .collect() + } + } + + pub(super) fn upgrade_notes_to_schema17(&self) -> Result<()> { + let notes: Result> = self + .db + .prepare_cached("select id, tags from notes")? + .query_and_then(NO_PARAMS, |row| -> Result<(NoteID, String)> { + let id = NoteID(row.get_raw(0).as_i64()?); + let tags: Vec = split_tags(row.get_raw(1).as_str()?) + .map(|t| human_tag_name_to_native(t)) + .collect(); + let tags = join_tags(&tags); + Ok((id, tags)) + })? + .collect(); + notes?.into_iter().try_for_each(|(id, tags)| -> Result<_> { + self.db + .prepare_cached("update notes set tags = ? where id = ?")? + .execute(params![tags, id])?; + Ok(()) + })?; + + Ok(()) + } + + pub(super) fn downgrade_notes_from_schema17(&self) -> Result<()> { + let notes: Result> = self + .db + .prepare_cached("select id, tags from notes")? + .query_and_then(NO_PARAMS, |row| -> Result<(NoteID, String)> { + let id = NoteID(row.get_raw(0).as_i64()?); + let tags: Vec = split_tags(row.get_raw(1).as_str()?) + .map(|t| native_tag_name_to_human(t)) + .collect(); + let tags = join_tags(&tags); + Ok((id, tags)) + })? + .collect(); + notes?.into_iter().try_for_each(|(id, tags)| -> Result<_> { + self.db + .prepare_cached("update notes set tags = ? where id = ?")? + .execute(params![tags, id])?; + Ok(()) + })?; + Ok(()) + } } diff --git a/rslib/src/storage/tag/add.sql b/rslib/src/storage/tag/add.sql index 211807a5f..16ee33d85 100644 --- a/rslib/src/storage/tag/add.sql +++ b/rslib/src/storage/tag/add.sql @@ -1,3 +1,4 @@ -INSERT - OR IGNORE INTO tags (tag, usn) -VALUES (?, ?) \ No newline at end of file +insert + or replace into tags (id, name, usn, config) +values + (?, ?, ?, ?) diff --git a/rslib/src/storage/tag/alloc_id.sql b/rslib/src/storage/tag/alloc_id.sql new file mode 100644 index 000000000..989c43749 --- /dev/null +++ b/rslib/src/storage/tag/alloc_id.sql @@ -0,0 +1,13 @@ +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..0aa958cce 100644 --- a/rslib/src/storage/tag/mod.rs +++ b/rslib/src/storage/tag/mod.rs @@ -2,36 +2,98 @@ // 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::{human_tag_name_to_native, native_tag_name_to_human, Tag, TagConfig, TagID}, + timestamp::TimestampMillis, + types::Usn, +}; +use prost::Message; +use rusqlite::{params, Row, NO_PARAMS}; use std::collections::HashMap; +fn row_to_tag(row: &Row) -> Result { + let config = TagConfig::decode(row.get_raw(3).as_blob()?)?; + Ok(Tag { + id: row.get(0)?, + name: row.get(1)?, + usn: row.get(2)?, + config, + }) +} + impl SqliteStorage { - pub(crate) fn all_tags(&self) -> Result> { + 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 id, name, usn, config from tags")? + .query_and_then(NO_PARAMS, row_to_tag)? + .collect() + } + + /// Get all tags in human form, sorted by name + pub(crate) fn all_tags_sorted(&self) -> Result> { + self.db + .prepare_cached("select id, name, usn, config from tags order by name")? + .query_and_then(NO_PARAMS, |row| { + let mut tag = row_to_tag(row)?; + tag.name = native_tag_name_to_human(&tag.name); + Ok(tag) })? .collect() } - pub(crate) fn register_tag(&self, tag: &str, usn: Usn) -> Result<()> { + pub(crate) fn get_tag(&self, id: TagID) -> Result> { + self.db + .prepare_cached("select id, name, usn, config from tags where id = ?")? + .query_and_then(&[id], |row| { + let mut tag = row_to_tag(row)?; + tag.name = native_tag_name_to_human(&tag.name); + Ok(tag) + })? + .next() + .transpose() + } + + fn alloc_id(&self) -> rusqlite::Result { + self.db + .prepare_cached(include_str!("alloc_id.sql"))? + .query_row(&[TimestampMillis::now()], |r| r.get(0)) + } + + pub(crate) fn register_tag(&self, tag: &mut Tag) -> Result<()> { + let mut config = vec![]; + tag.config.encode(&mut config)?; + tag.id = self.alloc_id()?; + self.update_tag(tag)?; + Ok(()) + } + + pub(crate) fn update_tag(&self, tag: &Tag) -> Result<()> { + let mut config = vec![]; + tag.config.encode(&mut config)?; self.db .prepare_cached(include_str!("add.sql"))? - .execute(params![tag, usn])?; + .execute(params![tag.id, tag.name, tag.usn, config])?; Ok(()) } pub(crate) fn preferred_tag_case(&self, tag: &str) -> Result> { self.db - .prepare_cached("select tag from tags where tag = ?")? + .prepare_cached("select name from tags where name = ?")? .query_and_then(params![tag], |row| row.get(0))? .next() .transpose() .map_err(Into::into) } + pub(crate) fn clear_tag(&self, tag: &str) -> Result<()> { + self.db + .prepare_cached("delete from tags where name regexp ?")? + .execute(&[format!("^{}($|\x1f)", regex::escape(tag))])?; + + Ok(()) + } + pub(crate) fn clear_tags(&self) -> Result<()> { self.db.execute("delete from tags", NO_PARAMS)?; Ok(()) @@ -48,7 +110,7 @@ impl SqliteStorage { pub(crate) fn tags_pending_sync(&self, usn: Usn) -> Result> { self.db .prepare_cached(&format!( - "select tag from tags where {}", + "select name from tags where {}", usn.pending_object_clause() ))? .query_and_then(&[usn], |r| r.get(0).map_err(Into::into))? @@ -58,7 +120,7 @@ impl SqliteStorage { pub(crate) fn update_tag_usns(&self, tags: &[String], new_usn: Usn) -> Result<()> { let mut stmt = self .db - .prepare_cached("update tags set usn=? where tag=?")?; + .prepare_cached("update tags set usn=? where name=?")?; for tag in tags { stmt.execute(params![new_usn, tag])?; } @@ -75,8 +137,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 +150,49 @@ 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 { + name: r.get(0)?, + usn: r.get(1)?, + ..Default::default() + }) + })? + .collect::>>()?; + self.db.execute_batch( + " + drop table tags; + create table tags ( + id integer primary key not null, + name text not null collate unicase, + usn integer not null, + config blob not null + ); + ", + )?; + tags.into_iter().try_for_each(|mut tag| -> Result<()> { + tag.name = human_tag_name_to_native(&tag.name); + self.register_tag(&mut tag) + }) + } + + pub(super) fn downgrade_tags_from_schema17(&self) -> Result<()> { + let tags = self.all_tags()?; + self.clear_tags()?; + tags.into_iter().try_for_each(|mut tag| -> Result<()> { + tag.name = native_tag_name_to_human(&tag.name); + self.register_tag(&mut tag) + }) + } } diff --git a/rslib/src/storage/upgrades/mod.rs b/rslib/src/storage/upgrades/mod.rs index 2d1896945..1167ecf59 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,11 @@ 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.upgrade_notes_to_schema17()?; + self.db.execute_batch("update col set ver = 17")?; + } Ok(()) } @@ -42,7 +47,9 @@ impl SqliteStorage { self.downgrade_decks_from_schema15()?; self.downgrade_notetypes_from_schema15()?; self.downgrade_config_from_schema14()?; + self.downgrade_tags_from_schema17()?; self.downgrade_tags_from_schema14()?; + self.downgrade_notes_from_schema17()?; self.db .execute_batch(include_str!("schema11_downgrade.sql"))?; diff --git a/rslib/src/sync/mod.rs b/rslib/src/sync/mod.rs index d8b718912..0a992f179 100644 --- a/rslib/src/sync/mod.rs +++ b/rslib/src/sync/mod.rs @@ -14,7 +14,7 @@ use crate::{ prelude::*, revlog::RevlogEntry, serde::{default_on_invalid, deserialize_int_from_number}, - tags::{join_tags, split_tags}, + tags::{join_tags, split_tags, Tag}, version::sync_client_version, }; use flate2::write::GzEncoder; @@ -888,7 +888,11 @@ 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 { + name: tag, + usn: latest_usn, + ..Default::default() + })?; } Ok(()) } @@ -1338,12 +1342,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..5c18d6879 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -1,17 +1,64 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html +pub use crate::backend_proto::TagConfig; use crate::{ + backend_proto::{Tag as TagProto, TagTreeNode}, collection::Collection, + define_newtype, 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; +define_newtype!(TagID, i64); + +#[derive(Debug, Clone, PartialEq)] +pub struct Tag { + pub id: TagID, + pub name: String, + pub usn: Usn, + pub config: TagConfig, +} + +impl Default for Tag { + fn default() -> Self { + Tag { + id: TagID(0), + name: "".to_string(), + usn: Usn(-1), + config: Default::default(), + } + } +} + +impl From for TagProto { + fn from(t: Tag) -> Self { + TagProto { + id: t.id.0, + name: t.name, + usn: t.usn.0, + config: Some(t.config), + } + } +} + +impl From for Tag { + fn from(t: TagProto) -> Self { + Tag { + id: TagID(t.id), + name: t.name, + usn: Usn(t.usn), + config: t.config.unwrap(), + } + } +} + pub(crate) fn split_tags(tags: &str) -> impl Iterator { tags.split(is_tag_separator).filter(|tag| !tag.is_empty()) } @@ -32,31 +79,117 @@ 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 + } +} + +pub(crate) fn human_tag_name_to_native(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('\x1f'); + } + out.trim_end_matches('\x1f').into() +} + +pub(crate) fn native_tag_name_to_human(name: &str) -> String { + name.replace('\x1f', "::") +} + +fn immediate_parent_name(native_name: &str) -> Option<&str> { + native_name.rsplitn(2, '\x1f').nth(1) +} + +fn tags_to_tree(tags: Vec) -> TagTreeNode { + 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("::").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 { + tag_id: tag.id.0, + name: (*split_name.last().unwrap()).into(), + children: vec![], + level: parent.level + 1, + collapsed: tag.config.browser_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 all_tags(&self) -> Result> { + self.storage + .all_tags()? + .into_iter() + .map(|t| { + Ok(Tag { + name: native_tag_name_to_human(&t.name), + ..t + }) + }) + .collect() + } + + pub fn tag_tree(&mut self) -> Result { + let tags = self.storage.all_tags_sorted()?; + 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(); - + let mut tags: Vec<_> = tags.iter().flat_map(|t| split_tags(t)).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 t = self.register_tag(Tag { + name: tag.to_string(), + usn, + ..Default::default() + })?; + if t.0.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)); } // exit early if no non-empty tags @@ -75,12 +208,40 @@ impl Collection { 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()) + fn create_missing_tag_parents(&self, mut native_name: &str, usn: Usn) -> Result { + let mut added = false; + while let Some(parent_name) = immediate_parent_name(native_name) { + if self.storage.preferred_tag_case(&parent_name)?.is_none() { + let mut t = Tag { + name: parent_name.to_string(), + usn, + ..Default::default() + }; + self.storage.register_tag(&mut t)?; + added = true; + } + native_name = parent_name; + } + Ok(added) + } + + pub(crate) fn register_tag<'a>(&self, tag: Tag) -> Result<(Cow<'a, str>, bool)> { + let native_name = human_tag_name_to_native(&tag.name); + if native_name.is_empty() { + return Ok(("".into(), false)); + } + let added_parents = self.create_missing_tag_parents(&native_name, tag.usn)?; + if let Some(preferred) = self.storage.preferred_tag_case(&native_name)? { + Ok((native_tag_name_to_human(&preferred).into(), added_parents)) } else { - self.storage.register_tag(tag, usn)?; - Ok(tag.into()) + let mut t = Tag { + name: native_name.clone(), + usn: tag.usn, + config: tag.config, + ..Default::default() + }; + self.storage.register_tag(&mut t)?; + Ok((native_tag_name_to_human(&native_name).into(), true)) } } @@ -90,14 +251,31 @@ impl Collection { self.storage.clear_tags()?; } for tag in split_tags(tags) { - let tag = self.register_tag(tag, usn)?; - if matches!(tag, Cow::Borrowed(_)) { - changed = true; - } + let t = self.register_tag(Tag { + name: tag.to_string(), + usn, + ..Default::default() + })?; + changed |= t.1; } Ok(changed) } + pub(crate) fn update_tag(&self, tag: &Tag) -> Result<()> { + let native_name = human_tag_name_to_native(&tag.name); + self.storage.update_tag(&Tag { + id: tag.id, + name: native_name, + usn: tag.usn, + config: tag.config.clone(), + }) + } + + pub(crate) fn clear_tag(&self, tag: &str) -> Result<()> { + let native_name = human_tag_name_to_native(tag); + self.storage.clear_tag(&native_name) + } + fn replace_tags_for_notes_inner( &mut self, nids: &[NoteID], @@ -135,11 +313,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 { @@ -222,6 +399,11 @@ mod test { col.update_note(&mut note)?; assert_eq!(¬e.tags, &["one", "two"]); + // note.tags is in human form + note.tags = vec!["foo::bar".into()]; + col.update_note(&mut note)?; + assert_eq!(¬e.tags, &["foo::bar"]); + Ok(()) } @@ -263,6 +445,26 @@ 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",]); + + // missing tag parents are registered too when registering their children + col.storage.clear_tags()?; + let mut note = col.storage.get_note(note.id)?.unwrap(); + note.tags = vec!["animal::mammal::cat".into()]; + col.update_note(&mut note)?; + let tags: Vec = col.all_tags()?.into_iter().map(|t| t.name).collect(); + assert_eq!(&tags, &["animal::mammal", "animal", "animal::mammal::cat"]); + Ok(()) } } diff --git a/rslib/src/text.rs b/rslib/src/text.rs index 0f892ecd0..70264b4c6 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") -} - /// Compare text with a possible glob, folding case. pub(crate) fn matches_glob(text: &str, search: &str) -> bool { if is_glob(search) { @@ -399,7 +391,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*")); From b33267f7543c376260eaa2a2396c8ae32bd20508 Mon Sep 17 00:00:00 2001 From: abdo Date: Sat, 9 Jan 2021 04:49:10 +0300 Subject: [PATCH 02/23] Do not check for missing tag parents at registration time --- pylib/anki/tags.py | 5 ++ qt/aqt/browser.py | 25 ++++++++-- qt/aqt/sidebar.py | 4 +- rslib/backend.proto | 2 +- rslib/src/backend/mod.rs | 5 +- rslib/src/storage/tag/mod.rs | 19 ++------ rslib/src/tags.rs | 89 ++++++++++++++++++++++-------------- 7 files changed, 88 insertions(+), 61 deletions(-) diff --git a/pylib/anki/tags.py b/pylib/anki/tags.py index ffd288e15..78adba0eb 100644 --- a/pylib/anki/tags.py +++ b/pylib/anki/tags.py @@ -80,6 +80,11 @@ class TagManager: res = self.col.db.list(query) return list(set(self.split(" ".join(res)))) + def toggle_browser_collapse(self, name: str): + tag = self.col.backend.get_tag(name) + tag.config.browser_collapsed = not tag.config.browser_collapsed + self.col.backend.update_tag(tag) + # Bulk addition/removal from notes ############################################################# diff --git a/qt/aqt/browser.py b/qt/aqt/browser.py index 63ffdf002..3d7161a4c 100644 --- a/qt/aqt/browser.py +++ b/qt/aqt/browser.py @@ -21,7 +21,13 @@ from anki.consts import * from anki.lang import without_unicode_isolation from anki.models import NoteType from anki.notes import Note -from anki.rsbackend import ConcatSeparator, DeckTreeNode, InvalidInput, TagTreeNode +from anki.rsbackend import ( + ConcatSeparator, + DeckTreeNode, + InvalidInput, + NotFoundError, + TagTreeNode, +) from anki.stats import CardStats from anki.utils import htmlToTextLine, ids2str, isMac, isWin from aqt import AnkiQt, gui_hooks @@ -451,8 +457,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 @@ -1142,12 +1152,16 @@ QTableView {{ gridline-color: {grid} }} return lambda: self.setFilter("tag", full_name) def toggle_expand(): - tid = node.tag_id # pylint: disable=cell-var-from-loop + + full_name = head + node.name # pylint: disable=cell-var-from-loop def toggle(_): - tag = self.mw.col.backend.get_tag(tid) - tag.config.browser_collapsed = not tag.config.browser_collapsed - self.mw.col.backend.update_tag(tag) + try: + self.mw.col.tags.toggle_browser_collapse(full_name) + except NotFoundError: + # tag is missing, register it first + self.mw.col.tags.register([full_name]) + self.mw.col.tags.toggle_browser_collapse(full_name) return toggle @@ -1159,6 +1173,7 @@ QTableView {{ gridline-color: {grid} }} not node.collapsed, item_type=SidebarItemType.TAG, id=node.tag_id, + full_name=head + node.name, ) root.addChild(item) newhead = head + node.name + "::" diff --git a/qt/aqt/sidebar.py b/qt/aqt/sidebar.py index f616bf7f1..d53b63563 100644 --- a/qt/aqt/sidebar.py +++ b/qt/aqt/sidebar.py @@ -118,7 +118,7 @@ class NewSidebarTreeView(SidebarTreeViewBase): self.browser.editor.saveNow(lambda: self._remove_tag(item)) def _remove_tag(self, item: "aqt.browser.SidebarItem") -> None: - old_name = self.mw.col.backend.get_tag(item.id).name + old_name = item.full_name def do_remove(): self.mw.col.backend.clear_tag(old_name) @@ -138,7 +138,7 @@ class NewSidebarTreeView(SidebarTreeViewBase): self.browser.editor.saveNow(lambda: self._rename_tag(item)) def _rename_tag(self, item: "aqt.browser.SidebarItem") -> None: - old_name = self.mw.col.backend.get_tag(item.id).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 diff --git a/rslib/backend.proto b/rslib/backend.proto index 10a3d0a95..80aafe3ad 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -212,7 +212,7 @@ service BackendService { rpc RegisterTags (RegisterTagsIn) returns (Bool); rpc AllTags (Empty) returns (AllTagsOut); - rpc GetTag (TagID) returns (Tag); + rpc GetTag (String) returns (Tag); rpc UpdateTag (Tag) returns (Bool); rpc ClearTag (String) returns (Bool); rpc TagTree (Empty) returns (TagTreeNode); diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index ab78d8f0d..caa6e5318 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -44,7 +44,6 @@ use crate::{ get_remote_sync_meta, sync_abort, sync_login, FullSyncProgress, NormalSyncProgress, SyncActionRequired, SyncAuth, SyncMeta, SyncOutput, SyncStage, }, - tags::TagID, template::RenderedNode, text::{extract_av_tags, strip_av_tags, AVTag}, timestamp::TimestampSecs, @@ -1300,9 +1299,9 @@ impl BackendService for Backend { Ok(pb::AllTagsOut { tags }) } - fn get_tag(&self, input: pb::TagId) -> BackendResult { + fn get_tag(&self, name: pb::String) -> BackendResult { self.with_col(|col| { - if let Some(tag) = col.storage.get_tag(TagID(input.tid))? { + if let Some(tag) = col.storage.get_tag(name.val.as_str())? { Ok(tag.into()) } else { Err(AnkiError::NotFound) diff --git a/rslib/src/storage/tag/mod.rs b/rslib/src/storage/tag/mod.rs index 0aa958cce..51e497835 100644 --- a/rslib/src/storage/tag/mod.rs +++ b/rslib/src/storage/tag/mod.rs @@ -30,22 +30,11 @@ impl SqliteStorage { .collect() } - /// Get all tags in human form, sorted by name - pub(crate) fn all_tags_sorted(&self) -> Result> { + /// Get tag by human name + pub(crate) fn get_tag(&self, name: &str) -> Result> { self.db - .prepare_cached("select id, name, usn, config from tags order by name")? - .query_and_then(NO_PARAMS, |row| { - let mut tag = row_to_tag(row)?; - tag.name = native_tag_name_to_human(&tag.name); - Ok(tag) - })? - .collect() - } - - pub(crate) fn get_tag(&self, id: TagID) -> Result> { - self.db - .prepare_cached("select id, name, usn, config from tags where id = ?")? - .query_and_then(&[id], |row| { + .prepare_cached("select id, name, usn, config from tags where name = ?")? + .query_and_then(&[human_tag_name_to_native(name)], |row| { let mut tag = row_to_tag(row)?; tag.name = native_tag_name_to_human(&tag.name); Ok(tag) diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index 5c18d6879..8468bc151 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -13,12 +13,17 @@ use crate::{ }; use regex::{NoExpand, Regex, Replacer}; -use std::{borrow::Cow, collections::HashSet, iter::Peekable}; +use std::cmp::Ordering; +use std::{ + borrow::Cow, + collections::{HashMap, HashSet}, + iter::Peekable, +}; use unicase::UniCase; define_newtype!(TagID, i64); -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub struct Tag { pub id: TagID, pub name: String, @@ -26,6 +31,26 @@ pub struct Tag { pub config: TagConfig, } +impl Ord for Tag { + fn cmp(&self, other: &Self) -> Ordering { + self.name.cmp(&other.name) + } +} + +impl PartialOrd for Tag { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl PartialEq for Tag { + fn eq(&self, other: &Self) -> bool { + self.name == other.name + } +} + +impl Eq for Tag {} + impl Default for Tag { fn default() -> Self { Tag { @@ -107,11 +132,33 @@ pub(crate) fn native_tag_name_to_human(name: &str) -> String { name.replace('\x1f', "::") } -fn immediate_parent_name(native_name: &str) -> Option<&str> { - native_name.rsplitn(2, '\x1f').nth(1) +fn fill_missing_tags(tags: Vec) -> Vec { + let mut filled_tags: HashMap = HashMap::new(); + for tag in tags.into_iter() { + let name = tag.name.to_owned(); + let split: Vec<&str> = (&tag.name).split("::").collect(); + for i in 0..split.len() - 1 { + let comp = split[0..i + 1].join("::"); + let t = Tag { + name: comp.to_owned(), + ..Default::default() + }; + if filled_tags.get(&comp).is_none() { + filled_tags.insert(comp, t); + } + } + if filled_tags.get(&name).is_none() { + filled_tags.insert(name, tag); + } + } + let mut tags: Vec = filled_tags.values().map(|t| (*t).clone()).collect(); + tags.sort_unstable(); + + tags } fn tags_to_tree(tags: Vec) -> TagTreeNode { + let tags = fill_missing_tags(tags); let mut top = TagTreeNode::default(); let mut it = tags.into_iter().peekable(); add_child_nodes(&mut it, &mut top); @@ -166,7 +213,7 @@ impl Collection { } pub fn tag_tree(&mut self) -> Result { - let tags = self.storage.all_tags_sorted()?; + let tags = self.all_tags()?; let tree = tags_to_tree(tags); Ok(tree) @@ -208,37 +255,17 @@ impl Collection { Ok((tags, added)) } - fn create_missing_tag_parents(&self, mut native_name: &str, usn: Usn) -> Result { - let mut added = false; - while let Some(parent_name) = immediate_parent_name(native_name) { - if self.storage.preferred_tag_case(&parent_name)?.is_none() { - let mut t = Tag { - name: parent_name.to_string(), - usn, - ..Default::default() - }; - self.storage.register_tag(&mut t)?; - added = true; - } - native_name = parent_name; - } - Ok(added) - } - pub(crate) fn register_tag<'a>(&self, tag: Tag) -> Result<(Cow<'a, str>, bool)> { let native_name = human_tag_name_to_native(&tag.name); if native_name.is_empty() { return Ok(("".into(), false)); } - let added_parents = self.create_missing_tag_parents(&native_name, tag.usn)?; if let Some(preferred) = self.storage.preferred_tag_case(&native_name)? { - Ok((native_tag_name_to_human(&preferred).into(), added_parents)) + Ok((native_tag_name_to_human(&preferred).into(), false)) } else { let mut t = Tag { name: native_name.clone(), - usn: tag.usn, - config: tag.config, - ..Default::default() + ..tag }; self.storage.register_tag(&mut t)?; Ok((native_tag_name_to_human(&native_name).into(), true)) @@ -457,14 +484,6 @@ mod test { let note = col.storage.get_note(note.id)?.unwrap(); assert_eq!(¬e.tags, &["foo::bar", "foo::bar::bar", "foo::bar::foo",]); - // missing tag parents are registered too when registering their children - col.storage.clear_tags()?; - let mut note = col.storage.get_note(note.id)?.unwrap(); - note.tags = vec!["animal::mammal::cat".into()]; - col.update_note(&mut note)?; - let tags: Vec = col.all_tags()?.into_iter().map(|t| t.name).collect(); - assert_eq!(&tags, &["animal::mammal", "animal", "animal::mammal::cat"]); - Ok(()) } } From c6e3d554006d4ee95719ad389f128779fadef9f1 Mon Sep 17 00:00:00 2001 From: abdo Date: Sat, 9 Jan 2021 14:15:30 +0300 Subject: [PATCH 03/23] fill_missing_tags's input should be sorted I assumed that fill_missing_tags will work correctly with un unsorted tag list previously so I replaced the all_tags_sorted call, but take the following the list for example: ["foo::bar", "foo"] This will cause "foo" to be counted like a missing tag, since it's encountered the first time when looking at "foo::bar"", and its config and other associated data will be lost. --- rslib/src/storage/tag/mod.rs | 12 ++++++++++++ rslib/src/tags.rs | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/rslib/src/storage/tag/mod.rs b/rslib/src/storage/tag/mod.rs index 51e497835..8d9817856 100644 --- a/rslib/src/storage/tag/mod.rs +++ b/rslib/src/storage/tag/mod.rs @@ -30,6 +30,18 @@ impl SqliteStorage { .collect() } + /// Get all tags in human form, sorted by name + pub(crate) fn all_tags_sorted(&self) -> Result> { + self.db + .prepare_cached("select id, name, usn, config from tags order by name")? + .query_and_then(NO_PARAMS, |row| { + let mut tag = row_to_tag(row)?; + tag.name = native_tag_name_to_human(&tag.name); + Ok(tag) + })? + .collect() + } + /// Get tag by human name pub(crate) fn get_tag(&self, name: &str) -> Result> { self.db diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index 8468bc151..7b2621561 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -213,7 +213,7 @@ impl Collection { } pub fn tag_tree(&mut self) -> Result { - let tags = self.all_tags()?; + let tags = self.storage.all_tags_sorted()?; let tree = tags_to_tree(tags); Ok(tree) From 1be789f25f66af82e36106430497c400753eff81 Mon Sep 17 00:00:00 2001 From: abdo Date: Sat, 9 Jan 2021 14:36:38 +0300 Subject: [PATCH 04/23] Move sql code for upgrading to schema 17 to a separate file --- rslib/src/storage/tag/mod.rs | 13 ++----------- rslib/src/storage/upgrades/schema17_upgrade.sql | 7 +++++++ 2 files changed, 9 insertions(+), 11 deletions(-) create mode 100644 rslib/src/storage/upgrades/schema17_upgrade.sql diff --git a/rslib/src/storage/tag/mod.rs b/rslib/src/storage/tag/mod.rs index 8d9817856..bfbbce227 100644 --- a/rslib/src/storage/tag/mod.rs +++ b/rslib/src/storage/tag/mod.rs @@ -171,17 +171,8 @@ impl SqliteStorage { }) })? .collect::>>()?; - self.db.execute_batch( - " - drop table tags; - create table tags ( - id integer primary key not null, - name text not null collate unicase, - usn integer not null, - config blob not null - ); - ", - )?; + self.db + .execute_batch(include_str!["../upgrades/schema17_upgrade.sql"])?; tags.into_iter().try_for_each(|mut tag| -> Result<()> { tag.name = human_tag_name_to_native(&tag.name); self.register_tag(&mut tag) diff --git a/rslib/src/storage/upgrades/schema17_upgrade.sql b/rslib/src/storage/upgrades/schema17_upgrade.sql new file mode 100644 index 000000000..a324181f9 --- /dev/null +++ b/rslib/src/storage/upgrades/schema17_upgrade.sql @@ -0,0 +1,7 @@ +drop table tags; +create table tags ( + id integer primary key not null, + name text not null collate unicase, + usn integer not null, + config blob not null +); \ No newline at end of file From f7f509c70dc4fcb553a8dbfd8e5e29f65bc8df6c Mon Sep 17 00:00:00 2001 From: abdo Date: Sat, 9 Jan 2021 16:55:08 +0300 Subject: [PATCH 05/23] Move tag collapse method to the backend --- pylib/anki/tags.py | 7 +++---- qt/aqt/browser.py | 22 ++++------------------ rslib/backend.proto | 6 ++++++ rslib/src/backend/mod.rs | 19 +++++++++++++++++++ rslib/src/tags.rs | 27 +++++++++++++-------------- 5 files changed, 45 insertions(+), 36 deletions(-) diff --git a/pylib/anki/tags.py b/pylib/anki/tags.py index 78adba0eb..2fd52375e 100644 --- a/pylib/anki/tags.py +++ b/pylib/anki/tags.py @@ -80,10 +80,9 @@ class TagManager: res = self.col.db.list(query) return list(set(self.split(" ".join(res)))) - def toggle_browser_collapse(self, name: str): - tag = self.col.backend.get_tag(name) - tag.config.browser_collapsed = not tag.config.browser_collapsed - self.col.backend.update_tag(tag) + 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 ############################################################# diff --git a/qt/aqt/browser.py b/qt/aqt/browser.py index 3d7161a4c..dd008c657 100644 --- a/qt/aqt/browser.py +++ b/qt/aqt/browser.py @@ -21,13 +21,7 @@ from anki.consts import * from anki.lang import without_unicode_isolation from anki.models import NoteType from anki.notes import Note -from anki.rsbackend import ( - ConcatSeparator, - DeckTreeNode, - InvalidInput, - NotFoundError, - TagTreeNode, -) +from anki.rsbackend import ConcatSeparator, DeckTreeNode, InvalidInput, TagTreeNode from anki.stats import CardStats from anki.utils import htmlToTextLine, ids2str, isMac, isWin from aqt import AnkiQt, gui_hooks @@ -1152,18 +1146,10 @@ QTableView {{ gridline-color: {grid} }} return lambda: self.setFilter("tag", full_name) def toggle_expand(): - full_name = head + node.name # pylint: disable=cell-var-from-loop - - def toggle(_): - try: - self.mw.col.tags.toggle_browser_collapse(full_name) - except NotFoundError: - # tag is missing, register it first - self.mw.col.tags.register([full_name]) - self.mw.col.tags.toggle_browser_collapse(full_name) - - return toggle + return lambda _: self.mw.col.tags.set_collapsed( + full_name, not node.collapsed + ) item = SidebarItem( node.name, diff --git a/rslib/backend.proto b/rslib/backend.proto index 80aafe3ad..6a8165cdf 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -214,6 +214,7 @@ service BackendService { rpc AllTags (Empty) returns (AllTagsOut); rpc GetTag (String) returns (Tag); rpc UpdateTag (Tag) returns (Bool); + rpc SetTagCollapsed (SetTagCollapsedIn) returns (Bool); rpc ClearTag (String) returns (Bool); rpc TagTree (Empty) returns (TagTreeNode); @@ -796,6 +797,11 @@ message AllTagsOut { repeated Tag tags = 1; } +message SetTagCollapsedIn { + string name = 1; + bool collapsed = 2; +} + message TagConfig { bool browser_collapsed = 1; } diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index caa6e5318..e2da5d64e 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -44,6 +44,7 @@ use crate::{ get_remote_sync_meta, sync_abort, sync_login, FullSyncProgress, NormalSyncProgress, SyncActionRequired, SyncAuth, SyncMeta, SyncOutput, SyncStage, }, + tags::Tag, template::RenderedNode, text::{extract_av_tags, strip_av_tags, AVTag}, timestamp::TimestampSecs, @@ -1316,6 +1317,24 @@ impl BackendService for Backend { }) } + fn set_tag_collapsed(&self, input: pb::SetTagCollapsedIn) -> BackendResult { + self.with_col(|col| { + let name = &input.name; + let tag: Result = if let Some(tag) = col.storage.get_tag(name)? { + Ok(tag) + } else { + // tag is missing, register it + let t = Tag { + name: name.to_owned(), + ..Default::default() + }; + Ok(col.register_tag(t)?.0) + }; + tag?.config.browser_collapsed = input.collapsed; + Ok(pb::Bool { val: true }) + }) + } + fn register_tags(&self, input: pb::RegisterTagsIn) -> BackendResult { self.with_col(|col| { col.transact(None, |col| { diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index 7b2621561..26ff9b486 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -232,11 +232,11 @@ impl Collection { usn, ..Default::default() })?; - if t.0.is_empty() { + if t.0.name.is_empty() { continue; } added |= t.1; - seen.insert(UniCase::new(t.0)); + seen.insert(UniCase::new(t.0.name)); } // exit early if no non-empty tags @@ -247,28 +247,27 @@ 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: Tag) -> Result<(Cow<'a, str>, bool)> { + pub(crate) fn register_tag(&self, tag: Tag) -> Result<(Tag, bool)> { let native_name = human_tag_name_to_native(&tag.name); + let mut t = Tag { + name: native_name.clone(), + ..tag + }; if native_name.is_empty() { - return Ok(("".into(), false)); + return Ok((t, false)); } if let Some(preferred) = self.storage.preferred_tag_case(&native_name)? { - Ok((native_tag_name_to_human(&preferred).into(), false)) + t.name = native_tag_name_to_human(&preferred); + Ok((t, false)) } else { - let mut t = Tag { - name: native_name.clone(), - ..tag - }; self.storage.register_tag(&mut t)?; - Ok((native_tag_name_to_human(&native_name).into(), true)) + t.name = native_tag_name_to_human(&t.name); + Ok((t, true)) } } From 97b4c2124c7ed3d78a84137509d6fccec683584d Mon Sep 17 00:00:00 2001 From: abdo Date: Sat, 9 Jan 2021 17:38:16 +0300 Subject: [PATCH 06/23] sql formatting --- rslib/src/storage/tag/add.sql | 7 +++--- rslib/src/storage/tag/alloc_id.sql | 23 ++++++++----------- .../src/storage/upgrades/schema17_upgrade.sql | 12 +++++----- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/rslib/src/storage/tag/add.sql b/rslib/src/storage/tag/add.sql index 16ee33d85..57db3bb0d 100644 --- a/rslib/src/storage/tag/add.sql +++ b/rslib/src/storage/tag/add.sql @@ -1,4 +1,3 @@ -insert - or replace into tags (id, name, usn, config) -values - (?, ?, ?, ?) +INSERT + OR REPLACE INTO tags (id, name, usn, config) +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 index 989c43749..9a5c80562 100644 --- a/rslib/src/storage/tag/alloc_id.sql +++ b/rslib/src/storage/tag/alloc_id.sql @@ -1,13 +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 +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/upgrades/schema17_upgrade.sql b/rslib/src/storage/upgrades/schema17_upgrade.sql index a324181f9..288a14a07 100644 --- a/rslib/src/storage/upgrades/schema17_upgrade.sql +++ b/rslib/src/storage/upgrades/schema17_upgrade.sql @@ -1,7 +1,7 @@ -drop table tags; -create table tags ( - id integer primary key not null, - name text not null collate unicase, - usn integer not null, - config blob not null +DROP TABLE tags; +CREATE TABLE tags ( + id integer PRIMARY KEY NOT NULL, + name text NOT NULL COLLATE unicase, + usn integer NOT NULL, + config blob NOT NULL ); \ No newline at end of file From 5919d9273f1d892727a23c0d629e375e65c02419 Mon Sep 17 00:00:00 2001 From: abdo Date: Sat, 9 Jan 2021 17:46:52 +0300 Subject: [PATCH 07/23] Fix tag collapse state not getting updated --- rslib/src/backend/mod.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index e2da5d64e..4c88c6264 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -1320,17 +1320,18 @@ impl BackendService for Backend { fn set_tag_collapsed(&self, input: pb::SetTagCollapsedIn) -> BackendResult { self.with_col(|col| { let name = &input.name; - let tag: Result = if let Some(tag) = col.storage.get_tag(name)? { - Ok(tag) + let mut tag = if let Some(tag) = col.storage.get_tag(name)? { + tag } else { // tag is missing, register it let t = Tag { name: name.to_owned(), ..Default::default() }; - Ok(col.register_tag(t)?.0) + col.register_tag(t)?.0 }; - tag?.config.browser_collapsed = input.collapsed; + tag.config.browser_collapsed = input.collapsed; + col.update_tag(&tag)?; Ok(pb::Bool { val: true }) }) } From 0b5bb711a18202edde4b0d6f6b3d511ac9cd08f1 Mon Sep 17 00:00:00 2001 From: abdo Date: Sat, 9 Jan 2021 17:48:34 +0300 Subject: [PATCH 08/23] Remove unused backend methods & formatting --- rslib/backend.proto | 279 +++++++++++++++++++-------------------- rslib/src/backend/mod.rs | 17 --- 2 files changed, 137 insertions(+), 159 deletions(-) diff --git a/rslib/backend.proto b/rslib/backend.proto index 6a8165cdf..4f9651054 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -66,167 +66,162 @@ message DeckConfigID { int64 dcid = 1; } -message TagID { - int64 tid = 1; -} - // New style RPC definitions /////////////////////////////////////////////////////////// service BackendService { - rpc LatestProgress (Empty) returns (Progress); - rpc SetWantsAbort (Empty) returns (Empty); + rpc LatestProgress(Empty) returns (Progress); + rpc SetWantsAbort(Empty) returns (Empty); - // card rendering + // card rendering - rpc ExtractAVTags (ExtractAVTagsIn) returns (ExtractAVTagsOut); - rpc ExtractLatex (ExtractLatexIn) returns (ExtractLatexOut); - rpc GetEmptyCards (Empty) returns (EmptyCardsReport); - rpc RenderExistingCard (RenderExistingCardIn) returns (RenderCardOut); - rpc RenderUncommittedCard (RenderUncommittedCardIn) returns (RenderCardOut); - rpc StripAVTags (String) returns (String); + rpc ExtractAVTags(ExtractAVTagsIn) returns (ExtractAVTagsOut); + rpc ExtractLatex(ExtractLatexIn) returns (ExtractLatexOut); + rpc GetEmptyCards(Empty) returns (EmptyCardsReport); + rpc RenderExistingCard(RenderExistingCardIn) returns (RenderCardOut); + rpc RenderUncommittedCard(RenderUncommittedCardIn) returns (RenderCardOut); + rpc StripAVTags(String) returns (String); - // searching + // searching - rpc NormalizeSearch (String) returns (String); - rpc SearchCards (SearchCardsIn) returns (SearchCardsOut); - rpc SearchNotes (SearchNotesIn) returns (SearchNotesOut); - rpc NegateSearch (String) returns (String); - rpc ConcatenateSearches (ConcatenateSearchesIn) returns (String); - rpc ReplaceSearchTerm (ReplaceSearchTermIn) returns (String); - rpc FindAndReplace (FindAndReplaceIn) returns (UInt32); + rpc NormalizeSearch(String) returns (String); + rpc SearchCards(SearchCardsIn) returns (SearchCardsOut); + rpc SearchNotes(SearchNotesIn) returns (SearchNotesOut); + rpc NegateSearch(String) returns (String); + rpc ConcatenateSearches(ConcatenateSearchesIn) returns (String); + rpc ReplaceSearchTerm(ReplaceSearchTermIn) returns (String); + rpc FindAndReplace(FindAndReplaceIn) returns (UInt32); - // scheduling + // scheduling - rpc LocalMinutesWest (Int64) returns (Int32); - rpc SetLocalMinutesWest (Int32) returns (Empty); - rpc SchedTimingToday (Empty) returns (SchedTimingTodayOut); - rpc StudiedToday (Empty) returns (String); - rpc StudiedTodayMessage (StudiedTodayMessageIn) returns (String); - rpc UpdateStats (UpdateStatsIn) returns (Empty); - rpc ExtendLimits (ExtendLimitsIn) returns (Empty); - rpc CountsForDeckToday (DeckID) returns (CountsForDeckTodayOut); - rpc CongratsInfo (Empty) returns (CongratsInfoOut); - rpc RestoreBuriedAndSuspendedCards (CardIDs) returns (Empty); - rpc UnburyCardsInCurrentDeck (UnburyCardsInCurrentDeckIn) returns (Empty); - rpc BuryOrSuspendCards (BuryOrSuspendCardsIn) returns (Empty); - rpc EmptyFilteredDeck (DeckID) returns (Empty); - rpc RebuildFilteredDeck (DeckID) returns (UInt32); - rpc ScheduleCardsAsReviews (ScheduleCardsAsReviewsIn) returns (Empty); - rpc ScheduleCardsAsNew (ScheduleCardsAsNewIn) returns (Empty); - rpc SortCards (SortCardsIn) returns (Empty); - rpc SortDeck (SortDeckIn) returns (Empty); + rpc LocalMinutesWest(Int64) returns (Int32); + rpc SetLocalMinutesWest(Int32) returns (Empty); + rpc SchedTimingToday(Empty) returns (SchedTimingTodayOut); + rpc StudiedToday(Empty) returns (String); + rpc StudiedTodayMessage(StudiedTodayMessageIn) returns (String); + rpc UpdateStats(UpdateStatsIn) returns (Empty); + rpc ExtendLimits(ExtendLimitsIn) returns (Empty); + rpc CountsForDeckToday(DeckID) returns (CountsForDeckTodayOut); + rpc CongratsInfo(Empty) returns (CongratsInfoOut); + rpc RestoreBuriedAndSuspendedCards(CardIDs) returns (Empty); + rpc UnburyCardsInCurrentDeck(UnburyCardsInCurrentDeckIn) returns (Empty); + rpc BuryOrSuspendCards(BuryOrSuspendCardsIn) returns (Empty); + rpc EmptyFilteredDeck(DeckID) returns (Empty); + rpc RebuildFilteredDeck(DeckID) returns (UInt32); + rpc ScheduleCardsAsReviews(ScheduleCardsAsReviewsIn) returns (Empty); + rpc ScheduleCardsAsNew(ScheduleCardsAsNewIn) returns (Empty); + rpc SortCards(SortCardsIn) returns (Empty); + rpc SortDeck(SortDeckIn) returns (Empty); - // stats + // stats - rpc CardStats (CardID) returns (String); - rpc Graphs(GraphsIn) returns (GraphsOut); + rpc CardStats(CardID) returns (String); + rpc Graphs(GraphsIn) returns (GraphsOut); - // media + // media - rpc CheckMedia (Empty) returns (CheckMediaOut); - rpc TrashMediaFiles (TrashMediaFilesIn) returns (Empty); - rpc AddMediaFile (AddMediaFileIn) returns (String); - rpc EmptyTrash (Empty) returns (Empty); - rpc RestoreTrash (Empty) returns (Empty); + rpc CheckMedia(Empty) returns (CheckMediaOut); + rpc TrashMediaFiles(TrashMediaFilesIn) returns (Empty); + rpc AddMediaFile(AddMediaFileIn) returns (String); + rpc EmptyTrash(Empty) returns (Empty); + rpc RestoreTrash(Empty) returns (Empty); - // decks + // decks - rpc AddOrUpdateDeckLegacy (AddOrUpdateDeckLegacyIn) returns (DeckID); - rpc DeckTree (DeckTreeIn) returns (DeckTreeNode); - rpc DeckTreeLegacy (Empty) returns (Json); - rpc GetAllDecksLegacy (Empty) returns (Json); - rpc GetDeckIDByName (String) returns (DeckID); - rpc GetDeckLegacy (DeckID) returns (Json); - rpc GetDeckNames (GetDeckNamesIn) returns (DeckNames); - rpc NewDeckLegacy (Bool) returns (Json); - rpc RemoveDeck (DeckID) returns (Empty); + rpc AddOrUpdateDeckLegacy(AddOrUpdateDeckLegacyIn) returns (DeckID); + rpc DeckTree(DeckTreeIn) returns (DeckTreeNode); + rpc DeckTreeLegacy(Empty) returns (Json); + rpc GetAllDecksLegacy(Empty) returns (Json); + rpc GetDeckIDByName(String) returns (DeckID); + rpc GetDeckLegacy(DeckID) returns (Json); + rpc GetDeckNames(GetDeckNamesIn) returns (DeckNames); + rpc NewDeckLegacy(Bool) returns (Json); + rpc RemoveDeck(DeckID) returns (Empty); - // deck config + // deck config - rpc AddOrUpdateDeckConfigLegacy (AddOrUpdateDeckConfigLegacyIn) returns (DeckConfigID); - rpc AllDeckConfigLegacy (Empty) returns (Json); - rpc GetDeckConfigLegacy (DeckConfigID) returns (Json); - rpc NewDeckConfigLegacy (Empty) returns (Json); - rpc RemoveDeckConfig (DeckConfigID) returns (Empty); + rpc AddOrUpdateDeckConfigLegacy(AddOrUpdateDeckConfigLegacyIn) + returns (DeckConfigID); + rpc AllDeckConfigLegacy(Empty) returns (Json); + rpc GetDeckConfigLegacy(DeckConfigID) returns (Json); + rpc NewDeckConfigLegacy(Empty) returns (Json); + rpc RemoveDeckConfig(DeckConfigID) returns (Empty); - // cards + // cards - rpc GetCard (CardID) returns (Card); - rpc UpdateCard (Card) returns (Empty); - rpc AddCard (Card) returns (CardID); - rpc RemoveCards (RemoveCardsIn) returns (Empty); - rpc SetDeck (SetDeckIn) returns (Empty); + rpc GetCard(CardID) returns (Card); + rpc UpdateCard(Card) returns (Empty); + rpc AddCard(Card) returns (CardID); + rpc RemoveCards(RemoveCardsIn) returns (Empty); + rpc SetDeck(SetDeckIn) returns (Empty); - // notes + // notes - rpc NewNote (NoteTypeID) returns (Note); - rpc AddNote (AddNoteIn) returns (NoteID); - rpc UpdateNote (Note) returns (Empty); - rpc GetNote (NoteID) returns (Note); - rpc RemoveNotes (RemoveNotesIn) returns (Empty); - rpc AddNoteTags (AddNoteTagsIn) returns (UInt32); - rpc UpdateNoteTags (UpdateNoteTagsIn) returns (UInt32); - rpc GetNoteTags(GetNoteTagsIn) returns (GetNoteTagsOut); - rpc ClozeNumbersInNote (Note) returns (ClozeNumbersInNoteOut); - rpc AfterNoteUpdates (AfterNoteUpdatesIn) returns (Empty); - rpc FieldNamesForNotes (FieldNamesForNotesIn) returns (FieldNamesForNotesOut); - rpc NoteIsDuplicateOrEmpty (Note) returns (NoteIsDuplicateOrEmptyOut); - rpc CardsOfNote (NoteID) returns (CardIDs); + rpc NewNote(NoteTypeID) returns (Note); + rpc AddNote(AddNoteIn) returns (NoteID); + rpc UpdateNote(Note) returns (Empty); + rpc GetNote(NoteID) returns (Note); + rpc RemoveNotes(RemoveNotesIn) returns (Empty); + rpc AddNoteTags(AddNoteTagsIn) returns (UInt32); + rpc UpdateNoteTags(UpdateNoteTagsIn) returns (UInt32); + rpc GetNoteTags(GetNoteTagsIn) returns (GetNoteTagsOut); + rpc ClozeNumbersInNote(Note) returns (ClozeNumbersInNoteOut); + rpc AfterNoteUpdates(AfterNoteUpdatesIn) returns (Empty); + rpc FieldNamesForNotes(FieldNamesForNotesIn) returns (FieldNamesForNotesOut); + rpc NoteIsDuplicateOrEmpty(Note) returns (NoteIsDuplicateOrEmptyOut); + rpc CardsOfNote(NoteID) returns (CardIDs); - // note types + // note types - rpc AddOrUpdateNotetype (AddOrUpdateNotetypeIn) returns (NoteTypeID); - rpc GetStockNotetypeLegacy (GetStockNotetypeIn) returns (Json); - rpc GetNotetypeLegacy (NoteTypeID) returns (Json); - rpc GetNotetypeNames (Empty) returns (NoteTypeNames); - rpc GetNotetypeNamesAndCounts (Empty) returns (NoteTypeUseCounts); - rpc GetNotetypeIDByName (String) returns (NoteTypeID); - rpc RemoveNotetype (NoteTypeID) returns (Empty); + rpc AddOrUpdateNotetype(AddOrUpdateNotetypeIn) returns (NoteTypeID); + rpc GetStockNotetypeLegacy(GetStockNotetypeIn) returns (Json); + rpc GetNotetypeLegacy(NoteTypeID) returns (Json); + rpc GetNotetypeNames(Empty) returns (NoteTypeNames); + rpc GetNotetypeNamesAndCounts(Empty) returns (NoteTypeUseCounts); + rpc GetNotetypeIDByName(String) returns (NoteTypeID); + rpc RemoveNotetype(NoteTypeID) returns (Empty); - // collection + // collection - rpc OpenCollection (OpenCollectionIn) returns (Empty); - rpc CloseCollection (CloseCollectionIn) returns (Empty); - rpc CheckDatabase (Empty) returns (CheckDatabaseOut); + rpc OpenCollection(OpenCollectionIn) returns (Empty); + rpc CloseCollection(CloseCollectionIn) returns (Empty); + rpc CheckDatabase(Empty) returns (CheckDatabaseOut); - // sync + // sync - rpc SyncMedia (SyncAuth) returns (Empty); - rpc AbortSync (Empty) returns (Empty); - rpc AbortMediaSync (Empty) returns (Empty); - rpc BeforeUpload (Empty) returns (Empty); - rpc SyncLogin (SyncLoginIn) returns (SyncAuth); - rpc SyncStatus (SyncAuth) returns (SyncStatusOut); - rpc SyncCollection (SyncAuth) returns (SyncCollectionOut); - rpc FullUpload (SyncAuth) returns (Empty); - rpc FullDownload (SyncAuth) returns (Empty); + rpc SyncMedia(SyncAuth) returns (Empty); + rpc AbortSync(Empty) returns (Empty); + rpc AbortMediaSync(Empty) returns (Empty); + rpc BeforeUpload(Empty) returns (Empty); + rpc SyncLogin(SyncLoginIn) returns (SyncAuth); + rpc SyncStatus(SyncAuth) returns (SyncStatusOut); + rpc SyncCollection(SyncAuth) returns (SyncCollectionOut); + rpc FullUpload(SyncAuth) returns (Empty); + rpc FullDownload(SyncAuth) returns (Empty); - // translation/messages + // translation/messages - rpc TranslateString (TranslateStringIn) returns (String); - rpc FormatTimespan (FormatTimespanIn) returns (String); - rpc I18nResources (Empty) returns (Json); + rpc TranslateString(TranslateStringIn) returns (String); + rpc FormatTimespan(FormatTimespanIn) returns (String); + rpc I18nResources(Empty) returns (Json); - // tags + // tags - rpc RegisterTags (RegisterTagsIn) returns (Bool); - rpc AllTags (Empty) returns (AllTagsOut); - rpc GetTag (String) returns (Tag); - rpc UpdateTag (Tag) returns (Bool); - rpc SetTagCollapsed (SetTagCollapsedIn) returns (Bool); - rpc ClearTag (String) returns (Bool); - rpc TagTree (Empty) returns (TagTreeNode); + rpc RegisterTags(RegisterTagsIn) returns (Bool); + rpc AllTags(Empty) returns (AllTagsOut); + rpc SetTagCollapsed(SetTagCollapsedIn) returns (Bool); + rpc ClearTag(String) returns (Bool); + rpc TagTree(Empty) returns (TagTreeNode); - // config/preferences + // config/preferences - rpc GetConfigJson (String) returns (Json); - rpc SetConfigJson (SetConfigJsonIn) returns (Empty); - rpc RemoveConfig (String) returns (Empty); - rpc SetAllConfig (Json) returns (Empty); - rpc GetAllConfig (Empty) returns (Json); - rpc GetPreferences (Empty) returns (Preferences); - rpc SetPreferences (Preferences) returns (Empty); + rpc GetConfigJson(String) returns (Json); + rpc SetConfigJson(SetConfigJsonIn) returns (Empty); + rpc RemoveConfig(String) returns (Empty); + rpc SetAllConfig(Json) returns (Empty); + rpc GetAllConfig(Empty) returns (Json); + rpc GetPreferences(Empty) returns (Preferences); + rpc SetPreferences(Preferences) returns (Empty); } // Protobuf stored in .anki2 files @@ -794,23 +789,23 @@ message RegisterTagsIn { } message AllTagsOut { - repeated Tag tags = 1; + repeated Tag tags = 1; } message SetTagCollapsedIn { - string name = 1; - bool collapsed = 2; + string name = 1; + bool collapsed = 2; } message TagConfig { - bool browser_collapsed = 1; + bool browser_collapsed = 1; } message Tag { - int64 id = 1; - string name = 2; - sint32 usn = 3; - TagConfig config = 4; + int64 id = 1; + string name = 2; + sint32 usn = 3; + TagConfig config = 4; } message GetChangedTagsOut { @@ -818,11 +813,11 @@ message GetChangedTagsOut { } message TagTreeNode { - int64 tag_id = 1; - string name = 2; - repeated TagTreeNode children = 3; - uint32 level = 5; - bool collapsed = 4; + int64 tag_id = 1; + string name = 2; + repeated TagTreeNode children = 3; + uint32 level = 5; + bool collapsed = 4; } message SetConfigJsonIn { @@ -932,11 +927,11 @@ message UpdateNoteTagsIn { } message GetNoteTagsIn { - repeated int64 nids = 1; + repeated int64 nids = 1; } message GetNoteTagsOut { - repeated string tags = 1; + repeated string tags = 1; } message CheckDatabaseOut { diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index 4c88c6264..926a59dac 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -1300,23 +1300,6 @@ impl BackendService for Backend { Ok(pb::AllTagsOut { tags }) } - fn get_tag(&self, name: pb::String) -> BackendResult { - self.with_col(|col| { - if let Some(tag) = col.storage.get_tag(name.val.as_str())? { - Ok(tag.into()) - } else { - Err(AnkiError::NotFound) - } - }) - } - - fn update_tag(&self, tag: pb::Tag) -> BackendResult { - self.with_col(|col| { - col.update_tag(&tag.into())?; - Ok(pb::Bool { val: true }) - }) - } - fn set_tag_collapsed(&self, input: pb::SetTagCollapsedIn) -> BackendResult { self.with_col(|col| { let name = &input.name; From 9a68d844837e31e70c509083a01c0f38ab195017 Mon Sep 17 00:00:00 2001 From: abdo Date: Tue, 12 Jan 2021 23:12:35 +0300 Subject: [PATCH 09/23] Keep tags in human form and update the tags table structure See https://github.com/ankitects/anki/pull/900#issuecomment-758284016 - Leave tag names alone and add the collapsed and config columns to the tags table. - Update The DB check code to preserve the collapse state of used tags. - Add a simple test for clearing tags and their children --- qt/aqt/browser.py | 1 - rslib/backend.proto | 18 ++-- rslib/src/backend/mod.rs | 31 +++---- rslib/src/dbcheck.rs | 13 +++ rslib/src/search/sqlwriter.rs | 10 +-- rslib/src/storage/note/mod.rs | 72 ++------------- rslib/src/storage/tag/add.sql | 4 +- rslib/src/storage/tag/mod.rs | 88 ++++++------------- rslib/src/storage/upgrades/mod.rs | 3 - .../src/storage/upgrades/schema17_upgrade.sql | 8 +- rslib/src/tags.rs | 88 ++++++------------- 11 files changed, 106 insertions(+), 230 deletions(-) diff --git a/qt/aqt/browser.py b/qt/aqt/browser.py index dd008c657..9b6e76565 100644 --- a/qt/aqt/browser.py +++ b/qt/aqt/browser.py @@ -1158,7 +1158,6 @@ QTableView {{ gridline-color: {grid} }} toggle_expand(), not node.collapsed, item_type=SidebarItemType.TAG, - id=node.tag_id, full_name=head + node.name, ) root.addChild(item) diff --git a/rslib/backend.proto b/rslib/backend.proto index 4f9651054..8f733096f 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -797,15 +797,10 @@ message SetTagCollapsedIn { bool collapsed = 2; } -message TagConfig { - bool browser_collapsed = 1; -} - message Tag { - int64 id = 1; - string name = 2; - sint32 usn = 3; - TagConfig config = 4; + string name = 1; + sint32 usn = 2; + bool collapsed = 3; } message GetChangedTagsOut { @@ -813,10 +808,9 @@ message GetChangedTagsOut { } message TagTreeNode { - int64 tag_id = 1; - string name = 2; - repeated TagTreeNode children = 3; - uint32 level = 5; + string name = 1; + repeated TagTreeNode children = 2; + uint32 level = 3; bool collapsed = 4; } diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index 926a59dac..c66f63f57 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -44,7 +44,6 @@ use crate::{ get_remote_sync_meta, sync_abort, sync_login, FullSyncProgress, NormalSyncProgress, SyncActionRequired, SyncAuth, SyncMeta, SyncOutput, SyncStage, }, - tags::Tag, template::RenderedNode, text::{extract_av_tags, strip_av_tags, AVTag}, timestamp::TimestampSecs, @@ -1295,27 +1294,23 @@ impl BackendService for Backend { //------------------------------------------------------------------- fn all_tags(&self, _input: Empty) -> BackendResult { - let tags: Vec = - self.with_col(|col| Ok(col.all_tags()?.into_iter().map(|t| t.into()).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 set_tag_collapsed(&self, input: pb::SetTagCollapsedIn) -> BackendResult { self.with_col(|col| { - let name = &input.name; - let mut tag = if let Some(tag) = col.storage.get_tag(name)? { - tag - } else { - // tag is missing, register it - let t = Tag { - name: name.to_owned(), - ..Default::default() - }; - col.register_tag(t)?.0 - }; - tag.config.browser_collapsed = input.collapsed; - col.update_tag(&tag)?; - Ok(pb::Bool { val: true }) + col.transact(None, |col| { + col.set_tag_collapsed(&input.name, input.collapsed)?; + Ok(pb::Bool { val: true }) + }) }) } @@ -1336,7 +1331,7 @@ impl BackendService for Backend { fn clear_tag(&self, tag: pb::String) -> BackendResult { self.with_col(|col| { col.transact(None, |col| { - col.clear_tag(tag.val.as_str())?; + col.storage.clear_tag(tag.val.as_str())?; Ok(pb::Bool { val: true }) }) }) diff --git a/rslib/src/dbcheck.rs b/rslib/src/dbcheck.rs index dc78e2bc2..80cccbc30 100644 --- a/rslib/src/dbcheck.rs +++ b/rslib/src/dbcheck.rs @@ -243,6 +243,7 @@ impl Collection { let stamp = TimestampMillis::now(); // will rebuild tag list below + let old_tags = self.storage.all_tags_sorted()?; self.storage.clear_tags()?; let total_notes = self.storage.total_notes()?; @@ -294,6 +295,18 @@ impl Collection { } } + let new_tags = self.storage.all_tags_sorted()?; + for old in old_tags.into_iter() { + for new in new_tags.iter() { + if new.name == old.name { + self.storage.set_tag_collapsed(&new.name, new.collapsed)?; + break; + } else if new.name.starts_with(&old.name) { + self.set_tag_collapsed(&old.name, old.collapsed)?; + } + } + } + // 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() { diff --git a/rslib/src/search/sqlwriter.rs b/rslib/src/search/sqlwriter.rs index aba59b8dd..b3863f6d1 100644 --- a/rslib/src/search/sqlwriter.rs +++ b/rslib/src/search/sqlwriter.rs @@ -10,7 +10,6 @@ use crate::{ notes::field_checksum, notetype::NoteTypeID, storage::ids_to_string, - tags::human_tag_name_to_native, text::{ is_glob, matches_glob, normalize_to_nfc, strip_html_preserving_media_filenames, to_custom_re, to_re, to_sql, to_text, without_combining, @@ -204,8 +203,7 @@ impl SqlWriter<'_> { text => { write!(self.sql, "n.tags regexp ?").unwrap(); let re = &to_custom_re(text, r"\S"); - let native_name = human_tag_name_to_native(re); - self.args.push(format!("(?i).* {}(\x1f| ).*", native_name)); + self.args.push(format!("(?i).* {}(::| ).*", re)); } } } @@ -681,14 +679,14 @@ mod test { s(ctx, r"tag:one"), ( "(n.tags regexp ?)".into(), - vec!["(?i).* one(\x1f| ).*".into()] + vec!["(?i).* one(::| ).*".into()] ) ); assert_eq!( s(ctx, r"tag:foo::bar"), ( "(n.tags regexp ?)".into(), - vec!["(?i).* foo\x1fbar(\x1f| ).*".into()] + vec!["(?i).* foo::bar(::| ).*".into()] ) ); @@ -696,7 +694,7 @@ mod test { s(ctx, r"tag:o*n\*et%w%oth_re\_e"), ( "(n.tags regexp ?)".into(), - vec!["(?i).* o\\S*n\\*et%w%oth\\Sre_e(\u{1f}| ).*".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 2ddaa8655..e109d9f33 100644 --- a/rslib/src/storage/note/mod.rs +++ b/rslib/src/storage/note/mod.rs @@ -5,7 +5,7 @@ use crate::{ err::Result, notes::{Note, NoteID}, notetype::NoteTypeID, - tags::{human_tag_name_to_native, join_tags, native_tag_name_to_human, split_tags}, + tags::{join_tags, split_tags}, timestamp::TimestampMillis, }; use rusqlite::{params, Row, NO_PARAMS}; @@ -18,11 +18,6 @@ pub(crate) fn join_fields(fields: &[String]) -> String { fields.join("\x1f") } -fn native_tags_str(tags: &[String]) -> String { - let s: Vec<_> = tags.iter().map(|t| human_tag_name_to_native(t)).collect(); - join_tags(&s) -} - fn row_to_note(row: &Row) -> Result { Ok(Note { id: row.get(0)?, @@ -31,7 +26,7 @@ fn row_to_note(row: &Row) -> Result { mtime: row.get(3)?, usn: row.get(4)?, tags: split_tags(row.get_raw(5).as_str()?) - .map(|t| native_tag_name_to_human(t)) + .map(Into::into) .collect(), fields: split_fields(row.get_raw(6).as_str()?), sort_field: None, @@ -57,7 +52,7 @@ impl super::SqliteStorage { note.notetype_id, note.mtime, note.usn, - native_tags_str(¬e.tags), + join_tags(¬e.tags), join_fields(¬e.fields()), note.sort_field.as_ref().unwrap(), note.checksum.unwrap(), @@ -75,7 +70,7 @@ impl super::SqliteStorage { note.notetype_id, note.mtime, note.usn, - native_tags_str(¬e.tags), + join_tags(¬e.tags), join_fields(¬e.fields()), note.sort_field.as_ref().unwrap(), note.checksum.unwrap(), @@ -93,7 +88,7 @@ impl super::SqliteStorage { note.notetype_id, note.mtime, note.usn, - native_tags_str(¬e.tags), + join_tags(¬e.tags), join_fields(¬e.fields()), note.sort_field.as_ref().unwrap(), note.checksum.unwrap(), @@ -162,69 +157,18 @@ impl super::SqliteStorage { .map_err(Into::into) } - // get distinct note tags in human form + // get distinct note tags pub(crate) fn get_note_tags(&self, nids: Vec) -> Result> { if nids.is_empty() { self.db .prepare_cached("select distinct tags from notes")? - .query_and_then(NO_PARAMS, |r| { - let t = r.get_raw(0).as_str()?; - Ok(native_tag_name_to_human(t)) - })? + .query_and_then(NO_PARAMS, |r| Ok(r.get_raw(0).as_str()?.to_owned()))? .collect() } else { self.db .prepare_cached("select distinct tags from notes where id in ?")? - .query_and_then(nids, |r| { - let t = r.get_raw(0).as_str()?; - Ok(native_tag_name_to_human(t)) - })? + .query_and_then(nids, |r| Ok(r.get_raw(0).as_str()?.to_owned()))? .collect() } } - - pub(super) fn upgrade_notes_to_schema17(&self) -> Result<()> { - let notes: Result> = self - .db - .prepare_cached("select id, tags from notes")? - .query_and_then(NO_PARAMS, |row| -> Result<(NoteID, String)> { - let id = NoteID(row.get_raw(0).as_i64()?); - let tags: Vec = split_tags(row.get_raw(1).as_str()?) - .map(|t| human_tag_name_to_native(t)) - .collect(); - let tags = join_tags(&tags); - Ok((id, tags)) - })? - .collect(); - notes?.into_iter().try_for_each(|(id, tags)| -> Result<_> { - self.db - .prepare_cached("update notes set tags = ? where id = ?")? - .execute(params![tags, id])?; - Ok(()) - })?; - - Ok(()) - } - - pub(super) fn downgrade_notes_from_schema17(&self) -> Result<()> { - let notes: Result> = self - .db - .prepare_cached("select id, tags from notes")? - .query_and_then(NO_PARAMS, |row| -> Result<(NoteID, String)> { - let id = NoteID(row.get_raw(0).as_i64()?); - let tags: Vec = split_tags(row.get_raw(1).as_str()?) - .map(|t| native_tag_name_to_human(t)) - .collect(); - let tags = join_tags(&tags); - Ok((id, tags)) - })? - .collect(); - notes?.into_iter().try_for_each(|(id, tags)| -> Result<_> { - self.db - .prepare_cached("update notes set tags = ? where id = ?")? - .execute(params![tags, id])?; - Ok(()) - })?; - Ok(()) - } } diff --git a/rslib/src/storage/tag/add.sql b/rslib/src/storage/tag/add.sql index 57db3bb0d..3f1f63656 100644 --- a/rslib/src/storage/tag/add.sql +++ b/rslib/src/storage/tag/add.sql @@ -1,3 +1,3 @@ INSERT - OR REPLACE INTO tags (id, name, usn, config) -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/mod.rs b/rslib/src/storage/tag/mod.rs index bfbbce227..866c5b6e6 100644 --- a/rslib/src/storage/tag/mod.rs +++ b/rslib/src/storage/tag/mod.rs @@ -2,30 +2,23 @@ // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html use super::SqliteStorage; -use crate::{ - err::Result, - tags::{human_tag_name_to_native, native_tag_name_to_human, Tag, TagConfig, TagID}, - timestamp::TimestampMillis, - types::Usn, -}; -use prost::Message; +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 { - let config = TagConfig::decode(row.get_raw(3).as_blob()?)?; Ok(Tag { - id: row.get(0)?, - name: row.get(1)?, - usn: row.get(2)?, - config, + name: row.get(0)?, + usn: row.get(1)?, + collapsed: row.get(2)?, }) } impl SqliteStorage { pub(crate) fn all_tags(&self) -> Result> { self.db - .prepare_cached("select id, name, usn, config from tags")? + .prepare_cached("select tag, usn, collapsed from tags")? .query_and_then(NO_PARAMS, row_to_tag)? .collect() } @@ -33,54 +26,30 @@ impl SqliteStorage { /// Get all tags in human form, sorted by name pub(crate) fn all_tags_sorted(&self) -> Result> { self.db - .prepare_cached("select id, name, usn, config from tags order by name")? - .query_and_then(NO_PARAMS, |row| { - let mut tag = row_to_tag(row)?; - tag.name = native_tag_name_to_human(&tag.name); - Ok(tag) - })? + .prepare_cached("select tag, usn, collapsed from tags order by tag")? + .query_and_then(NO_PARAMS, row_to_tag)? .collect() } /// Get tag by human name pub(crate) fn get_tag(&self, name: &str) -> Result> { self.db - .prepare_cached("select id, name, usn, config from tags where name = ?")? - .query_and_then(&[human_tag_name_to_native(name)], |row| { - let mut tag = row_to_tag(row)?; - tag.name = native_tag_name_to_human(&tag.name); - Ok(tag) - })? + .prepare_cached("select tag, usn, collapsed from tags where tag = ?")? + .query_and_then(&[name], row_to_tag)? .next() .transpose() } - fn alloc_id(&self) -> rusqlite::Result { - self.db - .prepare_cached(include_str!("alloc_id.sql"))? - .query_row(&[TimestampMillis::now()], |r| r.get(0)) - } - - pub(crate) fn register_tag(&self, tag: &mut Tag) -> Result<()> { - let mut config = vec![]; - tag.config.encode(&mut config)?; - tag.id = self.alloc_id()?; - self.update_tag(tag)?; - Ok(()) - } - - pub(crate) fn update_tag(&self, tag: &Tag) -> Result<()> { - let mut config = vec![]; - tag.config.encode(&mut config)?; + pub(crate) fn register_tag(&self, tag: &Tag) -> Result<()> { self.db .prepare_cached(include_str!("add.sql"))? - .execute(params![tag.id, tag.name, tag.usn, config])?; + .execute(params![tag.name, tag.usn, tag.collapsed])?; Ok(()) } pub(crate) fn preferred_tag_case(&self, tag: &str) -> Result> { self.db - .prepare_cached("select name from tags where name = ?")? + .prepare_cached("select tag from tags where tag = ?")? .query_and_then(params![tag], |row| row.get(0))? .next() .transpose() @@ -89,8 +58,16 @@ impl SqliteStorage { pub(crate) fn clear_tag(&self, tag: &str) -> Result<()> { self.db - .prepare_cached("delete from tags where name regexp ?")? - .execute(&[format!("^{}($|\x1f)", regex::escape(tag))])?; + .prepare_cached("delete from tags where tag regexp ?")? + .execute(&[format!("^{}($|::)", 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(()) } @@ -111,7 +88,7 @@ impl SqliteStorage { pub(crate) fn tags_pending_sync(&self, usn: Usn) -> Result> { self.db .prepare_cached(&format!( - "select name from tags where {}", + "select tag from tags where {}", usn.pending_object_clause() ))? .query_and_then(&[usn], |r| r.get(0).map_err(Into::into))? @@ -121,7 +98,7 @@ impl SqliteStorage { pub(crate) fn update_tag_usns(&self, tags: &[String], new_usn: Usn) -> Result<()> { let mut stmt = self .db - .prepare_cached("update tags set usn=? where name=?")?; + .prepare_cached("update tags set usn=? where tag=?")?; for tag in tags { stmt.execute(params![new_usn, tag])?; } @@ -173,18 +150,7 @@ impl SqliteStorage { .collect::>>()?; self.db .execute_batch(include_str!["../upgrades/schema17_upgrade.sql"])?; - tags.into_iter().try_for_each(|mut tag| -> Result<()> { - tag.name = human_tag_name_to_native(&tag.name); - self.register_tag(&mut tag) - }) - } - - pub(super) fn downgrade_tags_from_schema17(&self) -> Result<()> { - let tags = self.all_tags()?; - self.clear_tags()?; - tags.into_iter().try_for_each(|mut tag| -> Result<()> { - tag.name = native_tag_name_to_human(&tag.name); - self.register_tag(&mut tag) - }) + 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 1167ecf59..acccf7328 100644 --- a/rslib/src/storage/upgrades/mod.rs +++ b/rslib/src/storage/upgrades/mod.rs @@ -33,7 +33,6 @@ impl SqliteStorage { } if ver < 17 { self.upgrade_tags_to_schema17()?; - self.upgrade_notes_to_schema17()?; self.db.execute_batch("update col set ver = 17")?; } @@ -47,9 +46,7 @@ impl SqliteStorage { self.downgrade_decks_from_schema15()?; self.downgrade_notetypes_from_schema15()?; self.downgrade_config_from_schema14()?; - self.downgrade_tags_from_schema17()?; self.downgrade_tags_from_schema14()?; - self.downgrade_notes_from_schema17()?; self.db .execute_batch(include_str!("schema11_downgrade.sql"))?; diff --git a/rslib/src/storage/upgrades/schema17_upgrade.sql b/rslib/src/storage/upgrades/schema17_upgrade.sql index 288a14a07..c8abf52ee 100644 --- a/rslib/src/storage/upgrades/schema17_upgrade.sql +++ b/rslib/src/storage/upgrades/schema17_upgrade.sql @@ -1,7 +1,7 @@ DROP TABLE tags; CREATE TABLE tags ( - id integer PRIMARY KEY NOT NULL, - name text NOT NULL COLLATE unicase, + tag text NOT NULL PRIMARY KEY COLLATE unicase, usn integer NOT NULL, - config blob NOT NULL -); \ No newline at end of file + collapsed boolean NOT NULL, + config blob NULL +) without rowid; \ No newline at end of file diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index 26ff9b486..6de9d2031 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -1,11 +1,9 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html -pub use crate::backend_proto::TagConfig; use crate::{ backend_proto::{Tag as TagProto, TagTreeNode}, collection::Collection, - define_newtype, err::{AnkiError, Result}, notes::{NoteID, TransformNoteOutput}, text::{normalize_to_nfc, to_re}, @@ -21,14 +19,11 @@ use std::{ }; use unicase::UniCase; -define_newtype!(TagID, i64); - #[derive(Debug, Clone)] pub struct Tag { - pub id: TagID, pub name: String, pub usn: Usn, - pub config: TagConfig, + pub collapsed: bool, } impl Ord for Tag { @@ -54,10 +49,9 @@ impl Eq for Tag {} impl Default for Tag { fn default() -> Self { Tag { - id: TagID(0), name: "".to_string(), usn: Usn(-1), - config: Default::default(), + collapsed: false, } } } @@ -65,10 +59,9 @@ impl Default for Tag { impl From for TagProto { fn from(t: Tag) -> Self { TagProto { - id: t.id.0, name: t.name, usn: t.usn.0, - config: Some(t.config), + collapsed: t.collapsed, } } } @@ -76,10 +69,9 @@ impl From for TagProto { impl From for Tag { fn from(t: TagProto) -> Self { Tag { - id: TagID(t.id), name: t.name, usn: Usn(t.usn), - config: t.config.unwrap(), + collapsed: t.collapsed, } } } @@ -119,17 +111,13 @@ fn normalized_tag_name_component(comp: &str) -> Cow { } } -pub(crate) fn human_tag_name_to_native(name: &str) -> String { +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('\x1f'); + out.push_str("::"); } - out.trim_end_matches('\x1f').into() -} - -pub(crate) fn native_tag_name_to_human(name: &str) -> String { - name.replace('\x1f', "::") + out.trim_end_matches("::").into() } fn fill_missing_tags(tags: Vec) -> Vec { @@ -177,11 +165,10 @@ fn add_child_nodes(tags: &mut Peekable>, parent: &mut l if l == parent.level + 1 => { // next item is an immediate descendent of parent parent.children.push(TagTreeNode { - tag_id: tag.id.0, name: (*split_name.last().unwrap()).into(), children: vec![], level: parent.level + 1, - collapsed: tag.config.browser_collapsed, + collapsed: tag.collapsed, }); tags.next(); } @@ -199,19 +186,6 @@ fn add_child_nodes(tags: &mut Peekable>, parent: &mut } impl Collection { - pub fn all_tags(&self) -> Result> { - self.storage - .all_tags()? - .into_iter() - .map(|t| { - Ok(Tag { - name: native_tag_name_to_human(&t.name), - ..t - }) - }) - .collect() - } - pub fn tag_tree(&mut self) -> Result { let tags = self.storage.all_tags_sorted()?; let tree = tags_to_tree(tags); @@ -253,20 +227,19 @@ impl Collection { } pub(crate) fn register_tag(&self, tag: Tag) -> Result<(Tag, bool)> { - let native_name = human_tag_name_to_native(&tag.name); + let normalized_name = normalize_tag_name(&tag.name); let mut t = Tag { - name: native_name.clone(), + name: normalized_name.clone(), ..tag }; - if native_name.is_empty() { + if normalized_name.is_empty() { return Ok((t, false)); } - if let Some(preferred) = self.storage.preferred_tag_case(&native_name)? { - t.name = native_tag_name_to_human(&preferred); + if let Some(preferred) = self.storage.preferred_tag_case(&normalized_name)? { + t.name = preferred; Ok((t, false)) } else { - self.storage.register_tag(&mut t)?; - t.name = native_tag_name_to_human(&t.name); + self.storage.register_tag(&t)?; Ok((t, true)) } } @@ -287,19 +260,16 @@ impl Collection { Ok(changed) } - pub(crate) fn update_tag(&self, tag: &Tag) -> Result<()> { - let native_name = human_tag_name_to_native(&tag.name); - self.storage.update_tag(&Tag { - id: tag.id, - name: native_name, - usn: tag.usn, - config: tag.config.clone(), - }) - } - - pub(crate) fn clear_tag(&self, tag: &str) -> Result<()> { - let native_name = human_tag_name_to_native(tag); - self.storage.clear_tag(&native_name) + 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 + let t = Tag { + name: name.to_owned(), + ..Default::default() + }; + self.register_tag(t)?; + } + self.storage.set_tag_collapsed(name, collapsed) } fn replace_tags_for_notes_inner( @@ -425,11 +395,6 @@ mod test { col.update_note(&mut note)?; assert_eq!(¬e.tags, &["one", "two"]); - // note.tags is in human form - note.tags = vec!["foo::bar".into()]; - col.update_note(&mut note)?; - assert_eq!(¬e.tags, &["foo::bar"]); - Ok(()) } @@ -483,6 +448,11 @@ mod test { 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.register_tags("a a::b a::b::c", Usn(-1), true)?; + col.storage.clear_tag("a")?; + assert_eq!(col.storage.all_tags()?, vec![]); + Ok(()) } } From 9c1d7c522a1d136ce8d18752984c06b30928678f Mon Sep 17 00:00:00 2001 From: abdo Date: Thu, 14 Jan 2021 02:53:52 +0300 Subject: [PATCH 10/23] Refactor code for clearing unused tags and saving collapse state --- pylib/anki/tags.py | 28 ++++++---------------------- rslib/backend.proto | 22 +++------------------- rslib/src/backend/mod.rs | 31 +++++++++++-------------------- rslib/src/dbcheck.rs | 13 ++----------- rslib/src/storage/note/mod.rs | 17 +++++------------ rslib/src/tags.rs | 16 ++++++++++++++++ 6 files changed, 43 insertions(+), 84 deletions(-) diff --git a/pylib/anki/tags.py b/pylib/anki/tags.py index 2fd52375e..bb48e47c3 100644 --- a/pylib/anki/tags.py +++ b/pylib/anki/tags.py @@ -42,30 +42,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( - self.col.backend.get_note_tags(nids), - 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" diff --git a/rslib/backend.proto b/rslib/backend.proto index 974e9d759..1bfecfbce 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -165,7 +165,6 @@ service BackendService { rpc RemoveNotes(RemoveNotesIn) returns (Empty); rpc AddNoteTags(AddNoteTagsIn) returns (UInt32); rpc UpdateNoteTags(UpdateNoteTagsIn) returns (UInt32); - rpc GetNoteTags(GetNoteTagsIn) returns (GetNoteTagsOut); rpc ClozeNumbersInNote(Note) returns (ClozeNumbersInNoteOut); rpc AfterNoteUpdates(AfterNoteUpdatesIn) returns (Empty); rpc FieldNamesForNotes(FieldNamesForNotesIn) returns (FieldNamesForNotesOut); @@ -208,10 +207,10 @@ service BackendService { // tags - rpc RegisterTags(RegisterTagsIn) returns (Bool); + rpc ClearUnusedTags(Empty) returns (Empty); rpc AllTags(Empty) returns (AllTagsOut); - rpc SetTagCollapsed(SetTagCollapsedIn) returns (Bool); - rpc ClearTag(String) returns (Bool); + rpc SetTagCollapsed(SetTagCollapsedIn) returns (Empty); + rpc ClearTag(String) returns (Empty); rpc TagTree(Empty) returns (TagTreeNode); // config/preferences @@ -811,13 +810,6 @@ 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 Tag tags = 1; } @@ -950,14 +942,6 @@ message UpdateNoteTagsIn { bool regex = 4; } -message GetNoteTagsIn { - repeated int64 nids = 1; -} - -message GetNoteTagsOut { - repeated string tags = 1; -} - message CheckDatabaseOut { repeated string problems = 1; } diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index 170f978f9..15863b5dc 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -44,6 +44,7 @@ use crate::{ get_remote_sync_meta, sync_abort, sync_login, FullSyncProgress, NormalSyncProgress, SyncActionRequired, SyncAuth, SyncMeta, SyncOutput, SyncStage, }, + tags::join_tags, template::RenderedNode, text::{escape_anki_wildcards, extract_av_tags, strip_av_tags, AVTag}, timestamp::TimestampSecs, @@ -974,14 +975,6 @@ impl BackendService for Backend { }) } - fn get_note_tags(&self, input: pb::GetNoteTagsIn) -> BackendResult { - self.with_col(|col| { - Ok(pb::GetNoteTagsOut { - tags: col.storage.get_note_tags(to_nids(input.nids))?, - }) - }) - } - fn cloze_numbers_in_note(&self, note: pb::Note) -> BackendResult { let mut set = HashSet::with_capacity(4); for field in ¬e.fields { @@ -1360,34 +1353,32 @@ impl BackendService for Backend { Ok(pb::AllTagsOut { tags }) } - fn set_tag_collapsed(&self, input: pb::SetTagCollapsedIn) -> BackendResult { + fn set_tag_collapsed(&self, input: pb::SetTagCollapsedIn) -> BackendResult { self.with_col(|col| { col.transact(None, |col| { col.set_tag_collapsed(&input.name, input.collapsed)?; - Ok(pb::Bool { val: true }) + Ok(().into()) }) }) } - fn register_tags(&self, input: pb::RegisterTagsIn) -> BackendResult { + fn clear_unused_tags(&self, _input: pb::Empty) -> 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 }) + let old_tags = col.storage.all_tags()?; + let note_tags = join_tags(&col.storage.get_note_tags()?); + col.register_tags(¬e_tags, col.usn()?, true)?; + col.update_tags_collapse(old_tags)?; + Ok(().into()) }) }) } - fn clear_tag(&self, tag: pb::String) -> BackendResult { + 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(pb::Bool { val: true }) + Ok(().into()) }) }) } diff --git a/rslib/src/dbcheck.rs b/rslib/src/dbcheck.rs index 80cccbc30..ac76b0147 100644 --- a/rslib/src/dbcheck.rs +++ b/rslib/src/dbcheck.rs @@ -295,17 +295,8 @@ impl Collection { } } - let new_tags = self.storage.all_tags_sorted()?; - for old in old_tags.into_iter() { - for new in new_tags.iter() { - if new.name == old.name { - self.storage.set_tag_collapsed(&new.name, new.collapsed)?; - break; - } else if new.name.starts_with(&old.name) { - self.set_tag_collapsed(&old.name, old.collapsed)?; - } - } - } + // restore tags collapse state and re-register old tags that are parents of used ones + self.update_tags_collapse(old_tags)?; // if the collection is empty and the user has deleted all note types, ensure at least // one note type exists diff --git a/rslib/src/storage/note/mod.rs b/rslib/src/storage/note/mod.rs index e109d9f33..b2d582a93 100644 --- a/rslib/src/storage/note/mod.rs +++ b/rslib/src/storage/note/mod.rs @@ -158,17 +158,10 @@ impl super::SqliteStorage { } // get distinct note tags - pub(crate) fn get_note_tags(&self, nids: Vec) -> Result> { - if nids.is_empty() { - self.db - .prepare_cached("select distinct tags from notes")? - .query_and_then(NO_PARAMS, |r| Ok(r.get_raw(0).as_str()?.to_owned()))? - .collect() - } else { - self.db - .prepare_cached("select distinct tags from notes where id in ?")? - .query_and_then(nids, |r| Ok(r.get_raw(0).as_str()?.to_owned()))? - .collect() - } + pub(crate) fn get_note_tags(&self) -> Result> { + self.db + .prepare_cached("select distinct tags from notes")? + .query_and_then(NO_PARAMS, |r| Ok(r.get_raw(0).as_str()?.to_owned()))? + .collect() } } diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index 6de9d2031..f7c61ace4 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -272,6 +272,22 @@ impl Collection { self.storage.set_tag_collapsed(name, collapsed) } + /// Update collapse state of existing tags and register tags in old_tags that are parents of those tags + pub(crate) fn update_tags_collapse(&self, old_tags: Vec) -> Result<()> { + let new_tags = self.storage.all_tags_sorted()?; + for old in old_tags.into_iter() { + for new in new_tags.iter() { + if new.name == old.name { + self.storage.set_tag_collapsed(&new.name, old.collapsed)?; + break; + } else if new.name.starts_with(&old.name) { + self.set_tag_collapsed(&old.name, old.collapsed)?; + } + } + } + Ok(()) + } + fn replace_tags_for_notes_inner( &mut self, nids: &[NoteID], From ee3c01980446dfce2906977b7a7f12e6a2e9a778 Mon Sep 17 00:00:00 2001 From: abdo Date: Thu, 14 Jan 2021 06:41:46 +0300 Subject: [PATCH 11/23] Remove Default impl of Tag --- rslib/src/storage/tag/mod.rs | 8 +----- rslib/src/sync/mod.rs | 6 +---- rslib/src/tags.rs | 49 +++++++++++++----------------------- 3 files changed, 20 insertions(+), 43 deletions(-) diff --git a/rslib/src/storage/tag/mod.rs b/rslib/src/storage/tag/mod.rs index 866c5b6e6..fcd4f2c8a 100644 --- a/rslib/src/storage/tag/mod.rs +++ b/rslib/src/storage/tag/mod.rs @@ -140,13 +140,7 @@ impl SqliteStorage { let tags = self .db .prepare_cached("select tag, usn from tags")? - .query_and_then(NO_PARAMS, |r| { - Ok(Tag { - name: r.get(0)?, - usn: r.get(1)?, - ..Default::default() - }) - })? + .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"])?; diff --git a/rslib/src/sync/mod.rs b/rslib/src/sync/mod.rs index 0a992f179..3d00e2361 100644 --- a/rslib/src/sync/mod.rs +++ b/rslib/src/sync/mod.rs @@ -888,11 +888,7 @@ impl Collection { fn merge_tags(&self, tags: Vec, latest_usn: Usn) -> Result<()> { for tag in tags { - self.register_tag(Tag { - name: tag, - usn: latest_usn, - ..Default::default() - })?; + self.register_tag(Tag::new(tag, latest_usn))?; } Ok(()) } diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index f7c61ace4..1ae6a69c9 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -46,16 +46,6 @@ impl PartialEq for Tag { impl Eq for Tag {} -impl Default for Tag { - fn default() -> Self { - Tag { - name: "".to_string(), - usn: Usn(-1), - collapsed: false, - } - } -} - impl From for TagProto { fn from(t: Tag) -> Self { TagProto { @@ -76,6 +66,16 @@ impl From for Tag { } } +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()) } @@ -127,10 +127,7 @@ fn fill_missing_tags(tags: Vec) -> Vec { let split: Vec<&str> = (&tag.name).split("::").collect(); for i in 0..split.len() - 1 { let comp = split[0..i + 1].join("::"); - let t = Tag { - name: comp.to_owned(), - ..Default::default() - }; + let t = Tag::new(comp.to_owned(), Usn(-1)); if filled_tags.get(&comp).is_none() { filled_tags.insert(comp, t); } @@ -199,13 +196,9 @@ impl Collection { let mut seen = HashSet::new(); let mut added = false; - let mut tags: Vec<_> = tags.iter().flat_map(|t| split_tags(t)).collect(); - for tag in &mut tags { - let t = self.register_tag(Tag { - name: tag.to_string(), - usn, - ..Default::default() - })?; + 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; } @@ -226,6 +219,8 @@ impl Collection { Ok((tags, added)) } + /// 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 { @@ -250,11 +245,7 @@ impl Collection { self.storage.clear_tags()?; } for tag in split_tags(tags) { - let t = self.register_tag(Tag { - name: tag.to_string(), - usn, - ..Default::default() - })?; + let t = self.register_tag(Tag::new(tag.to_string(), usn))?; changed |= t.1; } Ok(changed) @@ -263,11 +254,7 @@ impl Collection { 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 - let t = Tag { - name: name.to_owned(), - ..Default::default() - }; - self.register_tag(t)?; + self.register_tag(Tag::new(name.to_string(), self.usn()?))?; } self.storage.set_tag_collapsed(name, collapsed) } From 831942c2e2ec86ff7d8b1e576fbb61eb1d7ef9c5 Mon Sep 17 00:00:00 2001 From: abdo Date: Thu, 14 Jan 2021 06:46:31 +0300 Subject: [PATCH 12/23] Fix unicode tag sorting --- rslib/src/tags.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index 1ae6a69c9..00c52adf3 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -28,7 +28,7 @@ pub struct Tag { impl Ord for Tag { fn cmp(&self, other: &Self) -> Ordering { - self.name.cmp(&other.name) + UniCase::new(&self.name).cmp(&UniCase::new(&other.name)) } } @@ -40,7 +40,7 @@ impl PartialOrd for Tag { impl PartialEq for Tag { fn eq(&self, other: &Self) -> bool { - self.name == other.name + self.cmp(other) == Ordering::Equal } } @@ -121,7 +121,7 @@ fn normalize_tag_name(name: &str) -> String { } fn fill_missing_tags(tags: Vec) -> Vec { - let mut filled_tags: HashMap = HashMap::new(); + let mut filled_tags: HashMap = HashMap::with_capacity(tags.len()); for tag in tags.into_iter() { let name = tag.name.to_owned(); let split: Vec<&str> = (&tag.name).split("::").collect(); From 2ff584c44de729446dc99edf2e6fc94a535a78bb Mon Sep 17 00:00:00 2001 From: abdo Date: Thu, 14 Jan 2021 07:56:43 +0300 Subject: [PATCH 13/23] Pass escaped name to bulk_update in rename_tag --- pylib/anki/tags.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pylib/anki/tags.py b/pylib/anki/tags.py index bb48e47c3..3302b8f9d 100644 --- a/pylib/anki/tags.py +++ b/pylib/anki/tags.py @@ -87,11 +87,11 @@ 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) + quote_escaped = escaped_name.replace('"', '\\"') + nids = self.col.find_notes(f'tag:"{quote_escaped}"') if not nids: return 0 - return self.col.tags.bulk_update(nids, old, new, False) + return self.bulk_update(nids, escaped_name, new, False) # legacy routines From 6f7c68b6610c386802439cc7bebc817bb469d199 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sat, 16 Jan 2021 17:18:09 +1000 Subject: [PATCH 14/23] add a (currently failing) test for duplicate parent names --- rslib/src/tags.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index 00c52adf3..977649f0c 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -458,4 +458,53 @@ mod test { 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::a".into()); + note.tags.push("foo::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![leaf("a", 2), leaf("b", 2)])] + ) + ); + + // differing case should result in only one parent case being added - + // the first one + *(&mut note.tags[1]) = "FOO::b".into(); + col.storage.clear_tags()?; + col.update_note(&mut note)?; + assert_eq!( + col.tag_tree()?, + node( + "", + 0, + vec![node("foo", 1, vec![leaf("a", 2), leaf("b", 2)])] + ) + ); + + Ok(()) + } } From a390a7781566756e551bfc484a820f46ee1b4f7e Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sat, 16 Jan 2021 19:28:06 +1000 Subject: [PATCH 15/23] handle missing parent names with varying case Also convert to \x1f before sorting, so that numbers (with have a lower ascii order than '::') don't mess up the sort. --- rslib/src/tags.rs | 125 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 92 insertions(+), 33 deletions(-) diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index 977649f0c..2a0355f14 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -12,11 +12,7 @@ use crate::{ use regex::{NoExpand, Regex, Replacer}; use std::cmp::Ordering; -use std::{ - borrow::Cow, - collections::{HashMap, HashSet}, - iter::Peekable, -}; +use std::{borrow::Cow, collections::HashSet, iter::Peekable}; use unicase::UniCase; #[derive(Debug, Clone)] @@ -120,30 +116,50 @@ fn normalize_tag_name(name: &str) -> String { out.trim_end_matches("::").into() } -fn fill_missing_tags(tags: Vec) -> Vec { - let mut filled_tags: HashMap = HashMap::with_capacity(tags.len()); - for tag in tags.into_iter() { - let name = tag.name.to_owned(); - let split: Vec<&str> = (&tag.name).split("::").collect(); - for i in 0..split.len() - 1 { - let comp = split[0..i + 1].join("::"); - let t = Tag::new(comp.to_owned(), Usn(-1)); - if filled_tags.get(&comp).is_none() { - filled_tags.insert(comp, t); - } - } - if filled_tags.get(&name).is_none() { - filled_tags.insert(name, tag); - } - } - let mut tags: Vec = filled_tags.values().map(|t| (*t).clone()).collect(); - tags.sort_unstable(); - - tags +fn immediate_parent_name(tag_name: UniCase<&str>) -> Option> { + tag_name.rsplitn(2, '\x1f').nth(1).map(UniCase::new) } -fn tags_to_tree(tags: Vec) -> TagTreeNode { - let tags = fill_missing_tags(tags); +/// 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_self_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.clone()); + add_self_and_missing_parents(all, missing, parent); + } + } + // finally, add self + 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_self_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); @@ -153,7 +169,7 @@ fn tags_to_tree(tags: Vec) -> TagTreeNode { fn add_child_nodes(tags: &mut Peekable>, parent: &mut TagTreeNode) { while let Some(tag) = tags.peek() { - let split_name: Vec<_> = tag.name.split("::").collect(); + 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 @@ -477,8 +493,8 @@ mod test { 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::a".into()); - note.tags.push("foo::b".into()); + 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 @@ -487,21 +503,64 @@ mod test { node( "", 0, - vec![node("foo", 1, vec![leaf("a", 2), leaf("b", 2)])] + 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 - *(&mut note.tags[1]) = "FOO::b".into(); 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![leaf("a", 2), leaf("b", 2)])] + 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)] ) ); From d54acba81fbf80da7d8de8fa34e45d609f56eb4a Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sat, 16 Jan 2021 19:29:19 +1000 Subject: [PATCH 16/23] custom ord/partialeq is not required; fix clippy lint --- rslib/src/tags.rs | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index 2a0355f14..f01fabcbf 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -11,37 +11,16 @@ use crate::{ }; use regex::{NoExpand, Regex, Replacer}; -use std::cmp::Ordering; use std::{borrow::Cow, collections::HashSet, iter::Peekable}; use unicase::UniCase; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct Tag { pub name: String, pub usn: Usn, pub collapsed: bool, } -impl Ord for Tag { - fn cmp(&self, other: &Self) -> Ordering { - UniCase::new(&self.name).cmp(&UniCase::new(&other.name)) - } -} - -impl PartialOrd for Tag { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl PartialEq for Tag { - fn eq(&self, other: &Self) -> bool { - self.cmp(other) == Ordering::Equal - } -} - -impl Eq for Tag {} - impl From for TagProto { fn from(t: Tag) -> Self { TagProto { @@ -132,7 +111,7 @@ fn add_self_and_missing_parents<'a, 'b>( ) { if let Some(parent) = immediate_parent_name(tag_name) { if !all.contains(&parent) { - missing.push(parent.clone()); + missing.push(parent); add_self_and_missing_parents(all, missing, parent); } } From d80a5c56e39e041ea328caee8280c55e77b83973 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sat, 16 Jan 2021 19:46:58 +1000 Subject: [PATCH 17/23] no need for separate all_tags_sorted() tag is the primary key, so sqlite will give it back to us in sorted order already. --- rslib/src/dbcheck.rs | 2 +- rslib/src/storage/tag/mod.rs | 9 +-------- rslib/src/tags.rs | 4 ++-- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/rslib/src/dbcheck.rs b/rslib/src/dbcheck.rs index ac76b0147..c216800bc 100644 --- a/rslib/src/dbcheck.rs +++ b/rslib/src/dbcheck.rs @@ -243,7 +243,7 @@ impl Collection { let stamp = TimestampMillis::now(); // will rebuild tag list below - let old_tags = self.storage.all_tags_sorted()?; + let old_tags = self.storage.all_tags()?; self.storage.clear_tags()?; let total_notes = self.storage.total_notes()?; diff --git a/rslib/src/storage/tag/mod.rs b/rslib/src/storage/tag/mod.rs index fcd4f2c8a..7620f314f 100644 --- a/rslib/src/storage/tag/mod.rs +++ b/rslib/src/storage/tag/mod.rs @@ -16,6 +16,7 @@ fn row_to_tag(row: &Row) -> Result { } impl SqliteStorage { + /// All tags in the collection, in alphabetical order. pub(crate) fn all_tags(&self) -> Result> { self.db .prepare_cached("select tag, usn, collapsed from tags")? @@ -23,14 +24,6 @@ impl SqliteStorage { .collect() } - /// Get all tags in human form, sorted by name - pub(crate) fn all_tags_sorted(&self) -> Result> { - self.db - .prepare_cached("select tag, usn, collapsed from tags order by tag")? - .query_and_then(NO_PARAMS, row_to_tag)? - .collect() - } - /// Get tag by human name pub(crate) fn get_tag(&self, name: &str) -> Result> { self.db diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index f01fabcbf..fd6a068f6 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -179,7 +179,7 @@ fn add_child_nodes(tags: &mut Peekable>, parent: &mut impl Collection { pub fn tag_tree(&mut self) -> Result { - let tags = self.storage.all_tags_sorted()?; + let tags = self.storage.all_tags()?; let tree = tags_to_tree(tags); Ok(tree) @@ -256,7 +256,7 @@ impl Collection { /// Update collapse state of existing tags and register tags in old_tags that are parents of those tags pub(crate) fn update_tags_collapse(&self, old_tags: Vec) -> Result<()> { - let new_tags = self.storage.all_tags_sorted()?; + let new_tags = self.storage.all_tags()?; for old in old_tags.into_iter() { for new in new_tags.iter() { if new.name == old.name { From 9f964916aba664d469b094ad6ba6850af41cc784 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sat, 16 Jan 2021 20:38:16 +1000 Subject: [PATCH 18/23] simplify unused tags and DB check - backend routines should contain minimal logic, and should call into a routine on the collection - instead of copying the giant-string approach the Python code was taking, we use a HashSet to keep track of seen tags as we loop through the notes, which should be more efficient --- rslib/src/backend/mod.rs | 11 +------ rslib/src/dbcheck.rs | 27 +++++++++++++--- rslib/src/storage/note/mod.rs | 22 +++++++++---- rslib/src/storage/tag/mod.rs | 18 ++++++++++- rslib/src/tags.rs | 59 ++++++++++++++++++++--------------- 5 files changed, 90 insertions(+), 47 deletions(-) diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index 0eaf49955..92080e62c 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -44,7 +44,6 @@ use crate::{ LocalServer, NormalSyncProgress, SyncActionRequired, SyncAuth, SyncMeta, SyncOutput, SyncStage, }, - tags::join_tags, template::RenderedNode, text::{escape_anki_wildcards, extract_av_tags, strip_av_tags, AVTag}, timestamp::TimestampSecs, @@ -1361,15 +1360,7 @@ impl BackendService for Backend { } fn clear_unused_tags(&self, _input: pb::Empty) -> BackendResult { - self.with_col(|col| { - col.transact(None, |col| { - let old_tags = col.storage.all_tags()?; - let note_tags = join_tags(&col.storage.get_note_tags()?); - col.register_tags(¬e_tags, col.usn()?, true)?; - col.update_tags_collapse(old_tags)?; - Ok(().into()) - }) - }) + self.with_col(|col| col.transact(None, |col| col.clear_unused_tags().map(Into::into))) } fn clear_tag(&self, tag: pb::String) -> BackendResult { diff --git a/rslib/src/dbcheck.rs b/rslib/src/dbcheck.rs index c216800bc..0ca55361b 100644 --- a/rslib/src/dbcheck.rs +++ b/rslib/src/dbcheck.rs @@ -242,8 +242,7 @@ impl Collection { let usn = self.usn()?; let stamp = TimestampMillis::now(); - // will rebuild tag list below - let old_tags = self.storage.all_tags()?; + let collapsed_tags = self.storage.collapsed_tags()?; self.storage.clear_tags()?; let total_notes = self.storage.total_notes()?; @@ -295,8 +294,9 @@ impl Collection { } } - // restore tags collapse state and re-register old tags that are parents of used ones - self.update_tags_collapse(old_tags)?; + // 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 @@ -636,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/storage/note/mod.rs b/rslib/src/storage/note/mod.rs index b2d582a93..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}, @@ -157,11 +159,19 @@ impl super::SqliteStorage { .map_err(Into::into) } - // get distinct note tags - pub(crate) fn get_note_tags(&self) -> Result> { - self.db - .prepare_cached("select distinct tags from notes")? - .query_and_then(NO_PARAMS, |r| Ok(r.get_raw(0).as_str()?.to_owned()))? - .collect() + 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/mod.rs b/rslib/src/storage/tag/mod.rs index 7620f314f..6cd44e193 100644 --- a/rslib/src/storage/tag/mod.rs +++ b/rslib/src/storage/tag/mod.rs @@ -24,7 +24,23 @@ impl SqliteStorage { .collect() } - /// Get tag by human name + 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 = ?")? diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index fd6a068f6..0120688ce 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -234,16 +234,19 @@ impl Collection { } } - 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 t = self.register_tag(Tag::new(tag.to_string(), usn))?; - changed |= t.1; - } - Ok(changed) + + Ok(()) } pub(crate) fn set_tag_collapsed(&self, name: &str, collapsed: bool) -> Result<()> { @@ -254,22 +257,6 @@ impl Collection { self.storage.set_tag_collapsed(name, collapsed) } - /// Update collapse state of existing tags and register tags in old_tags that are parents of those tags - pub(crate) fn update_tags_collapse(&self, old_tags: Vec) -> Result<()> { - let new_tags = self.storage.all_tags()?; - for old in old_tags.into_iter() { - for new in new_tags.iter() { - if new.name == old.name { - self.storage.set_tag_collapsed(&new.name, old.collapsed)?; - break; - } else if new.name.starts_with(&old.name) { - self.set_tag_collapsed(&old.name, old.collapsed)?; - } - } - } - Ok(()) - } - fn replace_tags_for_notes_inner( &mut self, nids: &[NoteID], @@ -447,7 +434,10 @@ mod test { assert_eq!(¬e.tags, &["foo::bar", "foo::bar::bar", "foo::bar::foo",]); // tag children are also cleared when clearing their parent - col.register_tags("a a::b a::b::c", Usn(-1), true)?; + 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![]); @@ -545,4 +535,21 @@ mod test { 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(()) + } } From 34245e6f72d5effc569f010116960f8735936a73 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sat, 16 Jan 2021 21:14:55 +1000 Subject: [PATCH 19/23] use of 'self' in function name was confusing --- rslib/src/tags.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index 0120688ce..876b96f8e 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -104,7 +104,7 @@ fn immediate_parent_name(tag_name: UniCase<&str>) -> Option> { /// 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_self_and_missing_parents<'a, 'b>( +fn add_tag_and_missing_parents<'a, 'b>( all: &'a mut HashSet>, missing: &'a mut Vec>, tag_name: UniCase<&'b str>, @@ -112,10 +112,10 @@ fn add_self_and_missing_parents<'a, 'b>( if let Some(parent) = immediate_parent_name(tag_name) { if !all.contains(&parent) { missing.push(parent); - add_self_and_missing_parents(all, missing, parent); + add_tag_and_missing_parents(all, missing, parent); } } - // finally, add self + // finally, add provided tag all.insert(tag_name); } @@ -124,7 +124,7 @@ fn add_missing_parents(tags: &mut Vec) { let mut all_names: HashSet> = HashSet::new(); let mut missing = vec![]; for tag in &*tags { - add_self_and_missing_parents(&mut all_names, &mut missing, UniCase::new(&tag.name)) + add_tag_and_missing_parents(&mut all_names, &mut missing, UniCase::new(&tag.name)) } let mut missing: Vec<_> = missing .into_iter() From bf0086d565f2a6eba9566f3025d47c56230d5f0e Mon Sep 17 00:00:00 2001 From: abdo Date: Sat, 16 Jan 2021 18:49:01 +0300 Subject: [PATCH 20/23] Use new backend filters in rename_tag() --- pylib/anki/tags.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pylib/anki/tags.py b/pylib/anki/tags.py index 3302b8f9d..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 @@ -86,11 +87,11 @@ 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) - quote_escaped = escaped_name.replace('"', '\\"') - nids = self.col.find_notes(f'tag:"{quote_escaped}"') + search = self.col.backend.filter_to_search(FilterToSearchIn(tag=old)) + nids = self.col.find_notes(search) if not nids: return 0 + escaped_name = re.sub(r"[*_\\]", r"\\\g<0>", old) return self.bulk_update(nids, escaped_name, new, False) # legacy routines From dbd0334f97b18f13225c49d9ddd08aa515cc991d Mon Sep 17 00:00:00 2001 From: abdo Date: Sat, 16 Jan 2021 18:51:31 +0300 Subject: [PATCH 21/23] Remove unused set_filter() --- qt/aqt/browser.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/qt/aqt/browser.py b/qt/aqt/browser.py index ffc84cf7c..20ee40e71 100644 --- a/qt/aqt/browser.py +++ b/qt/aqt/browser.py @@ -1166,10 +1166,6 @@ QTableView {{ gridline-color: {grid} }} def fillGroups(root, nodes: Sequence[TagTreeNode], head=""): for node in nodes: - def set_filter(): - full_name = head + node.name # pylint: disable=cell-var-from-loop - return lambda: self.setFilter("tag", full_name) - def toggle_expand(): full_name = head + node.name # pylint: disable=cell-var-from-loop return lambda _: self.mw.col.tags.set_collapsed( From 0ac97cf358a9789fa1c675241cf29cc594e280ca Mon Sep 17 00:00:00 2001 From: abdo Date: Mon, 18 Jan 2021 03:52:28 +0300 Subject: [PATCH 22/23] clear_tag() should be case-insensitive --- rslib/src/storage/tag/mod.rs | 2 +- rslib/src/tags.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rslib/src/storage/tag/mod.rs b/rslib/src/storage/tag/mod.rs index 6cd44e193..e0f1b0bd4 100644 --- a/rslib/src/storage/tag/mod.rs +++ b/rslib/src/storage/tag/mod.rs @@ -68,7 +68,7 @@ impl SqliteStorage { pub(crate) fn clear_tag(&self, tag: &str) -> Result<()> { self.db .prepare_cached("delete from tags where tag regexp ?")? - .execute(&[format!("^{}($|::)", regex::escape(tag))])?; + .execute(&[format!("(?i)^{}($|::)", regex::escape(tag))])?; Ok(()) } diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index 876b96f8e..0e70d77bb 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -435,7 +435,7 @@ mod test { // tag children are also cleared when clearing their parent col.storage.clear_tags()?; - for name in vec!["a", "a::b", "a::b::c"] { + 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")?; From 5ac69d6dc6b7cbe835e3aa2cab262b7dc6b00fdc Mon Sep 17 00:00:00 2001 From: abdo Date: Mon, 18 Jan 2021 06:50:29 +0300 Subject: [PATCH 23/23] Fix wrong tag collapse state being used --- qt/aqt/browser.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qt/aqt/browser.py b/qt/aqt/browser.py index 20ee40e71..386fc7aa6 100644 --- a/qt/aqt/browser.py +++ b/qt/aqt/browser.py @@ -1168,8 +1168,8 @@ QTableView {{ gridline-color: {grid} }} def toggle_expand(): full_name = head + node.name # pylint: disable=cell-var-from-loop - return lambda _: self.mw.col.tags.set_collapsed( - full_name, not node.collapsed + return lambda expanded: self.mw.col.tags.set_collapsed( + full_name, not expanded ) item = SidebarItem(