Refactor grid header and footer resolving#5919
Conversation
abf77f1 to
838bdc0
Compare
- Clearer code
They weren't doing much for us
laurmaedje
left a comment
There was a problem hiding this comment.
To be honest, I'm not familiar enough with the old code to properly review this change. I'd trust you on the change, but if you want we can also have a quick call where you walk me through the changes. Just trying to understanding everything from scratch by myself would be a bit too time-consuming right now.
Yeah sure, we can do that, even if just for a quick sanity check. |
PgBiel
left a comment
There was a problem hiding this comment.
Overall, the refactor was pretty "light" (I didn't move all the context variables I could into struct fields), but I think the effects were quite significant already.
|
What do you think about turning |
Not sure, in theory I can see it being useful from a library perspective in the future as well by providing a method or something which lets one access (and generate) the cellgrid before layout, within typst code. But i guess it doesn't really matter? |
(Draft as there's some polishing still missing, but keeping it open to track progress so far.)
Fixes #5359
This refactors grid resolving - that is, the step before grid and table layout where we parse the parameters, place cells in their appropriate locations and generate the final grid - in preparation for #5375 and #5377 to ensure the code can handle any amount of arbitrary row groups, that is, table children which aren't cells directly but span groups of entire rows, which right now only include headers and footers.
This also adjusts how manually-positioned cells behave with headers and footers. Before, you could place a cell before the header and then, even though the header would normally try to occupy the following row (so as to not conflict with that existing cell), you could force the header to occupy the first row as well with
grid.cell(y: 0)[...]inside the header. Instead of throwing an error, Typst would just happily accept that the cell outside of the header is now part of header, which is very unlikely to be the intention. So, headers and footers can no longer expand to include cells outside them. Instead, they will try to skip them, and cells outside headers and footers which don't pick their rows will always skip headers and footers, avoiding conflicts.In other words, conflicts between headers and footers and cells outside them are now only possible when you have cells with fixed
y, in which case this should always be an error now. This fixes #5359.TODO
Mostly just doing some final polishing before marking the PR as ready for review. There are some TODOs in the code, I'll tackle some now and the rest should be tackled in further PRs.