Fix SearchBuilder (#1754)

* Fix missing search grouping

* Fix SearchBuilder::or_join

* Unify search concatenations

rslib/src/notetype/notetypechange.rs

@@ -295,7 +295,7 @@ impl Collection {
             let ords =
                 SearchBuilder::any(map.removed.iter().map(|o| TemplateKind::Ordinal(*o as u16)));
             self.search_cards_into_table(
-                SearchBuilder::from(nids).and_join(&mut ords.group()),
+                SearchBuilder::from(nids).and(ords.group()),
                 SortMode::NoOrder,
             )?;
             for card in self.storage.all_searched_cards()? {
@@ -320,7 +320,7 @@ impl Collection {
                     .map(|o| TemplateKind::Ordinal(*o as u16)),
             );
             self.search_cards_into_table(
-                SearchBuilder::from(nids).and_join(&mut ords.group()),
+                SearchBuilder::from(nids).and(ords.group()),
                 SortMode::NoOrder,
             )?;
             for mut card in self.storage.all_searched_cards()? {

rslib/src/notetype/schemachange.rs

@@ -143,10 +143,9 @@ impl Collection {
 
         // remove any cards where the template was deleted
         if !changes.removed.is_empty() {
-            let mut ords =
+            let ords = SearchBuilder::any(changes.removed.into_iter().map(TemplateKind::Ordinal));
-                SearchBuilder::any(changes.removed.into_iter().map(TemplateKind::Ordinal));
             self.search_cards_into_table(
-                SearchBuilder::from(nt.id).and_join(&mut ords),
+                SearchBuilder::from(nt.id).and(ords.group()),
                 SortMode::NoOrder,
             )?;
             for card in self.storage.all_searched_cards()? {
@@ -157,10 +156,9 @@ impl Collection {
 
         // update ordinals for cards with a repositioned template
         if !changes.moved.is_empty() {
-            let mut ords =
+            let ords = SearchBuilder::any(changes.moved.keys().cloned().map(TemplateKind::Ordinal));
-                SearchBuilder::any(changes.moved.keys().cloned().map(TemplateKind::Ordinal));
             self.search_cards_into_table(
-                SearchBuilder::from(nt.id).and_join(&mut ords),
+                SearchBuilder::from(nt.id).and(ords.group()),
                 SortMode::NoOrder,
             )?;
             for mut card in self.storage.all_searched_cards()? {

rslib/src/scheduler/filtered/custom_study.rs

@@ -237,10 +237,7 @@ fn cram_config(deck_name: String, cram: &Cram) -> Result<FilteredDeck> {
     };
 
     let search = nodes
-        .and_join(&mut tags_to_nodes(
+        .and(tags_to_nodes(&cram.tags_to_include, &cram.tags_to_exclude))
-            &cram.tags_to_include,
-            &cram.tags_to_exclude,
-        ))
         .and(SearchNode::from_deck_name(&deck_name))
         .write();
 
@@ -258,13 +255,13 @@ fn tags_to_nodes(tags_to_include: &[String], tags_to_exclude: &[String]) -> Sear
             .iter()
             .map(|tag| SearchNode::from_tag_name(tag)),
     );
-    let mut exclude_nodes = SearchBuilder::all(
+    let exclude_nodes = SearchBuilder::all(
         tags_to_exclude
             .iter()
             .map(|tag| SearchNode::from_tag_name(tag).negated()),
     );
 
-    include_nodes.group().and_join(&mut exclude_nodes)
+    include_nodes.group().and(exclude_nodes)
 }
 
 #[cfg(test)]

rslib/src/search/builder.rs

@@ -59,19 +59,23 @@ impl SearchBuilder {
         self.0.len()
     }
 
-    pub fn and<N: Into<Node>>(mut self, node: N) -> Self {
+    /// Concatenates the two sets of [Node]s, inserting [Node::And] if appropriate.
-        if !self.is_empty() {
+    /// No implicit grouping is done.
-            self.0.push(Node::And)
+    pub fn and(self, other: impl Into<SearchBuilder>) -> Self {
-        }
+        self.join_other(other.into(), Node::And)
-        self.0.push(node.into());
-        self
     }
 
-    pub fn or<N: Into<Node>>(mut self, node: N) -> Self {
+    /// Concatenates the two sets of [Node]s, inserting [Node::Or] if appropriate.
-        if !self.is_empty() {
+    /// No implicit grouping is done.
-            self.0.push(Node::Or)
+    pub fn or(self, other: impl Into<SearchBuilder>) -> Self {
+        self.join_other(other.into(), Node::Or)
     }
-        self.0.push(node.into());
+
+    fn join_other(mut self, mut other: Self, joiner: Node) -> Self {
+        if !(self.is_empty() || other.is_empty()) {
+            self.0.push(joiner);
+        }
+        self.0.append(&mut other.0);
         self
     }
 
@@ -83,26 +87,6 @@ impl SearchBuilder {
         self
     }
 
-    /// Concatenate [Node]s of `other`, inserting [Node::And] if appropriate.
-    /// No implicit grouping is done.
-    pub fn and_join(mut self, other: &mut Self) -> Self {
-        if !(self.is_empty() || other.is_empty()) {
-            self.0.push(Node::And);
-        }
-        self.0.append(&mut other.0);
-        self
-    }
-
-    /// Concatenate [Node]s of `other`, inserting [Node::Or] if appropriate.
-    /// No implicit grouping is done.
-    pub fn or_join(mut self, other: &mut Self) -> Self {
-        if !(self.is_empty() || other.is_empty()) {
-            self.0.push(Node::And);
-        }
-        self.0.append(&mut other.0);
-        self
-    }
-
     pub fn write(&self) -> String {
         write_nodes(&self.0)
     }