fix(template): implement layout chaining via front matter directive#278
Conversation
…ective Layouts can now reference a parent layout via front matter, enabling multi-level composition (e.g., page → has-toc → base). The pipeline renders inside-out, stripping front matter at each level. - StripLayoutFrontMatter: removes YAML front matter from layout content - ResolveLayoutChain: follows parent references up to 10 levels deep - Build pipeline: renders through full chain in Build(), BuildWithContent() - BuildWithContent: now populates RenderedContent and resolves layouts Closes #276 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
zeroedin
left a comment
There was a problem hiding this comment.
PR #278 Review — Implement Layout Chaining (#276)
Scope: layout.go (+36/-3), build.go (+123/-43). Implements StripLayoutFrontMatter, ResolveLayoutChain, and wires chaining into all three build paths.
StripLayoutFrontMatter
func StripLayoutFrontMatter(s string) string {
if !strings.HasPrefix(s, "---") { return s }
rest := s[3:]
// skip newline after opening ---
idx := strings.Index(rest, "\n---")
// return content after closing ---
}Correct: finds --- at start, then \n--- as the closer, returns everything after. Handles optional newline after delimiters. Returns original content if no front matter found. Simple and sufficient.
ResolveLayoutChain
func ResolveLayoutChain(layoutPath string, layoutsDir string, engine string) ([]string, error) {
chain := []string{layoutPath}
for i := 0; i < maxDepth; i++ {
parent := ExtractLayoutParent(current)
if parent == "" { return chain, nil }
parentPath := filepath.Join(layoutsDir, parent+ext)
// verify exists, append, continue
}
return nil, fmt.Errorf("layout chain exceeds maximum depth of %d levels", maxDepth)
}Correct: starts with the initial layout, follows layout: directives up the chain, stops at root (no parent) or max depth 10. Returns ordered list innermost → root. Error on missing parent or exceeded depth.
Pipeline Wiring (build.go)
Both i18n and single-language paths updated identically:
ResolveLayoutChainreplaces single layout read- Track ALL layouts in chain for cache invalidation
- Render inside-out loop: for each layout in chain, strip front matter, parse, render with
{{ content }}from previous level
BuildWithContent — Gets layout resolution + chaining added (was previously missing). Creates a template engine, resolves layouts from the temp directory, renders through the chain. Also adds RenderedContent to the result (was missing from BuildWithContent).
Issues
1. Cache tracking only stores the LAST layout in the chain (Medium)
for _, lp := range chain {
trackedLayout := filepath.ToSlash(filepath.Clean(lp))
if relLayout, relErr := filepath.Rel(cfg.ProjectRoot, lp); relErr == nil {
trackedLayout = filepath.ToSlash(relLayout)
}
templateUsage[page.RelPath] = trackedLayout // overwrites each iteration
}templateUsage is map[string]string — each page maps to ONE layout. The loop overwrites the value on each iteration, so only the last layout in the chain (the root) is tracked. If the middle layout changes, the page won't be invalidated.
The spec (IMPLEMENTATION.md step 12b) says: "Track ALL layouts in the chain (not just the innermost)." templateUsage needs to be map[string][]string or cache.TrackTemplateUsage needs to be called per layout (it already supports multiple calls per page — it appends to a slice internally).
Fix: replace the templateUsage[page.RelPath] = trackedLayout loop with direct TrackTemplateUsage calls:
for _, lp := range chain {
relLayout := filepath.ToSlash(filepath.Clean(lp))
if r, err := filepath.Rel(cfg.ProjectRoot, lp); err == nil {
relLayout = filepath.ToSlash(r)
}
buildCache.TrackTemplateUsage(page.RelPath, relLayout)
}This is the same bug in both the i18n and single-language paths.
2. BuildWithContent creates a second template engine (Low)
BuildWithContent now creates its own tmpl.TemplateEngine for layout rendering. The main Build function already has an engine created earlier. BuildWithContent creates a separate one with RegisterBuiltinFilters but without plugin filter/shortcode bridging. Content rendered by BuildWithContent through layouts won't have access to plugin-registered filters or shortcodes. This is acceptable for test usage but would break if BuildWithContent is ever used with plugin-dependent layouts.
3. BuildWithContent now returns RenderedContent — Good
Previously BuildWithContent didn't populate RenderedContent. Now it does, matching Build(). This enables PR #277's integration tests to assert on result.RenderedContent["page.md"].
Verdict
Core implementation is correct — StripLayoutFrontMatter and ResolveLayoutChain work, inside-out rendering is properly wired in all three build paths. One bug: cache tracking overwrites instead of accumulating — only the root layout is tracked for invalidation, not the full chain. Both pipeline paths have this issue. Otherwise clean.
|
Follow-up on observation #2 (BuildWithContent creates a second engine): On reflection, this is worse than "acceptable for tests."
Options:
Option 1 is the cleanest — This should be tracked as a separate issue. |
There was a problem hiding this comment.
Pull request overview
Implements layout chaining for template layouts by allowing layouts to declare a parent via layout: in YAML front matter, stripping that front matter before template parsing, and rendering pages through the resolved chain (innermost → root). This addresses issue #276 where layout front matter was previously emitted as literal output.
Changes:
- Implement
StripLayoutFrontMatterandResolveLayoutChainininternal/template/layout.go. - Update
Build()to resolve and render a full layout chain (both i18n/batched and single-language paths). - Update
BuildWithContent()to resolve/apply layouts and populateBuildResult.RenderedContent.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| internal/template/layout.go | Adds front-matter stripping and parent-chain resolution for layouts. |
| internal/pipeline/build.go | Wires layout chaining into the build pipeline and BuildWithContent() output. |
Comments suppressed due to low confidence (1)
internal/pipeline/build.go:453
renderPageFormatsreads/parsesfmtLayoutPathas-is and doesn’t applyStripLayoutFrontMatterorResolveLayoutChain. That means format-specific layouts (e.g.,single.json.liquid) can still leak---\nlayout: ...\n---into output and won’t support chaining, even though HTML layout rendering now does. Consider applying the same front-matter stripping and chaining logic for format layouts before rendering.
// Multi-format output: render additional formats (spec §1e)
if err := renderPageFormats(page, layoutsDir, engineName, engine, cfg, siteData, langContexts, pages, batch.collections); err != nil {
return nil, err
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Architectural concern:
func BuildWithContent(cfg *config.Config, contentMap map[string]string) (*BuildResult, error) {
tmpDir := writeTempFiles(contentMap)
defer os.RemoveAll(tmpDir)
cfg.ProjectRoot = tmpDir
return Build(cfg)
}Write the files, point the config at the temp dir, call The current implementation has grown organically — each PR that added a feature to For the developer: If this refactor needs specification work (e.g., how |
- Track all layouts in the chain for cache invalidation by changing templateUsage to map[string][]string and calling TrackTemplateUsage for each layout in the chain - Fix StripLayoutFrontMatter edge case with empty front matter (---\n---\n) - Strip front matter in renderPageFormats for format-specific layouts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
layout:in front matter (e.g.,page → has-toc → base)ResolveLayoutChain()follows parent references up to 10 levels deep, failing on excessive depthBuild()(both batched and single-language paths) andBuildWithContent()BuildWithContent()now also resolves layouts and populatesRenderedContentin the resultTest plan
go test ./...— all 20 packages pass, zero failuresextractLayoutParentreadslayout:from layout front matterextractLayoutParentreturns empty for root layoutsStripLayoutFrontMatterremoves front matter delimiters and directivesResolveLayoutChainbuilds ordered chain (innermost → root)Closes #276
🤖 Generated with Claude Code