update template when fields renamed

rslib/src/notetype/mod.rs

@@ -97,20 +97,21 @@ impl NoteType {
         }
     }
 
-    fn update_requirements(&mut self) {
+    fn updated_requirements(
+        &self,
+        parsed: &[(Option<ParsedTemplate>, Option<ParsedTemplate>)],
+    ) -> Vec<CardRequirement> {
         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
+        parsed
-            .templates
             .iter()
             .enumerate()
-            .map(|(ord, tmpl)| {
+            .map(|(ord, (qtmpl, _atmpl))| {
-                let conf = &tmpl.config;
+                if let Some(tmpl) = qtmpl {
-                if let Ok(tmpl) = ParsedTemplate::from_text(&conf.q_format) {
                     let mut req = match tmpl.requirements(&field_map) {
                         FieldRequirements::Any(ords) => CardRequirement {
                             card_ord: ord as u32,
@@ -139,8 +140,7 @@ impl NoteType {
                     }
                 }
             })
-            .collect();
+            .collect()
-        self.config.reqs = reqs;
     }
 
     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<String, String> {
+        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<String, String>,
+        parsed: Vec<(Option<ParsedTemplate>, Option<ParsedTemplate>)>,
+    ) -> Vec<(Option<String>, Option<String>)> {
+        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<ParsedTemplate>, Option<ParsedTemplate>)> {
+        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())?;
             }

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();

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> {
+        ParsedTemplate::from_text(&self.config.a_format).ok()
+    }
+
     pub(crate) fn target_deck_id(&self) -> Option<DeckID> {
         if self.config.target_deck_id > 0 {
             Some(DeckID(self.config.target_deck_id))

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<String, String>) -> bool {
+        rename_fields(&mut self.0, fields)
+    }
+}
+
+fn rename_fields(nodes: &mut [ParsedNode], fields: &HashMap<String, String>) -> 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::<Vec<_>>()
+                        .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]