From e8eeec0b3eab1cec427ac05c768b94af79fce927 Mon Sep 17 00:00:00 2001 From: PgBiel <9021226+PgBiel@users.noreply.github.com> Date: Thu, 6 Mar 2025 01:06:47 -0300 Subject: [PATCH] improve header/footer expansion error message --- .../typst-library/src/layout/grid/resolve.rs | 43 ++++++++++--------- tests/suite/layout/grid/footers.typ | 16 +++---- tests/suite/layout/grid/headers.typ | 16 +++---- 3 files changed, 39 insertions(+), 36 deletions(-) diff --git a/crates/typst-library/src/layout/grid/resolve.rs b/crates/typst-library/src/layout/grid/resolve.rs index 9ea543a61..353785770 100644 --- a/crates/typst-library/src/layout/grid/resolve.rs +++ b/crates/typst-library/src/layout/grid/resolve.rs @@ -908,11 +908,21 @@ struct CellGridResolver<'a, 'b, 'x> { span: Span, } +#[derive(Debug, Clone, Copy)] enum RowGroupKind { Header, Footer, } +impl RowGroupKind { + fn name(self) -> &'static str { + match self { + Self::Header => "header", + Self::Footer => "footer", + } + } +} + struct RowGroupData { /// The range of rows of cells inside this grid row group. The /// first and last rows are guaranteed to have cells (an exception @@ -1456,11 +1466,14 @@ impl<'x> CellGridResolver<'_, '_, 'x> { // Cell's header or footer must expand to include the cell's // occupied positions, if possible. - if let Some(RowGroupData { range: group_range, .. }) = &mut row_group_data { + if let Some(RowGroupData { range: group_range, kind, .. }) = + &mut row_group_data + { *group_range = Some( expand_row_group( resolved_cells, group_range.as_ref(), + *kind, first_available_row, y, rowspan, @@ -1683,6 +1696,7 @@ impl<'x> CellGridResolver<'_, '_, 'x> { fn expand_row_group( resolved_cells: &[Option>], group_range: Option<&Range>, + group_kind: RowGroupKind, first_available_row: usize, cell_y: usize, rowspan: usize, @@ -1706,9 +1720,10 @@ fn expand_row_group( // 'y = 1' is the earliest row it can occupy. if new_group_start < first_available_row { bail!( - "cell would cause header or footer to expand to non-empty row {}", + "cell would cause {} to expand to non-empty row {}", + group_kind.name(), first_available_row.saturating_sub(1); - hint: "try moving its cells to later rows" + hint: "try moving its cells to available rows" ); } @@ -1770,23 +1785,11 @@ fn expand_row_group( .map(|cells| &cells[..columns.min(cells.len())]) { if new_row.iter().any(Option::is_some) { - // TODO: - // - Later/earlier rows might be confusing - // (moving to the end always works...) - // - Detect when header or footer collided with - // another header or footer and provide a - // better error message if so. - if group_range.is_none_or(|r| new_y < r.start) { - bail!( - "cell would cause header or footer to expand to non-empty row {new_y}"; - hint: "try moving its cells to later rows" - ); - } else { - bail!( - "cell would cause header or footer to expand to non-empty row {new_y}"; - hint: "try moving its cells to earlier rows" - ); - } + bail!( + "cell would cause {} to expand to non-empty row {new_y}", + group_kind.name(); + hint: "try moving its cells to available rows", + ) } } else { // Received 'None' or an empty slice, so we are expanding the diff --git a/tests/suite/layout/grid/footers.typ b/tests/suite/layout/grid/footers.typ index 263530f83..1d01a041b 100644 --- a/tests/suite/layout/grid/footers.typ +++ b/tests/suite/layout/grid/footers.typ @@ -458,8 +458,8 @@ table.footer( table.hline(stroke: red), table.hline(y: 1, stroke: aqua), - // Error: 5-24 cell would cause header or footer to expand to non-empty row 0 - // Hint: 5-24 try moving its cells to later rows + // Error: 5-24 cell would cause footer to expand to non-empty row 0 + // Hint: 5-24 try moving its cells to available rows table.cell(y: 0)[b], [c] ) @@ -474,8 +474,8 @@ table.cell(y: 1)[a], table.footer( [b], [c], - // Error: 6-7 cell would cause header or footer to expand to non-empty row 1 - // Hint: 6-7 try moving its cells to earlier rows + // Error: 6-7 cell would cause footer to expand to non-empty row 1 + // Hint: 6-7 try moving its cells to available rows [d], ), ) @@ -489,8 +489,8 @@ table.cell(y: 2)[a], table.footer( [b], [c], - // Error: 5-24 cell would cause header or footer to expand to non-empty row 2 - // Hint: 5-24 try moving its cells to earlier rows + // Error: 5-24 cell would cause footer to expand to non-empty row 2 + // Hint: 5-24 try moving its cells to available rows table.cell(y: 3)[d], ), ) @@ -500,8 +500,8 @@ columns: 2, table.header(), table.footer( - // Error: 5-24 cell would cause header or footer to expand to non-empty row 0 - // Hint: 5-24 try moving its cells to later rows + // Error: 5-24 cell would cause footer to expand to non-empty row 0 + // Hint: 5-24 try moving its cells to available rows table.cell(y: 0)[a] ), ) diff --git a/tests/suite/layout/grid/headers.typ b/tests/suite/layout/grid/headers.typ index 28ae753f1..229bce614 100644 --- a/tests/suite/layout/grid/headers.typ +++ b/tests/suite/layout/grid/headers.typ @@ -403,8 +403,8 @@ table.header( table.hline(stroke: red), table.hline(y: 1, stroke: aqua), - // Error: 5-24 cell would cause header or footer to expand to non-empty row 0 - // Hint: 5-24 try moving its cells to later rows + // Error: 5-24 cell would cause header to expand to non-empty row 0 + // Hint: 5-24 try moving its cells to available rows table.cell(y: 0)[b], [c] ) @@ -423,8 +423,8 @@ table.header( table.hline(stroke: red), table.hline(y: 3, stroke: aqua), - // Error: 5-24 cell would cause header or footer to expand to non-empty row 2 - // Hint: 5-24 try moving its cells to later rows + // Error: 5-24 cell would cause header to expand to non-empty row 2 + // Hint: 5-24 try moving its cells to available rows table.cell(y: 2)[b], ) ) @@ -438,8 +438,8 @@ table.cell(y: 1)[a], table.header( [b], [c], - // Error: 6-7 cell would cause header or footer to expand to non-empty row 1 - // Hint: 6-7 try moving its cells to earlier rows + // Error: 6-7 cell would cause header to expand to non-empty row 1 + // Hint: 6-7 try moving its cells to available rows [d], ), ) @@ -453,8 +453,8 @@ table.cell(y: 2)[a], table.header( [b], [c], - // Error: 5-24 cell would cause header or footer to expand to non-empty row 2 - // Hint: 5-24 try moving its cells to earlier rows + // Error: 5-24 cell would cause header to expand to non-empty row 2 + // Hint: 5-24 try moving its cells to available rows table.cell(y: 3)[d], ), )