introduce separate routine to remove tags from specific notes

We were (ab)using the bulk update routine to do deletions, but that
code was really intended to be used for finding&replacing, where an
exact match is not a requirement.

pylib/anki/tags.py

@@ -68,9 +68,15 @@ class TagManager:
     # Bulk addition/removal from specific notes
     #############################################################
 
-    def bulk_add(self, nids: Sequence[int], tags: str) -> OpChangesWithCount:
+    def bulk_add(self, note_ids: Sequence[int], tags: str) -> OpChangesWithCount:
         """Add space-separate tags to provided notes, returning changed count."""
-        return self.col._backend.add_note_tags(nids=nids, tags=tags)
+        return self.col._backend.add_note_tags(note_ids=note_ids, tags=tags)
+
+    def bulk_remove(self, note_ids: Sequence[int], tags: str) -> OpChangesWithCount:
+        return self.col._backend.remove_note_tags(note_ids=note_ids, tags=tags)
+
+    # Find&replace
+    #############################################################
 
     def bulk_update(
         self, nids: Sequence[int], tags: str, replacement: str, regex: bool
@@ -81,9 +87,6 @@ class TagManager:
             nids=nids, tags=tags, replacement=replacement, regex=regex
         )
 
-    def bulk_remove(self, nids: Sequence[int], tags: str) -> OpChangesWithCount:
-        return self.bulk_update(nids, tags, "", False)
-
     # Bulk addition/removal based on tag
     #############################################################
 
@@ -167,7 +170,7 @@ class TagManager:
         if add:
             self.bulk_add(ids, tags)
         else:
-            self.bulk_update(ids, tags, "", False)
+            self.bulk_remove(ids, tags)
 
     def bulkRem(self, ids: List[int], tags: str) -> None:
         self.bulkAdd(ids, tags, False)

qt/aqt/browser.py

@@ -1353,7 +1353,14 @@ where id in %s"""
             tags := tags or self._prompt_for_tags(tr(TR.BROWSING_ENTER_TAGS_TO_ADD))
         ):
             return
-        add_tags(mw=self.mw, note_ids=self.selected_notes(), space_separated_tags=tags)
+        add_tags(
+            mw=self.mw,
+            note_ids=self.selected_notes(),
+            space_separated_tags=tags,
+            success=lambda out: tooltip(
+                tr(TR.BROWSING_NOTES_UPDATED, count=out.count), parent=self
+            ),
+        )
 
     @ensure_editor_saved_on_trigger
     def remove_tags_from_selected_notes(self, tags: Optional[str] = None) -> None:
@@ -1363,7 +1370,12 @@ where id in %s"""
         ):
             return
         remove_tags_for_notes(
-            mw=self.mw, note_ids=self.selected_notes(), space_separated_tags=tags
+            mw=self.mw,
+            note_ids=self.selected_notes(),
+            space_separated_tags=tags,
+            success=lambda out: tooltip(
+                tr(TR.BROWSING_NOTES_UPDATED, count=out.count), parent=self
+            ),
         )
 
     def _prompt_for_tags(self, prompt: str) -> Optional[str]:

qt/aqt/note_ops.py

