87 refactor loom tree builder - #88
Merged
Merged
Conversation
…stry DocraftLoomTreeBuilder::build() was a ~17-branch if/else over tag_name dispatching to one build_<Tag>() method each, all living in a single 1480-line .cc -- adding or maintaining a Craft-language tag meant editing that one shared file. Replace it with DocraftLoomTagHandlerRegistry, a tag -> IDocraftLoomTagHandler map mirroring the registry docraft::craft::DocraftCraftLanguageParser already uses one stage earlier, and the DocraftChartBuilderRegistry precedent. Adding a tag is now one handler file plus one registration line in docraft_loom_builtin_tag_handlers.cc. Handlers reach the builder's state (color/template resolution, child recursion, Foreach item) through two new interfaces -- DocraftLoomTreeBuilderContext (most tags) and the wider DocraftLoomTableHandlerContext (Table only) -- rather than being friended into all of DocraftLoomTreeBuilder's internals. Table (the most cross-cutting tag, see .local/TABELLE_LOOM.md) was migrated last into its own substantial handler, moved close to verbatim: this isolates its complexity rather than solving it. DocraftLoomTreeBuilder shrinks from 1480 to 244 lines (.cc) and 275 to 169 lines (.h). Also, smaller pipeline cleanup: - DocraftLoomVStack gains resolve_vertical_child_gaps(), closing the asymmetry with DocraftLoomHStack's existing resolve_horizontal_child_gaps(); MeasureProcessor and LayoutProcessor's VStack visits now share it instead of hand-rolling the same gap loop. - DocraftLoomMeasureProcessor gains a private incoming_width() helper, mirroring DocraftLoomLayoutProcessor's existing one, deduplicating 5 identical inline ternaries. - parse_page_size/parse_page_orientation move into a new docraft_craft_enum_parsers.h/.cc, and parse_chart_axis_position into docraft_chart_types.h/.cc, out of anonymous namespaces that made them unreachable from anywhere but their original file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
DocraftLoomTable::row_height() (and every other reader of frame/
page_index) relied on a doc comment ("once Layout has resolved it")
to keep call sites from reading pipeline-stage output before that
stage ran -- nothing enforced it. Before row_height() existed, exactly
that kind of cross-phase drift caused bug #54 (see
.local/TABELLE_LOOM.md): Pagination re-derived a row's height
differently in different call sites, so try_split_table's fit_rows
never made progress and looped forever on an oversized row after a
header.
Generalize the fix to the whole LayoutBox rather than special-casing
Table: `frame` and `page_index` move behind two capability tokens,
LayoutProof and PageIndexProof, each mintable only through a
dedicated, protected-only accessor -- DocraftLoomLayoutBoxLayoutAccessor
and DocraftLoomLayoutBoxPaginationAccessor. DocraftLoomLayoutProcessor
inherits only the first (can seal a frame, never a page index);
DocraftLoomPaginationProcessor inherits only the second, and its proof
constructor requires an existing LayoutProof, so minting one is itself
evidence Layout already ran -- Measure -> Layout -> Pagination is now
enforced by the type system, not convention. Code with no proof in
hand (Measure, or any future pass) simply cannot name the call.
The two accessors are the only friends LayoutProof/PageIndexProof
declare -- docraft_loom_node.h has no knowledge of pipeline or test
class names, and adding a future legitimate minter never means editing
it again. Tests needing to fabricate LayoutBox state directly (rather
than run the real pipeline) go through
docraft::test::utils::LayoutBoxTestAccess, which inherits both
accessors privately -- kept out of the production header entirely,
the same way charts/handlers keep their own registries out of the
core model.
measured_size stays ungated (Measure is unconditionally first, so
there's no early-read case to guard). page_index gets an additional
ungated page_index_or_unpaginated() accessor for
DocraftLoomRenderingProcessor::should_render(), since "not yet
paginated" has one safe answer (render on every page) -- unlike frame,
which has no safe placeholder geometry to fall back to.
docraft/loom/nodes/docraft_loom_layout_box_access.h adds
sealed_frame()/sealed_edit_frame()/layout_proof_or_throw() for the
~80 production call sites (Pagination, Rendering, Table row/cell
splitting, DocraftLoomPdfCreator) that read an already-sealed proof
rather than minting a new one.
Verified: full test suite green, and showcase.craft/charts.craft
render to byte-identical PDFs before and after.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.