Fix the TOC generation with unique links

symfony-tools · Apr 14, 2023 · 87859d7 · 87859d7
1 parent e74e0c6
commit 87859d7
Show file tree

Hide file tree

Showing 7 changed files with 122 additions and 29 deletions.
diff --git 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
@@ -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
@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Docs Builder package.
+ * (c) Ryan Weaver <[email protected]>
+ * 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
@@ -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
@@ -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
@@ -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
@@ -11,18 +11,30 @@ 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
 ---------
 
 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