Skip to content

feat: add sub-section support for pages#2321

Open
ArnaudLigny wants to merge 15 commits intomasterfrom
sub-sections
Open

feat: add sub-section support for pages#2321
ArnaudLigny wants to merge 15 commits intomasterfrom
sub-sections

Conversation

@ArnaudLigny
Copy link
Copy Markdown
Member

Introduce nested sub-sections (folders with index.md) and related APIs and docs. Key changes:

  • Core: add Page parent/subSections properties and helpers (parent, add/get sub-sections, depth, breadcrumb, all-pages-recursive).
  • Generator: Section generator now detects nested index.md, assigns pages to the deepest matching section, creates section pages per language, and builds parent/child relationships.
  • Renderer: add Twig functions/tests (subsections, parent_section, section_breadcrumb, all_pages_recursive, section_tree, subsection, has_subsections) and extend layout lookup to consider parent section templates.
  • Config/docs: add pages.sections option (nested flag), update Content/Templates/Configuration docs and README release note.
  • Tests/fixtures: add SubSectionTests and sample pages for tutorials/advanced; register test suite in phpunit.xml.dist.

Sub-sections are disabled by default and require an index.md to be recognized; enable via pages.sections.nested: true.

Introduce nested sub-sections (folders with index.md) and related APIs and docs. Key changes:

- Core: add Page parent/subSections properties and helpers (parent, add/get sub-sections, depth, breadcrumb, all-pages-recursive).
- Generator: Section generator now detects nested index.md, assigns pages to the deepest matching section, creates section pages per language, and builds parent/child relationships.
- Renderer: add Twig functions/tests (subsections, parent_section, section_breadcrumb, all_pages_recursive, section_tree, subsection, has_subsections) and extend layout lookup to consider parent section templates.
- Config/docs: add pages.sections option (nested flag), update Content/Templates/Configuration docs and README release note.
- Tests/fixtures: add SubSectionTests and sample pages for tutorials/advanced; register test suite in phpunit.xml.dist.

Sub-sections are disabled by default and require an index.md to be recognized; enable via pages.sections.nested: true.
Consolidate logic that ensures nested section paths and their ancestors exist by collecting all paths first and delegating creation to a new ensureSectionExists() helper. This removes inline duplication, uses Page::slugify to find index pages and determine language, and replaces the previous nestedSectionPaths flow. Also updated buildSectionTree() to drop the redundant nestedSectionPaths parameter and adjusted its call site accordingly.
Add a guard to ensure section-specific template names are only prepended if $section is truthy. Previously the code checked only $page->getPath(), which could lead to added layout paths with an empty section and unnecessary or invalid template lookups; this restricts those additions to when a section is present.
@ArnaudLigny ArnaudLigny marked this pull request as ready for review February 26, 2026 14:49
Copilot AI review requested due to automatic review settings February 26, 2026 14:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces hierarchical sub-sections support to Cecil, enabling nested folder structures with index.md files to be recognized as sub-sections. The implementation adds parent/child relationships between sections, new Twig functions for working with sub-sections, and extends template lookup to support sub-section layouts.

