From 57ea6a899b0a17366dfb0770f8e769068d1e7577 Mon Sep 17 00:00:00 2001 From: Travis Cross Date: Thu, 17 Apr 2025 17:18:44 +0000 Subject: [PATCH] Refactor rendering with `RenderCtx` We thread certain contextual information through to most of our rendering functions. Currently this information includes whether the rendering is for the summary page and the two link maps. Adding new information to thread through, or changing something that was only using a subset to use more of the context, requires touching a lot of places. Let's improve this by refactoring these details into a `RenderCtx` that we'll thread through everywhere instead. In this way, new details can simply be added to this struct, and callees can use whatever details are needed. --- mdbook-spec/src/grammar.rs | 35 +++++----- mdbook-spec/src/grammar/render_markdown.rs | 70 ++++++++------------ mdbook-spec/src/grammar/render_railroad.rs | 76 +++++++++------------- 3 files changed, 71 insertions(+), 110 deletions(-) diff --git a/mdbook-spec/src/grammar.rs b/mdbook-spec/src/grammar.rs index c6a850c6e..cc5e8a24d 100644 --- a/mdbook-spec/src/grammar.rs +++ b/mdbook-spec/src/grammar.rs @@ -88,6 +88,13 @@ enum Characters { Range(char, char), } +#[derive(Debug)] +pub struct RenderCtx { + md_link_map: HashMap, + rr_link_map: HashMap, + for_summary: bool, +} + impl Grammar { fn visit_nt(&self, callback: &mut dyn FnMut(&str)) { for p in self.productions.values() { @@ -301,7 +308,7 @@ fn render_names( output.push_str("
\n"); // Convert the link map to add the id. - let updated_link_map = |get_id: fn(&str, bool) -> String| -> HashMap { + let update_link_map = |get_id: fn(&str, bool) -> String| -> HashMap { link_map .iter() .map(|(name, path)| { @@ -316,19 +323,13 @@ fn render_names( .collect() }; - let markdown_link_map = updated_link_map(render_markdown::markdown_id); - // Modify the link map so that it contains the exact destination needed to - // link to the railroad productions, and to accommodate the summary - // chapter. - let railroad_link_map = updated_link_map(render_railroad::railroad_id); - - if let Err(e) = grammar.render_markdown( - &names, - &markdown_link_map, - &railroad_link_map, - &mut output, + let render_ctx = RenderCtx { + md_link_map: update_link_map(render_markdown::markdown_id), + rr_link_map: update_link_map(render_railroad::railroad_id), for_summary, - ) { + }; + + if let Err(e) = grammar.render_markdown(&render_ctx, &names, &mut output) { warn_or_err!( diag, "grammar failed in chapter {:?}: {e}", @@ -348,13 +349,7 @@ fn render_names( \n", ); - if let Err(e) = grammar.render_railroad( - &names, - &railroad_link_map, - &markdown_link_map, - &mut output, - for_summary, - ) { + if let Err(e) = grammar.render_railroad(&render_ctx, &names, &mut output) { warn_or_err!( diag, "grammar failed in chapter {:?}: {e}", diff --git a/mdbook-spec/src/grammar/render_markdown.rs b/mdbook-spec/src/grammar/render_markdown.rs index 1cf0cde82..625eaea51 100644 --- a/mdbook-spec/src/grammar/render_markdown.rs +++ b/mdbook-spec/src/grammar/render_markdown.rs @@ -1,29 +1,26 @@ //! Renders the grammar to markdown. -use super::{Characters, Expression, ExpressionKind, Production}; +use super::{Characters, Expression, ExpressionKind, Production, RenderCtx}; use crate::grammar::Grammar; use anyhow::bail; use regex::Regex; use std::borrow::Cow; -use std::collections::HashMap; use std::fmt::Write; use std::sync::LazyLock; impl Grammar { pub fn render_markdown( &self, + cx: &RenderCtx, names: &[&str], - link_map: &HashMap, - rr_link_map: &HashMap, output: &mut String, - for_summary: bool, ) -> anyhow::Result<()> { let mut iter = names.into_iter().peekable(); while let Some(name) = iter.next() { let Some(prod) = self.productions.get(*name) else { bail!("could not find grammar production named `{name}`"); }; - prod.render_markdown(link_map, rr_link_map, output, for_summary); + prod.render_markdown(cx, output); if iter.peek().is_some() { output.push_str("\n"); } @@ -42,14 +39,9 @@ pub fn markdown_id(name: &str, for_summary: bool) -> String { } impl Production { - fn render_markdown( - &self, - link_map: &HashMap, - rr_link_map: &HashMap, - output: &mut String, - for_summary: bool, - ) { - let dest = rr_link_map + fn render_markdown(&self, cx: &RenderCtx, output: &mut String) { + let dest = cx + .rr_link_map .get(&self.name) .map(|path| path.to_string()) .unwrap_or_else(|| format!("missing")); @@ -60,12 +52,11 @@ impl Production { >\ [{name}]({dest})\ → ", - id = markdown_id(&self.name, for_summary), + id = markdown_id(&self.name, cx.for_summary), name = self.name, ) .unwrap(); - self.expression - .render_markdown(link_map, output, for_summary); + self.expression.render_markdown(cx, output); output.push('\n'); } } @@ -92,16 +83,11 @@ impl Expression { } } - fn render_markdown( - &self, - link_map: &HashMap, - output: &mut String, - for_summary: bool, - ) { + fn render_markdown(&self, cx: &RenderCtx, output: &mut String) { match &self.kind { ExpressionKind::Grouped(e) => { output.push_str("( "); - e.render_markdown(link_map, output, for_summary); + e.render_markdown(cx, output); if !matches!(e.last(), ExpressionKind::Break(_)) { output.push(' '); } @@ -110,7 +96,7 @@ impl Expression { ExpressionKind::Alt(es) => { let mut iter = es.iter().peekable(); while let Some(e) = iter.next() { - e.render_markdown(link_map, output, for_summary); + e.render_markdown(cx, output); if iter.peek().is_some() { if !matches!(e.last(), ExpressionKind::Break(_)) { output.push(' '); @@ -122,34 +108,34 @@ impl Expression { ExpressionKind::Sequence(es) => { let mut iter = es.iter().peekable(); while let Some(e) = iter.next() { - e.render_markdown(link_map, output, for_summary); + e.render_markdown(cx, output); if iter.peek().is_some() && !matches!(e.last(), ExpressionKind::Break(_)) { output.push(' '); } } } ExpressionKind::Optional(e) => { - e.render_markdown(link_map, output, for_summary); + e.render_markdown(cx, output); output.push_str("?"); } ExpressionKind::Repeat(e) => { - e.render_markdown(link_map, output, for_summary); + e.render_markdown(cx, output); output.push_str("\\*"); } ExpressionKind::RepeatNonGreedy(e) => { - e.render_markdown(link_map, output, for_summary); + e.render_markdown(cx, output); output.push_str("\\* (non-greedy)"); } ExpressionKind::RepeatPlus(e) => { - e.render_markdown(link_map, output, for_summary); + e.render_markdown(cx, output); output.push_str("+"); } ExpressionKind::RepeatPlusNonGreedy(e) => { - e.render_markdown(link_map, output, for_summary); + e.render_markdown(cx, output); output.push_str("+ (non-greedy)"); } ExpressionKind::RepeatRange(e, a, b) => { - e.render_markdown(link_map, output, for_summary); + e.render_markdown(cx, output); write!( output, "{}..{}", @@ -159,7 +145,7 @@ impl Expression { .unwrap(); } ExpressionKind::Nt(nt) => { - let dest = link_map.get(nt).map_or("missing", |d| d.as_str()); + let dest = cx.md_link_map.get(nt).map_or("missing", |d| d.as_str()); write!(output, "[{nt}]({dest})").unwrap(); } ExpressionKind::Terminal(t) => { @@ -177,10 +163,10 @@ impl Expression { output.push_str("\\\n"); output.push_str(&" ".repeat(*indent)); } - ExpressionKind::Charset(set) => charset_render_markdown(set, link_map, output), + ExpressionKind::Charset(set) => charset_render_markdown(cx, set, output), ExpressionKind::NegExpression(e) => { output.push('~'); - e.render_markdown(link_map, output, for_summary); + e.render_markdown(cx, output); } ExpressionKind::Unicode(s) => { output.push_str("U+"); @@ -190,7 +176,7 @@ impl Expression { if let Some(suffix) = &self.suffix { write!(output, "{suffix}").unwrap(); } - if !for_summary { + if !cx.for_summary { if let Some(footnote) = &self.footnote { // The `ZeroWidthSpace` is to avoid conflicts with markdown link // references. @@ -200,15 +186,11 @@ impl Expression { } } -fn charset_render_markdown( - set: &[Characters], - link_map: &HashMap, - output: &mut String, -) { +fn charset_render_markdown(cx: &RenderCtx, set: &[Characters], output: &mut String) { output.push_str("\\["); let mut iter = set.iter().peekable(); while let Some(chars) = iter.next() { - chars.render_markdown(link_map, output); + chars.render_markdown(cx, output); if iter.peek().is_some() { output.push(' '); } @@ -217,10 +199,10 @@ fn charset_render_markdown( } impl Characters { - fn render_markdown(&self, link_map: &HashMap, output: &mut String) { + fn render_markdown(&self, cx: &RenderCtx, output: &mut String) { match self { Characters::Named(s) => { - let dest = link_map.get(s).map_or("missing", |d| d.as_str()); + let dest = cx.md_link_map.get(s).map_or("missing", |d| d.as_str()); write!(output, "[{s}]({dest})").unwrap(); } Characters::Terminal(s) => write!( diff --git a/mdbook-spec/src/grammar/render_railroad.rs b/mdbook-spec/src/grammar/render_railroad.rs index 5974fc9fd..93d68a589 100644 --- a/mdbook-spec/src/grammar/render_railroad.rs +++ b/mdbook-spec/src/grammar/render_railroad.rs @@ -1,29 +1,26 @@ //! Converts a [`Grammar`] to an SVG railroad diagram. -use super::{Characters, Expression, ExpressionKind, Production}; +use super::{Characters, Expression, ExpressionKind, Production, RenderCtx}; use crate::grammar::Grammar; use anyhow::bail; use railroad::*; use regex::Regex; -use std::collections::HashMap; use std::fmt::Write; use std::sync::LazyLock; impl Grammar { pub fn render_railroad( &self, + cx: &RenderCtx, names: &[&str], - link_map: &HashMap, - md_link_map: &HashMap, output: &mut String, - for_summary: bool, ) -> anyhow::Result<()> { for name in names { let prod = match self.productions.get(*name) { Some(p) => p, None => bail!("could not find grammar production named `{name}`"), }; - prod.render_railroad(link_map, md_link_map, output, for_summary); + prod.render_railroad(cx, output); } Ok(()) } @@ -39,20 +36,14 @@ pub fn railroad_id(name: &str, for_summary: bool) -> String { } impl Production { - fn render_railroad( - &self, - link_map: &HashMap, - md_link_map: &HashMap, - output: &mut String, - for_summary: bool, - ) { - let mut dia = self.make_diagram(false, link_map, md_link_map); + fn render_railroad(&self, cx: &RenderCtx, output: &mut String) { + let mut dia = self.make_diagram(cx, false); // If the diagram is very wide, try stacking it to reduce the width. // This 900 is somewhat arbitrary based on looking at productions that // looked too squished. If your diagram is still too squished, // consider adding more rules to shorten it. if dia.width() > 900 { - dia = self.make_diagram(true, link_map, md_link_map); + dia = self.make_diagram(cx, true); } writeln!( output, @@ -60,19 +51,15 @@ impl Production { class=\"railroad-production\" \ id=\"{id}\">{dia}", width = dia.width(), - id = railroad_id(&self.name, for_summary), + id = railroad_id(&self.name, cx.for_summary), ) .unwrap(); } - fn make_diagram( - &self, - stack: bool, - link_map: &HashMap, - md_link_map: &HashMap, - ) -> Diagram> { - let n = self.expression.render_railroad(stack, link_map); - let dest = md_link_map + fn make_diagram(&self, cx: &RenderCtx, stack: bool) -> Diagram> { + let n = self.expression.render_railroad(cx, stack); + let dest = cx + .md_link_map .get(&self.name) .map(|path| path.to_string()) .unwrap_or_else(|| format!("missing")); @@ -88,11 +75,7 @@ impl Production { } impl Expression { - fn render_railroad( - &self, - stack: bool, - link_map: &HashMap, - ) -> Option> { + fn render_railroad(&self, cx: &RenderCtx, stack: bool) -> Option> { let mut state; let mut state_ref = &self.kind; let n: Box = 'l: loop { @@ -101,12 +84,12 @@ impl Expression { // Render grouped nodes and `e{1..1}` repeats directly. ExpressionKind::Grouped(e) | ExpressionKind::RepeatRange(e, Some(1), Some(1)) => { - e.render_railroad(stack, link_map)? + e.render_railroad(cx, stack)? } ExpressionKind::Alt(es) => { let choices: Vec<_> = es .iter() - .map(|e| e.render_railroad(stack, link_map)) + .map(|e| e.render_railroad(cx, stack)) .filter_map(|n| n) .collect(); Box::new(Choice::>::new(choices)) @@ -116,7 +99,7 @@ impl Expression { let make_seq = |es: &[&Expression]| { let seq: Vec<_> = es .iter() - .map(|e| e.render_railroad(stack, link_map)) + .map(|e| e.render_railroad(cx, stack)) .filter_map(|n| n) .collect(); let seq: Sequence> = Sequence::new(seq); @@ -159,17 +142,17 @@ impl Expression { // Treat `e?` and `e{..1}` / `e{0..1}` equally. ExpressionKind::Optional(e) | ExpressionKind::RepeatRange(e, None | Some(0), Some(1)) => { - let n = e.render_railroad(stack, link_map)?; + let n = e.render_railroad(cx, stack)?; Box::new(Optional::new(n)) } // Treat `e*` and `e{..}` / `e{0..}` equally. ExpressionKind::Repeat(e) | ExpressionKind::RepeatRange(e, None | Some(0), None) => { - let n = e.render_railroad(stack, link_map)?; + let n = e.render_railroad(cx, stack)?; Box::new(Optional::new(Repeat::new(n, railroad::Empty))) } ExpressionKind::RepeatNonGreedy(e) => { - let n = e.render_railroad(stack, link_map)?; + let n = e.render_railroad(cx, stack)?; let r = Box::new(Optional::new(Repeat::new(n, railroad::Empty))); let lbox = LabeledBox::new(r, Comment::new("non-greedy".to_string())); Box::new(lbox) @@ -177,11 +160,11 @@ impl Expression { // Treat `e+` and `e{1..}` equally. ExpressionKind::RepeatPlus(e) | ExpressionKind::RepeatRange(e, Some(1), None) => { - let n = e.render_railroad(stack, link_map)?; + let n = e.render_railroad(cx, stack)?; Box::new(Repeat::new(n, railroad::Empty)) } ExpressionKind::RepeatPlusNonGreedy(e) => { - let n = e.render_railroad(stack, link_map)?; + let n = e.render_railroad(cx, stack)?; let r = Repeat::new(n, railroad::Empty); let lbox = LabeledBox::new(r, Comment::new("non-greedy".to_string())); Box::new(lbox) @@ -197,7 +180,7 @@ impl Expression { } // Render `e{1..b}` directly. ExpressionKind::RepeatRange(e, Some(1), Some(b @ 2..)) => { - let n = e.render_railroad(stack, link_map)?; + let n = e.render_railroad(cx, stack)?; let cmt = format!("at most {b} more times", b = b - 1); let r = Repeat::new(n, Comment::new(cmt)); Box::new(r) @@ -218,17 +201,17 @@ impl Expression { state = ExpressionKind::Sequence(es); break 'cont &state; } - ExpressionKind::Nt(nt) => node_for_nt(link_map, nt), + ExpressionKind::Nt(nt) => node_for_nt(cx, nt), ExpressionKind::Terminal(t) => Box::new(Terminal::new(t.clone())), ExpressionKind::Prose(s) => Box::new(Terminal::new(s.clone())), ExpressionKind::Break(_) => return None, ExpressionKind::Charset(set) => { - let ns: Vec<_> = set.iter().map(|c| c.render_railroad(link_map)).collect(); + let ns: Vec<_> = set.iter().map(|c| c.render_railroad(cx)).collect(); Box::new(Choice::>::new(ns)) } ExpressionKind::NegExpression(e) => { - let n = e.render_railroad(stack, link_map)?; - let ch = node_for_nt(link_map, "CHAR"); + let n = e.render_railroad(cx, stack)?; + let ch = node_for_nt(cx, "CHAR"); Box::new(Except::new(Box::new(ch), n)) } ExpressionKind::Unicode(s) => Box::new(Terminal::new(format!("U+{}", s))), @@ -248,17 +231,18 @@ impl Expression { } impl Characters { - fn render_railroad(&self, link_map: &HashMap) -> Box { + fn render_railroad(&self, cx: &RenderCtx) -> Box { match self { - Characters::Named(s) => node_for_nt(link_map, s), + Characters::Named(s) => node_for_nt(cx, s), Characters::Terminal(s) => Box::new(Terminal::new(s.clone())), Characters::Range(a, b) => Box::new(Terminal::new(format!("{a}-{b}"))), } } } -fn node_for_nt(link_map: &HashMap, name: &str) -> Box { - let dest = link_map +fn node_for_nt(cx: &RenderCtx, name: &str) -> Box { + let dest = cx + .rr_link_map .get(name) .map(|path| path.to_string()) .unwrap_or_else(|| format!("missing"));