From ea8e0ef6a239030f03b2ccc77e0ff151ecc39f38 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Fri, 24 Apr 2020 14:46:59 +1000 Subject: [PATCH] update template when fields renamed --- rslib/src/notetype/mod.rs | 118 +++++++++++++++++++++++++---- rslib/src/notetype/schemachange.rs | 17 +++++ rslib/src/notetype/templates.rs | 4 + rslib/src/template.rs | 100 +++++++++++++++++++++++- 4 files changed, 223 insertions(+), 16 deletions(-) diff --git a/rslib/src/notetype/mod.rs b/rslib/src/notetype/mod.rs index a9d73c0ab..6c39ca7b1 100644 --- a/rslib/src/notetype/mod.rs +++ b/rslib/src/notetype/mod.rs @@ -97,20 +97,21 @@ impl NoteType { } } - fn update_requirements(&mut self) { + fn updated_requirements( + &self, + parsed: &[(Option, Option)], + ) -> Vec { let field_map: HashMap<&str, u16> = self .fields .iter() .enumerate() .map(|(idx, field)| (field.name.as_str(), idx as u16)) .collect(); - let reqs: Vec<_> = self - .templates + parsed .iter() .enumerate() - .map(|(ord, tmpl)| { - let conf = &tmpl.config; - if let Ok(tmpl) = ParsedTemplate::from_text(&conf.q_format) { + .map(|(ord, (qtmpl, _atmpl))| { + if let Some(tmpl) = qtmpl { let mut req = match tmpl.requirements(&field_map) { FieldRequirements::Any(ords) => CardRequirement { card_ord: ord as u32, @@ -139,8 +140,7 @@ impl NoteType { } } }) - .collect(); - self.config.reqs = reqs; + .collect() } fn reposition_sort_idx(&mut self) { @@ -182,24 +182,111 @@ impl NoteType { if self.config.target_deck_id == 0 { self.config.target_deck_id = 1; } + self.prepare_for_update(None) + } + + pub(crate) fn prepare_for_update(&mut self, existing: Option<&NoteType>) -> Result<()> { if self.fields.is_empty() { return Err(AnkiError::invalid_input("1 field required")); } if self.templates.is_empty() { return Err(AnkiError::invalid_input("1 template required")); } - self.prepare_for_update() - } - - pub(crate) fn prepare_for_update(&mut self) -> Result<()> { self.normalize_names(); self.ensure_names_unique(); - self.update_requirements(); self.reposition_sort_idx(); + + let parsed_templates = self.parsed_templates(); + let invalid_card_idx = parsed_templates + .iter() + .enumerate() + .find_map(|(idx, (q, a))| { + if q.is_none() || a.is_none() { + Some(idx) + } else { + None + } + }); + if let Some(idx) = invalid_card_idx { + return Err(AnkiError::TemplateError { + info: format!("invalid card {}", idx + 1), + }); + } + let reqs = self.updated_requirements(&parsed_templates); + + // handle renamed fields + if let Some(existing) = existing { + let renamed_fields = self.renamed_fields(existing); + if !renamed_fields.is_empty() { + let updated_templates = + self.updated_templates_for_renamed_fields(renamed_fields, parsed_templates); + for (idx, (q, a)) in updated_templates.into_iter().enumerate() { + if let Some(q) = q { + self.templates[idx].config.q_format = q + } + if let Some(a) = a { + self.templates[idx].config.a_format = a + } + } + } + } + self.config.reqs = reqs; + // fixme: deal with duplicate note type names on update Ok(()) } + fn renamed_fields(&self, current: &NoteType) -> HashMap { + self.fields + .iter() + .filter_map(|field| { + if let Some(existing_ord) = field.ord { + if let Some(existing_field) = current.fields.get(existing_ord as usize) { + if existing_field.name != field.name { + return Some((existing_field.name.clone(), field.name.clone())); + } + } + } + None + }) + .collect() + } + + fn updated_templates_for_renamed_fields( + &self, + renamed_fields: HashMap, + parsed: Vec<(Option, Option)>, + ) -> Vec<(Option, Option)> { + parsed + .into_iter() + .map(|(q, a)| { + let q = q.and_then(|mut q| { + if q.rename_fields(&renamed_fields) { + Some(q.template_to_string()) + } else { + None + } + }); + let a = a.and_then(|mut a| { + if a.rename_fields(&renamed_fields) { + Some(a.template_to_string()) + } else { + None + } + }); + + (q, a) + }) + .collect() + } + + fn parsed_templates(&self) -> Vec<(Option, Option)> { + self.templates + .iter() + .map(|t| (t.parsed_question(), t.parsed_answer())) + .collect() + } + pub fn new_note(&self) -> Note { Note::new(&self) } @@ -233,13 +320,14 @@ impl Collection { /// Saves changes to a note type. This will force a full sync if templates /// or fields have been added/removed/reordered. pub fn update_notetype(&mut self, nt: &mut NoteType, preserve_usn: bool) -> Result<()> { - nt.prepare_for_update()?; + let existing = self.get_notetype(nt.id)?; + nt.prepare_for_update(existing.as_ref().map(AsRef::as_ref))?; if !preserve_usn { nt.mtime_secs = TimestampSecs::now(); nt.usn = self.usn()?; } self.transact(None, |col| { - if let Some(existing_notetype) = col.get_notetype(nt.id)? { + if let Some(existing_notetype) = existing { col.update_notes_for_changed_fields(nt, existing_notetype.fields.len())?; col.update_cards_for_changed_templates(nt, existing_notetype.templates.len())?; } diff --git a/rslib/src/notetype/schemachange.rs b/rslib/src/notetype/schemachange.rs index 0f0105873..43164f438 100644 --- a/rslib/src/notetype/schemachange.rs +++ b/rslib/src/notetype/schemachange.rs @@ -210,6 +210,23 @@ mod test { Ok(()) } + #[test] + fn field_renaming() -> Result<()> { + let mut col = open_test_collection(); + let mut nt = col + .storage + .get_notetype(col.get_current_notetype_id().unwrap())? + .unwrap(); + nt.templates[0].config.q_format += "\n{{#Front}}{{some:Front}}{{/Front}}"; + nt.fields[0].name = "Test".into(); + col.update_notetype(&mut nt, false)?; + assert_eq!( + &nt.templates[0].config.q_format, + "{{Test}}\n{{#Test}}{{some:Test}}{{/Test}}" + ); + Ok(()) + } + #[test] fn cards() -> Result<()> { let mut col = open_test_collection(); diff --git a/rslib/src/notetype/templates.rs b/rslib/src/notetype/templates.rs index d56678621..c2ef18d5d 100644 --- a/rslib/src/notetype/templates.rs +++ b/rslib/src/notetype/templates.rs @@ -23,6 +23,10 @@ impl CardTemplate { ParsedTemplate::from_text(&self.config.q_format).ok() } + pub(crate) fn parsed_answer(&self) -> Option { + ParsedTemplate::from_text(&self.config.a_format).ok() + } + pub(crate) fn target_deck_id(&self) -> Option { if self.config.target_deck_id > 0 { Some(DeckID(self.config.target_deck_id)) diff --git a/rslib/src/template.rs b/rslib/src/template.rs index ed5fe7e91..3cf05cae6 100644 --- a/rslib/src/template.rs +++ b/rslib/src/template.rs @@ -13,6 +13,7 @@ use nom::{ }; use regex::Regex; use std::collections::{HashMap, HashSet}; +use std::fmt::Write; use std::iter; pub type FieldMap<'a> = HashMap<&'a str, u16>; @@ -591,6 +592,96 @@ impl ParsedTemplate { } } +// Renaming fields +//---------------------------------------- + +impl ParsedTemplate { + /// Given a map of old to new field names, update references to the new names. + /// Returns true if any changes made. + pub(crate) fn rename_fields(&mut self, fields: &HashMap) -> bool { + rename_fields(&mut self.0, fields) + } +} + +fn rename_fields(nodes: &mut [ParsedNode], fields: &HashMap) -> bool { + let mut changed = false; + for node in nodes { + match node { + ParsedNode::Text(_) => (), + ParsedNode::Replacement { key, .. } => { + if let Some(new_name) = fields.get(key) { + *key = new_name.clone(); + changed = true; + } + } + ParsedNode::Conditional { key, children } => { + if let Some(new_name) = fields.get(key) { + *key = new_name.clone(); + changed = true; + }; + if rename_fields(children, fields) { + changed = true; + } + } + ParsedNode::NegatedConditional { key, children } => { + if let Some(new_name) = fields.get(key) { + *key = new_name.clone(); + changed = true; + }; + if rename_fields(children, fields) { + changed = true; + } + } + } + } + changed +} + +// Writing back to a string +//---------------------------------------- + +impl ParsedTemplate { + pub(crate) fn template_to_string(&self) -> String { + let mut buf = String::new(); + nodes_to_string(&mut buf, &self.0); + buf + } +} + +fn nodes_to_string(buf: &mut String, nodes: &[ParsedNode]) { + for node in nodes { + match node { + ParsedNode::Text(text) => buf.push_str(text), + ParsedNode::Replacement { key, filters } => { + write!( + buf, + "{{{{{}}}}}", + filters + .iter() + .rev() + .chain(iter::once(key)) + .map(|s| s.to_string()) + .collect::>() + .join(":") + ) + .unwrap(); + } + ParsedNode::Conditional { key, children } => { + write!(buf, "{{{{#{}}}}}", key).unwrap(); + nodes_to_string(buf, &children); + write!(buf, "{{{{/{}}}}}", key).unwrap(); + } + ParsedNode::NegatedConditional { key, children } => { + write!(buf, "{{{{^{}}}}}", key).unwrap(); + nodes_to_string(buf, &children); + write!(buf, "{{{{/{}}}}}", key).unwrap(); + } + } + } +} + +// fixme: unit test filter order, etc + // Tests //--------------------------------------- @@ -615,7 +706,8 @@ mod test { #[test] fn parsing() { - let tmpl = PT::from_text("foo {{bar}} {{#baz}} quux {{/baz}}").unwrap(); + let orig = "foo {{bar}} {{#baz}} quux {{/baz}}"; + let tmpl = PT::from_text(orig).unwrap(); assert_eq!( tmpl.0, vec![ @@ -631,6 +723,7 @@ mod test { } ] ); + assert_eq!(orig, &tmpl.template_to_string()); let tmpl = PT::from_text("{{^baz}}{{/baz}}").unwrap(); assert_eq!( @@ -663,6 +756,11 @@ mod test { PT::from_text("{{").unwrap_err(); PT::from_text(" {{").unwrap_err(); PT::from_text(" {{ ").unwrap_err(); + + // make sure filters and so on are round-tripped correctly + let orig = "foo {{one:two}} {{one:two:three}} {{^baz}} {{/baz}} {{foo:}}"; + let tmpl = PT::from_text(orig).unwrap(); + assert_eq!(orig, &tmpl.template_to_string()); } #[test]