From dc56a2ca7d9815aaa4c954c929c7527621d8a92b Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Tue, 27 Jun 2023 00:37:41 +1000 Subject: [PATCH] Rework RenderCardOutput::question/answer Instead of flattening the output (which was missing FrontSide), alter the behaviour of render() instead. The non-partial output is now exposed via Protobuf, so the non-Python clients can take advantage of it. --- proto/anki/card_rendering.proto | 12 ++ pylib/anki/template.py | 3 +- rslib/src/browser_table.rs | 2 +- rslib/src/card_rendering/service.rs | 6 +- rslib/src/cloze.rs | 4 +- rslib/src/import_export/text/csv/export.rs | 3 +- rslib/src/notetype/render.rs | 70 +++++-- rslib/src/template.rs | 204 ++++++++++++--------- rslib/src/template_filters.rs | 26 ++- 9 files changed, 213 insertions(+), 117 deletions(-) diff --git a/proto/anki/card_rendering.proto b/proto/anki/card_rendering.proto index a0f13af6b..2c544dd9b 100644 --- a/proto/anki/card_rendering.proto +++ b/proto/anki/card_rendering.proto @@ -93,6 +93,10 @@ message EmptyCardsReport { message RenderExistingCardRequest { int64 card_id = 1; bool browser = 2; + // If true, rendering will stop when an unknown filter is encountered, + // and caller will need to complete rendering. This is done to allow + // Python code to modify the rendering. + bool partial_render = 3; } message RenderUncommittedCardRequest { @@ -100,6 +104,10 @@ message RenderUncommittedCardRequest { uint32 card_ord = 2; notetypes.Notetype.Template template = 3; bool fill_empty = 4; + // If true, rendering will stop when an unknown filter is encountered, + // and caller will need to complete rendering. This is done to allow + // Python code to modify the rendering. + bool partial_render = 5; } message RenderUncommittedCardLegacyRequest { @@ -107,6 +115,10 @@ message RenderUncommittedCardLegacyRequest { uint32 card_ord = 2; bytes template = 3; bool fill_empty = 4; + // If true, rendering will stop when an unknown filter is encountered, + // and caller will need to complete rendering. This is done to allow + // Python code to modify the rendering. + bool partial_render = 5; } message RenderCardResponse { diff --git a/pylib/anki/template.py b/pylib/anki/template.py index 3c0267949..c4a31c2ef 100644 --- a/pylib/anki/template.py +++ b/pylib/anki/template.py @@ -262,6 +262,7 @@ class TemplateRenderContext: card_ord=self._card.ord, template=to_json_bytes(self._template), fill_empty=self._fill_empty, + partial_render=True, ) # when rendering card layout, the css changes have not been # committed; we need the current notetype instance instead @@ -269,7 +270,7 @@ class TemplateRenderContext: else: # existing card (eg study mode) out = self._col._backend.render_existing_card( - card_id=self._card.id, browser=self._browser + card_id=self._card.id, browser=self._browser, partial_render=True ) return PartiallyRenderedCard.from_proto(out) diff --git a/rslib/src/browser_table.rs b/rslib/src/browser_table.rs index a2343eb10..9218996b6 100644 --- a/rslib/src/browser_table.rs +++ b/rslib/src/browser_table.rs @@ -257,7 +257,7 @@ impl RenderContext { fn new(col: &mut Collection, card: &Card, note: &Note, notetype: &Notetype) -> Self { match notetype .get_template(card.template_idx) - .and_then(|template| col.render_card(note, card, notetype, template, true)) + .and_then(|template| col.render_card(note, card, notetype, template, true, true)) { Ok(render) => RenderContext::Ok { question: rendered_nodes_to_str(&render.qnodes), diff --git a/rslib/src/card_rendering/service.rs b/rslib/src/card_rendering/service.rs index fe4bfe7ed..c4bde891c 100644 --- a/rslib/src/card_rendering/service.rs +++ b/rslib/src/card_rendering/service.rs @@ -90,7 +90,7 @@ impl crate::services::CardRenderingService for Collection { &mut self, input: anki_proto::card_rendering::RenderExistingCardRequest, ) -> Result { - self.render_existing_card(CardId(input.card_id), input.browser) + self.render_existing_card(CardId(input.card_id), input.browser, input.partial_render) .map(Into::into) } @@ -103,7 +103,7 @@ impl crate::services::CardRenderingService for Collection { let ord = input.card_ord as u16; let fill_empty = input.fill_empty; - self.render_uncommitted_card(&mut note, &template, ord, fill_empty) + self.render_uncommitted_card(&mut note, &template, ord, fill_empty, input.partial_render) .map(Into::into) } @@ -117,7 +117,7 @@ impl crate::services::CardRenderingService for Collection { let ord = input.card_ord as u16; let fill_empty = input.fill_empty; - self.render_uncommitted_card(&mut note, &template, ord, fill_empty) + self.render_uncommitted_card(&mut note, &template, ord, fill_empty, input.partial_render) .map(Into::into) } diff --git a/rslib/src/cloze.rs b/rslib/src/cloze.rs index 2983c5b4a..c00447487 100644 --- a/rslib/src/cloze.rs +++ b/rslib/src/cloze.rs @@ -410,14 +410,14 @@ fn strip_html_inside_mathjax(text: &str) -> Cow { pub(crate) fn cloze_filter<'a>(text: &'a str, context: &RenderContext) -> Cow<'a, str> { strip_html_inside_mathjax( - reveal_cloze_text(text, context.card_ord + 1, context.question_side).as_ref(), + reveal_cloze_text(text, context.card_ord + 1, context.frontside.is_none()).as_ref(), ) .into_owned() .into() } pub(crate) fn cloze_only_filter<'a>(text: &'a str, context: &RenderContext) -> Cow<'a, str> { - reveal_cloze_text_only(text, context.card_ord + 1, context.question_side) + reveal_cloze_text_only(text, context.card_ord + 1, context.frontside.is_none()) } #[cfg(test)] diff --git a/rslib/src/import_export/text/csv/export.rs b/rslib/src/import_export/text/csv/export.rs index 25f1d1014..ff7714b9c 100644 --- a/rslib/src/import_export/text/csv/export.rs +++ b/rslib/src/import_export/text/csv/export.rs @@ -69,7 +69,8 @@ impl Collection { } fn card_record(&mut self, card: CardId, with_html: bool) -> Result<[String; 2]> { - let RenderCardOutput { qnodes, anodes, .. } = self.render_existing_card(card, false)?; + let RenderCardOutput { qnodes, anodes, .. } = + self.render_existing_card(card, false, false)?; Ok([ rendered_nodes_to_record_field(&qnodes, with_html, false), rendered_nodes_to_record_field(&anodes, with_html, true), diff --git a/rslib/src/notetype/render.rs b/rslib/src/notetype/render.rs index 4e3eb3439..e739ce9d7 100644 --- a/rslib/src/notetype/render.rs +++ b/rslib/src/notetype/render.rs @@ -9,11 +9,12 @@ use super::Notetype; use super::NotetypeKind; use crate::prelude::*; use crate::template::field_is_empty; -use crate::template::flatten_nodes; use crate::template::render_card; use crate::template::ParsedTemplate; +use crate::template::RenderCardRequest; use crate::template::RenderedNode; +#[derive(Debug)] pub struct RenderCardOutput { pub qnodes: Vec, pub anodes: Vec, @@ -22,20 +23,31 @@ pub struct RenderCardOutput { } impl RenderCardOutput { - /// The question text, ignoring any unknown field replacements. + /// The question text. This is only valid to call when partial_render=false. pub fn question(&self) -> Cow { - flatten_nodes(&self.qnodes) + match self.qnodes.as_slice() { + [RenderedNode::Text { text }] => text.into(), + _ => "not fully rendered".into(), + } } - /// The answer text, ignoring any unknown field replacements. + /// The answer text. This is only valid to call when partial_render=false. pub fn answer(&self) -> Cow { - flatten_nodes(&self.anodes) + match self.anodes.as_slice() { + [RenderedNode::Text { text }] => text.into(), + _ => "not fully rendered".into(), + } } } impl Collection { /// Render an existing card saved in the database. - pub fn render_existing_card(&mut self, cid: CardId, browser: bool) -> Result { + pub fn render_existing_card( + &mut self, + cid: CardId, + browser: bool, + partial_render: bool, + ) -> Result { let card = self.storage.get_card(cid)?.or_invalid("no such card")?; let note = self .storage @@ -50,7 +62,7 @@ impl Collection { } .or_invalid("missing template")?; - self.render_card(¬e, &card, &nt, template, browser) + self.render_card(¬e, &card, &nt, template, browser, partial_render) } /// Render a card that may not yet have been added. @@ -62,6 +74,7 @@ impl Collection { template: &CardTemplate, card_ord: u16, fill_empty: bool, + partial_render: bool, ) -> Result { let card = self.existing_or_synthesized_card(note.id, template.ord, card_ord)?; let nt = self @@ -72,7 +85,7 @@ impl Collection { fill_empty_fields(note, &template.config.q_format, &nt, &self.tr); } - self.render_card(note, &card, &nt, template, false) + self.render_card(note, &card, &nt, template, false, partial_render) } fn existing_or_synthesized_card( @@ -102,6 +115,7 @@ impl Collection { nt: &Notetype, template: &CardTemplate, browser: bool, + partial_render: bool, ) -> Result { let mut field_map = note.fields_map(&nt.fields); @@ -122,15 +136,16 @@ impl Collection { ) }; - let (qnodes, anodes) = render_card( + let (qnodes, anodes) = render_card(RenderCardRequest { qfmt, afmt, - &field_map, - card.template_idx, - nt.is_cloze(), + field_map: &field_map, + card_ord: card.template_idx, + is_cloze: nt.is_cloze(), browser, - &self.tr, - )?; + tr: &self.tr, + partial_render, + })?; Ok(RenderCardOutput { qnodes, anodes, @@ -190,3 +205,30 @@ fn fill_empty_fields(note: &mut Note, qfmt: &str, nt: &Notetype, tr: &I18n) { } } } + +#[cfg(test)] +mod test { + use super::*; + use crate::collection::CollectionBuilder; + + #[test] + fn can_render_fully() -> Result<()> { + let mut col = CollectionBuilder::default().build()?; + let nt = col.get_notetype_by_name("Basic")?.unwrap(); + let mut note = Note::new(&nt); + note.set_field(0, "front")?; + note.set_field(1, "back")?; + let out: RenderCardOutput = + col.render_uncommitted_card(&mut note, &nt.templates[0], 0, false, false)?; + assert_eq!(&out.question(), "front"); + assert_eq!(&out.answer(), "front\n\n
\n\nback"); + + // should work even if unknown filters are encountered + let mut tmpl = nt.templates[0].clone(); + tmpl.config.q_format = "{{some_filter:Front}}".into(); + let out = col.render_uncommitted_card(&mut note, &nt.templates[0], 0, false, false)?; + assert_eq!(&out.question(), "front"); + + Ok(()) + } +} diff --git a/rslib/src/template.rs b/rslib/src/template.rs index 3af2418f7..2144678e0 100644 --- a/rslib/src/template.rs +++ b/rslib/src/template.rs @@ -22,6 +22,7 @@ use crate::cloze::add_cloze_numbers_in_string; use crate::error::AnkiError; use crate::error::Result; use crate::error::TemplateError; +use crate::invalid_input; use crate::template_filters::apply_filters; pub type FieldMap<'a> = HashMap<&'a str, u16>; @@ -387,8 +388,14 @@ pub enum RenderedNode { pub(crate) struct RenderContext<'a> { pub fields: &'a HashMap<&'a str, Cow<'a, str>>, pub nonempty_fields: &'a HashSet<&'a str>, - pub question_side: bool, pub card_ord: u16, + /// Should be set before rendering the answer, even if `partial_for_python` + /// is true. + pub frontside: Option<&'a str>, + /// If true, question/answer will not be fully rendered if an unknown filter + /// is encountered, and the frontend code will need to complete the + /// rendering. + pub partial_for_python: bool, } impl ParsedTemplate { @@ -418,62 +425,75 @@ fn render_into( append_str_to_nodes(rendered_nodes, text); } Replacement { key, .. } if key == "FrontSide" => { - // defer FrontSide rendering to Python, as extra - // filters may be required - rendered_nodes.push(RenderedNode::Replacement { - field_name: (*key).to_string(), - filters: vec![], - current_text: "".into(), - }); - } - Replacement { key, filters } if key.is_empty() && !filters.is_empty() => { - // if a filter is provided, we accept an empty field name to - // mean 'pass an empty string to the filter, and it will add - // its own text' - rendered_nodes.push(RenderedNode::Replacement { - field_name: "".to_string(), - current_text: "".to_string(), - filters: filters.clone(), - }) + if let Some(frontside) = &context.frontside { + if context.partial_for_python { + // defer FrontSide rendering to Python, as extra + // filters may be required + rendered_nodes.push(RenderedNode::Replacement { + field_name: (*key).to_string(), + filters: vec![], + current_text: "".into(), + }); + } else { + append_str_to_nodes(rendered_nodes, frontside); + } + } else { + // Not valid on the question side + return Err(TemplateError::FieldNotFound { + field: "FrontSide".into(), + filters: "".into(), + }); + } } Replacement { key, filters } => { - // apply built in filters if field exists - let (text, remaining_filters) = match context.fields.get(key.as_str()) { - Some(text) => apply_filters( - text, - filters - .iter() - .map(|s| s.as_str()) - .collect::>() - .as_slice(), - key, - context, - ), - None => { - // unknown field encountered - let filters_str = filters - .iter() - .rev() - .cloned() - .chain(iter::once("".into())) - .collect::>() - .join(":"); - return Err(TemplateError::FieldNotFound { - field: (*key).to_string(), - filters: filters_str, + if context.partial_for_python && key.is_empty() && !filters.is_empty() { + // if a filter is provided, we accept an empty field name to + // mean 'pass an empty string to the filter, and it will add + // its own text' + rendered_nodes.push(RenderedNode::Replacement { + field_name: "".to_string(), + current_text: "".to_string(), + filters: filters.clone(), + }); + } else { + // apply built in filters if field exists + let (text, remaining_filters) = match context.fields.get(key.as_str()) { + Some(text) => apply_filters( + text, + filters + .iter() + .map(|s| s.as_str()) + .collect::>() + .as_slice(), + key, + context, + ), + None => { + // unknown field encountered + let filters_str = filters + .iter() + .rev() + .cloned() + .chain(iter::once("".into())) + .collect::>() + .join(":"); + return Err(TemplateError::FieldNotFound { + field: (*key).to_string(), + filters: filters_str, + }); + } + }; + + // fully processed? + if remaining_filters.is_empty() { + append_str_to_nodes(rendered_nodes, text.as_ref()) + } else { + rendered_nodes.push(RenderedNode::Replacement { + field_name: (*key).to_string(), + filters: remaining_filters, + current_text: text.into(), }); } - }; - - // fully processed? - if remaining_filters.is_empty() { - append_str_to_nodes(rendered_nodes, text.as_ref()) - } else { - rendered_nodes.push(RenderedNode::Replacement { - field_name: (*key).to_string(), - filters: remaining_filters, - current_text: text.into(), - }); } } Conditional { key, children } => { @@ -529,24 +549,6 @@ fn append_str_to_nodes(nodes: &mut Vec, text: &str) { } } -/// Return the resolved text of a list of nodes, ignoring any unknown -/// filters that were encountered. -pub(crate) fn flatten_nodes(nodes: &[RenderedNode]) -> Cow { - match nodes { - [RenderedNode::Text { text }] => text.into(), - nodes => { - let mut buf = String::new(); - for node in nodes { - match node { - RenderedNode::Text { text } => buf.push_str(text), - RenderedNode::Replacement { current_text, .. } => buf.push_str(current_text), - } - } - buf.into() - } - } -} - /// True if provided text contains only whitespace and/or empty BR/DIV tags. pub(crate) fn field_is_empty(text: &str) -> bool { lazy_static! { @@ -583,22 +585,36 @@ where // Rendering both sides //---------------------------------------- -#[allow(clippy::implicit_hasher)] +pub struct RenderCardRequest<'a> { + pub qfmt: &'a str, + pub afmt: &'a str, + pub field_map: &'a HashMap<&'a str, Cow<'a, str>>, + pub card_ord: u16, + pub is_cloze: bool, + pub browser: bool, + pub tr: &'a I18n, + pub partial_render: bool, +} + pub fn render_card( - qfmt: &str, - afmt: &str, - field_map: &HashMap<&str, Cow>, - card_ord: u16, - is_cloze: bool, - browser: bool, - tr: &I18n, + RenderCardRequest { + qfmt, + afmt, + field_map, + card_ord, + is_cloze, + browser, + tr, + partial_render: partial_for_python, + }: RenderCardRequest<'_>, ) -> Result<(Vec, Vec)> { // prepare context let mut context = RenderContext { fields: field_map, nonempty_fields: &nonempty_fields(field_map), - question_side: true, + frontside: None, card_ord, + partial_for_python, }; // question side @@ -630,7 +646,14 @@ pub fn render_card( } // answer side - context.question_side = false; + context.frontside = if context.partial_for_python { + Some("") + } else { + let Some(RenderedNode::Text {text }) = &qnodes.get(0) else { + invalid_input!("should not happen: first node not text"); + }; + Some(text) + }; let anodes = ParsedTemplate::from_text(afmt) .and_then(|tmpl| tmpl.render(&context, tr)) .map_err(|e| template_error_to_anki_error(e, false, browser, tr))?; @@ -873,6 +896,7 @@ mod test { use crate::template::field_is_empty; use crate::template::nonempty_fields; use crate::template::FieldRequirements; + use crate::template::RenderCardRequest; use crate::template::RenderContext; #[test] @@ -1081,8 +1105,9 @@ mod test { let ctx = RenderContext { fields: &map, nonempty_fields: &nonempty_fields(&map), - question_side: true, + frontside: None, card_ord: 1, + partial_for_python: true, }; use crate::template::RenderedNode as FN; @@ -1198,9 +1223,18 @@ mod test { let tr = I18n::template_only(); use crate::template::RenderedNode as FN; - let qnodes = super::render_card("test{{E}}", "", &map, 1, false, false, &tr) - .unwrap() - .0; + let qnodes = super::render_card(RenderCardRequest { + qfmt: "test{{E}}", + afmt: "", + field_map: &map, + card_ord: 1, + is_cloze: false, + browser: false, + tr: &tr, + partial_render: true, + }) + .unwrap() + .0; assert_eq!( qnodes[0], FN::Text { diff --git a/rslib/src/template_filters.rs b/rslib/src/template_filters.rs index 9f5cc2a5b..e762a8a22 100644 --- a/rslib/src/template_filters.rs +++ b/rslib/src/template_filters.rs @@ -19,8 +19,9 @@ use crate::text::strip_html; /// Applies built in filters, returning the resulting text and remaining /// filters. /// -/// The first non-standard filter that is encountered will terminate processing, -/// so non-standard filters must come at the end. +/// If [context.partial_for_python] is true, the first non-standard filter that +/// is encountered will terminate processing, so non-standard filters must come +/// at the end. If false, missing filters are ignored. pub(crate) fn apply_filters<'a>( text: &'a str, filters: &[&str], @@ -46,11 +47,14 @@ pub(crate) fn apply_filters<'a>( text = output.into(); } (false, _) => { - // unrecognized filter, return current text and remaining filters - return ( - text, - filters.iter().skip(idx).map(ToString::to_string).collect(), - ); + // unrecognized filter + if context.partial_for_python { + // return current text and remaining filters + return ( + text, + filters.iter().skip(idx).map(ToString::to_string).collect(), + ); + } } } } @@ -236,8 +240,9 @@ field let ctx = RenderContext { fields: &Default::default(), nonempty_fields: &Default::default(), - question_side: false, + frontside: Some(""), card_ord: 0, + partial_for_python: true, }; assert_eq!( apply_filters("ignored", &["cloze", "type"], "Text", &ctx), @@ -251,8 +256,9 @@ field let mut ctx = RenderContext { fields: &Default::default(), nonempty_fields: &Default::default(), - question_side: true, + frontside: None, card_ord: 0, + partial_for_python: true, }; assert_eq!(strip_html(&cloze_filter(text, &ctx)).as_ref(), "[...] two"); assert_eq!( @@ -263,7 +269,7 @@ field ctx.card_ord = 1; assert_eq!(strip_html(&cloze_filter(text, &ctx)).as_ref(), "one [hint]"); - ctx.question_side = false; + ctx.frontside = Some(""); assert_eq!(strip_html(&cloze_filter(text, &ctx)).as_ref(), "one two"); // if the provided ordinal did not match any cloze deletions,