diff --git a/rslib/src/notetype/mod.rs b/rslib/src/notetype/mod.rs index a7f06a857..5a2bb5411 100644 --- a/rslib/src/notetype/mod.rs +++ b/rslib/src/notetype/mod.rs @@ -214,20 +214,11 @@ impl NoteType { } let reqs = self.updated_requirements(&parsed_templates); - // handle renamed fields + // handle renamed+deleted 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 - } - } + let fields = self.renamed_and_removed_fields(existing); + if !fields.is_empty() { + self.update_templates_for_renamed_and_removed_fields(fields, parsed_templates); } } self.config.reqs = reqs; @@ -236,48 +227,51 @@ impl NoteType { Ok(()) } - fn renamed_fields(&self, current: &NoteType) -> HashMap { - self.fields + fn renamed_and_removed_fields(&self, current: &NoteType) -> HashMap> { + let mut remaining_ords = HashSet::new(); + // gather renames + let mut map: HashMap> = self + .fields .iter() .filter_map(|field| { if let Some(existing_ord) = field.ord { + remaining_ords.insert(existing_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())); + return Some((existing_field.name.clone(), Some(field.name.clone()))); } } } None }) - .collect() + .collect(); + // and add any fields that have been removed + for (idx, field) in current.fields.iter().enumerate() { + if !remaining_ords.contains(&(idx as u32)) { + map.insert(field.name.clone(), None); + } + } + + map } - fn updated_templates_for_renamed_fields( - &self, - renamed_fields: HashMap, + /// Update templates to reflect field deletions and renames. + /// Any templates that failed to parse will be ignored. + fn update_templates_for_renamed_and_removed_fields( + &mut self, + 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() + ) { + for (idx, (q, a)) in parsed.into_iter().enumerate() { + if let Some(q) = q { + let updated = q.rename_and_remove_fields(&fields); + self.templates[idx].config.q_format = updated.template_to_string(); + } + if let Some(a) = a { + let updated = a.rename_and_remove_fields(&fields); + self.templates[idx].config.a_format = updated.template_to_string(); + } + } } fn parsed_templates(&self) -> Vec<(Option, Option)> { diff --git a/rslib/src/notetype/schemachange.rs b/rslib/src/notetype/schemachange.rs index e8c843bc6..f7bd3da33 100644 --- a/rslib/src/notetype/schemachange.rs +++ b/rslib/src/notetype/schemachange.rs @@ -222,19 +222,23 @@ mod test { } #[test] - fn field_renaming() -> Result<()> { + fn field_renaming_and_deleting() -> 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.templates[0].config.q_format += "\n{{#Front}}{{some:Front}}{{Back}}{{/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}}" + "{{Test}}\n{{#Test}}{{some:Test}}{{Back}}{{/Test}}" ); + nt.fields.remove(0); + col.update_notetype(&mut nt, false)?; + assert_eq!(&nt.templates[0].config.q_format, "\n{{Back}}"); + Ok(()) } diff --git a/rslib/src/template.rs b/rslib/src/template.rs index 3cf05cae6..f3e3bcfe7 100644 --- a/rslib/src/template.rs +++ b/rslib/src/template.rs @@ -592,49 +592,73 @@ impl ParsedTemplate { } } -// Renaming fields +// Renaming & deleting 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) + pub(crate) fn rename_and_remove_fields( + self, + fields: &HashMap>, + ) -> ParsedTemplate { + let out = rename_and_remove_fields(self.0, fields); + ParsedTemplate(out) } } -fn rename_fields(nodes: &mut [ParsedNode], fields: &HashMap) -> bool { - let mut changed = false; +fn rename_and_remove_fields( + nodes: Vec, + fields: &HashMap>, +) -> Vec { + let mut out = vec![]; 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::Text(text) => out.push(ParsedNode::Text(text)), + ParsedNode::Replacement { key, filters } => { + match fields.get(&key) { + // delete the field + Some(None) => (), + // rename it + Some(Some(new_name)) => out.push(ParsedNode::Replacement { + key: new_name.into(), + filters, + }), + // or leave it alone + None => out.push(ParsedNode::Replacement { key, filters }), } } 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; + let children = rename_and_remove_fields(children, fields); + match fields.get(&key) { + // remove the field, preserving children + Some(None) => out.extend(children), + // rename it + Some(Some(new_name)) => out.push(ParsedNode::Conditional { + key: new_name.into(), + children, + }), + // or leave it alone + None => out.push(ParsedNode::Conditional { key, children }), } } 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; + let children = rename_and_remove_fields(children, fields); + match fields.get(&key) { + // remove the field, preserving children + Some(None) => out.extend(children), + // rename it + Some(Some(new_name)) => out.push(ParsedNode::Conditional { + key: new_name.into(), + children, + }), + // or leave it alone + None => out.push(ParsedNode::Conditional { key, children }), } } } } - changed + out } // Writing back to a string