From 242b4ea5050d6d0adf34b84fe2b9de01919f0c5a Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Thu, 11 Feb 2021 12:19:36 +1000 Subject: [PATCH] switch search parser to using owned values I was a bit too enthusiastic with using borrowed values in structs earlier on in the Rust porting. In this case any performance gains are dwarfed by the cost of querying the DB, and using owned values here simplifies the code, and will make it easier to parse a fragment in the From impl. --- rslib/src/backend/mod.rs | 35 ++++++++------------- rslib/src/search/parser.rs | 58 +++++++++++++++++------------------ rslib/src/search/sqlwriter.rs | 8 +++-- rslib/src/search/writer.rs | 6 ++-- rslib/src/text.rs | 4 +-- 5 files changed, 52 insertions(+), 59 deletions(-) diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index 7eae49dc3..700b3a038 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -296,41 +296,32 @@ impl From for DeckConfID { } } -impl From for Node<'_> { +impl From for Node { fn from(msg: pb::SearchTerm) -> Self { use pb::search_term::group::Operator; use pb::search_term::Filter; use pb::search_term::Flag; if let Some(filter) = msg.filter { match filter { - Filter::Tag(s) => Node::Search(SearchNode::Tag( - escape_anki_wildcards(&s).into_owned().into(), - )), - Filter::Deck(s) => Node::Search(SearchNode::Deck( - if s == "*" { - s - } else { - escape_anki_wildcards(&s).into_owned() - } - .into(), - )), - Filter::Note(s) => Node::Search(SearchNode::NoteType( - escape_anki_wildcards(&s).into_owned().into(), - )), + Filter::Tag(s) => Node::Search(SearchNode::Tag(escape_anki_wildcards(&s))), + Filter::Deck(s) => Node::Search(SearchNode::Deck(if s == "*" { + s + } else { + escape_anki_wildcards(&s) + })), + Filter::Note(s) => Node::Search(SearchNode::NoteType(escape_anki_wildcards(&s))), Filter::Template(u) => { Node::Search(SearchNode::CardTemplate(TemplateKind::Ordinal(u as u16))) } - Filter::Nid(nid) => Node::Search(SearchNode::NoteIDs(nid.to_string().into())), - Filter::Nids(nids) => { - Node::Search(SearchNode::NoteIDs(nids.into_id_string().into())) - } + Filter::Nid(nid) => Node::Search(SearchNode::NoteIDs(nid.to_string())), + Filter::Nids(nids) => Node::Search(SearchNode::NoteIDs(nids.into_id_string())), Filter::Dupe(dupe) => Node::Search(SearchNode::Duplicates { note_type_id: dupe.notetype_id.into(), - text: dupe.first_field.into(), + text: dupe.first_field, }), Filter::FieldName(s) => Node::Search(SearchNode::SingleField { - field: escape_anki_wildcards(&s).into_owned().into(), - text: "*".to_string().into(), + field: escape_anki_wildcards(&s), + text: "*".to_string(), is_re: false, }), Filter::Rated(rated) => Node::Search(SearchNode::Rated { diff --git a/rslib/src/search/parser.rs b/rslib/src/search/parser.rs index 33d5b6adc..427251a6d 100644 --- a/rslib/src/search/parser.rs +++ b/rslib/src/search/parser.rs @@ -17,7 +17,6 @@ use nom::{ sequence::{preceded, separated_pair}, }; use regex::{Captures, Regex}; -use std::borrow::Cow; type IResult<'a, O> = std::result::Result<(&'a str, O), nom::Err>>; type ParseResult<'a, O> = std::result::Result>>; @@ -31,52 +30,52 @@ fn parse_error(input: &str) -> nom::Err> { } #[derive(Debug, PartialEq, Clone)] -pub enum Node<'a> { +pub enum Node { And, Or, - Not(Box>), - Group(Vec>), - Search(SearchNode<'a>), + Not(Box), + Group(Vec), + Search(SearchNode), } #[derive(Debug, PartialEq, Clone)] -pub enum SearchNode<'a> { +pub enum SearchNode { // text without a colon - UnqualifiedText(Cow<'a, str>), + UnqualifiedText(String), // foo:bar, where foo doesn't match a term below SingleField { - field: Cow<'a, str>, - text: Cow<'a, str>, + field: String, + text: String, is_re: bool, }, AddedInDays(u32), EditedInDays(u32), - CardTemplate(TemplateKind<'a>), - Deck(Cow<'a, str>), + CardTemplate(TemplateKind), + Deck(String), DeckID(DeckID), NoteTypeID(NoteTypeID), - NoteType(Cow<'a, str>), + NoteType(String), Rated { days: u32, ease: RatingKind, }, - Tag(Cow<'a, str>), + Tag(String), Duplicates { note_type_id: NoteTypeID, - text: Cow<'a, str>, + text: String, }, State(StateKind), Flag(u8), - NoteIDs(Cow<'a, str>), - CardIDs(&'a str), + NoteIDs(String), + CardIDs(String), Property { operator: String, kind: PropertyKind, }, WholeCollection, - Regex(Cow<'a, str>), - NoCombining(Cow<'a, str>), - WordBoundary(Cow<'a, str>), + Regex(String), + NoCombining(String), + WordBoundary(String), } #[derive(Debug, PartialEq, Clone)] @@ -103,9 +102,9 @@ pub enum StateKind { } #[derive(Debug, PartialEq, Clone)] -pub enum TemplateKind<'a> { +pub enum TemplateKind { Ordinal(u16), - Name(Cow<'a, str>), + Name(String), } #[derive(Debug, PartialEq, Clone)] @@ -303,7 +302,7 @@ fn search_node_for_text(s: &str) -> ParseResult { fn search_node_for_text_with_argument<'a>( key: &'a str, val: &'a str, -) -> ParseResult<'a, SearchNode<'a>> { +) -> ParseResult<'a, SearchNode> { Ok(match key.to_ascii_lowercase().as_str() { "deck" => SearchNode::Deck(unescape(val)?), "note" => SearchNode::NoteType(unescape(val)?), @@ -319,7 +318,7 @@ fn search_node_for_text_with_argument<'a>( "did" => parse_did(val)?, "mid" => parse_mid(val)?, "nid" => SearchNode::NoteIDs(check_id_list(val, key)?.into()), - "cid" => SearchNode::CardIDs(check_id_list(val, key)?), + "cid" => SearchNode::CardIDs(check_id_list(val, key)?.into()), "re" => SearchNode::Regex(unescape_quotes(val)), "nc" => SearchNode::NoCombining(unescape(val)?), "w" => SearchNode::WordBoundary(unescape(val)?), @@ -579,7 +578,7 @@ fn parse_dupe(s: &str) -> ParseResult { } } -fn parse_single_field<'a>(key: &'a str, val: &'a str) -> ParseResult<'a, SearchNode<'a>> { +fn parse_single_field<'a>(key: &'a str, val: &'a str) -> ParseResult<'a, SearchNode> { Ok(if let Some(stripped) = val.strip_prefix("re:") { SearchNode::SingleField { field: unescape(key)?, @@ -596,25 +595,25 @@ fn parse_single_field<'a>(key: &'a str, val: &'a str) -> ParseResult<'a, SearchN } /// For strings without unescaped ", convert \" to " -fn unescape_quotes(s: &str) -> Cow { +fn unescape_quotes(s: &str) -> String { if s.contains('"') { - s.replace(r#"\""#, "\"").into() + s.replace(r#"\""#, "\"") } else { s.into() } } /// For non-globs like dupe text without any assumption about the content -fn unescape_quotes_and_backslashes(s: &str) -> Cow { +fn unescape_quotes_and_backslashes(s: &str) -> String { if s.contains('"') || s.contains('\\') { - s.replace(r#"\""#, "\"").replace(r"\\", r"\").into() + s.replace(r#"\""#, "\"").replace(r"\\", r"\") } else { s.into() } } /// Unescape chars with special meaning to the parser. -fn unescape(txt: &str) -> ParseResult> { +fn unescape(txt: &str) -> ParseResult { if let Some(seq) = invalid_escape_sequence(txt) { Err(parse_failure(txt, FailKind::UnknownEscape(seq))) } else { @@ -631,6 +630,7 @@ fn unescape(txt: &str) -> ParseResult> { r"\-" => "-", _ => unreachable!(), }) + .into() } else { txt.into() }) diff --git a/rslib/src/search/sqlwriter.rs b/rslib/src/search/sqlwriter.rs index a3e009d5e..f65f93832 100644 --- a/rslib/src/search/sqlwriter.rs +++ b/rslib/src/search/sqlwriter.rs @@ -134,7 +134,9 @@ impl SqlWriter<'_> { SearchNode::EditedInDays(days) => self.write_edited(*days)?, SearchNode::CardTemplate(template) => match template { TemplateKind::Ordinal(_) => self.write_template(template)?, - TemplateKind::Name(name) => self.write_template(&TemplateKind::Name(norm(name)))?, + TemplateKind::Name(name) => { + self.write_template(&TemplateKind::Name(norm(name).into()))? + } }, SearchNode::Deck(deck) => self.write_deck(&norm(deck))?, SearchNode::NoteTypeID(ntid) => { @@ -532,7 +534,7 @@ impl RequiredTable { } } -impl Node<'_> { +impl Node { fn required_table(&self) -> RequiredTable { match self { Node::And => RequiredTable::CardsOrNotes, @@ -546,7 +548,7 @@ impl Node<'_> { } } -impl SearchNode<'_> { +impl SearchNode { fn required_table(&self) -> RequiredTable { match self { SearchNode::AddedInDays(_) => RequiredTable::Cards, diff --git a/rslib/src/search/writer.rs b/rslib/src/search/writer.rs index ac4490e9e..cede01ec8 100644 --- a/rslib/src/search/writer.rs +++ b/rslib/src/search/writer.rs @@ -67,8 +67,8 @@ pub fn replace_search_term(search: &str, replacement: &str) -> Result { let mut nodes = parse(search)?; let new = parse(replacement)?; if let [Node::Search(search_node)] = &new[..] { - fn update_node_vec<'a>(old_nodes: &mut [Node<'a>], new_node: &SearchNode<'a>) { - fn update_node<'a>(old_node: &mut Node<'a>, new_node: &SearchNode<'a>) { + fn update_node_vec(old_nodes: &mut [Node], new_node: &SearchNode) { + fn update_node(old_node: &mut Node, new_node: &SearchNode) { match old_node { Node::Not(n) => update_node(n, new_node), Node::Group(ns) => update_node_vec(ns, new_node), @@ -89,7 +89,7 @@ pub fn replace_search_term(search: &str, replacement: &str) -> Result { pub fn write_nodes<'a, I>(nodes: I) -> String where - I: IntoIterator>, + I: IntoIterator, { nodes.into_iter().map(|node| write_node(node)).collect() } diff --git a/rslib/src/text.rs b/rslib/src/text.rs index be985cbd0..003bdd967 100644 --- a/rslib/src/text.rs +++ b/rslib/src/text.rs @@ -336,11 +336,11 @@ pub(crate) fn to_text(txt: &str) -> Cow { } /// Escape Anki wildcards and the backslash for escaping them: \*_ -pub(crate) fn escape_anki_wildcards(txt: &str) -> Cow { +pub(crate) fn escape_anki_wildcards(txt: &str) -> String { lazy_static! { static ref RE: Regex = Regex::new(r"[\\*_]").unwrap(); } - RE.replace_all(&txt, r"\$0") + RE.replace_all(&txt, r"\$0").into() } /// Compare text with a possible glob, folding case.