undo support for bulk tag add/remove

ftl/core/undo.ftl

@@ -6,6 +6,10 @@ undo-undo-action = Undo { $val }
 undo-action-undone = { $action } undone
 undo-redo-action = Redo { $action }
 undo-action-redone = { $action } redone
+
+## Action that can be undone
+
 undo-answer-card = Answer Card
 undo-unbury-unsuspend = Unbury/Unsuspend
 undo-add-note = Add Note
+undo-update-tag = Update Tag

pylib/anki/tags.py

@@ -88,6 +88,9 @@ class TagManager:
             nids=nids, tags=tags, replacement=replacement, regex=regex
         )
 
+    def bulk_remove(self, nids: Sequence[int], tags: str) -> int:
+        return self.bulk_update(nids, tags, "", False)
+
     def rename(self, old: str, new: str) -> int:
         "Rename provided tag, returning number of changed notes."
         nids = self.col.find_notes(anki.collection.SearchNode(tag=old))

pylib/tests/test_collection.py

@@ -104,13 +104,13 @@ def test_addDelTags():
     note2["Front"] = "2"
     col.addNote(note2)
     # adding for a given id
-    col.tags.bulkAdd([note.id], "foo")
+    col.tags.bulk_add([note.id], "foo")
     note.load()
     note2.load()
     assert "foo" in note.tags
     assert "foo" not in note2.tags
     # should be canonified
-    col.tags.bulkAdd([note.id], "foo aaa")
+    col.tags.bulk_add([note.id], "foo aaa")
     note.load()
     assert note.tags[0] == "aaa"
     assert len(note.tags) == 2

pylib/tests/test_find.py

@@ -61,7 +61,7 @@ def test_findCards():
     assert len(col.findCards("tag:monkey")) == 1
     assert len(col.findCards("tag:sheep -tag:monkey")) == 1
     assert len(col.findCards("-tag:sheep")) == 4
-    col.tags.bulkAdd(col.db.list("select id from notes"), "foo bar")
+    col.tags.bulk_add(col.db.list("select id from notes"), "foo bar")
     assert len(col.findCards("tag:foo")) == len(col.findCards("tag:bar")) == 5
     col.tags.bulkRem(col.db.list("select id from notes"), "foo")
     assert len(col.findCards("tag:foo")) == 0

qt/aqt/browser.py

@@ -490,8 +490,11 @@ class Browser(QMainWindow):
         f.actionCreateFilteredDeck.setShortcuts(["Ctrl+G", "Ctrl+Alt+G"])
         # notes
         qconnect(f.actionAdd.triggered, self.mw.onAddCard)
-        qconnect(f.actionAdd_Tags.triggered, lambda: self.addTags())
+        qconnect(f.actionAdd_Tags.triggered, lambda: self.add_tags_to_selected_notes())
-        qconnect(f.actionRemove_Tags.triggered, lambda: self.deleteTags())
+        qconnect(
+            f.actionRemove_Tags.triggered,
+            lambda: self.remove_tags_from_selected_notes(),
+        )
         qconnect(f.actionClear_Unused_Tags.triggered, self.clearUnusedTags)
         qconnect(f.actionToggle_Mark.triggered, lambda: self.onMark())
         qconnect(f.actionChangeModel.triggered, self.onChangeModel)
@@ -1193,53 +1196,46 @@ where id in %s"""
     # Tags
     ######################################################################
 
-    def addTags(
+    def add_tags_to_selected_notes(
         self,
         tags: Optional[str] = None,
-        label: Optional[str] = None,
-        prompt: Optional[str] = None,
-        func: Optional[Callable] = None,
     ) -> None:
-        self.editor.saveNow(lambda: self._addTags(tags, label, prompt, func))
+        "Shows prompt if tags not provided."
+        self.editor.saveNow(
+            lambda: self._update_tags_of_selected_notes(
+                func=self.col.tags.bulk_add,
+                tags=tags,
+                prompt=tr(TR.BROWSING_ENTER_TAGS_TO_ADD),
+            )
+        )
 
-    def _addTags(
+    def remove_tags_from_selected_notes(self, tags: Optional[str] = None) -> None:
+        "Shows prompt if tags not provided."
+        self.editor.saveNow(
+            lambda: self._update_tags_of_selected_notes(
+                func=self.col.tags.bulk_remove,
+                tags=tags,
+                prompt=tr(TR.BROWSING_ENTER_TAGS_TO_DELETE),
+            )
+        )
+
+    def _update_tags_of_selected_notes(
         self,
+        func: Callable[[List[int], str], int],
         tags: Optional[str],
-        label: Optional[str],
         prompt: Optional[str],
-        func: Optional[Callable],
     ) -> None:
-        if prompt is None:
+        "If tags provided, prompt skipped. If tags not provided, prompt must be."
-            prompt = tr(TR.BROWSING_ENTER_TAGS_TO_ADD)
         if tags is None:
-            (tags, r) = getTag(self, self.col, prompt)
+            (tags, ok) = getTag(self, self.col, prompt)
-        else:
+            if not ok:
-            r = True
-        if not r:
                 return
-        if func is None:
+
-            func = self.col.tags.bulkAdd
-        if label is None:
-            label = tr(TR.BROWSING_ADD_TAGS)
-        if label:
-            self.mw.checkpoint(label)
         self.model.beginReset()
         func(self.selectedNotes(), tags)
         self.model.endReset()
         self.mw.requireReset(reason=ResetReason.BrowserAddTags, context=self)
 
-    def deleteTags(
-        self, tags: Optional[str] = None, label: Optional[str] = None
-    ) -> None:
-        if label is None:
-            label = tr(TR.BROWSING_DELETE_TAGS)
-        self.addTags(
-            tags,
-            label,
-            tr(TR.BROWSING_ENTER_TAGS_TO_DELETE),
-            func=self.col.tags.bulkRem,
-        )
-
     def clearUnusedTags(self) -> None:
         self.editor.saveNow(self._clearUnusedTags)
 
@@ -1250,6 +1246,9 @@ where id in %s"""
 
         self.mw.taskman.run_in_background(self.col.tags.registerNotes, on_done)
 
