add Deref to Repeatable

honestly, all the unwrapping was just generating noise.

crates/typst-layout/src/grid/layouter.rs

@@ -236,54 +236,12 @@ impl<'a> GridLayouter<'a> {
         let mut y = 0;
         let mut consecutive_header_count = 0;
         while y < self.grid.rows.len() {
-            if let Some(first_header) =
+            if let Some(next_header) = self.upcoming_headers.get(consecutive_header_count)
-                self.upcoming_headers.get(consecutive_header_count)
             {
-                if first_header.unwrap().range().contains(&y) {
+                if next_header.range().contains(&y) {
-                    consecutive_header_count += 1;
+                    self.place_new_headers(&mut consecutive_header_count, engine)?;
+                    y = next_header.end;
 
-                    // TODO: surely there is a better way to do this
-                    match self.upcoming_headers.get(consecutive_header_count) {
-                        // No more headers, so place the latest headers.
-                        None => {
-                            self.place_new_headers(
-                                consecutive_header_count,
-                                None,
-                                engine,
-                            )?;
-                            consecutive_header_count = 0;
-                        }
-                        // Next header is not consecutive, so place the latest headers.
-                        Some(next_header)
-                            if next_header.unwrap().start > first_header.unwrap().end =>
-                        {
-                            self.place_new_headers(
-                                consecutive_header_count,
-                                None,
-                                engine,
-                            )?;
-                            consecutive_header_count = 0;
-                        }
-                        // Next header is consecutive and conflicts with one or
-                        // more of the latest consecutive headers, so we must
-                        // place them before proceeding.
-                        Some(next_header)
-                            if next_header.unwrap().level
-                                <= first_header.unwrap().level =>
-                        {
-                            self.place_new_headers(
-                                consecutive_header_count,
-                                Some(next_header),
-                                engine,
-                            )?;
-                            consecutive_header_count = 0;
-                        }
-                        // Next header is a non-conflicting consecutive header.
-                        // Keep collecting more headers.
-                        _ => {}
-                    }
-
-                    y = first_header.unwrap().end;
                     // Skip header rows during normal layout.
                     continue;
                 }
@@ -1504,63 +1462,59 @@ impl<'a> GridLayouter<'a> {
                 self.current.repeated_header_rows.min(self.lrows.len());
         }
 
-        let footer_would_be_widow = if let Some(last_header_row) = self
+        let footer_would_be_widow =
-            .current
+            if !self.lrows.is_empty() && self.current.repeated_header_rows > 0 {
-            .repeated_header_rows
+                // If headers are repeating, then we already know they are not
-            .checked_sub(1)
+                // short-lived as that is checked, so they have orphan prevention.
-            .and_then(|last_header_index| self.lrows.get(last_header_index))
+                if self.lrows.len() == self.current.repeated_header_rows
-        {
+                    && may_progress_with_offset(
-            let last_header_end = last_header_row.index();
+                        self.regions,
-            if self.grid.rows.len() > last_header_end
+                        // Since we're trying to find a region where to place all
-                && self
+                        // repeating + pending headers, it makes sense to use
-                    .grid
+                        // 'header_height' and include even non-repeating pending
-                    .footer
+                        // headers for this check.
-                    .as_ref()
+                        self.current.header_height + self.current.footer_height,
-                    .and_then(Repeatable::as_repeated)
+                    )
-                    .is_none_or(|footer| footer.start != last_header_end)
+                {
-                && self.lrows.len() == self.current.repeated_header_rows
+                    // Header and footer would be alone in this region, but
-                && may_progress_with_offset(
+                    // there are more rows beyond the headers and the footer.
-                    self.regions,
+                    // Push an empty region.
-                    // Since we're trying to find a region where to place all
+                    self.lrows.clear();
-                    // repeating + pending headers, it makes sense to use
+                    self.current.last_repeated_header_end = 0;
-                    // 'header_height' and include even non-repeating pending
+                    self.current.repeated_header_rows = 0;
-                    // headers for this check.
+                    true
-                    self.current.header_height + self.current.footer_height,
+                } else {
-                )
+                    false
-            {
+                }
-                // Header and footer would be alone in this region, but there are more
+            } else if let Some(Repeatable::Repeated(footer)) = &self.grid.footer {
-                // rows beyond the header and the footer. Push an empty region.
+                // If no rows other than the footer have been laid out so far,
-                self.lrows.clear();
+                // and there are rows beside the footer, then don't lay it out
-                self.current.last_repeated_header_end = 0;
+                // at all. (Similar check from above, but for the case without
-                self.current.repeated_header_rows = 0;
+                // headers.)
-                true
+                // TODO: widow prevention for non-repeated footers with a
+                // similar mechanism / when implementing multiple footers.
+                self.lrows.is_empty()
+                    && may_progress_with_offset(
+                        self.regions,
+                        // This header height isn't doing much as we just
+                        // confirmed that there are no headers in this region,
+                        // but let's keep it here for correctness. It will add
+                        // zero anyway.
+                        self.current.header_height + self.current.footer_height,
+                    )
+                    && footer.start != 0
             } else {
                 false
-            }
+            };
-        } else if let Some(Repeatable::Repeated(footer)) = &self.grid.footer {
-            // If no rows other than the footer have been laid out so far, and
-            // there are rows beside the footer, then don't lay it out at all.
-            // (Similar check from above, but for the case without headers.)
-            // TODO: widow prevention for non-repeated footers with a similar
-            // mechanism / when implementing multiple footers.
-            self.lrows.is_empty()
-                && may_progress_with_offset(
-                    self.regions,
-                    // This header height isn't doing much as we just confirmed
-                    // that there are no headers in this region, but let's keep
-                    // it here for correctness. It will add zero anyway.
-                    self.current.header_height + self.current.footer_height,
-                )
-                && footer.start != 0
-        } else {
-            false
-        };
 
         let mut laid_out_footer_start = None;
         if !footer_would_be_widow {
             if let Some(Repeatable::Repeated(footer)) = &self.grid.footer {
-                // Don't layout the footer if it would be alone with the header in
+                // Don't layout the footer if it would be alone with the header
-                // the page (hence the widow check), and don't layout it twice.
+                // in the page (hence the widow check), and don't layout it
+                // twice.
+                // TODO: this check can be replaced by a vector of repeating
+                // footers in the future.
                 if self.lrows.iter().all(|row| row.index() < footer.start) {
                     laid_out_footer_start = Some(footer.start);
                     self.layout_footer(footer, engine, self.finished.len())?;

crates/typst-layout/src/grid/repeated.rs

@@ -9,71 +9,43 @@ use super::rowspans::UnbreakableRowGroup;
 impl<'a> GridLayouter<'a> {
     pub fn place_new_headers(
         &mut self,
-        consecutive_header_count: usize,
+        consecutive_header_count: &mut usize,
-        conflicting_header: Option<&Repeatable<Header>>,
         engine: &mut Engine,
     ) -> SourceResult<()> {
+        *consecutive_header_count += 1;
         let (consecutive_headers, new_upcoming_headers) =
-            self.upcoming_headers.split_at(consecutive_header_count);
+            self.upcoming_headers.split_at(*consecutive_header_count);
+
+        if new_upcoming_headers.first().is_some_and(|next_header| {
+            consecutive_headers.last().is_none_or(|latest_header| {
+                !latest_header.short_lived && next_header.start == latest_header.end
+            }) && !next_header.short_lived
+        }) {
+            // More headers coming, so wait until we reach them.
+            // TODO: refactor
+            return Ok(());
+        }
+
         self.upcoming_headers = new_upcoming_headers;
+        *consecutive_header_count = 0;
 
-        let (non_conflicting_headers, conflicting_headers) = match conflicting_header {
+        // Layout short-lived headers immediately.
-            // Headers succeeded by end of grid or footer are short lived and
+        if consecutive_headers.last().is_some_and(|h| h.short_lived) {
-            // can be placed in separate regions (no orphan prevention).
+            // No chance of orphans as we're immediately placing conflicting
-            // TODO: do this during grid resolving?
+            // headers afterwards, which basically are not headers, for all intents
-            // might be needed for multiple footers. Or maybe not if we check
+            // and purposes. It is therefore guaranteed that all new headers have
-            // "upcoming_footers" (O(1) here), however that looks like.
+            // been placed at least once.
-            _ if consecutive_headers
-                .last()
-                .is_some_and(|x| x.unwrap().end == self.grid.rows.len())
-                || self
-                    .grid
-                    .footer
-                    .as_ref()
-                    .zip(consecutive_headers.last())
-                    .is_some_and(|(f, h)| f.unwrap().start == h.unwrap().end) =>
-            {
-                (Default::default(), consecutive_headers)
-            }
-
-            Some(conflicting_header) => {
-                // All immediately conflicting headers will
-                // be laid out without orphan prevention.
-                consecutive_headers.split_at(consecutive_headers.partition_point(|h| {
-                    conflicting_header.unwrap().level > h.unwrap().level
-                }))
-            }
-            _ => (consecutive_headers, Default::default()),
-        };
-
-        self.layout_new_pending_headers(non_conflicting_headers, engine)?;
-
-        // Layout each conflicting header independently, without orphan
-        // prevention (as they don't go into 'pending_headers').
-        // These headers are short-lived as they are immediately followed by a
-        // header of the same or lower level, such that they never actually get
-        // to repeat.
-        for conflicting_header in conflicting_headers.chunks_exact(1) {
-            self.layout_new_headers(
-                // Using 'chunks_exact", we pass a slice of length one instead
-                // of a reference for type consistency.
-                // In addition, this is the only place where we layout
-                // short-lived headers.
-                conflicting_header,
-                true,
-                engine,
-            )?
-        }
-
-        // No chance of orphans as we're immediately placing conflicting
-        // headers afterwards, which basically are not headers, for all intents
-        // and purposes. It is therefore guaranteed that all new headers have
-        // been placed at least once.
-        if !conflicting_headers.is_empty() {
             self.flush_pending_headers();
-        }
 
-        Ok(())
+            // Layout each conflicting header independently, without orphan
+            // prevention (as they don't go into 'pending_headers').
+            // These headers are short-lived as they are immediately followed by a
+            // header of the same or lower level, such that they never actually get
+            // to repeat.
+            self.layout_new_headers(consecutive_headers, true, engine)
+        } else {
+            self.layout_new_pending_headers(consecutive_headers, engine)
+        }
     }
 
     /// Lays out a row while indicating that it should store its persistent
@@ -131,7 +103,7 @@ impl<'a> GridLayouter<'a> {
 
         // Assuming non-conflicting headers sorted by increasing y, this must
         // be the header with the lowest level (sorted by increasing levels).
-        let first_level = first_header.unwrap().level;
+        let first_level = first_header.level;
 
         // Stop repeating conflicting headers.
         // If we go to a new region before the pending headers fit alongside

crates/typst-library/src/layout/grid/resolve.rs

@@ -1,5 +1,5 @@
 use std::num::{NonZeroU32, NonZeroUsize};
-use std::ops::Range;
+use std::ops::{Deref, Range};
 use std::sync::Arc;
 
 use ecow::eco_format;
@@ -438,6 +438,12 @@ pub struct Header {
     /// lower level header stops repeating, all higher level headers do as
     /// well.
     pub level: u32,
+    /// Whether this header cannot be repeated nor should have orphan
+    /// prevention because it would be about to cease repetition, either
+    /// because it is followed by headers of conflicting levels, or because
+    /// it is at the end of the table (possibly followed by some footers at the
+    /// end).
+    pub short_lived: bool,
 }
 
 impl Header {
@@ -469,15 +475,26 @@ impl Footer {
     }
 }
 
-/// A possibly repeatable grid object.
+/// A possibly repeatable grid child.
+///
 /// It still exists even when not repeatable, but must not have additional
 /// considerations by grid layout, other than for consistency (such as making
 /// a certain group of rows unbreakable).
 pub enum Repeatable<T> {
+    /// The user asked this grid child to repeat.
     Repeated(T),
+    /// The user asked this grid child to not repeat.
     NotRepeated(T),
 }
 
+impl<T> Deref for Repeatable<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        self.unwrap()
+    }
+}
+
 impl<T> Repeatable<T> {
     /// Gets the value inside this repeatable, regardless of whether
     /// it repeats.
@@ -490,7 +507,7 @@ impl<T> Repeatable<T> {
     }
 
     /// Gets the value inside this repeatable, regardless of whether
-    /// it repeats.
+    /// it repeats (mutably).
     #[inline]
     pub fn unwrap_mut(&mut self) -> &mut T {
         match self {
@@ -1540,8 +1557,29 @@ impl<'x> CellGridResolver<'_, '_, 'x> {
                         end: group_range.end,
 
                         level: row_group.repeatable_level.get(),
+
+                        // This can only change at a later iteration, if we
+                        // find a conflicting header or footer right away.
+                        short_lived: false,
                     };
 
+                    // Mark consecutive headers right before this one as short
+                    // lived if they would have a higher or equal level, as
+                    // then they would immediately stop repeating during
+                    // layout.
+                    let mut consecutive_header_start = data.start;
+                    for conflicting_header in
+                        headers.iter_mut().rev().take_while(move |h| {
+                            let conflicts = h.end == consecutive_header_start
+                                && h.level >= data.level;
+
+                            consecutive_header_start = h.start;
+                            conflicts
+                        })
+                    {
+                        conflicting_header.unwrap_mut().short_lived = true;
+                    }
+
                     headers.push(if row_group.repeat {
                         Repeatable::Repeated(data)
                     } else {
@@ -1788,8 +1826,7 @@ impl<'x> CellGridResolver<'_, '_, 'x> {
 
                 // TODO: will need a global slice of headers and footers for
                 // when we have multiple footers
-                let last_header_end =
+                let last_header_end = headers.last().map(|header| header.end);
-                    headers.last().map(Repeatable::unwrap).map(|header| header.end);
 
                 if has_gutter {
                     // Convert the footer's start index to post-gutter coordinates.
@@ -1826,6 +1863,20 @@ impl<'x> CellGridResolver<'_, '_, 'x> {
                 }
             });
 
+        // Mark consecutive headers right before the end of the table, or the
+        // final footer, as short lived, given that there are no normal rows
+        // after them, so repeating them is pointless.
+        let mut consecutive_header_start =
+            footer.as_ref().map(|f| f.start).unwrap_or(row_amount);
+        for header_at_the_end in headers.iter_mut().rev().take_while(move |h| {
+            let at_the_end = h.end == consecutive_header_start;
+
+            consecutive_header_start = h.start;
+            at_the_end
+        }) {
+            header_at_the_end.unwrap_mut().short_lived = true;
+        }
+
         Ok(footer)
     }
 
@@ -2008,9 +2059,10 @@ fn check_for_conflicting_cell_row(
     // `y + 1 = header.start` holds, that means `y < header.start`, and it
     // only occupies one row (`y`), so the cell is actually not in
     // conflict.
-    if headers.iter().any(|header| {
+    if headers
-        cell_y < header.unwrap().end && cell_y + rowspan > header.unwrap().start
+        .iter()
-    }) {
+        .any(|header| cell_y < header.end && cell_y + rowspan > header.start)
+    {
         bail!(
             "cell would conflict with header spanning the same position";
             hint: "try moving the cell or the header"
@@ -2204,11 +2256,9 @@ fn find_next_available_position<const SKIP_ROWS: bool>(
             }
         // TODO: consider keeping vector of upcoming headers to make this check
         // non-quadratic (O(cells) instead of O(headers * cells)).
-        } else if let Some(header) =
+        } else if let Some(header) = headers.iter().find(|header| {
-            headers.iter().map(Repeatable::unwrap).find(|header| {
+            (header.start * columns..header.end * columns).contains(&resolved_index)
-                (header.start * columns..header.end * columns).contains(&resolved_index)
+        }) {
-            })
-        {
             // Skip header (can't place a cell inside it from outside it).
             resolved_index = header.end * columns;
 

crates/typst-library/src/model/table.rs

@@ -293,13 +293,13 @@ fn show_cellgrid_html(grid: CellGrid, styles: StyleChain) -> Content {
     };
 
     let footer = grid.footer.map(|ft| {
-        let rows = rows.drain(ft.unwrap().start..);
+        let rows = rows.drain(ft.start..);
         elem(tag::tfoot, Content::sequence(rows.map(|row| tr(tag::td, row))))
     });
     // TODO: Headers and footers in arbitrary positions
     // Right now, only those at either end are accepted
-    let header = grid.headers.first().filter(|h| h.unwrap().start == 0).map(|hd| {
+    let header = grid.headers.first().filter(|h| h.start == 0).map(|hd| {
-        let rows = rows.drain(..hd.unwrap().end);
+        let rows = rows.drain(..hd.end);
         elem(tag::thead, Content::sequence(rows.map(|row| tr(tag::th, row))))
     });