From 87859d794e031d8f1b615d0712f123895024239a Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Thu, 13 Apr 2023 18:02:38 +0200 Subject: [PATCH] Fix the TOC generation with unique links --- src/DocsKernel.php | 6 +++ src/Generator/JsonGenerator.php | 41 ++++++++++------- src/Listener/DuplicatedHeaderIdListener.php | 25 +++++++++++ src/Renderers/TitleNodeRenderer.php | 5 +++ tests/IntegrationTest.php | 9 ---- tests/JsonIntegrationTest.php | 49 ++++++++++++++++++++- tests/fixtures/source/json/design.rst | 16 ++++++- 7 files changed, 122 insertions(+), 29 deletions(-) create mode 100644 src/Listener/DuplicatedHeaderIdListener.php diff --git a/src/DocsKernel.php b/src/DocsKernel.php index 1f234425..8ffbedd7 100644 --- a/src/DocsKernel.php +++ b/src/DocsKernel.php @@ -20,6 +20,7 @@ use SymfonyDocsBuilder\Listener\AdmonitionListener; use SymfonyDocsBuilder\Listener\AssetsCopyListener; use SymfonyDocsBuilder\Listener\CopyImagesListener; +use SymfonyDocsBuilder\Listener\DuplicatedHeaderIdListener; class DocsKernel extends Kernel { @@ -49,6 +50,11 @@ private function initializeListeners(EventManager $eventManager, ErrorManager $e new AdmonitionListener() ); + $eventManager->addEventListener( + PreParseDocumentEvent::PRE_PARSE_DOCUMENT, + new DuplicatedHeaderIdListener() + ); + $eventManager->addEventListener( PreNodeRenderEvent::PRE_NODE_RENDER, new CopyImagesListener($this->buildConfig, $errorManager) diff --git a/src/Generator/JsonGenerator.php b/src/Generator/JsonGenerator.php index 7d429be7..51de0dee 100644 --- a/src/Generator/JsonGenerator.php +++ b/src/Generator/JsonGenerator.php @@ -69,7 +69,7 @@ public function generateJson(string $masterDocument = 'index'): array $crawler = new Crawler(file_get_contents($this->buildConfig->getOutputDir().'/'.$filename.'.html')); // happens when some doc is a partial included in other doc an it doesn't have any titles - $toc = false === current($metaEntry->getTitles()) ? [] : $this->generateToc($metaEntry, current($metaEntry->getTitles())[1]); + $toc = $this->generateToc($metaEntry, $crawler); $next = $this->determineNext($parserFilename, $flattenedTocTree, $masterDocument); $prev = $this->determinePrev($parserFilename, $flattenedTocTree); $data = [ @@ -102,26 +102,35 @@ public function setOutput(SymfonyStyle $output) $this->output = $output; } - private function generateToc(MetaEntry $metaEntry, ?array $titles, int $level = 1): array + private function generateToc(MetaEntry $metaEntry, Crawler $crawler): array { - if (null === $titles) { - return []; + $flatTocTree = []; + + foreach ($crawler->filter('h2, h3') as $heading) { + $headerId = $heading->getAttribute('id') ?? Environment::slugify($heading->textContent); + + // this tocTree stores items sequentially (h2, h2, h3, h3, h2, h3, etc.) + $flatTocTree[] = [ + 'level' => 'h2' === $heading->tagName ? 1 : 2, + 'url' => sprintf('%s#%s', $metaEntry->getUrl(), $headerId), + 'page' => u($metaEntry->getUrl())->beforeLast('.html')->toString(), + 'fragment' => $headerId, + 'title' => $heading->textContent, + 'children' => [], + ]; } - $tocTree = []; - - foreach ($titles as $title) { - $tocTree[] = [ - 'level' => $level, - 'url' => sprintf('%s#%s', $metaEntry->getUrl(), Environment::slugify($title[0])), - 'page' => u($metaEntry->getUrl())->beforeLast('.html'), - 'fragment' => Environment::slugify($title[0]), - 'title' => $title[0], - 'children' => $this->generateToc($metaEntry, $title[1], $level + 1), - ]; + // this tocTree stores items nested by level (h2, h2[h3, h3], h2[h3], etc.) + $nestedTocTree = []; + foreach ($flatTocTree as $tocItem) { + if (1 === $tocItem['level']) { + $nestedTocTree[] = $tocItem; + } else { + $nestedTocTree[\count($nestedTocTree) - 1]['children'][] = $tocItem; + } } - return $tocTree; + return $nestedTocTree; } private function determineNext(string $parserFilename, array $flattenedTocTree): ?array diff --git a/src/Listener/DuplicatedHeaderIdListener.php b/src/Listener/DuplicatedHeaderIdListener.php new file mode 100644 index 00000000..b0d139bf --- /dev/null +++ b/src/Listener/DuplicatedHeaderIdListener.php @@ -0,0 +1,25 @@ + + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace SymfonyDocsBuilder\Listener; + +use Doctrine\RST\Event\PreParseDocumentEvent; +use SymfonyDocsBuilder\Renderers\TitleNodeRenderer; + +final class DuplicatedHeaderIdListener +{ + public function preParseDocument(PreParseDocumentEvent $event): void + { + // needed because we only need to handle duplicated headers within + // the same file, not across all the files being generated + TitleNodeRenderer::resetHeaderIdCache(); + } +} diff --git a/src/Renderers/TitleNodeRenderer.php b/src/Renderers/TitleNodeRenderer.php index 2012be65..10042c5d 100644 --- a/src/Renderers/TitleNodeRenderer.php +++ b/src/Renderers/TitleNodeRenderer.php @@ -32,6 +32,11 @@ public function __construct(TitleNode $titleNode, TemplateRenderer $templateRend $this->templateRenderer = $templateRenderer; } + public static function resetHeaderIdCache(): void + { + self::$idUsagesCountByFilename = []; + } + public function render(): string { $filename = $this->titleNode->getEnvironment()->getCurrentFileName(); diff --git a/tests/IntegrationTest.php b/tests/IntegrationTest.php index ca97f976..a9f9c403 100644 --- a/tests/IntegrationTest.php +++ b/tests/IntegrationTest.php @@ -21,15 +21,6 @@ class IntegrationTest extends AbstractIntegrationTest { - public static function setUpBeforeClass(): void - { - $reflection = new \ReflectionClass(TitleNodeRenderer::class); - $property = $reflection->getProperty('idUsagesCountByFilename'); - $property->setAccessible(true); - - $property->setValue([]); - } - /** * @dataProvider integrationProvider */ diff --git a/tests/JsonIntegrationTest.php b/tests/JsonIntegrationTest.php index 7d3d07ba..b9ab0924 100644 --- a/tests/JsonIntegrationTest.php +++ b/tests/JsonIntegrationTest.php @@ -10,6 +10,7 @@ namespace SymfonyDocsBuilder\Tests; use SymfonyDocsBuilder\DocBuilder; +use SymfonyDocsBuilder\Renderers\TitleNodeRenderer; class JsonIntegrationTest extends AbstractIntegrationTest { @@ -26,7 +27,7 @@ public function testJsonGeneration(string $filename, array $expectedData) $actualFileData = $fJsons[$filename]; foreach ($expectedData as $key => $expectedKeyData) { $this->assertArrayHasKey($key, $actualFileData, sprintf('Missing key "%s" in file "%s"', $key, $filename)); - $this->assertSame($expectedData[$key], $actualFileData[$key], sprintf('Invalid data for key "%s" in file "%s"', $key, $filename)); + $this->assertSame($expectedKeyData, $actualFileData[$key], sprintf('Invalid data for key "%s" in file "%s"', $key, $filename)); } } @@ -76,9 +77,53 @@ public function getJsonTests() 'title' => 'Design', 'toc_options' => [ 'maxDepth' => 2, - 'numVisibleItems' => 3, + 'numVisibleItems' => 5, 'size' => 'md' ], + 'toc' => [ + [ + 'level' => 1, + 'url' => 'design.html#section-1', + 'page' => 'design', + 'fragment' => 'section-1', + 'title' => 'Section 1', + 'children' => [ + [ + 'level' => 2, + 'url' => 'design.html#some-subsection', + 'page' => 'design', + 'fragment' => 'some-subsection', + 'title' => 'Some subsection', + 'children' => [], + ], + [ + 'level' => 2, + 'url' => 'design.html#some-subsection-1', + 'page' => 'design', + 'fragment' => 'some-subsection-1', + 'title' => 'Some subsection', + 'children' => [], + ], + ], + ], + [ + 'level' => 1, + 'url' => 'design.html#section-2', + 'page' => 'design', + 'fragment' => 'section-2', + 'title' => 'Section 2', + 'children' => [ + [ + 'level' => 2, + 'url' => 'design.html#some-subsection-2', + 'page' => 'design', + 'fragment' => 'some-subsection-2', + 'title' => 'Some subsection', + 'children' => [], + ], + ], + ], + ], ], ]; diff --git a/tests/fixtures/source/json/design.rst b/tests/fixtures/source/json/design.rst index b9a4537e..2711ac4a 100644 --- a/tests/fixtures/source/json/design.rst +++ b/tests/fixtures/source/json/design.rst @@ -11,11 +11,17 @@ The toctree below should affects the next/prev. The first entry is effectively ignored, as it was already included by the toctree in index.rst (which is parsed first). -Subsection 1 -~~~~~~~~~~~~ +Some subsection +~~~~~~~~~~~~~~~ This is a subsection of the first section. That's all. +Some subsection +~~~~~~~~~~~~~~~ + +This sub-section uses the same title as before to test that the tool +never generated two or more headings with the same ID. + Section 2 --------- @@ -23,6 +29,12 @@ However, crud (which is ALSO included in the toctree in index.rst), WILL be read here, as the "crud" in index.rst has not been read yet (design comes first). Also, design/sub-page WILL be considered. +Some subsection +~~~~~~~~~~~~~~~ + +This sub-section also uses the same title as in the previous section +to test that the tool never generated two or more headings with the same ID. + .. toctree:: :maxdepth: 1