+    addTags = add_tags_to_selected_notes
+    deleteTags = remove_tags_from_selected_notes
+
     # Suspending
     ######################################################################
 
@@ -1313,9 +1312,9 @@ where id in %s"""
         if mark is None:
             mark = not self.isMarked()
         if mark:
-            self.addTags(tags="marked")
+            self.add_tags_to_selected_notes(tags="marked")
         else:
-            self.deleteTags(tags="marked")
+            self.remove_tags_from_selected_notes(tags="marked")
 
     def isMarked(self) -> bool:
         return bool(self.card and self.card.note().hasTag("Marked"))
@@ -1643,7 +1642,7 @@ where id in %s"""
         nids = set()
         for _, nidlist in res:
             nids.update(nidlist)
-        self.col.tags.bulkAdd(list(nids), tr(TR.BROWSING_DUPLICATE))
+        self.col.tags.bulk_add(list(nids), tr(TR.BROWSING_DUPLICATE))
         self.mw.progress.finish()
         self.model.endReset()
         self.mw.requireReset(reason=ResetReason.BrowserTagDupes, context=self)

rslib/src/backend/mod.rs

@@ -1087,7 +1087,7 @@ impl BackendService for Backend {
 
     fn add_note_tags(&self, input: pb::AddNoteTagsIn) -> BackendResult<pb::UInt32> {
         self.with_col(|col| {
-            col.add_tags_for_notes(&to_nids(input.nids), &input.tags)
+            col.add_tags_to_notes(&to_nids(input.nids), &input.tags)
                 .map(|n| n as u32)
         })
         .map(Into::into)

rslib/src/tags.rs

@@ -6,6 +6,7 @@ use crate::{
     collection::Collection,
     err::{AnkiError, Result},
     notes::{NoteID, TransformNoteOutput},
+    prelude::*,
     text::{normalize_to_nfc, to_re},
     types::Usn,
     undo::Undo,
@@ -320,7 +321,7 @@ impl Collection {
         tags: &[Regex],
         mut repl: R,
     ) -> Result<usize> {
-        self.transact(None, |col| {
+        self.transact(Some(UndoableOp::UpdateTag), |col| {
             col.transform_notes(nids, |note, _nt| {
                 let mut changed = false;
                 for re in tags {
@@ -362,7 +363,7 @@ impl Collection {
         }
     }
 
-    pub fn add_tags_for_notes(&mut self, nids: &[NoteID], tags: &str) -> Result<usize> {
+    pub fn add_tags_to_notes(&mut self, nids: &[NoteID], tags: &str) -> Result<usize> {
         let tags: Vec<_> = split_tags(tags).collect();
         let matcher = regex::RegexSet::new(
             tags.iter()
@@ -371,7 +372,7 @@ impl Collection {
         )
         .map_err(|_| AnkiError::invalid_input("invalid regex"))?;
 
-        self.transact(None, |col| {
+        self.transact(Some(UndoableOp::UpdateTag), |col| {
             col.transform_notes(nids, |note, _nt| {
                 let mut need_to_add = true;
                 let mut match_count = 0;
@@ -575,13 +576,13 @@ mod test {
         let note = col.storage.get_note(note.id)?.unwrap();
         assert_eq!(note.tags[0], "baz");
 
-        let cnt = col.add_tags_for_notes(&[note.id], "cee aye")?;
+        let cnt = col.add_tags_to_notes(&[note.id], "cee aye")?;
         assert_eq!(cnt, 1);
         let note = col.storage.get_note(note.id)?.unwrap();
         assert_eq!(&note.tags, &["aye", "baz", "cee"]);
 
         // if all tags already on note, it doesn't get updated
-        let cnt = col.add_tags_for_notes(&[note.id], "cee aye")?;
+        let cnt = col.add_tags_to_notes(&[note.id], "cee aye")?;
         assert_eq!(cnt, 0);
 
         // empty replacement deletes tag

rslib/src/undo.rs

@@ -15,6 +15,7 @@ pub enum UndoableOp {
     UnburyUnsuspend,
     AddNote,
     RemoveNote,
+    UpdateTag,
 }
 
 impl UndoableOp {
@@ -33,6 +34,7 @@ impl Collection {
             UndoableOp::UnburyUnsuspend => TR::UndoUnburyUnsuspend,
             UndoableOp::AddNote => TR::UndoAddNote,
             UndoableOp::RemoveNote => TR::StudyingDeleteNote,
+            UndoableOp::UpdateTag => TR::UndoUpdateTag,
         };
 
         self.i18n.tr(key).to_string()