From d97d71948ebadebe87341649eeb4aae69c746ae1 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Fri, 16 Aug 2024 12:53:12 +0200 Subject: [PATCH] Fix document set rules (#4768) --- crates/typst-pdf/src/catalog.rs | 14 +++---- crates/typst/src/foundations/styles.rs | 9 +++++ crates/typst/src/layout/mod.rs | 4 +- crates/typst/src/model/document.rs | 48 +++++++++++++++++------ crates/typst/src/realize/mod.rs | 43 ++++++++++---------- tests/ref/document-set-after-content.png | Bin 0 -> 245 bytes tests/src/custom.rs | 46 ++++++++++++++++++++++ tests/src/run.rs | 13 ++++++ tests/src/tests.rs | 1 + tests/suite/model/document.typ | 28 ++++++++++--- 10 files changed, 158 insertions(+), 48 deletions(-) create mode 100644 tests/ref/document-set-after-content.png create mode 100644 tests/src/custom.rs diff --git a/crates/typst-pdf/src/catalog.rs b/crates/typst-pdf/src/catalog.rs index 7d52cc58d..8f3253eee 100644 --- a/crates/typst-pdf/src/catalog.rs +++ b/crates/typst-pdf/src/catalog.rs @@ -44,12 +44,12 @@ pub fn write_catalog( let info_ref = alloc.bump(); let mut info = pdf.document_info(info_ref); let mut xmp = XmpWriter::new(); - if let Some(title) = &ctx.document.title { + if let Some(title) = &ctx.document.info.title { info.title(TextStr(title)); xmp.title([(None, title.as_str())]); } - let authors = &ctx.document.author; + let authors = &ctx.document.info.author; if !authors.is_empty() { // Turns out that if the authors are given in both the document // information dictionary and the XMP metadata, Acrobat takes a little @@ -76,15 +76,15 @@ pub fn write_catalog( info.creator(TextStr(&creator)); xmp.creator_tool(&creator); - let keywords = &ctx.document.keywords; + let keywords = &ctx.document.info.keywords; if !keywords.is_empty() { let joined = keywords.join(", "); info.keywords(TextStr(&joined)); xmp.pdf_keywords(&joined); } - if let Some(date) = ctx.document.date.unwrap_or(timestamp) { - let tz = ctx.document.date.is_auto(); + if let Some(date) = ctx.document.info.date.unwrap_or(timestamp) { + let tz = ctx.document.info.date.is_auto(); if let Some(pdf_date) = pdf_date(date, tz) { info.creation_date(pdf_date); info.modified_date(pdf_date); @@ -109,10 +109,10 @@ pub fn write_catalog( let doc_id = if let Smart::Custom(ident) = ident { // We were provided with a stable ID. Yay! hash_base64(&(PDF_VERSION, ident)) - } else if ctx.document.title.is_some() && !ctx.document.author.is_empty() { + } else if ctx.document.info.title.is_some() && !ctx.document.info.author.is_empty() { // If not provided from the outside, but title and author were given, we // compute a hash of them, which should be reasonably stable and unique. - hash_base64(&(PDF_VERSION, &ctx.document.title, &ctx.document.author)) + hash_base64(&(PDF_VERSION, &ctx.document.info.title, &ctx.document.info.author)) } else { // The user provided no usable metadata which we can use as an `/ID`. instance_id.clone() diff --git a/crates/typst/src/foundations/styles.rs b/crates/typst/src/foundations/styles.rs index 48009c8c3..55bb348a0 100644 --- a/crates/typst/src/foundations/styles.rs +++ b/crates/typst/src/foundations/styles.rs @@ -133,6 +133,15 @@ impl Styles { self } + /// Whether there is a style for the given field of the given element. + pub fn has(&self, field: u8) -> bool { + let elem = T::elem(); + self.0 + .iter() + .filter_map(|style| style.property()) + .any(|property| property.is_of(elem) && property.id == field) + } + /// Returns `Some(_)` with an optional span if this list contains /// styles for the given element. pub fn interruption(&self) -> Option> { diff --git a/crates/typst/src/layout/mod.rs b/crates/typst/src/layout/mod.rs index 739d09224..f7f5c5b3b 100644 --- a/crates/typst/src/layout/mod.rs +++ b/crates/typst/src/layout/mod.rs @@ -149,9 +149,9 @@ impl Content { route: Route::extend(route).unnested(), }; let arenas = Arenas::default(); - let (document, styles) = + let (document, styles, info) = realize_doc(&mut engine, locator.next(&()), &arenas, content, styles)?; - document.layout(&mut engine, locator.next(&()), styles) + document.layout(&mut engine, locator.next(&()), styles, info) } cached( diff --git a/crates/typst/src/model/document.rs b/crates/typst/src/model/document.rs index 77044112d..71e9e7969 100644 --- a/crates/typst/src/model/document.rs +++ b/crates/typst/src/model/document.rs @@ -3,8 +3,8 @@ use ecow::EcoString; use crate::diag::{bail, HintedStrResult, SourceResult}; use crate::engine::Engine; use crate::foundations::{ - cast, elem, Args, Array, Construct, Content, Datetime, Packed, Smart, StyleChain, - Value, + cast, elem, Args, Array, Construct, Content, Datetime, Fields, Packed, Smart, + StyleChain, Styles, Value, }; use crate::introspection::{Introspector, Locator, ManualPageCounter}; use crate::layout::{Page, PageElem}; @@ -78,6 +78,7 @@ impl Packed { engine: &mut Engine, locator: Locator, styles: StyleChain, + info: DocumentInfo, ) -> SourceResult { let children = self.children(); let mut peekable = children.chain(&styles).peekable(); @@ -107,14 +108,7 @@ impl Packed { pages.extend(result?.finalize(engine, &mut page_counter)?); } - Ok(Document { - pages, - title: DocumentElem::title_in(styles).map(|content| content.plain_text()), - author: DocumentElem::author_in(styles).0, - keywords: DocumentElem::keywords_in(styles).0, - date: DocumentElem::date_in(styles), - introspector: Introspector::default(), - }) + Ok(Document { pages, info, introspector: Introspector::default() }) } } @@ -145,6 +139,15 @@ cast! { pub struct Document { /// The document's finished pages. pub pages: Vec, + /// Details about the document. + pub info: DocumentInfo, + /// Provides the ability to execute queries on the document. + pub introspector: Introspector, +} + +/// Details about the document. +#[derive(Debug, Default, Clone, PartialEq, Hash)] +pub struct DocumentInfo { /// The document's title. pub title: Option, /// The document's author. @@ -153,8 +156,29 @@ pub struct Document { pub keywords: Vec, /// The document's creation date. pub date: Smart>, - /// Provides the ability to execute queries on the document. - pub introspector: Introspector, +} + +impl DocumentInfo { + /// Populate this document info with details from the given styles. + /// + /// Document set rules are a bit special, so we need to do this manually. + pub fn populate(&mut self, styles: &Styles) { + let chain = StyleChain::new(styles); + let has = |field| styles.has::(field as _); + if has(::Enum::Title) { + self.title = + DocumentElem::title_in(chain).map(|content| content.plain_text()); + } + if has(::Enum::Author) { + self.author = DocumentElem::author_in(chain).0; + } + if has(::Enum::Keywords) { + self.keywords = DocumentElem::keywords_in(chain).0; + } + if has(::Enum::Date) { + self.date = DocumentElem::date_in(chain); + } + } } #[cfg(test)] diff --git a/crates/typst/src/realize/mod.rs b/crates/typst/src/realize/mod.rs index a67660c1f..721d4b4a6 100644 --- a/crates/typst/src/realize/mod.rs +++ b/crates/typst/src/realize/mod.rs @@ -30,8 +30,8 @@ use crate::layout::{ }; use crate::math::{EquationElem, LayoutMath}; use crate::model::{ - CiteElem, CiteGroup, DocumentElem, EnumElem, EnumItem, ListElem, ListItem, ParElem, - ParbreakElem, TermItem, TermsElem, + CiteElem, CiteGroup, DocumentElem, DocumentInfo, EnumElem, EnumItem, ListElem, + ListItem, ParElem, ParbreakElem, TermItem, TermsElem, }; use crate::syntax::Span; use crate::text::{LinebreakElem, SmartQuoteElem, SpaceElem, TextElem}; @@ -45,7 +45,7 @@ pub fn realize_doc<'a>( arenas: &'a Arenas<'a>, content: &'a Content, styles: StyleChain<'a>, -) -> SourceResult<(Packed, StyleChain<'a>)> { +) -> SourceResult<(Packed, StyleChain<'a>, DocumentInfo)> { let mut builder = Builder::new(engine, locator, arenas, true); builder.accept(content, styles)?; builder.interrupt_page(Some(styles), true)?; @@ -198,11 +198,21 @@ impl<'a, 'v, 't> Builder<'a, 'v, 't> { styled: &'a StyledElem, styles: StyleChain<'a>, ) -> SourceResult<()> { + let local = &styled.styles; let stored = self.arenas.store(styles); - let styles = stored.chain(&styled.styles); - self.interrupt_style(&styled.styles, None)?; + let styles = stored.chain(local); + + if let Some(Some(span)) = local.interruption::() { + let Some(doc) = &mut self.doc else { + bail!(span, "document set rules are not allowed inside of containers"); + }; + doc.info.populate(local); + } + + self.interrupt_style(local, None)?; self.accept(&styled.child, styles)?; - self.interrupt_style(&styled.styles, Some(styles))?; + self.interrupt_style(local, Some(styles))?; + Ok(()) } @@ -211,20 +221,6 @@ impl<'a, 'v, 't> Builder<'a, 'v, 't> { local: &Styles, outer: Option>, ) -> SourceResult<()> { - if let Some(Some(span)) = local.interruption::() { - let Some(doc) = &self.doc else { - bail!(span, "document set rules are not allowed inside of containers"); - }; - if outer.is_none() - && (!doc.pages.is_empty() - || !self.flow.0.is_empty() - || !self.par.0.is_empty() - || !self.list.items.is_empty() - || !self.cites.items.is_empty()) - { - bail!(span, "document set rules must appear before any content"); - } - } if let Some(Some(span)) = local.interruption::() { if self.doc.is_none() { bail!(span, "page configuration is not allowed inside of containers"); @@ -314,6 +310,8 @@ struct DocBuilder<'a> { keep_next: bool, /// Whether the next page should be cleared to an even or odd number. clear_next: Option, + /// Details about the document. + info: DocumentInfo, } impl<'a> DocBuilder<'a> { @@ -354,9 +352,9 @@ impl<'a> DocBuilder<'a> { /// Turns this builder into the resulting document, along with /// its [style chain][StyleChain]. - fn finish(self) -> (Packed, StyleChain<'a>) { + fn finish(self) -> (Packed, StyleChain<'a>, DocumentInfo) { let (children, trunk, span) = self.pages.finish(); - (Packed::new(DocumentElem::new(children)).spanned(span), trunk) + (Packed::new(DocumentElem::new(children)).spanned(span), trunk, self.info) } } @@ -366,6 +364,7 @@ impl Default for DocBuilder<'_> { pages: BehavedBuilder::new(), keep_next: true, clear_next: None, + info: DocumentInfo::default(), } } } diff --git a/tests/ref/document-set-after-content.png b/tests/ref/document-set-after-content.png new file mode 100644 index 0000000000000000000000000000000000000000..37e137732089dda55ea1e81a9ce645988bbf2b08 GIT binary patch literal 245 zcmV { + if $lhs != $rhs { + writeln!(&mut $sink, "{:?} != {:?}", $lhs, $rhs).unwrap(); + } + }; +} + +/// Run special checks for specific tests for which it is not worth it to create +/// custom annotations. +pub fn check(test: &Test, world: &TestWorld, doc: Option<&Document>) -> String { + let mut sink = String::new(); + match test.name.as_str() { + "document-set-author-date" => { + let info = info(doc); + test_eq!(sink, info.author, ["A", "B"]); + test_eq!(sink, info.date, Smart::Custom(world.today(None))); + } + "issue-4065-document-context" => { + let info = info(doc); + test_eq!(sink, info.title.as_deref(), Some("Top level")); + } + "issue-4769-document-context-conditional" => { + let info = info(doc); + test_eq!(sink, info.author, ["Changed"]); + test_eq!(sink, info.title.as_deref(), Some("Alternative")); + } + _ => {} + } + sink +} + +/// Extract the document information. +fn info(doc: Option<&Document>) -> DocumentInfo { + doc.map(|doc| doc.info.clone()).unwrap_or_default() +} diff --git a/tests/src/run.rs b/tests/src/run.rs index 9681ae4cc..c65f5e389 100644 --- a/tests/src/run.rs +++ b/tests/src/run.rs @@ -89,6 +89,7 @@ impl<'a> Runner<'a> { log!(self, "no document, but also no errors"); } + self.check_custom(doc.as_ref()); self.check_document(doc.as_ref()); for error in &errors { @@ -129,6 +130,18 @@ impl<'a> Runner<'a> { } } + /// Run custom checks for which it is not worth to create special + /// annotations. + fn check_custom(&mut self, doc: Option<&Document>) { + let errors = crate::custom::check(self.test, &self.world, doc); + if !errors.is_empty() { + log!(self, "custom check failed"); + for line in errors.lines() { + log!(self, " {line}"); + } + } + } + /// Check that the document output is correct. fn check_document(&mut self, document: Option<&Document>) { let live_path = format!("{}/render/{}.png", crate::STORE_PATH, self.test.name); diff --git a/tests/src/tests.rs b/tests/src/tests.rs index 3e29c0ffb..77c8b210b 100644 --- a/tests/src/tests.rs +++ b/tests/src/tests.rs @@ -2,6 +2,7 @@ mod args; mod collect; +mod custom; mod logger; mod run; mod world; diff --git a/tests/suite/model/document.typ b/tests/suite/model/document.typ index 6f8e71315..2ed199fee 100644 --- a/tests/suite/model/document.typ +++ b/tests/suite/model/document.typ @@ -1,12 +1,10 @@ // Test document and page-level styles. --- document-set-title --- -// This is okay. #set document(title: [Hello]) What's up? --- document-set-author-date --- -// This, too. #set document(author: ("A", "B"), date: datetime.today()) --- document-date-bad --- @@ -14,15 +12,14 @@ What's up? #set document(date: "today") --- document-author-bad --- -// This, too. // Error: 23-29 expected string, found integer #set document(author: (123,)) What's up? --- document-set-after-content --- +// Document set rules can appear anywhere in top-level realization, also after +// content. Hello - -// Error: 2-30 document set rules must appear before any content #set document(title: [Hello]) --- document-constructor --- @@ -34,3 +31,24 @@ Hello // Error: 4-32 document set rules are not allowed inside of containers #set document(title: [Hello]) ] + +--- issue-4065-document-context --- +// Test that we can set document properties based on context. +#show: body => context { + let all = query(heading) + let title = if all.len() > 0 { all.first().body } + set document(title: title) + body +} + +#show heading: none += Top level + +--- issue-4769-document-context-conditional --- +// Test that document set rule can be conditional on document information +// itself. +#set document(author: "Normal", title: "Alternative") +#context { + set document(author: "Changed") if "Normal" in document.author + set document(title: "Changed") if document.title == "Normal" +}