From ab57fa484b8ef13d45f1164c747d3df256a27101 Mon Sep 17 00:00:00 2001 From: RumovZ Date: Tue, 29 Mar 2022 02:53:02 +0200 Subject: [PATCH] Fix SearchBuilder (#1754) * Fix missing search grouping * Fix SearchBuilder::or_join * Unify search concatenations --- rslib/src/notetype/notetypechange.rs | 4 +- rslib/src/notetype/schemachange.rs | 10 ++--- rslib/src/scheduler/filtered/custom_study.rs | 9 ++-- rslib/src/search/builder.rs | 44 +++++++------------- 4 files changed, 23 insertions(+), 44 deletions(-) diff --git a/rslib/src/notetype/notetypechange.rs b/rslib/src/notetype/notetypechange.rs index 000030744..35c796b99 100644 --- a/rslib/src/notetype/notetypechange.rs +++ b/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()? { diff --git a/rslib/src/notetype/schemachange.rs b/rslib/src/notetype/schemachange.rs index 630712833..cb68f69f2 100644 --- a/rslib/src/notetype/schemachange.rs +++ b/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 = - SearchBuilder::any(changes.removed.into_iter().map(TemplateKind::Ordinal)); + let ords = 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 = - SearchBuilder::any(changes.moved.keys().cloned().map(TemplateKind::Ordinal)); + let ords = 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()? { diff --git a/rslib/src/scheduler/filtered/custom_study.rs b/rslib/src/scheduler/filtered/custom_study.rs index c224c44dd..1ab390bdc 100644 --- a/rslib/src/scheduler/filtered/custom_study.rs +++ b/rslib/src/scheduler/filtered/custom_study.rs @@ -237,10 +237,7 @@ fn cram_config(deck_name: String, cram: &Cram) -> Result { }; let search = nodes - .and_join(&mut tags_to_nodes( - &cram.tags_to_include, - &cram.tags_to_exclude, - )) + .and(tags_to_nodes(&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)] diff --git a/rslib/src/search/builder.rs b/rslib/src/search/builder.rs index de4560b3e..8a1f4a6bd 100644 --- a/rslib/src/search/builder.rs +++ b/rslib/src/search/builder.rs @@ -59,19 +59,23 @@ impl SearchBuilder { self.0.len() } - pub fn and>(mut self, node: N) -> Self { - if !self.is_empty() { - self.0.push(Node::And) - } - self.0.push(node.into()); - self + /// Concatenates the two sets of [Node]s, inserting [Node::And] if appropriate. + /// No implicit grouping is done. + pub fn and(self, other: impl Into) -> Self { + self.join_other(other.into(), Node::And) } - pub fn or>(mut self, node: N) -> Self { - if !self.is_empty() { - self.0.push(Node::Or) + /// Concatenates the two sets of [Node]s, inserting [Node::Or] if appropriate. + /// No implicit grouping is done. + pub fn or(self, other: impl Into) -> Self { + self.join_other(other.into(), Node::Or) + } + + fn join_other(mut self, mut other: Self, joiner: Node) -> Self { + if !(self.is_empty() || other.is_empty()) { + self.0.push(joiner); } - self.0.push(node.into()); + 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) }