@@ -39,14 +39,28 @@ def remove_notes(
     mw.perform_op(lambda: mw.col.remove_notes(note_ids), success=success)
 
 
-def add_tags(*, mw: AnkiQt, note_ids: Sequence[int], space_separated_tags: str) -> None:
+def add_tags(
-    mw.perform_op(lambda: mw.col.tags.bulk_add(note_ids, space_separated_tags))
+    *,
+    mw: AnkiQt,
+    note_ids: Sequence[int],
+    space_separated_tags: str,
+    success: PerformOpOptionalSuccessCallback = None,
+) -> None:
+    mw.perform_op(
+        lambda: mw.col.tags.bulk_add(note_ids, space_separated_tags), success=success
+    )
 
 
 def remove_tags_for_notes(
-    *, mw: AnkiQt, note_ids: Sequence[int], space_separated_tags: str
+    *,
+    mw: AnkiQt,
+    note_ids: Sequence[int],
+    space_separated_tags: str,
+    success: PerformOpOptionalSuccessCallback = None,
 ) -> None:
-    mw.perform_op(lambda: mw.col.tags.bulk_remove(note_ids, space_separated_tags))
+    mw.perform_op(
+        lambda: mw.col.tags.bulk_remove(note_ids, space_separated_tags), success=success
+    )
 
 
 def clear_unused_tags(*, mw: AnkiQt, parent: QWidget) -> None:

rslib/backend.proto

@@ -152,8 +152,6 @@ service NotesService {
   rpc UpdateNote(UpdateNoteIn) returns (OpChanges);
   rpc GetNote(NoteID) returns (Note);
   rpc RemoveNotes(RemoveNotesIn) returns (OpChanges);
-  rpc AddNoteTags(AddNoteTagsIn) returns (OpChangesWithCount);
-  rpc UpdateNoteTags(UpdateNoteTagsIn) returns (OpChangesWithCount);
   rpc ClozeNumbersInNote(Note) returns (ClozeNumbersInNoteOut);
   rpc AfterNoteUpdates(AfterNoteUpdatesIn) returns (OpChanges);
   rpc FieldNamesForNotes(FieldNamesForNotesIn) returns (FieldNamesForNotesOut);
@@ -224,6 +222,9 @@ service TagsService {
   rpc TagTree(Empty) returns (TagTreeNode);
   rpc DragDropTags(DragDropTagsIn) returns (Empty);
   rpc RenameTags(RenameTagsIn) returns (OpChangesWithCount);
+  rpc AddNoteTags(NoteIDsAndTagsIn) returns (OpChangesWithCount);
+  rpc RemoveNoteTags(NoteIDsAndTagsIn) returns (OpChangesWithCount);
+  rpc UpdateNoteTags(UpdateNoteTagsIn) returns (OpChangesWithCount);
 }
 
 service SearchService {
@@ -1043,8 +1044,8 @@ message AfterNoteUpdatesIn {
   bool generate_cards = 3;
 }
 
-message AddNoteTagsIn {
+message NoteIDsAndTagsIn {
-  repeated int64 nids = 1;
+  repeated int64 note_ids = 1;
   string tags = 2;
 }
 

rslib/src/backend/notes.rs

@@ -87,25 +87,6 @@ impl NotesService for Backend {
         })
     }
 
-    fn add_note_tags(&self, input: pb::AddNoteTagsIn) -> Result<pb::OpChangesWithCount> {
-        self.with_col(|col| {
-            col.add_tags_to_notes(&to_note_ids(input.nids), &input.tags)
-                .map(Into::into)
-        })
-    }
-
-    fn update_note_tags(&self, input: pb::UpdateNoteTagsIn) -> Result<pb::OpChangesWithCount> {
-        self.with_col(|col| {
-            col.replace_tags_for_notes(
-                &to_note_ids(input.nids),
-                &input.tags,
-                &input.replacement,
-                input.regex,
-            )
-            .map(Into::into)
-        })
-    }
-
     fn cloze_numbers_in_note(&self, note: pb::Note) -> Result<pb::ClozeNumbersInNoteOut> {
         let mut set = HashSet::with_capacity(4);
         for field in &note.fields {
@@ -158,6 +139,6 @@ impl NotesService for Backend {
     }
 }
 
-fn to_note_ids(ids: Vec<i64>) -> Vec<NoteID> {
+pub(super) fn to_note_ids(ids: Vec<i64>) -> Vec<NoteID> {
     ids.into_iter().map(NoteID).collect()
 }

rslib/src/backend/tags.rs

@@ -1,7 +1,7 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
-use super::Backend;
+use super::{notes::to_note_ids, Backend};
 use crate::{backend_proto as pb, prelude::*};
 pub(super) use pb::tags_service::Service as TagsService;
 
@@ -55,4 +55,30 @@ impl TagsService for Backend {
         self.with_col(|col| col.rename_tag(&input.current_prefix, &input.new_prefix))
             .map(Into::into)
     }
+
+    fn add_note_tags(&self, input: pb::NoteIDsAndTagsIn) -> Result<pb::OpChangesWithCount> {
+        self.with_col(|col| {
+            col.add_tags_to_notes(&to_note_ids(input.note_ids), &input.tags)
+                .map(Into::into)
+        })
+    }
+
+    fn remove_note_tags(&self, input: pb::NoteIDsAndTagsIn) -> Result<pb::OpChangesWithCount> {
+        self.with_col(|col| {
+            col.remove_tags_from_notes(&to_note_ids(input.note_ids), &input.tags)
+                .map(Into::into)
+        })
+    }
+
+    fn update_note_tags(&self, input: pb::UpdateNoteTagsIn) -> Result<pb::OpChangesWithCount> {
+        self.with_col(|col| {
+            col.replace_tags_for_notes(
+                &to_note_ids(input.nids),
+                &input.tags,
+                &input.replacement,
+                input.regex,
+            )
+            .map(Into::into)
+        })
+    }
 }

rslib/src/storage/card/mod.rs

@@ -445,7 +445,7 @@ impl super::SqliteStorage {
 
     /// Injects the provided card IDs into the search_cids table, for
     /// when ids have arrived outside of a search.
-    /// Clear with clear_searched_cards().
+    /// Clear with clear_searched_cards_table().
     pub(crate) fn set_search_table_to_card_ids(
         &mut self,
         cards: &[CardID],

rslib/src/storage/note/mod.rs

@@ -20,22 +20,6 @@ pub(crate) fn join_fields(fields: &[String]) -> String {
     fields.join("\x1f")
 }
 
-fn row_to_note(row: &Row) -> Result<Note> {
-    Ok(Note::new_from_storage(
-        row.get(0)?,
-        row.get(1)?,
-        row.get(2)?,
-        row.get(3)?,
-        row.get(4)?,
-        split_tags(row.get_raw(5).as_str()?)
-            .map(Into::into)
-            .collect(),
-        split_fields(row.get_raw(6).as_str()?),
-        Some(row.get(7)?),
-        Some(row.get(8).unwrap_or_default()),
-    ))
-}
-
 impl super::SqliteStorage {
     pub fn get_note(&self, nid: NoteID) -> Result<Option<Note>> {
         self.db
@@ -193,20 +177,28 @@ impl super::SqliteStorage {
     pub(crate) fn get_note_tags_by_id(&mut self, note_id: NoteID) -> Result<Option<NoteTags>> {
         self.db
             .prepare_cached(&format!("{} where id = ?", include_str!("get_tags.sql")))?
-            .query_and_then(&[note_id], |row| -> Result<_> {
+            .query_and_then(&[note_id], row_to_note_tags)?
-                {
-                    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_id_list(
+        &mut self,
+        note_ids: &[NoteID],
+    ) -> Result<Vec<NoteTags>> {
+        self.set_search_table_to_note_ids(note_ids)?;
+        let out = self
+            .db
+            .prepare_cached(&format!(
+                "{} where id in (select nid from search_nids)",
+                include_str!("get_tags.sql")
+            ))?
+            .query_and_then(NO_PARAMS, row_to_note_tags)?
+            .collect::<Result<Vec<_>>>()?;
+        self.clear_searched_notes_table()?;
+        Ok(out)
+    }
+
     pub(crate) fn get_note_tags_by_predicate<F>(&mut self, want: F) -> Result<Vec<NoteTags>>
     where
         F: Fn(&str) -> bool,
@@ -217,12 +209,7 @@ impl super::SqliteStorage {
         while let Some(row) = rows.next()? {
             let tags = row.get_raw(3).as_str()?;
             if want(tags) {
-                output.push(NoteTags {
+                output.push(row_to_note_tags(row)?)
-                    id: row.get(0)?,
-                    mtime: row.get(1)?,
-                    usn: row.get(2)?,
-                    tags: tags.to_owned(),
-                })
             }
         }
         Ok(output)
@@ -234,4 +221,56 @@ impl super::SqliteStorage {
             .execute(params![note.mtime, note.usn, note.tags, note.id])?;
         Ok(())
     }
+
+    fn setup_searched_notes_table(&self) -> Result<()> {
+        self.db
+            .execute_batch(include_str!("search_nids_setup.sql"))?;
+        Ok(())
+    }
+
+    fn clear_searched_notes_table(&self) -> Result<()> {
+        self.db
+            .execute("drop table if exists search_nids", NO_PARAMS)?;
+        Ok(())
+    }
+
+    /// Injects the provided card IDs into the search_nids table, for
+    /// when ids have arrived outside of a search.
+    /// Clear with clear_searched_notes_table().
+    fn set_search_table_to_note_ids(&mut self, notes: &[NoteID]) -> Result<()> {
+        self.setup_searched_notes_table()?;
+        let mut stmt = self
+            .db
+            .prepare_cached("insert into search_nids values (?)")?;
+        for nid in notes {
+            stmt.execute(&[nid])?;
+        }
+
+        Ok(())
+    }
+}
+
+fn row_to_note(row: &Row) -> Result<Note> {
+    Ok(Note::new_from_storage(
+        row.get(0)?,
+        row.get(1)?,
+        row.get(2)?,
+        row.get(3)?,
+        row.get(4)?,
+        split_tags(row.get_raw(5).as_str()?)
+            .map(Into::into)
+            .collect(),
+        split_fields(row.get_raw(6).as_str()?),
+        Some(row.get(7)?),
+        Some(row.get(8).unwrap_or_default()),
+    ))
+}
+
+fn row_to_note_tags(row: &Row) -> Result<NoteTags> {
+    Ok(NoteTags {
+        id: row.get(0)?,
+        mtime: row.get(1)?,
+        usn: row.get(2)?,
+        tags: row.get(3)?,
+    })
 }

rslib/src/storage/note/search_nids_setup.sql

@@ -0,0 +1,2 @@
+DROP TABLE IF EXISTS search_nids;
+CREATE TEMPORARY TABLE search_nids (nid integer PRIMARY KEY NOT NULL);

rslib/src/tags/remove.rs

@@ -10,6 +10,17 @@ impl Collection {
         self.transact(Op::RemoveTag, |col| col.remove_tags_inner(tags))
     }
 
+    /// Remove whitespace-separated tags from provided notes.
+    pub fn remove_tags_from_notes(
+        &mut self,
+        nids: &[NoteID],
+        tags: &str,
+    ) -> Result<OpOutput<usize>> {
+        self.transact(Op::RemoveTag, |col| {
+            col.remove_tags_from_notes_inner(nids, tags)
+        })
+    }
+
     /// Remove tags not referenced by notes, returning removed count.
     pub fn clear_unused_tags(&mut self) -> Result<OpOutput<usize>> {
         self.transact(Op::ClearUnusedTags, |col| col.clear_unused_tags_inner())
@@ -43,6 +54,28 @@ impl Collection {
         Ok(match_count)
     }
 
+    fn remove_tags_from_notes_inner(&mut self, nids: &[NoteID], tags: &str) -> Result<usize> {
+        let usn = self.usn()?;
+
+        let mut re = PrefixReplacer::new(tags)?;
+        let mut match_count = 0;
+        let notes = self.storage.get_note_tags_by_id_list(nids)?;
+
+        for mut note in notes {
+            if !re.is_match(&note.tags) {
+                continue;
+            }
+
+            match_count += 1;
+            let original = note.clone();
+            note.tags = re.remove(&note.tags);
+            note.set_modified(usn);
+            self.update_note_tags_undoable(&note, original)?;
+        }
+
+        Ok(match_count)
+    }
+
     fn clear_unused_tags_inner(&mut self) -> Result<usize> {
         let mut count = 0;
         let in_notes = self.storage.all_tags_in_notes()?;