Changes:

  • Core Page model extended with parent/child section properties and helper methods (depth, breadcrumb, recursive pages)
  • Section generator detects nested index.md files and builds section tree with parent/child relationships
  • Twig extension adds 5 new functions (subsections, parent_section, section_breadcrumb, all_pages_recursive, section_tree) and 2 tests (subsection, has_subsections)
  • Layout lookup extended to check parent section templates for sub-sections
  • Comprehensive test suite and documentation added

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/Collection/Page/Page.php Adds parent/child section properties and methods (getParentSection, addSubSection, hasSubSections, isSubSection, getSectionDepth, getSectionBreadcrumb, getAllPagesRecursive)
src/Generator/Section.php Implements nested section detection via index.md files, assigns pages to deepest matching section using filepath, builds section tree with parent/child relationships
src/Renderer/Extension/Core.php Adds 5 Twig functions and 2 Twig tests for sub-section manipulation and queries
src/Renderer/Layout.php Extends layout lookup for sub-sections to include parent section templates
config/default.php Adds commented-out pages.sections.nested configuration option
docs/2-Content.md Documents sub-section structure, requirements (index.md), and usage
docs/3-Templates.md Documents sub-section functions, tests, template variables, and layout lookup rules
docs/4-Configuration.md Documents pages.sections.nested configuration option to enable sub-sections
tests/SubSectionTests.php Comprehensive test suite covering section creation, relationships, depth, breadcrumb, and page assignment
tests/fixtures/website/pages/Blog/Tutorials/* Test fixture files: index.md and 2 tutorial pages
tests/fixtures/website/pages/Blog/Tutorials/Advanced/* Nested test fixture: index.md and 1 tutorial page
phpunit.xml.dist Registers SubSectionTests test suite
composer.lock Routine Symfony dependency updates (v7.4.4-7.4.5 → v7.4.6)
README.md Adds unrelated "Release" section documenting release process

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Generator/Section.php Outdated
Comment thread src/Renderer/Layout.php
Comment on lines +134 to +143
$layouts = array_merge(
[
"{$section}/index.$format.$ext",
"{$section}/list.$format.$ext",
"section/{$section}.$format.$ext",
"{$parentSection}/list.$format.$ext",
"section/{$parentSection}.$format.$ext",
],
$layouts
);
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is duplicated code in the layout lookup array. Lines 136-138 duplicate entries that are already added in lines 127-129 (in reverse order). The array merge on line 134 adds these layouts at the beginning, but they're already present later in the array. This could affect template resolution priority and should be cleaned up to avoid confusion.

Copilot uses AI. Check for mistakes.
Comment thread src/Generator/Section.php
Comment thread src/Generator/Section.php Outdated
Comment thread README.md
Comment on lines +141 to +153
## Release

To release a new version, create a new Git tag with the version number (e.g. `1.0.0`), push it to GitHub and the release will be automatically published by GitHub Actions.

```bash
git tag 1.0.0
git push origin 1.0.0
```

> [!TIP]
> To create a **pre-release**, add a suffix to the version number (e.g. `1.0.0-beta.1`).

The automated workflow also will publish the release to the [website](https://cecil.app/download), update the [Homebrew formula](https://github.com/Cecilapp/homebrew-tap) and the [Scoop manifest](https://cecil.app/scoop/cecil.json).
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This addition of a "Release" section in the README does not appear to be related to the sub-sections feature. While it's useful documentation, it should ideally be in a separate PR focused on improving release documentation, not bundled with the sub-sections feature PR.

Copilot uses AI. Check for mistakes.
Comment thread docs/3-Templates.md
Comment on lines +371 to +372
| `page.subsections` | Collection of child sub-sections (sections only). | _Collection_ |
| `page.parent` | Parent section (sub-sections only). | _Page_ |
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation states that page.subsections and page.parent can be accessed as page variables. However, Twig's property access for page.subsections will try to call getSubsections() (lowercase 's'), but the actual method is getSubSections() (camelCase with capital 'S'). Similarly, page.parent will try getParent() but the method is getParentSection(). Users should use the Twig functions subsections(page) and parent_section(page) instead, or the documentation should be updated to reflect the correct property accessors.

Suggested change
| `page.subsections` | Collection of child sub-sections (sections only). | _Collection_ |
| `page.parent` | Parent section (sub-sections only). | _Page_ |
| `subsections(page)` | Collection of child sub-sections (sections only). | _Collection_ |
| `parent_section(page)`| Parent section (sub-sections only). | _Page_ |

Copilot uses AI. Check for mistakes.
ArnaudLigny and others added 8 commits February 26, 2026 17:02
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 8 comments.

Comment thread src/Generator/Section.php
Comment on lines +86 to +94

// Don't add a section's own index page to its pages list.
// A page is a section index if its page ID matches a nested section path.
if ($pageId === $sectionPath || isset($nestedSectionPaths[$pageId])) {
continue;
}

// Root section index pages: their path equals their section.
$pagePath = $page->getPath();
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic that excludes a section’s own index page from its pages list does not work for localized section indexes (e.g. blog/tutorials/index.fr.md has page ID fr/blog/tutorials but $sectionPath is blog/tutorials). As a result, the localized index page can be added to the section’s pages collection and show up in listings/navigation. Consider excluding index pages by comparing $page->getPath() (which is language-agnostic) against $sectionPath, or by using $page->getIdWithoutLang() when comparing IDs, and avoid relying on isset($nestedSectionPaths[$pageId]) since keys are folder paths without language prefixes.

Suggested change
// Don't add a section's own index page to its pages list.
// A page is a section index if its page ID matches a nested section path.
if ($pageId === $sectionPath || isset($nestedSectionPaths[$pageId])) {
continue;
}
// Root section index pages: their path equals their section.
$pagePath = $page->getPath();
$pagePath = $page->getPath();
// Don't add a section's own index page to its pages list.
// Compare language-agnostic paths so localized section indexes are excluded too.
if ($pagePath === $sectionPath) {
continue;
}
// Root section index pages: their path equals their section.

Copilot uses AI. Check for mistakes.
Comment thread src/Generator/Section.php
Comment on lines +350 to +364
$slug = Page::slugify($sectionPath);

// Determine the language for this section. Prefer the language from an existing
// index page when available; otherwise, fall back to the default language.
if ($this->builder->getPages()->has($slug)) {
$lang = $this->builder->getPages()->get($slug)
->getVariable('language', $this->config->getLanguageDefault());
} else {
$lang = $this->config->getLanguageDefault();
}

// Ensure the section entry exists for the resolved language.
if (!isset($sections[$sectionPath][$lang])) {
$sections[$sectionPath][$lang] = [];
}
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ensureSectionExists() determines the section language by checking only the default-language page ID ($slug). For multilingual sites, localized section index pages use IDs like <lang>/$slug (see Page::createIdFromFile()), so this can fail to create empty section entries for non-default languages when a section exists only via index.<lang>.md (or has no direct pages). Consider iterating configured languages and creating entries for each language where the section index page exists (e.g. check has("$lang/$slug")) instead of only has($slug).

Copilot uses AI. Check for mistakes.
Comment thread src/Generator/Section.php
// in the configuration.
$nestedSectionPaths = [];
if ((bool) $this->config->get('pages.sections.nested') === true) {
// Returns a map of slugified-folder-path => page-id.
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline comment says detectNestedSectionPaths() “Returns a map of slugified-folder-path => page-id”, but the method actually returns array<string,true>. Please update the comment to reflect the real return type to avoid confusion when this map is consumed later.

Suggested change
// Returns a map of slugified-folder-path => page-id.
// Returns a set-like map of slugified-folder-path => true.

Copilot uses AI. Check for mistakes.
Comment on lines +614 to +617
while ($current->hasParentSection()) {
$depth++;
$current = $current->getParentSection();
}
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In getSectionDepth(), $current is assigned from getParentSection() which returns ?Page. Even though hasParentSection() guarantees non-null at runtime, static analysis (PHPStan) will still treat $current as Page|null after the assignment, potentially raising “call on null” warnings. Refactor the loop to assign the parent to a temporary variable and break when it’s null (e.g. while (($parent = $current->getParentSection()) !== null) { ... }).

Copilot uses AI. Check for mistakes.
Comment on lines +632 to +635
while ($current->hasParentSection()) {
$current = $current->getParentSection();
array_unshift($breadcrumb, $current);
}
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In getSectionBreadcrumb(), $current = $current->getParentSection() assigns a nullable return value, and array_unshift($breadcrumb, $current) will then pass Page|null. This is safe at runtime due to hasParentSection(), but PHPStan is likely to flag it. Consider using a $parent temp variable checked for non-null before unshifting/assigning.

Copilot uses AI. Check for mistakes.
Comment on lines +659 to +660
if ($this->hasSubSections()) {
foreach ($this->getSubSections() as $subSection) {
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getAllPagesRecursive() iterates foreach ($this->getSubSections() as $subSection) after hasSubSections(). Since getSubSections() returns ?Collection, static analyzers won’t infer it’s non-null here. Consider caching getSubSections() into a local variable and checking it for non-null before iterating to avoid PHPStan warnings.

Suggested change
if ($this->hasSubSections()) {
foreach ($this->getSubSections() as $subSection) {
$subSections = $this->getSubSections();
if ($subSections !== null) {
foreach ($subSections as $subSection) {

Copilot uses AI. Check for mistakes.
Comment on lines +299 to +300
if ($page->hasSubSections()) {
foreach ($page->getSubSections() as $child) {
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildTreeNode() iterates over $page->getSubSections() after hasSubSections(), but getSubSections() returns ?Collection. PHPStan typically won’t infer non-null from hasSubSections(), so this can introduce static analysis errors. Consider fetching into a local $subSections = $page->getSubSections() and null-checking before iterating.

Suggested change
if ($page->hasSubSections()) {
foreach ($page->getSubSections() as $child) {
$subSections = $page->getSubSections();
if ($page->hasSubSections() && $subSections !== null) {
foreach ($subSections as $child) {

Copilot uses AI. Check for mistakes.
Comment thread src/Renderer/Layout.php
Comment on lines +136 to +140
"{$section}/index.$format.$ext",
"{$section}/list.$format.$ext",
"section/{$section}.$format.$ext",
"{$parentSection}/list.$format.$ext",
"section/{$parentSection}.$format.$ext",
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sub-section layout lookup block prepends subsection layouts ({$section}/index, {$section}/list, section/{$section}) that were already prepended just above, causing duplicates and extra work. Also, the comment says it will try the parent {$parentSection}/index, but that layout is not actually added. Consider removing duplicates and either updating the comment or adding the missing parent index layout if that was intended.

Suggested change
"{$section}/index.$format.$ext",
"{$section}/list.$format.$ext",
"section/{$section}.$format.$ext",
"{$parentSection}/list.$format.$ext",
"section/{$parentSection}.$format.$ext",
"section/{$parentSection}.$format.$ext",
"{$parentSection}/list.$format.$ext",
"{$parentSection}/index.$format.$ext",

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.

Comment thread src/Generator/Section.php
Comment on lines +82 to +95

// Use the original file folder path to resolve the section.
$originalFolder = $pageIdToFolderPath[$pageId] ?? null;
$sectionPath = $this->resolveSection($originalFolder, $page->getSection(), $nestedSectionPaths);

// Don't add a section's own index page to its pages list.
// A page is a section index if its page ID matches a nested section path.
if ($pageId === $sectionPath || isset($nestedSectionPaths[$pageId])) {
continue;
}

// Root section index pages: their path equals their section.
$pagePath = $page->getPath();
if ($pagePath === $page->getSection()) {
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Localized section index pages (e.g. "section/index.fr.md" => page ID "fr/section") won't be excluded here: $pageId === $sectionPath only matches default language, and $nestedSectionPaths is keyed by folder path (e.g. "blog/tutorials"), not by page ID. This will incorrectly add localized index pages into the section’s pages list and can break section assignments. Consider excluding section index pages by comparing $page->getIdWithoutLang() or $page->getPath() against the resolved $sectionPath (and/or keying $nestedSectionPaths consistently).

Suggested change
// Use the original file folder path to resolve the section.
$originalFolder = $pageIdToFolderPath[$pageId] ?? null;
$sectionPath = $this->resolveSection($originalFolder, $page->getSection(), $nestedSectionPaths);
// Don't add a section's own index page to its pages list.
// A page is a section index if its page ID matches a nested section path.
if ($pageId === $sectionPath || isset($nestedSectionPaths[$pageId])) {
continue;
}
// Root section index pages: their path equals their section.
$pagePath = $page->getPath();
if ($pagePath === $page->getSection()) {
$pageIdWithoutLang = $page->getIdWithoutLang();
// Use the original file folder path to resolve the section.
$originalFolder = $pageIdToFolderPath[$pageId] ?? null;
$sectionPath = $this->resolveSection($originalFolder, $page->getSection(), $nestedSectionPaths);
// Don't add a section's own index page to its pages list.
// Compare using language-neutral identifiers and normalized paths,
// because nested section paths are keyed by folder path, not page ID.
$pagePath = $page->getPath();
if (
$pageIdWithoutLang === $sectionPath
|| $pagePath === $sectionPath
|| isset($nestedSectionPaths[$pagePath])
) {

Copilot uses AI. Check for mistakes.
Comment thread tests/SubSectionTests.php
Comment on lines +31 to +42
self::$source = Util::joinFile(__DIR__, 'fixtures/website');
self::$config = Util::joinFile(self::$source, 'config.yml');
self::$destination = self::$source;

putenv('CECIL_DEBUG=true');
self::$builder = Builder::create(Config::loadFile(self::$config), new PrintLogger(Builder::VERBOSITY_DEBUG))
->setSourceDir(self::$source)
->setDestinationDir(self::$destination);
self::$builder->build([
'drafts' => true,
'dry-run' => true,
]);
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests rely on sub-sections being enabled, but the fixture tests/fixtures/website/config.yml currently doesn’t set pages.sections.nested: true. With the feature disabled by default, build() will not generate blog/tutorials / blog/tutorials/advanced section pages and the assertions will fail. Consider enabling the config flag in the fixture config or setting it on the loaded Config instance before building.

Copilot uses AI. Check for mistakes.
Comment thread tests/SubSectionTests.php
namespace Cecil\Test;

use Cecil\Builder;
use Cecil\Collection\Page\Page;
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use Cecil\Collection\Page\Page; is unused in this test file. Removing it avoids unused-import lint failures.

Suggested change
use Cecil\Collection\Page\Page;

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants