diff --git a/src/export/lecture.rs b/src/export/lecture.rs index 79e728a..cba635d 100644 --- a/src/export/lecture.rs +++ b/src/export/lecture.rs @@ -110,7 +110,7 @@ fn is_tiered(course: &CourseFile, lecture: &str) -> bool { /// # Returns /// /// Registry ids in printed order. -fn numbered_targets(course: &CourseFile, lecture: &str) -> Vec { +pub fn numbered_targets(course: &CourseFile, lecture: &str) -> Vec { if !is_tiered(course, lecture) { // Untiered page: level groups, in the order they print, which is the // numbering this page has always had. diff --git a/src/export/practice.rs b/src/export/practice.rs index c92f267..47f97c0 100644 --- a/src/export/practice.rs +++ b/src/export/practice.rs @@ -259,11 +259,10 @@ pub fn render(catalog: &Catalog, record: &AssessmentFile, opts: &Options) -> Res /// The Quarto YAML front matter. fn front_matter(course: &CourseFile, record: &AssessmentFile, variant: Variant) -> String { - let title = if matches!(variant, Variant::Solutions) { - format!("{}: {}", record.assessment.title, variant.title_word()) - } else { - record.assessment.title.clone() - }; + // Both documents name themselves. Two PDFs called "Homework 1" are + // indistinguishable in a downloads folder, which is how a solutions copy + // gets posted in place of the worksheet. + let title = format!("{}: {}", record.assessment.title, variant.title_word()); let subtitle = format!("{} · {}", course.course.code, course.course.title); let mut out = String::from("---\n"); out.push_str(&format!("title: \"{}\"\n", yaml_quote(&title))); @@ -663,13 +662,18 @@ items: &Options::default().form, ) .expect("renders"); - assert!(ws.contains("title: \"Homework 1: Questions\"")); + assert!(ws.contains("title: \"Homework 1: Questions\""), "{ws}"); let sol = solutions( &catalog("front-matter-sol"), &record(), &Options::default().form, ) .expect("renders"); - assert!(sol.contains("title: \"Homework 1: Solutions\"")); + assert!(sol.contains("title: \"Homework 1: Solutions\""), "{sol}"); + // Neither document may title itself with the bare assessment name: two + // PDFs called "Homework 1" are indistinguishable once downloaded, and + // the pair that gets confused is the one with the answers in it. + assert!(!ws.contains("title: \"Homework 1\""), "{ws}"); + assert!(!sol.contains("title: \"Homework 1\""), "{sol}"); } } diff --git a/src/export/qti.rs b/src/export/qti.rs index bc0b331..86e31f9 100644 --- a/src/export/qti.rs +++ b/src/export/qti.rs @@ -1172,7 +1172,11 @@ mod tests { assert!(m.contains("-1")); assert!(m.contains("assignment")); assert!(m.contains("true")); - assert!(m.contains("false")); + // Canvas does the shuffling. The form fixes an option order for the + // printed paper, but an online quiz has no printed order to preserve and + // `data::canvas` matches responses back by answer text rather than by + // letter, so a per-student shuffle costs the analysis nothing. + assert!(m.contains("true")); // Default is unpublished, so an import never goes live unreviewed. assert!(m.contains("false")); assert!(m.contains("unpublished")); @@ -1180,6 +1184,22 @@ mod tests { assert!(m.contains(CANVAS_NS)); } + #[test] + fn shuffling_can_be_turned_off_per_assessment() { + // A record with `shuffle: false` is the case where the printed order has + // to be preserved online too, such as an item whose options are a + // sequence ("first ... then ...") that reads wrong in any other order. + let opts = QtiOptions { + shuffle_in_canvas: false, + ..QtiOptions::default() + }; + let m = build_assessment_meta("gq", "ga", "T", 1.0, &opts).render(0); + assert!( + m.contains("false"), + "{m}" + ); + } + #[test] fn hide_results_tracks_letting_students_see_responses() { // Default (on): hide_results is empty, so students may review and the diff --git a/src/export/typst.rs b/src/export/typst.rs index 92625a1..7c4113d 100644 --- a/src/export/typst.rs +++ b/src/export/typst.rs @@ -69,6 +69,7 @@ use crate::Layout; use crate::assessment::{AssessmentFile, Form}; use crate::catalog::Catalog; use crate::error::{Error, Result}; +use crate::markup; /// What to render. #[derive(Debug, Clone)] @@ -195,6 +196,17 @@ pub fn render(catalog: &Catalog, record: &AssessmentFile, opts: &Options) -> Res } let mut warnings = payload::check(&payload, &opts.config); + for (slot, body) in &bodies { + if markup::needs_chem_import(&template.source, body) { + warnings.push(format!( + "the `{}` slot carries a chemical formula, but the template {} does not import \ + whalogen, so Typst will stop at `unknown variable: ce`; add `{}`", + slot.as_str(), + template.origin, + markup::CHEM_IMPORT + )); + } + } if template.is_inert() { warnings.push(format!( "the template {} declares no coursebank markers, so no questions were injected; add \ diff --git a/src/export/typst/diagnostic.rs b/src/export/typst/diagnostic.rs index 74d65f4..a8b3009 100644 --- a/src/export/typst/diagnostic.rs +++ b/src/export/typst/diagnostic.rs @@ -1031,6 +1031,17 @@ fn render( } let mut warnings = Vec::new(); + for (slot, body) in &bodies { + if crate::markup::needs_chem_import(&template.source, body) { + warnings.push(format!( + "the `{}` slot carries a chemical formula, but the template {} does not import \ + whalogen, so Typst will stop at `unknown variable: ce`; add `{}`", + slot.as_str(), + template.origin, + crate::markup::CHEM_IMPORT + )); + } + } if template.is_inert() { warnings.push(format!( "the template {} declares no coursebank markers, so the report is empty; add `// \ @@ -1213,6 +1224,42 @@ mod tests { } } + #[test] + fn report_markup_takes_the_same_path_as_a_paper() { + for source in [ + "$\\ce{H2O <=> H+ + OH-}$", + "the backbone $\\ce{-C=O}$ group", + "$K_w = [\\text{H}^+][\\text{OH}^-]$", + "see @fig:x where x < y", + "costs \\$5, and just $5", + ] { + let expected = crate::markup::to_typst(source); + assert_eq!( + markup_value(source, true).to_typst(0), + format!("[{expected}]"), + "content mode diverged on {source:?}" + ); + assert_eq!( + markup_value(source, false).to_typst(0), + Value::str(expected).to_typst(0), + "string mode diverged on {source:?}" + ); + } + } + + #[test] + fn a_report_carrying_chemistry_names_a_template_missing_the_import() { + let body = "#let cb-data = (stem: [#ce(\"H2O\")])"; + assert!(crate::markup::needs_chem_import( + "#import \"@preview/mitex:0.2.7\": mi\n// coursebank:data\n", + body + )); + assert!(!crate::markup::needs_chem_import( + crate::markup::CHEM_IMPORT, + body + )); + } + #[test] fn the_headline_leads_with_the_distribution() { let lines = headline(&empty_cohort()); diff --git a/src/export/typst/payload.rs b/src/export/typst/payload.rs index 68171a2..da51acd 100644 --- a/src/export/typst/payload.rs +++ b/src/export/typst/payload.rs @@ -1066,61 +1066,27 @@ fn numeric_map(map: &BTreeMap) -> Value { ) } -/// Rewrites inline LaTeX math (`$...$`) into a call to mitex's `mi`, so Typst's -/// own math grammar never has to parse it. Typst's `\times`, `\Delta`, `\ln` and -/// friends are not valid Typst math — a bare backslash escapes the next -/// character instead of naming a symbol — which is why equations compile but -/// print wrong instead of failing outright. `\$` is left alone, matching LaTeX's -/// own convention for a literal dollar sign, and a `$` with no matching close is -/// left alone too, rather than swallowing the rest of the field. -fn rewrite_latex_math(source: &str) -> String { - let chars: Vec<(usize, char)> = source.char_indices().collect(); - let mut out = String::with_capacity(source.len()); - let mut i = 0; - while i < chars.len() { - let (_, c) = chars[i]; - if c == '\\' && i + 1 < chars.len() { - out.push('\\'); - out.push(chars[i + 1].1); - i += 2; - continue; - } - if c != '$' { - out.push(c); - i += 1; - continue; - } - let mut j = i + 1; - let close = loop { - if j >= chars.len() { - break None; - } - match chars[j].1 { - '\\' => j += 2, - '$' => break Some(j), - _ => j += 1, - } - }; - match close { - Some(close_idx) => { - let start = chars[i + 1].0; - let end = chars[close_idx].0; - out.push_str("#mi("); - out.push_str(&Value::str(&source[start..end]).to_typst(0)); - out.push(')'); - i = close_idx + 1; - } - None => { - out.push('$'); - i += 1; - } - } - } - out -} - +/// Emits one markup-bearing field for a diagnostic report. +/// +/// The conversion itself lives in [`markup::to_typst`], which is the single +/// entry point for turning authoring markup into Typst: it rewrites inline +/// LaTeX math, routes mhchem through whalogen, and escapes the characters Typst +/// treats specially in content mode. This path used to carry its own copy of +/// the math rewriting, which drifted — the reports escaped nothing outside math +/// and silently disagreed with the exam papers about an unmatched `$`. +/// +/// # Arguments +/// +/// * `source` - the authoring source. +/// * `content` - whether to emit a content block rather than a quoted string. +/// A string is evaluated by the template with `eval(.., mode: "markup")`, so +/// both modes need the same escaping. +/// +/// # Returns +/// +/// The value to place in the payload. pub(crate) fn markup_value(source: &str, content: bool) -> Value { - let source = rewrite_latex_math(source); + let source = markup::to_typst(source); if content { Value::content(source) } else { @@ -1270,17 +1236,3 @@ mod tests { assert!(!balanced("closing ] first")); } } - -#[test] -fn latex_math_becomes_a_mitex_call() { - assert_eq!( - rewrite_latex_math("angle $\\phi$ (phi)"), - "angle #mi(\"\\\\phi\") (phi)" - ); -} - -#[test] -fn escaped_and_unmatched_dollar_signs_are_left_alone() { - assert_eq!(rewrite_latex_math("costs \\$5 total"), "costs \\$5 total"); - assert_eq!(rewrite_latex_math("just $5"), "just $5"); -} diff --git a/src/export/typst/templates/student-report.typ b/src/export/typst/templates/student-report.typ index e208393..2324761 100644 --- a/src/export/typst/templates/student-report.typ +++ b/src/export/typst/templates/student-report.typ @@ -33,6 +33,7 @@ // it. If you change one thing here, keep that. #import "@preview/mitex:0.2.7": mi +#import "@preview/whalogen:0.3.0": ce // ───────────────────────────────────────────────────────────────────────────── // Data diff --git a/src/util/markup.rs b/src/util/markup.rs index d2aa64a..d0d6b1d 100644 --- a/src/util/markup.rs +++ b/src/util/markup.rs @@ -150,7 +150,9 @@ pub fn to_markdown(src: &str) -> String { /// rather than naming a symbol, so `\Delta`, `\times`, `\ln` compile without /// error and print wrong. Each `$...$` span is instead handed whole to /// mitex's `mi`, which parses LaTeX grammar on purpose: `$\phi$` becomes -/// `#mi("\\phi")`. The `@`/`<`/`>` escaping above is skipped for anything +/// `#mi("\\phi")`. mhchem's `\ce{...}` is the one exception, and goes to +/// whalogen's `ce` instead, via `push_math_span`. The `@`/`<`/`>` escaping +/// above is skipped for anything /// inside the span, since it reaches Typst as a string argument, not as /// markup — escaping `<` there would corrupt the LaTeX rather than protect /// anything. @@ -174,6 +176,26 @@ pub fn to_typst(src: &str) -> String { while i < chars.len() { let (_, c) = chars[i]; + // A `\ce{...}` written outside math is still chemistry and still cannot + // reach Typst as a backslash: `\c` is an escape in content mode. Checked + // before the escaped-pair rule below, which would otherwise copy `\c` + // through and leave `e{...}` as literal text. The cheap two-character + // guard keeps a stem full of `\Delta` and `\times` from rescanning the + // remainder at every backslash. + if c == '\\' + && chars.get(i + 1).map(|c| c.1) == Some('c') + && chars.get(i + 2).map(|c| c.1) == Some('e') + { + if let Some(span) = find_ce(&src[chars[i].0..]) { + if span.start == 0 { + let at = chars[i].0; + push_ce(&mut out, &src[at + span.arg.0..at + span.arg.1]); + i = char_index(&chars, at + span.end); + continue; + } + } + } + // An escaped pair is copied verbatim and never reconsidered, so `\$` // can't be mistaken for the start of math and an `\@`/`\<`/`\>` an // author already wrote is not escaped a second time. @@ -189,9 +211,7 @@ pub fn to_typst(src: &str) -> String { Some(close) => { let start = chars[i + 1].0; let end = chars[close].0; - out.push_str("#mi("); - push_typst_string(&mut out, &src[start..end]); - out.push(')'); + push_math_span(&mut out, &src[start..end]); i = close + 1; continue; } @@ -229,6 +249,190 @@ fn find_math_close(chars: &[(usize, char)], open: usize) -> Option { None } +/// The import line a template needs before it can receive a `#ce(...)` call. +/// +/// mitex renders the LaTeX in a stem, but it ships no package support, so +/// mhchem's `\ce` is simply an unknown command to it. whalogen is a Typst port +/// of mhchem, and `ce` is the function this module emits chemistry as. +pub const CHEM_IMPORT: &str = "#import \"@preview/whalogen:0.3.0\": ce"; + +/// Whether `body` needs [`CHEM_IMPORT`] and `template` does not provide it. +/// +/// A template is checked for the package name rather than for the exact import +/// line, so a template that pins a different whalogen version, imports the +/// package under an alias, or defines its own `ce` is left alone. +/// +/// # Arguments +/// +/// * `template` - the template source the body will be injected into. +/// * `body` - the generated Typst source. +/// +/// # Returns +/// +/// Whether the pair would fail to compile for want of the import. +pub fn needs_chem_import(template: &str, body: &str) -> bool { + body.contains("#ce(") && !template.contains("whalogen") && !template.contains("let ce") +} + +/// A `\ce{...}` command located in a source fragment, as byte offsets from the +/// start of that fragment. +struct CeSpan { + /// Offset of the backslash. + start: usize, + /// Offset just past the closing brace. + end: usize, + /// The argument, without its braces. + arg: (usize, usize), +} + +/// Finds the first `\ce{...}` in `s`. +/// +/// Three details matter, and all three come from the same place: the scan has to +/// agree with what LaTeX itself would read. +/// +/// A backslash-escaped pair is skipped as a unit, so the line break `\\` +/// followed by the letters `ce` is not read as the command. `$a \\ ce{x}$` is a +/// break and then literal text; mhchem's own command is a single backslash. +/// +/// The name must end at the `e`, so `\cellcolor` and `\century` are left for +/// mitex rather than half-consumed here. +/// +/// The argument is matched on brace depth rather than on the first `}`, so +/// `\ce{Fe^{2+}}` keeps its superscript. An unbalanced argument yields `None`: +/// there is nothing safe to convert, and mitex's own error names the line. +fn find_ce(s: &str) -> Option { + let chars: Vec<(usize, char)> = s.char_indices().collect(); + let mut i = 0; + while i < chars.len() { + if chars[i].1 != '\\' { + i += 1; + continue; + } + if matches!(chars.get(i + 1), Some((_, '\\'))) { + i += 2; + continue; + } + let named = chars.get(i + 1).map(|c| c.1) == Some('c') + && chars.get(i + 2).map(|c| c.1) == Some('e') + && !matches!(chars.get(i + 3), Some((_, c)) if c.is_ascii_alphabetic()); + if !named { + i += 1; + continue; + } + + // LaTeX allows whitespace between a control word and its argument. + let mut open = i + 3; + while matches!(chars.get(open), Some((_, c)) if c.is_whitespace()) { + open += 1; + } + if !matches!(chars.get(open), Some((_, '{'))) { + i += 1; + continue; + } + + let mut depth = 1usize; + let mut k = open + 1; + while k < chars.len() { + match chars[k].1 { + '\\' => k += 2, + '{' => { + depth += 1; + k += 1; + } + '}' => { + depth -= 1; + if depth == 0 { + return Some(CeSpan { + start: chars[i].0, + end: chars[k].0 + 1, + arg: (chars[open].0 + 1, chars[k].0), + }); + } + k += 1; + } + _ => k += 1, + } + } + return None; + } + None +} + +/// The index into `chars` of the entry at byte offset `byte`, or the length when +/// the offset is past the end. +fn char_index(chars: &[(usize, char)], byte: usize) -> usize { + chars + .iter() + .position(|(at, _)| *at >= byte) + .unwrap_or(chars.len()) +} + +/// Emits one `$...$` span as Typst content. +/// +/// Most of a span goes to mitex's `mi`, which parses LaTeX on purpose. The +/// exception is mhchem: `\ce{...}` is not a LaTeX primitive but a package +/// command with its own character-level grammar, and mitex implements no +/// packages, so the plugin aborts the whole document with `unknown command: +/// \ce` rather than degrading to something printable. Those runs are handed to +/// whalogen's `ce` instead and the rest of the span still goes to `mi`, so an +/// equation that mixes chemistry with ordinary math renders both. +/// +/// A span with no chemistry in it produces exactly one `mi` call, as it always +/// has. +/// +/// # Arguments +/// +/// * `out` - the buffer to append to. +/// * `latex` - the LaTeX between the dollar signs. +fn push_math_span(out: &mut String, latex: &str) { + if find_ce(latex).is_none() { + push_mi(out, latex); + return; + } + + let mut rest = latex; + while let Some(span) = find_ce(rest) { + push_run(out, &rest[..span.start]); + push_ce(out, &rest[span.arg.0..span.arg.1]); + rest = &rest[span.end..]; + } + push_run(out, rest); +} + +/// Emits a non-chemistry run of a math span, dropping an empty one and keeping a +/// whitespace-only one as the single space it separates two calls with. +fn push_run(out: &mut String, latex: &str) { + if latex.is_empty() { + return; + } + if latex.trim().is_empty() { + out.push(' '); + return; + } + push_mi(out, latex); +} + +/// Emits a `#mi(...)` call wrapping LaTeX math. +fn push_mi(out: &mut String, latex: &str) { + out.push_str("#mi("); + push_typst_string(out, latex); + out.push(')'); +} + +/// Emits a `#ce(...)` call wrapping an mhchem argument. +/// +/// The argument is passed through as written. whalogen reads the same formula, +/// charge, bond, and arrow syntax mhchem does, so `H2O`, `<=>`, `[AgCl2]-`, and +/// `->[H2O]` need no translation. Its isotope and oxidation-number spellings do +/// differ (`@Th,227,90@` against mhchem's `^{227}_{90}Th`), and so do `\pu` and +/// `\bond`, which whalogen has no equivalent for. Those print oddly rather than +/// failing the build, so a bank that uses them needs its own pass. +fn push_ce(out: &mut String, argument: &str) { + out.push_str("#ce("); + push_typst_string(out, argument.trim()); + out.push(')'); +} + /// Writes `s` as a quoted Typst string. Kept local, duplicating the five-case /// match in `typst::value::write_string`, rather than reaching into the /// Typst-specific value writer for one small helper. @@ -529,3 +733,96 @@ fn escaped_and_unmatched_dollar_signs_are_left_or_escaped() { assert_eq!(to_typst("costs \\$5 total"), "costs \\$5 total"); assert_eq!(to_typst("just $5"), "just \\$5"); } + +#[test] +fn mhchem_goes_to_whalogen_rather_than_mitex() { + // The regression: mitex implements no LaTeX packages, so `\ce` reached the + // plugin as an unknown command and failed the whole document rather than + // printing badly. + assert_eq!(to_typst("$\\ce{H2O}$"), "#ce(\"H2O\")"); + assert_eq!( + to_typst("water ionizes, $\\ce{H2O <=> H+ + OH-}$, giving"), + "water ionizes, #ce(\"H2O <=> H+ + OH-\"), giving" + ); +} + +#[test] +fn a_span_can_mix_chemistry_with_ordinary_math() { + // The chemistry leaves the span and the rest of it still reaches mitex, so + // an equation that needs both renders both. + assert_eq!(to_typst("$K_w = \\ce{H2O}$"), "#mi(\"K_w = \")#ce(\"H2O\")"); + // The space between the two runs stays inside the `mi` call rather than + // becoming content-mode whitespace, so the spacing is TeX's to decide. + assert_eq!( + to_typst("$\\ce{H2O} \\to \\Delta H$"), + "#ce(\"H2O\")#mi(\" \\\\to \\\\Delta H\")" + ); +} + +#[test] +fn mhchem_arguments_keep_their_nested_braces() { + // Matched on brace depth, not on the first `}`, or the charge is orphaned + // and the remaining `}` closes the `#ce(` call early. + assert_eq!(to_typst("$\\ce{Fe^{2+}}$"), "#ce(\"Fe^{2+}\")"); + assert_eq!( + to_typst("$\\ce{SO4^{2-} + Ba^{2+}}$"), + "#ce(\"SO4^{2-} + Ba^{2+}\")" + ); +} + +#[test] +fn a_latex_line_break_is_not_read_as_the_chemistry_command() { + // `\\` is an escaped backslash followed by the letters `ce`, not `\ce`. + // Scanning that skips escaped pairs as a unit keeps the two apart; scanning + // that does not would convert a line break into a formula. + assert_eq!(to_typst("$a \\\\ce{x}$"), "#mi(\"a \\\\\\\\ce{x}\")"); +} + +#[test] +fn commands_that_merely_start_with_ce_are_left_to_mitex() { + // The name has to end at the `e`, or `\cellcolor` is half-consumed and the + // conversion invents a formula out of its argument. + assert_eq!( + to_typst("$\\cellcolor{red} x$"), + "#mi(\"\\\\cellcolor{red} x\")" + ); +} + +#[test] +fn unbalanced_chemistry_is_left_for_mitex_to_report() { + // Nothing safe to convert. mitex's own error names the file and line, which + // is more useful than a silently truncated formula. + assert_eq!(to_typst("$\\ce{H2O$"), "#mi(\"\\\\ce{H2O\")"); +} + +#[test] +fn chemistry_outside_math_is_converted_too() { + // A bare `\ce` never reaches mitex at all, and `\c` is an escape in Typst + // content mode, so leaving it alone produces a document that either fails + // or prints the letters. + assert_eq!( + to_typst("the backbone \\ce{-NH} group"), + "the backbone #ce(\"-NH\") group" + ); +} + +#[test] +fn a_template_without_the_chemistry_import_is_named() { + let body = "#question((stem: [#ce(\"H2O\")]))"; + assert!(needs_chem_import( + "#import \"@preview/mitex:0.2.7\": mi", + body + )); + // An import of any whalogen version, or a template's own `ce`, is enough. + assert!(!needs_chem_import(CHEM_IMPORT, body)); + assert!(!needs_chem_import( + "#import \"@preview/whalogen:0.2.0\": ce as ce", + body + )); + assert!(!needs_chem_import("#let ce(f) = f", body)); + // No chemistry in the body, nothing to warn about. + assert!(!needs_chem_import( + "#import \"@preview/mitex:0.2.7\": mi", + "#mi(\"x\")" + )); +}