diff --git a/pylib/anki/tags.py b/pylib/anki/tags.py index 7deb379f0..71c9d14df 100644 --- a/pylib/anki/tags.py +++ b/pylib/anki/tags.py @@ -85,12 +85,9 @@ class TagManager: return self.bulk_update(nids, tags, "", False) def rename(self, old: str, new: str) -> OpChangesWithCount: - "Rename provided tag, returning number of changed notes." - nids = self.col.find_notes(anki.collection.SearchNode(tag=old)) - if not nids: - return OpChangesWithCount() - escaped_name = re.sub(r"[*_\\]", r"\\\g<0>", old) - return self.bulk_update(nids, escaped_name, new, False) + "Rename provided tag and its children, returning number of changed notes." + x = self.col._backend.rename_tags(current_prefix=old, new_prefix=new) + return x def remove(self, tag: str) -> None: self.col._backend.clear_tag(tag) diff --git a/qt/aqt/main.py b/qt/aqt/main.py index 79eb3041d..06d18617e 100644 --- a/qt/aqt/main.py +++ b/qt/aqt/main.py @@ -789,7 +789,7 @@ class AnkiQt(QMainWindow): after_hooks() will be called after hooks are fired, if it is provided. Components can use this to ignore change notices generated by operations - they invoke themselves. + they invoke themselves, or perform some subsequent action. """ self._increase_background_ops() diff --git a/qt/aqt/note_ops.py b/qt/aqt/note_ops.py index 091b93d55..b7a1af72a 100644 --- a/qt/aqt/note_ops.py +++ b/qt/aqt/note_ops.py @@ -5,6 +5,7 @@ from __future__ import annotations from typing import Callable, Optional, Sequence +from anki.collection import OpChangesWithCount from anki.lang import TR from anki.notes import Note from aqt import AnkiQt, QWidget @@ -57,6 +58,27 @@ def clear_unused_tags(*, mw: AnkiQt, parent: QWidget) -> None: ) +def rename_tag( + *, + mw: AnkiQt, + parent: QWidget, + current_name: str, + new_name: str, + after_rename: Callable[[], None], +) -> None: + def success(out: OpChangesWithCount) -> None: + if out.count: + tooltip(tr(TR.BROWSING_NOTES_UPDATED, count=out.count), parent=parent) + else: + showInfo(tr(TR.BROWSING_TAG_RENAME_WARNING_EMPTY), parent=parent) + + mw.perform_op( + lambda: mw.col.tags.rename(old=current_name, new=new_name), + success=success, + after_hooks=after_rename, + ) + + def find_and_replace( *, mw: AnkiQt, diff --git a/qt/aqt/sidebar.py b/qt/aqt/sidebar.py index b3b70447c..86f61cb89 100644 --- a/qt/aqt/sidebar.py +++ b/qt/aqt/sidebar.py @@ -19,6 +19,7 @@ from aqt.clayout import CardLayout from aqt.deck_ops import remove_decks from aqt.main import ResetReason from aqt.models import Models +from aqt.note_ops import rename_tag from aqt.qt import * from aqt.theme import ColoredIcon, theme_manager from aqt.utils import ( @@ -27,7 +28,6 @@ from aqt.utils import ( askUser, getOnlyText, show_invalid_search_error, - showInfo, showWarning, tooltip, tr, @@ -1217,40 +1217,26 @@ class SidebarTreeView(QTreeView): self.mw.taskman.with_progress(do_remove, on_done) def rename_tag(self, item: SidebarItem, new_name: str) -> None: - new_name = new_name.replace(" ", "") - if new_name and new_name != item.name: - # block repainting until collection is updated - self.setUpdatesEnabled(False) - self.browser.editor.call_after_note_saved( - lambda: self._rename_tag(item, new_name) - ) + if not new_name or new_name == item.name: + return + + new_name_base = new_name - def _rename_tag(self, item: SidebarItem, new_name: str) -> None: old_name = item.full_name new_name = item.name_prefix + new_name - def do_rename() -> int: - self.mw.col.tags.remove(old_name) - return self.col.tags.rename(old_name, new_name).count + item.name = new_name_base - def on_done(fut: Future) -> None: - self.setUpdatesEnabled(True) - self.mw.requireReset(reason=ResetReason.BrowserAddTags, context=self) - self.browser.model.endReset() - - count = fut.result() - if not count: - showInfo(tr(TR.BROWSING_TAG_RENAME_WARNING_EMPTY)) - else: - tooltip(tr(TR.BROWSING_NOTES_UPDATED, count=count), parent=self) - self.refresh( - lambda item: item.item_type == SidebarItemType.TAG - and item.full_name == new_name - ) - - self.mw.checkpoint(tr(TR.ACTIONS_RENAME_TAG)) - self.browser.model.beginReset() - self.mw.taskman.with_progress(do_rename, on_done) + rename_tag( + mw=self.mw, + parent=self.browser, + current_name=old_name, + new_name=new_name, + after_rename=lambda: self.refresh( + lambda item: item.item_type == SidebarItemType.TAG + and item.full_name == new_name + ), + ) # Saved searches #################################### diff --git a/rslib/backend.proto b/rslib/backend.proto index d8d1475f5..4bde1021b 100644 --- a/rslib/backend.proto +++ b/rslib/backend.proto @@ -224,6 +224,7 @@ service TagsService { rpc ClearTag(String) returns (Empty); rpc TagTree(Empty) returns (TagTreeNode); rpc DragDropTags(DragDropTagsIn) returns (Empty); + rpc RenameTags(RenameTagsIn) returns (OpChangesWithCount); } service SearchService { @@ -930,6 +931,11 @@ message DragDropTagsIn { string target_tag = 2; } +message RenameTagsIn { + string current_prefix = 1; + string new_prefix = 2; +} + message SetConfigJsonIn { string key = 1; bytes value_json = 2; diff --git a/rslib/src/backend/tags.rs b/rslib/src/backend/tags.rs index 4c24d38e3..3ba67f116 100644 --- a/rslib/src/backend/tags.rs +++ b/rslib/src/backend/tags.rs @@ -59,4 +59,9 @@ impl TagsService for Backend { self.with_col(|col| col.drag_drop_tags(&source_tags, target_tag)) .map(Into::into) } + + fn rename_tags(&self, input: pb::RenameTagsIn) -> Result { + self.with_col(|col| col.rename_tag(&input.current_prefix, &input.new_prefix)) + .map(Into::into) + } } diff --git a/rslib/src/notes/mod.rs b/rslib/src/notes/mod.rs index f0a58a1a3..fe415f8c3 100644 --- a/rslib/src/notes/mod.rs +++ b/rslib/src/notes/mod.rs @@ -49,6 +49,23 @@ pub struct Note { pub(crate) checksum: Option, } +/// Information required for updating tags while leaving note content alone. +/// Tags are stored in their DB form, separated by spaces. +#[derive(Debug, PartialEq, Clone)] +pub(crate) struct NoteTags { + pub id: NoteID, + pub mtime: TimestampSecs, + pub usn: Usn, + pub tags: String, +} + +impl NoteTags { + pub(crate) fn set_modified(&mut self, usn: Usn) { + self.mtime = TimestampSecs::now(); + self.usn = usn; + } +} + impl Note { pub(crate) fn new(notetype: &NoteType) -> Self { Note { diff --git a/rslib/src/notes/undo.rs b/rslib/src/notes/undo.rs index a0e176009..c79ebceda 100644 --- a/rslib/src/notes/undo.rs +++ b/rslib/src/notes/undo.rs @@ -3,6 +3,8 @@ use crate::{prelude::*, undo::UndoableChange}; +use super::NoteTags; + #[derive(Debug)] pub(crate) enum UndoableNoteChange { Added(Box), @@ -10,6 +12,7 @@ pub(crate) enum UndoableNoteChange { Removed(Box), GraveAdded(Box<(NoteID, Usn)>), GraveRemoved(Box<(NoteID, Usn)>), + TagsUpdated(Box), } impl Collection { @@ -26,6 +29,13 @@ impl Collection { UndoableNoteChange::Removed(note) => self.restore_deleted_note(*note), UndoableNoteChange::GraveAdded(e) => self.remove_note_grave(e.0, e.1), UndoableNoteChange::GraveRemoved(e) => self.add_note_grave(e.0, e.1), + UndoableNoteChange::TagsUpdated(note_tags) => { + let current = self + .storage + .get_note_tags_by_id(note_tags.id)? + .ok_or_else(|| AnkiError::invalid_input("note disappeared"))?; + self.update_note_tags_undoable(¬e_tags, current) + } } } @@ -81,6 +91,15 @@ impl Collection { Ok(()) } + pub(crate) fn update_note_tags_undoable( + &mut self, + tags: &NoteTags, + original: NoteTags, + ) -> Result<()> { + self.save_undo(UndoableNoteChange::TagsUpdated(Box::new(original))); + self.storage.update_note_tags(tags) + } + fn remove_note_without_grave(&mut self, note: Note) -> Result<()> { self.storage.remove_note(note.id)?; self.save_undo(UndoableNoteChange::Removed(Box::new(note))); diff --git a/rslib/src/ops.rs b/rslib/src/ops.rs index 91f9c785a..d6466c3c8 100644 --- a/rslib/src/ops.rs +++ b/rslib/src/ops.rs @@ -14,6 +14,7 @@ pub enum Op { RemoveDeck, RemoveNote, RenameDeck, + RenameTag, ScheduleAsNew, SetDeck, SetDueDate, @@ -52,6 +53,7 @@ impl Op { Op::FindAndReplace => TR::BrowsingFindAndReplace, Op::ClearUnusedTags => TR::BrowsingClearUnusedTags, Op::SortCards => TR::BrowsingReschedule, + Op::RenameTag => TR::ActionsRenameTag, }; i18n.tr(key).to_string() diff --git a/rslib/src/storage/note/get_tags.sql b/rslib/src/storage/note/get_tags.sql new file mode 100644 index 000000000..5bcd2be0e --- /dev/null +++ b/rslib/src/storage/note/get_tags.sql @@ -0,0 +1,5 @@ +SELECT id, + mod, + usn, + tags +FROM notes \ No newline at end of file diff --git a/rslib/src/storage/note/mod.rs b/rslib/src/storage/note/mod.rs index 9ea67cf0d..40a44194c 100644 --- a/rslib/src/storage/note/mod.rs +++ b/rslib/src/storage/note/mod.rs @@ -5,7 +5,7 @@ use std::collections::HashSet; use crate::{ err::Result, - notes::{Note, NoteID}, + notes::{Note, NoteID, NoteTags}, notetype::NoteTypeID, tags::{join_tags, split_tags}, timestamp::TimestampMillis, @@ -189,4 +189,49 @@ impl super::SqliteStorage { Ok(()) } + + pub(crate) fn get_note_tags_by_id(&mut self, note_id: NoteID) -> Result> { + self.db + .prepare_cached(&format!("{} where id = ?", include_str!("get_tags.sql")))? + .query_and_then(&[note_id], |row| -> Result<_> { + { + Ok(NoteTags { + id: row.get(0)?, + mtime: row.get(1)?, + usn: row.get(2)?, + tags: row.get(3)?, + }) + } + })? + .next() + .transpose() + } + + pub(crate) fn get_note_tags_by_predicate(&mut self, want: F) -> Result> + where + F: Fn(&str) -> bool, + { + let mut query_stmt = self.db.prepare_cached(include_str!("get_tags.sql"))?; + let mut rows = query_stmt.query(NO_PARAMS)?; + let mut output = vec![]; + while let Some(row) = rows.next()? { + let tags = row.get_raw(3).as_str()?; + if want(tags) { + output.push(NoteTags { + id: row.get(0)?, + mtime: row.get(1)?, + usn: row.get(2)?, + tags: tags.to_owned(), + }) + } + } + Ok(output) + } + + pub(crate) fn update_note_tags(&mut self, note: &NoteTags) -> Result<()> { + self.db + .prepare_cached(include_str!("update_tags.sql"))? + .execute(params![note.mtime, note.usn, note.tags, note.id])?; + Ok(()) + } } diff --git a/rslib/src/storage/note/update_tags.sql b/rslib/src/storage/note/update_tags.sql new file mode 100644 index 000000000..9bbfc13c2 --- /dev/null +++ b/rslib/src/storage/note/update_tags.sql @@ -0,0 +1,5 @@ +UPDATE notes +SET mod = ?, + usn = ?, + tags = ? +WHERE id = ? \ No newline at end of file diff --git a/rslib/src/storage/tag/mod.rs b/rslib/src/storage/tag/mod.rs index c2aac651e..ce300c522 100644 --- a/rslib/src/storage/tag/mod.rs +++ b/rslib/src/storage/tag/mod.rs @@ -65,8 +65,6 @@ impl SqliteStorage { .map_err(Into::into) } - // for undo in the future - #[allow(dead_code)] pub(crate) fn get_tag_and_children(&self, name: &str) -> Result> { self.db .prepare_cached("select tag, usn, collapsed from tags where tag regexp ?")? diff --git a/rslib/src/tags/mod.rs b/rslib/src/tags/mod.rs index 553069db0..2a0fa4238 100644 --- a/rslib/src/tags/mod.rs +++ b/rslib/src/tags/mod.rs @@ -1,6 +1,7 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html +mod prefix_replacer; pub(crate) mod undo; use crate::{ @@ -13,6 +14,7 @@ use crate::{ types::Usn, }; +use prefix_replacer::PrefixReplacer; use regex::{NoExpand, Regex, Replacer}; use std::{borrow::Cow, collections::HashSet, iter::Peekable}; use unicase::UniCase; @@ -231,6 +233,21 @@ impl Collection { /// In the case the tag is already registered, tag will be mutated to match the existing /// name. pub(crate) fn register_tag(&mut self, tag: &mut Tag) -> Result { + let is_new = self.prepare_tag_for_registering(tag)?; + if is_new { + self.register_tag_undoable(&tag)?; + } + Ok(is_new) + } + + fn register_tag_string(&mut self, tag: String, usn: Usn) -> Result { + let mut tag = Tag::new(tag, usn); + self.register_tag(&mut tag) + } + + /// Create a tag object, normalize text, and match parents/existing case if available. + /// True if tag is new. + fn prepare_tag_for_registering(&self, tag: &mut Tag) -> Result { let normalized_name = normalize_tag_name(&tag.name); if normalized_name.is_empty() { // this should not be possible @@ -245,7 +262,6 @@ impl Collection { } else if let Cow::Owned(new_name) = normalized_name { tag.name = new_name; } - self.register_tag_undoable(&tag)?; Ok(true) } } @@ -446,18 +462,7 @@ impl Collection { .iter() // convert the names into regexps/replacements .map(|(tag, output)| { - Regex::new(&format!( - r#"(?ix) - ^ - {} - # optional children - (::.+)? - $ - "#, - regex::escape(tag) - )) - .map_err(|_| AnkiError::invalid_input("invalid regex")) - .map(|regex| (regex, output)) + regex_matching_tag_and_children_in_single_tag(tag).map(|regex| (regex, output)) }) .collect::>>()?; @@ -505,6 +510,82 @@ impl Collection { Ok(()) } + + /// Rename a given tag and its children on all notes that reference it, returning changed + /// note count. + pub fn rename_tag(&mut self, old_prefix: &str, new_prefix: &str) -> Result> { + self.transact(Op::RenameTag, |col| { + col.rename_tag_inner(old_prefix, new_prefix) + }) + } + + fn rename_tag_inner(&mut self, old_prefix: &str, new_prefix: &str) -> Result { + if new_prefix.contains(' ') { + return Err(AnkiError::invalid_input( + "replacement name can not contain a space", + )); + } + if new_prefix.trim().is_empty() { + return Err(AnkiError::invalid_input( + "replacement name must not be empty", + )); + } + + let usn = self.usn()?; + + // match existing case if available, and ensure normalized. + let mut tag = Tag::new(new_prefix.to_string(), usn); + self.prepare_tag_for_registering(&mut tag)?; + let new_prefix = &tag.name; + + // gather tags that need replacing + let mut re = PrefixReplacer::new(old_prefix)?; + let matched_notes = self + .storage + .get_note_tags_by_predicate(|tags| re.is_match(tags))?; + let match_count = matched_notes.len(); + if match_count == 0 { + // no matches; exit early so we don't clobber the empty tag entries + return Ok(0); + } + + // remove old prefix from the tag list + for tag in self.storage.get_tag_and_children(old_prefix)? { + self.remove_single_tag_undoable(tag)?; + } + + // replace tags + for mut note in matched_notes { + let original = note.clone(); + note.tags = re.replace(¬e.tags, new_prefix); + note.set_modified(usn); + self.update_note_tags_undoable(¬e, original)?; + } + + // update tag list + for tag in re.into_seen_tags() { + self.register_tag_string(tag, usn)?; + } + + Ok(match_count) + } +} + +// fixme: merge with prefixmatcher + +/// A regex that will match a string tag that has been split from a list. +fn regex_matching_tag_and_children_in_single_tag(tag: &str) -> Result { + Regex::new(&format!( + r#"(?ix) + ^ + {} + # optional children + (::.+)? + $ + "#, + regex::escape(tag) + )) + .map_err(Into::into) } #[cfg(test)] diff --git a/rslib/src/tags/prefix_replacer.rs b/rslib/src/tags/prefix_replacer.rs new file mode 100644 index 000000000..a14ab4a77 --- /dev/null +++ b/rslib/src/tags/prefix_replacer.rs @@ -0,0 +1,119 @@ +// Copyright: Ankitects Pty Ltd and contributors +// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html + +use regex::{Captures, Regex}; +use std::{borrow::Cow, collections::HashSet}; + +use super::{join_tags, split_tags}; +use crate::prelude::*; +pub(crate) struct PrefixReplacer { + regex: Regex, + seen_tags: HashSet, +} + +/// Helper to match any of the provided space-separated tags in a space- +/// separated list of tags, and replace the prefix. +/// +/// Tracks seen tags during replacement, so the tag list can be updated as well. +impl PrefixReplacer { + pub fn new(space_separated_tags: &str) -> Result { + // convert "fo*o bar" into "fo\*o|bar" + let tags: Vec<_> = split_tags(space_separated_tags) + .map(regex::escape) + .collect(); + let tags = tags.join("|"); + + let regex = Regex::new(&format!( + r#"(?ix) + # start of string, or a space + (?:^|\ ) + # 1: the tag prefix + ( + {} + ) + (?: + # 2: an optional child separator + (::) + # or a space/end of string the end of the string + |\ |$ + ) + "#, + tags + ))?; + + Ok(Self { + regex, + seen_tags: HashSet::new(), + }) + } + + pub fn is_match(&self, space_separated_tags: &str) -> bool { + self.regex.is_match(space_separated_tags) + } + + pub fn replace(&mut self, space_separated_tags: &str, replacement: &str) -> String { + let tags: Vec<_> = split_tags(space_separated_tags) + .map(|tag| { + self.regex + .replace(tag, |caps: &Captures| { + // if we captured the child separator, add it to the replacement + if caps.get(2).is_some() { + Cow::Owned(format!("{}::", replacement)) + } else { + Cow::Borrowed(replacement) + } + }) + .to_string() + }) + .collect(); + + for tag in &tags { + // sadly HashSet doesn't have an entry API at the moment + if !self.seen_tags.contains(tag) { + self.seen_tags.insert(tag.clone()); + } + } + + join_tags(tags.as_slice()) + } + + pub fn into_seen_tags(self) -> HashSet { + self.seen_tags + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn regex() -> Result<()> { + let re = PrefixReplacer::new("one two")?; + assert_eq!(re.is_match(" foo "), false); + assert_eq!(re.is_match(" foo one "), true); + assert_eq!(re.is_match(" two foo "), true); + + let mut re = PrefixReplacer::new("foo")?; + assert_eq!(re.is_match("foo"), true); + assert_eq!(re.is_match(" foo "), true); + assert_eq!(re.is_match(" bar foo baz "), true); + assert_eq!(re.is_match(" bar FOO baz "), true); + assert_eq!(re.is_match(" bar foof baz "), false); + assert_eq!(re.is_match(" barfoo "), false); + + let mut as_xxx = |text| re.replace(text, "xxx"); + + assert_eq!(&as_xxx(" baz FOO "), " baz xxx "); + assert_eq!(&as_xxx(" x foo::bar x "), " x xxx::bar x "); + assert_eq!( + &as_xxx(" x foo::bar bar::foo x "), + " x xxx::bar bar::foo x " + ); + assert_eq!( + &as_xxx(" x foo::bar foo::bar::baz x "), + " x xxx::bar xxx::bar::baz x " + ); + + Ok(()) + } +} diff --git a/rslib/src/tags/undo.rs b/rslib/src/tags/undo.rs index 3775dcfb2..49062a881 100644 --- a/rslib/src/tags/undo.rs +++ b/rslib/src/tags/undo.rs @@ -17,13 +17,15 @@ impl Collection { UndoableTagChange::Removed(tag) => self.register_tag_undoable(&tag), } } - /// Adds an already-validated tag to the DB and undo list. + /// Adds an already-validated tag to the tag list, saving an undo entry. /// Caller is responsible for setting usn. pub(super) fn register_tag_undoable(&mut self, tag: &Tag) -> Result<()> { self.save_undo(UndoableTagChange::Added(Box::new(tag.clone()))); self.storage.register_tag(&tag) } + /// Remove a single tag from the tag list, saving an undo entry. Does not alter notes. + /// FIXME: caller will need to update usn when we make tags incrementally syncable. pub(super) fn remove_single_tag_undoable(&mut self, tag: Tag) -> Result<()> { self.storage.remove_single_tag(&tag.name)?; self.save_undo(UndoableTagChange::Removed(Box::new(tag)));