From 302b87032121125bcfdb0490ad865a585ee0e3aa Mon Sep 17 00:00:00 2001 From: Laurenz Date: Mon, 5 Feb 2024 10:56:09 +0100 Subject: [PATCH] Delay errors for all show rules (#3323) --- crates/typst/src/introspection/counter.rs | 54 +++++------ crates/typst/src/introspection/locate.rs | 6 +- crates/typst/src/introspection/state.rs | 14 ++- crates/typst/src/model/bibliography.rs | 74 +++++++------- crates/typst/src/model/cite.rs | 20 ++-- crates/typst/src/model/footnote.rs | 18 ++-- crates/typst/src/model/link.rs | 12 +-- crates/typst/src/model/reference.rs | 113 +++++++++++----------- crates/typst/src/realize/mod.rs | 62 ++++++------ tests/typ/compiler/delayed-error.typ | 11 +++ 10 files changed, 190 insertions(+), 194 deletions(-) create mode 100644 tests/typ/compiler/delayed-error.typ diff --git a/crates/typst/src/introspection/counter.rs b/crates/typst/src/introspection/counter.rs index b87c8cfea..0031ea5b7 100644 --- a/crates/typst/src/introspection/counter.rs +++ b/crates/typst/src/introspection/counter.rs @@ -635,37 +635,35 @@ struct DisplayElem { impl Show for Packed { #[typst_macros::time(name = "counter.display", span = self.span())] fn show(&self, engine: &mut Engine, styles: StyleChain) -> SourceResult { - Ok(engine.delayed(|engine| { - let location = self.location().unwrap(); - let counter = self.counter(); - let numbering = self - .numbering() - .clone() - .or_else(|| { - let CounterKey::Selector(Selector::Elem(func, _)) = counter.0 else { - return None; - }; + let location = self.location().unwrap(); + let counter = self.counter(); + let numbering = self + .numbering() + .clone() + .or_else(|| { + let CounterKey::Selector(Selector::Elem(func, _)) = counter.0 else { + return None; + }; - if func == HeadingElem::elem() { - HeadingElem::numbering_in(styles).clone() - } else if func == FigureElem::elem() { - FigureElem::numbering_in(styles).clone() - } else if func == EquationElem::elem() { - EquationElem::numbering_in(styles).clone() - } else { - None - } - }) - .unwrap_or_else(|| NumberingPattern::from_str("1.1").unwrap().into()); + if func == HeadingElem::elem() { + HeadingElem::numbering_in(styles).clone() + } else if func == FigureElem::elem() { + FigureElem::numbering_in(styles).clone() + } else if func == EquationElem::elem() { + EquationElem::numbering_in(styles).clone() + } else { + None + } + }) + .unwrap_or_else(|| NumberingPattern::from_str("1.1").unwrap().into()); - let state = if *self.both() { - counter.both(engine, location)? - } else { - counter.at(engine, location)? - }; + let state = if *self.both() { + counter.both(engine, location)? + } else { + counter.at(engine, location)? + }; - state.display(engine, &numbering) - })) + state.display(engine, &numbering) } } diff --git a/crates/typst/src/introspection/locate.rs b/crates/typst/src/introspection/locate.rs index 132714f14..46d037052 100644 --- a/crates/typst/src/introspection/locate.rs +++ b/crates/typst/src/introspection/locate.rs @@ -44,9 +44,7 @@ struct LocateElem { impl Show for Packed { #[typst_macros::time(name = "locate", span = self.span())] fn show(&self, engine: &mut Engine, _: StyleChain) -> SourceResult { - Ok(engine.delayed(|engine| { - let location = self.location().unwrap(); - Ok(self.func().call(engine, [location])?.display()) - })) + let location = self.location().unwrap(); + Ok(self.func().call(engine, [location])?.display()) } } diff --git a/crates/typst/src/introspection/state.rs b/crates/typst/src/introspection/state.rs index 4cfa6754b..a4a358c9a 100644 --- a/crates/typst/src/introspection/state.rs +++ b/crates/typst/src/introspection/state.rs @@ -388,14 +388,12 @@ struct DisplayElem { impl Show for Packed { #[typst_macros::time(name = "state.display", span = self.span())] fn show(&self, engine: &mut Engine, _: StyleChain) -> SourceResult { - Ok(engine.delayed(|engine| { - let location = self.location().unwrap(); - let value = self.state().at(engine, location)?; - Ok(match self.func() { - Some(func) => func.call(engine, [value])?.display(), - None => value.display(), - }) - })) + let location = self.location().unwrap(); + let value = self.state().at(engine, location)?; + Ok(match self.func() { + Some(func) => func.call(engine, [value])?.display(), + None => value.display(), + }) } } diff --git a/crates/typst/src/model/bibliography.rs b/crates/typst/src/model/bibliography.rs index 6f31fd071..84ab2fffb 100644 --- a/crates/typst/src/model/bibliography.rs +++ b/crates/typst/src/model/bibliography.rs @@ -225,51 +225,47 @@ impl Show for Packed { ); } - Ok(engine.delayed(|engine| { - let span = self.span(); - let works = Works::generate(engine.world, engine.introspector).at(span)?; - let references = works - .references - .as_ref() - .ok_or("CSL style is not suitable for bibliographies") - .at(span)?; + let span = self.span(); + let works = Works::generate(engine.world, engine.introspector).at(span)?; + let references = works + .references + .as_ref() + .ok_or("CSL style is not suitable for bibliographies") + .at(span)?; - let row_gutter = *BlockElem::below_in(styles).amount(); - if references.iter().any(|(prefix, _)| prefix.is_some()) { - let mut cells = vec![]; - for (prefix, reference) in references { - cells.push( - Packed::new(GridCell::new(prefix.clone().unwrap_or_default())) - .spanned(span), - ); - cells.push( - Packed::new(GridCell::new(reference.clone())).spanned(span), - ); - } - - seq.push(VElem::new(row_gutter).with_weakness(3).pack()); - seq.push( - GridElem::new(cells) - .with_columns(TrackSizings(smallvec![Sizing::Auto; 2])) - .with_column_gutter(TrackSizings(smallvec![COLUMN_GUTTER.into()])) - .with_row_gutter(TrackSizings(smallvec![(row_gutter).into()])) - .pack() - .spanned(self.span()), + let row_gutter = *BlockElem::below_in(styles).amount(); + if references.iter().any(|(prefix, _)| prefix.is_some()) { + let mut cells = vec![]; + for (prefix, reference) in references { + cells.push( + Packed::new(GridCell::new(prefix.clone().unwrap_or_default())) + .spanned(span), ); - } else { - for (_, reference) in references { - seq.push(VElem::new(row_gutter).with_weakness(3).pack()); - seq.push(reference.clone()); - } + cells.push(Packed::new(GridCell::new(reference.clone())).spanned(span)); } - let mut content = Content::sequence(seq); - if works.hanging_indent { - content = content.styled(ParElem::set_hanging_indent(INDENT.into())); + seq.push(VElem::new(row_gutter).with_weakness(3).pack()); + seq.push( + GridElem::new(cells) + .with_columns(TrackSizings(smallvec![Sizing::Auto; 2])) + .with_column_gutter(TrackSizings(smallvec![COLUMN_GUTTER.into()])) + .with_row_gutter(TrackSizings(smallvec![(row_gutter).into()])) + .pack() + .spanned(self.span()), + ); + } else { + for (_, reference) in references { + seq.push(VElem::new(row_gutter).with_weakness(3).pack()); + seq.push(reference.clone()); } + } - Ok(content) - })) + let mut content = Content::sequence(seq); + if works.hanging_indent { + content = content.styled(ParElem::set_hanging_indent(INDENT.into())); + } + + Ok(content) } } diff --git a/crates/typst/src/model/cite.rs b/crates/typst/src/model/cite.rs index 2cd86b001..088c75127 100644 --- a/crates/typst/src/model/cite.rs +++ b/crates/typst/src/model/cite.rs @@ -152,17 +152,13 @@ pub struct CiteGroup { impl Show for Packed { #[typst_macros::time(name = "cite", span = self.span())] fn show(&self, engine: &mut Engine, _: StyleChain) -> SourceResult { - Ok(engine.delayed(|engine| { - let location = self.location().unwrap(); - let span = self.span(); - Works::generate(engine.world, engine.introspector) - .at(span)? - .citations - .get(&location) - .cloned() - .unwrap_or_else(|| { - bail!(span, "failed to format citation (this is a bug)") - }) - })) + let location = self.location().unwrap(); + let span = self.span(); + Works::generate(engine.world, engine.introspector) + .at(span)? + .citations + .get(&location) + .cloned() + .unwrap_or_else(|| bail!(span, "failed to format citation (this is a bug)")) } } diff --git a/crates/typst/src/model/footnote.rs b/crates/typst/src/model/footnote.rs index dbff2c1ee..383389edd 100644 --- a/crates/typst/src/model/footnote.rs +++ b/crates/typst/src/model/footnote.rs @@ -126,16 +126,14 @@ impl Packed { impl Show for Packed { #[typst_macros::time(name = "footnote", span = self.span())] fn show(&self, engine: &mut Engine, styles: StyleChain) -> SourceResult { - Ok(engine.delayed(|engine| { - let loc = self.declaration_location(engine).at(self.span())?; - let numbering = self.numbering(styles); - let counter = Counter::of(FootnoteElem::elem()); - let num = counter.at(engine, loc)?.display(engine, numbering)?; - let sup = SuperElem::new(num).pack().spanned(self.span()); - let loc = loc.variant(1); - // Add zero-width weak spacing to make the footnote "sticky". - Ok(HElem::hole().pack() + sup.linked(Destination::Location(loc))) - })) + let loc = self.declaration_location(engine).at(self.span())?; + let numbering = self.numbering(styles); + let counter = Counter::of(FootnoteElem::elem()); + let num = counter.at(engine, loc)?.display(engine, numbering)?; + let sup = SuperElem::new(num).pack().spanned(self.span()); + let loc = loc.variant(1); + // Add zero-width weak spacing to make the footnote "sticky". + Ok(HElem::hole().pack() + sup.linked(Destination::Location(loc))) } } diff --git a/crates/typst/src/model/link.rs b/crates/typst/src/model/link.rs index 7b39e90b9..22500c3b4 100644 --- a/crates/typst/src/model/link.rs +++ b/crates/typst/src/model/link.rs @@ -95,13 +95,11 @@ impl Show for Packed { let body = self.body().clone(); let linked = match self.dest() { LinkTarget::Dest(dest) => body.linked(dest.clone()), - LinkTarget::Label(label) => engine - .delayed(|engine| { - let elem = engine.introspector.query_label(*label).at(self.span())?; - let dest = Destination::Location(elem.location().unwrap()); - Ok(Some(body.clone().linked(dest))) - }) - .unwrap_or(body), + LinkTarget::Label(label) => { + let elem = engine.introspector.query_label(*label).at(self.span())?; + let dest = Destination::Location(elem.location().unwrap()); + body.clone().linked(dest) + } }; Ok(linked.styled(TextElem::set_hyphenate(Hyphenate(Smart::Custom(false))))) diff --git a/crates/typst/src/model/reference.rs b/crates/typst/src/model/reference.rs index 7109f2f5b..00ab8b4dd 100644 --- a/crates/typst/src/model/reference.rs +++ b/crates/typst/src/model/reference.rs @@ -163,78 +163,73 @@ impl Synthesize for Packed { impl Show for Packed { #[typst_macros::time(name = "ref", span = self.span())] fn show(&self, engine: &mut Engine, styles: StyleChain) -> SourceResult { - Ok(engine.delayed(|engine| { - let target = *self.target(); - let elem = engine.introspector.query_label(target); - let span = self.span(); + let target = *self.target(); + let elem = engine.introspector.query_label(target); + let span = self.span(); - if BibliographyElem::has(engine, target) { - if elem.is_ok() { - bail!(span, "label occurs in the document and its bibliography"); - } - - return Ok(to_citation(self, engine, styles)?.pack().spanned(span)); + if BibliographyElem::has(engine, target) { + if elem.is_ok() { + bail!(span, "label occurs in the document and its bibliography"); } - let elem = elem.at(span)?; + return Ok(to_citation(self, engine, styles)?.pack().spanned(span)); + } - if elem.func() == FootnoteElem::elem() { - return Ok(FootnoteElem::with_label(target).pack().spanned(span)); - } + let elem = elem.at(span)?; - let elem = elem.clone(); - let refable = elem - .with::() - .ok_or_else(|| { - if elem.can::() { - eco_format!( - "cannot reference {} directly, try putting it into a figure", - elem.func().name() - ) - } else { - eco_format!("cannot reference {}", elem.func().name()) - } - }) - .at(span)?; + if elem.func() == FootnoteElem::elem() { + return Ok(FootnoteElem::with_label(target).pack().spanned(span)); + } - let numbering = refable - .numbering() - .ok_or_else(|| { + let elem = elem.clone(); + let refable = elem + .with::() + .ok_or_else(|| { + if elem.can::() { eco_format!( - "cannot reference {} without numbering", + "cannot reference {} directly, try putting it into a figure", elem.func().name() ) - }) - .hint(eco_format!( - "you can enable {} numbering with `#set {}(numbering: \"1.\")`", - elem.func().name(), - if elem.func() == EquationElem::elem() { - "math.equation" - } else { - elem.func().name() - } - )) - .at(span)?; + } else { + eco_format!("cannot reference {}", elem.func().name()) + } + }) + .at(span)?; - let loc = elem.location().unwrap(); - let numbers = refable - .counter() - .at(engine, loc)? - .display(engine, &numbering.clone().trimmed())?; + let numbering = refable + .numbering() + .ok_or_else(|| { + eco_format!("cannot reference {} without numbering", elem.func().name()) + }) + .hint(eco_format!( + "you can enable {} numbering with `#set {}(numbering: \"1.\")`", + elem.func().name(), + if elem.func() == EquationElem::elem() { + "math.equation" + } else { + elem.func().name() + } + )) + .at(span)?; - let supplement = match self.supplement(styles).as_ref() { - Smart::Auto => refable.supplement(), - Smart::Custom(None) => Content::empty(), - Smart::Custom(Some(supplement)) => supplement.resolve(engine, [elem])?, - }; + let loc = elem.location().unwrap(); + let numbers = refable + .counter() + .at(engine, loc)? + .display(engine, &numbering.clone().trimmed())?; - let mut content = numbers; - if !supplement.is_empty() { - content = supplement + TextElem::packed("\u{a0}") + content; - } + let supplement = match self.supplement(styles).as_ref() { + Smart::Auto => refable.supplement(), + Smart::Custom(None) => Content::empty(), + Smart::Custom(Some(supplement)) => supplement.resolve(engine, [elem])?, + }; - Ok(content.linked(Destination::Location(loc))) - })) + let mut content = numbers; + if !supplement.is_empty() { + content = supplement + TextElem::packed("\u{a0}") + content; + } + + Ok(content.linked(Destination::Location(loc))) } } diff --git a/crates/typst/src/realize/mod.rs b/crates/typst/src/realize/mod.rs index 7c0980f65..191d9eedc 100644 --- a/crates/typst/src/realize/mod.rs +++ b/crates/typst/src/realize/mod.rs @@ -92,18 +92,17 @@ pub fn realize( meta = prepare(engine, &mut target, &mut map, styles)?; } - // Apply the step. + // Apply a step, if there is one. let mut output = match step { - // Apply a user-defined show rule. - Some(Step::Recipe(recipe, guard)) => show(engine, target, recipe, guard)?, - - // If the verdict picks this step, the `target` is guaranteed - // to have a built-in show rule. - Some(Step::Builtin) => { - target.with::().unwrap().show(engine, styles.chain(&map))? + Some(step) => { + // Errors in show rules don't terminate compilation immediately. We + // just continue with empty content for them and show all errors + // together, if they remain by the end of the introspection loop. + // + // This way, we can ignore errors that only occur in earlier + // iterations and also show more useful errors at once. + engine.delayed(|engine| show(engine, target, step, styles.chain(&map))) } - - // Nothing to do. None => target, }; @@ -122,12 +121,12 @@ struct Verdict<'a> { prepared: bool, /// A map of styles to apply to the element. map: Styles, - /// An optional transformation step to apply to the element. - step: Option>, + /// An optional show rule transformation to apply to the element. + step: Option>, } -/// An optional transformation step to apply to an element. -enum Step<'a> { +/// An optional show rule transformation to apply to the element. +enum ShowStep<'a> { /// A user-defined transformational show rule. Recipe(&'a Recipe, RecipeIndex), /// The built-in show rule. @@ -200,7 +199,7 @@ fn verdict<'a>( // If we find a matching, unguarded replacement show rule, // remember it, but still continue searching for potential // show-set styles that might change the verdict. - step = Some(Step::Recipe(recipe, index)); + step = Some(ShowStep::Recipe(recipe, index)); // If we found a show rule and are already prepared, there is // nothing else to do, so we can just break. @@ -215,7 +214,7 @@ fn verdict<'a>( // If we found no user-defined rule, also consider the built-in show rule. if step.is_none() && target.can::() { - step = Some(Step::Builtin); + step = Some(ShowStep::Builtin); } // If there's no nothing to do, there is also no verdict. @@ -286,21 +285,30 @@ fn prepare( Ok(None) } -/// Apply a user-defined show rule. +/// Apply a step. fn show( engine: &mut Engine, target: Content, - recipe: &Recipe, - index: RecipeIndex, + step: ShowStep, + styles: StyleChain, ) -> SourceResult { - match &recipe.selector { - Some(Selector::Regex(regex)) => { - // If the verdict picks this rule, the `target` is guaranteed - // to be a text element. - let text = target.into_packed::().unwrap(); - show_regex(engine, &text, regex, recipe, index) - } - _ => recipe.apply(engine, target.guarded(index)), + match step { + // Apply a user-defined show rule. + ShowStep::Recipe(recipe, guard) => match &recipe.selector { + // If the selector is a regex, the `target` is guaranteed to be a + // text element. This invokes special regex handling. + Some(Selector::Regex(regex)) => { + let text = target.into_packed::().unwrap(); + show_regex(engine, &text, regex, recipe, guard) + } + + // Just apply the recipe. + _ => recipe.apply(engine, target.guarded(guard)), + }, + + // If the verdict picks this step, the `target` is guaranteed to have a + // built-in show rule. + ShowStep::Builtin => target.with::().unwrap().show(engine, styles), } } diff --git a/tests/typ/compiler/delayed-error.typ b/tests/typ/compiler/delayed-error.typ new file mode 100644 index 000000000..c0bfba4c4 --- /dev/null +++ b/tests/typ/compiler/delayed-error.typ @@ -0,0 +1,11 @@ +// Test that errors in show rules are delayed: There can be multiple at once. + +--- +// Error: 26-34 panicked with: "hey1" +#show heading: _ => panic("hey1") + +// Error: 25-33 panicked with: "hey2" +#show strong: _ => panic("hey2") + += Hello +*strong*