Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 57ea6a8

Browse files
traviscrosstraviscross
committedApr 17, 2025·
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.
1 parent 7fdb38f commit 57ea6a8

File tree

3 files changed

+71
-110
lines changed

3 files changed

+71
-110
lines changed
 

‎mdbook-spec/src/grammar.rs

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,13 @@ enum Characters {
8888
Range(char, char),
8989
}
9090

91+
#[derive(Debug)]
92+
pub struct RenderCtx {
93+
md_link_map: HashMap<String, String>,
94+
rr_link_map: HashMap<String, String>,
95+
for_summary: bool,
96+
}
97+
9198
impl Grammar {
9299
fn visit_nt(&self, callback: &mut dyn FnMut(&str)) {
93100
for p in self.productions.values() {
@@ -301,7 +308,7 @@ fn render_names(
301308
output.push_str("<br>\n");
302309

303310
// Convert the link map to add the id.
304-
let updated_link_map = |get_id: fn(&str, bool) -> String| -> HashMap<String, String> {
311+
let update_link_map = |get_id: fn(&str, bool) -> String| -> HashMap<String, String> {
305312
link_map
306313
.iter()
307314
.map(|(name, path)| {
@@ -316,19 +323,13 @@ fn render_names(
316323
.collect()
317324
};
318325

319-
let markdown_link_map = updated_link_map(render_markdown::markdown_id);
320-
// Modify the link map so that it contains the exact destination needed to
321-
// link to the railroad productions, and to accommodate the summary
322-
// chapter.
323-
let railroad_link_map = updated_link_map(render_railroad::railroad_id);
324-
325-
if let Err(e) = grammar.render_markdown(
326-
&names,
327-
&markdown_link_map,
328-
&railroad_link_map,
329-
&mut output,
326+
let render_ctx = RenderCtx {
327+
md_link_map: update_link_map(render_markdown::markdown_id),
328+
rr_link_map: update_link_map(render_railroad::railroad_id),
330329
for_summary,
331-
) {
330+
};
331+
332+
if let Err(e) = grammar.render_markdown(&render_ctx, &names, &mut output) {
332333
warn_or_err!(
333334
diag,
334335
"grammar failed in chapter {:?}: {e}",
@@ -348,13 +349,7 @@ fn render_names(
348349
\n",
349350
);
350351

351-
if let Err(e) = grammar.render_railroad(
352-
&names,
353-
&railroad_link_map,
354-
&markdown_link_map,
355-
&mut output,
356-
for_summary,
357-
) {
352+
if let Err(e) = grammar.render_railroad(&render_ctx, &names, &mut output) {
358353
warn_or_err!(
359354
diag,
360355
"grammar failed in chapter {:?}: {e}",

‎mdbook-spec/src/grammar/render_markdown.rs

Lines changed: 26 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,26 @@
11
//! Renders the grammar to markdown.
22
3-
use super::{Characters, Expression, ExpressionKind, Production};
3+
use super::{Characters, Expression, ExpressionKind, Production, RenderCtx};
44
use crate::grammar::Grammar;
55
use anyhow::bail;
66
use regex::Regex;
77
use std::borrow::Cow;
8-
use std::collections::HashMap;
98
use std::fmt::Write;
109
use std::sync::LazyLock;
1110

1211
impl Grammar {
1312
pub fn render_markdown(
1413
&self,
14+
cx: &RenderCtx,
1515
names: &[&str],
16-
link_map: &HashMap<String, String>,
17-
rr_link_map: &HashMap<String, String>,
1816
output: &mut String,
19-
for_summary: bool,
2017
) -> anyhow::Result<()> {
2118
let mut iter = names.into_iter().peekable();
2219
while let Some(name) = iter.next() {
2320
let Some(prod) = self.productions.get(*name) else {
2421
bail!("could not find grammar production named `{name}`");
2522
};
26-
prod.render_markdown(link_map, rr_link_map, output, for_summary);
23+
prod.render_markdown(cx, output);
2724
if iter.peek().is_some() {
2825
output.push_str("\n");
2926
}
@@ -42,14 +39,9 @@ pub fn markdown_id(name: &str, for_summary: bool) -> String {
4239
}
4340

4441
impl Production {
45-
fn render_markdown(
46-
&self,
47-
link_map: &HashMap<String, String>,
48-
rr_link_map: &HashMap<String, String>,
49-
output: &mut String,
50-
for_summary: bool,
51-
) {
52-
let dest = rr_link_map
42+
fn render_markdown(&self, cx: &RenderCtx, output: &mut String) {
43+
let dest = cx
44+
.rr_link_map
5345
.get(&self.name)
5446
.map(|path| path.to_string())
5547
.unwrap_or_else(|| format!("missing"));
@@ -60,12 +52,11 @@ impl Production {
6052
>\
6153
[{name}]({dest})\
6254
</span> → ",
63-
id = markdown_id(&self.name, for_summary),
55+
id = markdown_id(&self.name, cx.for_summary),
6456
name = self.name,
6557
)
6658
.unwrap();
67-
self.expression
68-
.render_markdown(link_map, output, for_summary);
59+
self.expression.render_markdown(cx, output);
6960
output.push('\n');
7061
}
7162
}
@@ -92,16 +83,11 @@ impl Expression {
9283
}
9384
}
9485

95-
fn render_markdown(
96-
&self,
97-
link_map: &HashMap<String, String>,
98-
output: &mut String,
99-
for_summary: bool,
100-
) {
86+
fn render_markdown(&self, cx: &RenderCtx, output: &mut String) {
10187
match &self.kind {
10288
ExpressionKind::Grouped(e) => {
10389
output.push_str("( ");
104-
e.render_markdown(link_map, output, for_summary);
90+
e.render_markdown(cx, output);
10591
if !matches!(e.last(), ExpressionKind::Break(_)) {
10692
output.push(' ');
10793
}
@@ -110,7 +96,7 @@ impl Expression {
11096
ExpressionKind::Alt(es) => {
11197
let mut iter = es.iter().peekable();
11298
while let Some(e) = iter.next() {
113-
e.render_markdown(link_map, output, for_summary);
99+
e.render_markdown(cx, output);
114100
if iter.peek().is_some() {
115101
if !matches!(e.last(), ExpressionKind::Break(_)) {
116102
output.push(' ');
@@ -122,34 +108,34 @@ impl Expression {
122108
ExpressionKind::Sequence(es) => {
123109
let mut iter = es.iter().peekable();
124110
while let Some(e) = iter.next() {
125-
e.render_markdown(link_map, output, for_summary);
111+
e.render_markdown(cx, output);
126112
if iter.peek().is_some() && !matches!(e.last(), ExpressionKind::Break(_)) {
127113
output.push(' ');
128114
}
129115
}
130116
}
131117
ExpressionKind::Optional(e) => {
132-
e.render_markdown(link_map, output, for_summary);
118+
e.render_markdown(cx, output);
133119
output.push_str("<sup>?</sup>");
134120
}
135121
ExpressionKind::Repeat(e) => {
136-
e.render_markdown(link_map, output, for_summary);
122+
e.render_markdown(cx, output);
137123
output.push_str("<sup>\\*</sup>");
138124
}
139125
ExpressionKind::RepeatNonGreedy(e) => {
140-
e.render_markdown(link_map, output, for_summary);
126+
e.render_markdown(cx, output);
141127
output.push_str("<sup>\\* (non-greedy)</sup>");
142128
}
143129
ExpressionKind::RepeatPlus(e) => {
144-
e.render_markdown(link_map, output, for_summary);
130+
e.render_markdown(cx, output);
145131
output.push_str("<sup>+</sup>");
146132
}
147133
ExpressionKind::RepeatPlusNonGreedy(e) => {
148-
e.render_markdown(link_map, output, for_summary);
134+
e.render_markdown(cx, output);
149135
output.push_str("<sup>+ (non-greedy)</sup>");
150136
}
151137
ExpressionKind::RepeatRange(e, a, b) => {
152-
e.render_markdown(link_map, output, for_summary);
138+
e.render_markdown(cx, output);
153139
write!(
154140
output,
155141
"<sup>{}..{}</sup>",
@@ -159,7 +145,7 @@ impl Expression {
159145
.unwrap();
160146
}
161147
ExpressionKind::Nt(nt) => {
162-
let dest = link_map.get(nt).map_or("missing", |d| d.as_str());
148+
let dest = cx.md_link_map.get(nt).map_or("missing", |d| d.as_str());
163149
write!(output, "<span class=\"grammar-text\">[{nt}]({dest})</span>").unwrap();
164150
}
165151
ExpressionKind::Terminal(t) => {
@@ -177,10 +163,10 @@ impl Expression {
177163
output.push_str("\\\n");
178164
output.push_str(&"&nbsp;".repeat(*indent));
179165
}
180-
ExpressionKind::Charset(set) => charset_render_markdown(set, link_map, output),
166+
ExpressionKind::Charset(set) => charset_render_markdown(cx, set, output),
181167
ExpressionKind::NegExpression(e) => {
182168
output.push('~');
183-
e.render_markdown(link_map, output, for_summary);
169+
e.render_markdown(cx, output);
184170
}
185171
ExpressionKind::Unicode(s) => {
186172
output.push_str("U+");
@@ -190,7 +176,7 @@ impl Expression {
190176
if let Some(suffix) = &self.suffix {
191177
write!(output, "<sub class=\"grammar-text\">{suffix}</sub>").unwrap();
192178
}
193-
if !for_summary {
179+
if !cx.for_summary {
194180
if let Some(footnote) = &self.footnote {
195181
// The `ZeroWidthSpace` is to avoid conflicts with markdown link
196182
// references.
@@ -200,15 +186,11 @@ impl Expression {
200186
}
201187
}
202188

203-
fn charset_render_markdown(
204-
set: &[Characters],
205-
link_map: &HashMap<String, String>,
206-
output: &mut String,
207-
) {
189+
fn charset_render_markdown(cx: &RenderCtx, set: &[Characters], output: &mut String) {
208190
output.push_str("\\[");
209191
let mut iter = set.iter().peekable();
210192
while let Some(chars) = iter.next() {
211-
chars.render_markdown(link_map, output);
193+
chars.render_markdown(cx, output);
212194
if iter.peek().is_some() {
213195
output.push(' ');
214196
}
@@ -217,10 +199,10 @@ fn charset_render_markdown(
217199
}
218200

219201
impl Characters {
220-
fn render_markdown(&self, link_map: &HashMap<String, String>, output: &mut String) {
202+
fn render_markdown(&self, cx: &RenderCtx, output: &mut String) {
221203
match self {
222204
Characters::Named(s) => {
223-
let dest = link_map.get(s).map_or("missing", |d| d.as_str());
205+
let dest = cx.md_link_map.get(s).map_or("missing", |d| d.as_str());
224206
write!(output, "[{s}]({dest})").unwrap();
225207
}
226208
Characters::Terminal(s) => write!(

‎mdbook-spec/src/grammar/render_railroad.rs

Lines changed: 30 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,26 @@
11
//! Converts a [`Grammar`] to an SVG railroad diagram.
22
3-
use super::{Characters, Expression, ExpressionKind, Production};
3+
use super::{Characters, Expression, ExpressionKind, Production, RenderCtx};
44
use crate::grammar::Grammar;
55
use anyhow::bail;
66
use railroad::*;
77
use regex::Regex;
8-
use std::collections::HashMap;
98
use std::fmt::Write;
109
use std::sync::LazyLock;
1110

1211
impl Grammar {
1312
pub fn render_railroad(
1413
&self,
14+
cx: &RenderCtx,
1515
names: &[&str],
16-
link_map: &HashMap<String, String>,
17-
md_link_map: &HashMap<String, String>,
1816
output: &mut String,
19-
for_summary: bool,
2017
) -> anyhow::Result<()> {
2118
for name in names {
2219
let prod = match self.productions.get(*name) {
2320
Some(p) => p,
2421
None => bail!("could not find grammar production named `{name}`"),
2522
};
26-
prod.render_railroad(link_map, md_link_map, output, for_summary);
23+
prod.render_railroad(cx, output);
2724
}
2825
Ok(())
2926
}
@@ -39,40 +36,30 @@ pub fn railroad_id(name: &str, for_summary: bool) -> String {
3936
}
4037

4138
impl Production {
42-
fn render_railroad(
43-
&self,
44-
link_map: &HashMap<String, String>,
45-
md_link_map: &HashMap<String, String>,
46-
output: &mut String,
47-
for_summary: bool,
48-
) {
49-
let mut dia = self.make_diagram(false, link_map, md_link_map);
39+
fn render_railroad(&self, cx: &RenderCtx, output: &mut String) {
40+
let mut dia = self.make_diagram(cx, false);
5041
// If the diagram is very wide, try stacking it to reduce the width.
5142
// This 900 is somewhat arbitrary based on looking at productions that
5243
// looked too squished. If your diagram is still too squished,
5344
// consider adding more rules to shorten it.
5445
if dia.width() > 900 {
55-
dia = self.make_diagram(true, link_map, md_link_map);
46+
dia = self.make_diagram(cx, true);
5647
}
5748
writeln!(
5849
output,
5950
"<div style=\"width: {width}px; height: auto; max-width: 100%; max-height: 100%\" \
6051
class=\"railroad-production\" \
6152
id=\"{id}\">{dia}</div>",
6253
width = dia.width(),
63-
id = railroad_id(&self.name, for_summary),
54+
id = railroad_id(&self.name, cx.for_summary),
6455
)
6556
.unwrap();
6657
}
6758

68-
fn make_diagram(
69-
&self,
70-
stack: bool,
71-
link_map: &HashMap<String, String>,
72-
md_link_map: &HashMap<String, String>,
73-
) -> Diagram<Box<dyn Node>> {
74-
let n = self.expression.render_railroad(stack, link_map);
75-
let dest = md_link_map
59+
fn make_diagram(&self, cx: &RenderCtx, stack: bool) -> Diagram<Box<dyn Node>> {
60+
let n = self.expression.render_railroad(cx, stack);
61+
let dest = cx
62+
.md_link_map
7663
.get(&self.name)
7764
.map(|path| path.to_string())
7865
.unwrap_or_else(|| format!("missing"));
@@ -88,11 +75,7 @@ impl Production {
8875
}
8976

9077
impl Expression {
91-
fn render_railroad(
92-
&self,
93-
stack: bool,
94-
link_map: &HashMap<String, String>,
95-
) -> Option<Box<dyn Node>> {
78+
fn render_railroad(&self, cx: &RenderCtx, stack: bool) -> Option<Box<dyn Node>> {
9679
let mut state;
9780
let mut state_ref = &self.kind;
9881
let n: Box<dyn Node> = 'l: loop {
@@ -101,12 +84,12 @@ impl Expression {
10184
// Render grouped nodes and `e{1..1}` repeats directly.
10285
ExpressionKind::Grouped(e)
10386
| ExpressionKind::RepeatRange(e, Some(1), Some(1)) => {
104-
e.render_railroad(stack, link_map)?
87+
e.render_railroad(cx, stack)?
10588
}
10689
ExpressionKind::Alt(es) => {
10790
let choices: Vec<_> = es
10891
.iter()
109-
.map(|e| e.render_railroad(stack, link_map))
92+
.map(|e| e.render_railroad(cx, stack))
11093
.filter_map(|n| n)
11194
.collect();
11295
Box::new(Choice::<Box<dyn Node>>::new(choices))
@@ -116,7 +99,7 @@ impl Expression {
11699
let make_seq = |es: &[&Expression]| {
117100
let seq: Vec<_> = es
118101
.iter()
119-
.map(|e| e.render_railroad(stack, link_map))
102+
.map(|e| e.render_railroad(cx, stack))
120103
.filter_map(|n| n)
121104
.collect();
122105
let seq: Sequence<Box<dyn Node>> = Sequence::new(seq);
@@ -159,29 +142,29 @@ impl Expression {
159142
// Treat `e?` and `e{..1}` / `e{0..1}` equally.
160143
ExpressionKind::Optional(e)
161144
| ExpressionKind::RepeatRange(e, None | Some(0), Some(1)) => {
162-
let n = e.render_railroad(stack, link_map)?;
145+
let n = e.render_railroad(cx, stack)?;
163146
Box::new(Optional::new(n))
164147
}
165148
// Treat `e*` and `e{..}` / `e{0..}` equally.
166149
ExpressionKind::Repeat(e)
167150
| ExpressionKind::RepeatRange(e, None | Some(0), None) => {
168-
let n = e.render_railroad(stack, link_map)?;
151+
let n = e.render_railroad(cx, stack)?;
169152
Box::new(Optional::new(Repeat::new(n, railroad::Empty)))
170153
}
171154
ExpressionKind::RepeatNonGreedy(e) => {
172-
let n = e.render_railroad(stack, link_map)?;
155+
let n = e.render_railroad(cx, stack)?;
173156
let r = Box::new(Optional::new(Repeat::new(n, railroad::Empty)));
174157
let lbox = LabeledBox::new(r, Comment::new("non-greedy".to_string()));
175158
Box::new(lbox)
176159
}
177160
// Treat `e+` and `e{1..}` equally.
178161
ExpressionKind::RepeatPlus(e)
179162
| ExpressionKind::RepeatRange(e, Some(1), None) => {
180-
let n = e.render_railroad(stack, link_map)?;
163+
let n = e.render_railroad(cx, stack)?;
181164
Box::new(Repeat::new(n, railroad::Empty))
182165
}
183166
ExpressionKind::RepeatPlusNonGreedy(e) => {
184-
let n = e.render_railroad(stack, link_map)?;
167+
let n = e.render_railroad(cx, stack)?;
185168
let r = Repeat::new(n, railroad::Empty);
186169
let lbox = LabeledBox::new(r, Comment::new("non-greedy".to_string()));
187170
Box::new(lbox)
@@ -197,7 +180,7 @@ impl Expression {
197180
}
198181
// Render `e{1..b}` directly.
199182
ExpressionKind::RepeatRange(e, Some(1), Some(b @ 2..)) => {
200-
let n = e.render_railroad(stack, link_map)?;
183+
let n = e.render_railroad(cx, stack)?;
201184
let cmt = format!("at most {b} more times", b = b - 1);
202185
let r = Repeat::new(n, Comment::new(cmt));
203186
Box::new(r)
@@ -218,17 +201,17 @@ impl Expression {
218201
state = ExpressionKind::Sequence(es);
219202
break 'cont &state;
220203
}
221-
ExpressionKind::Nt(nt) => node_for_nt(link_map, nt),
204+
ExpressionKind::Nt(nt) => node_for_nt(cx, nt),
222205
ExpressionKind::Terminal(t) => Box::new(Terminal::new(t.clone())),
223206
ExpressionKind::Prose(s) => Box::new(Terminal::new(s.clone())),
224207
ExpressionKind::Break(_) => return None,
225208
ExpressionKind::Charset(set) => {
226-
let ns: Vec<_> = set.iter().map(|c| c.render_railroad(link_map)).collect();
209+
let ns: Vec<_> = set.iter().map(|c| c.render_railroad(cx)).collect();
227210
Box::new(Choice::<Box<dyn Node>>::new(ns))
228211
}
229212
ExpressionKind::NegExpression(e) => {
230-
let n = e.render_railroad(stack, link_map)?;
231-
let ch = node_for_nt(link_map, "CHAR");
213+
let n = e.render_railroad(cx, stack)?;
214+
let ch = node_for_nt(cx, "CHAR");
232215
Box::new(Except::new(Box::new(ch), n))
233216
}
234217
ExpressionKind::Unicode(s) => Box::new(Terminal::new(format!("U+{}", s))),
@@ -248,17 +231,18 @@ impl Expression {
248231
}
249232

250233
impl Characters {
251-
fn render_railroad(&self, link_map: &HashMap<String, String>) -> Box<dyn Node> {
234+
fn render_railroad(&self, cx: &RenderCtx) -> Box<dyn Node> {
252235
match self {
253-
Characters::Named(s) => node_for_nt(link_map, s),
236+
Characters::Named(s) => node_for_nt(cx, s),
254237
Characters::Terminal(s) => Box::new(Terminal::new(s.clone())),
255238
Characters::Range(a, b) => Box::new(Terminal::new(format!("{a}-{b}"))),
256239
}
257240
}
258241
}
259242

260-
fn node_for_nt(link_map: &HashMap<String, String>, name: &str) -> Box<dyn Node> {
261-
let dest = link_map
243+
fn node_for_nt(cx: &RenderCtx, name: &str) -> Box<dyn Node> {
244+
let dest = cx
245+
.rr_link_map
262246
.get(name)
263247
.map(|path| path.to_string())
264248
.unwrap_or_else(|| format!("missing"));

0 commit comments

Comments
 (0)
Please sign in to comment.