[BUGFIX] Prevent Duplicates in toctree and menu

linawolf · jaapio · commit e10e781433b2 · 2023-08-10T22:50:21.000+02:00
A page that has already been explicitely added to a toctree does not get added by the glob directive once more. As having multiple occurences of the same page within one toctree or menu does not make sense in general, I applied array_unique to the menu_items of one level of the toctree or menu.
diff --git a/packages/guides/src/Compiler/NodeTransformers/MenuNodeAddEntryTransformer.php b/packages/guides/src/Compiler/NodeTransformers/MenuNodeAddEntryTransformer.php
@@ -16,6 +16,7 @@
 use Psr\Log\LoggerInterface;
 
 use function array_pop;
+use function array_unique;
 use function assert;
 use function explode;
 use function implode;
@@ -85,6 +86,8 @@ public function leaveNode(Node $node, CompilerContext $compilerContext): Node|nu
             }
         }
 
+        $menuEntries = array_unique($menuEntries);
+
         $node = $node->withMenuEntries($menuEntries);
 
         return $node;
diff --git a/packages/guides/src/Nodes/Menu/MenuEntryNode.php b/packages/guides/src/Nodes/Menu/MenuEntryNode.php
@@ -72,4 +72,9 @@ public function addSection(MenuEntryNode $section): void
     {
         $this->sections[] = $section;
     }
+
+    public function __toString(): string
+    {
+        return $this->url . '#' . $this->anchor;
+    }
 }
diff --git a/packages/guides/tests/unit/Compiler/NodeTransformers/MenuNodeAddSubDocumentsTransformerTest.php b/packages/guides/tests/unit/Compiler/NodeTransformers/MenuNodeAddSubDocumentsTransformerTest.php
@@ -79,9 +79,15 @@ public static function tocTreeEntryProvider(): array
             'testRelativeTocUrlInSubdir' => [
                 'current' => 'doc1/index',
                 'paths' =>  ['index', 'doc1', 'doc2','doc1/index' , 'doc1/subdoc1', 'doc1/subdoc2', 'doc1/subdoc3', 'doc3/index'],
-                'tocFiles' => ['subdoc1', 'subdoc1'],
+                'tocFiles' => ['subdoc1', 'subdoc2'],
                 'expectedCount' => 2,
             ],
+            'testMultipleGetRemoved' => [
+                'current' => 'doc1/index',
+                'paths' =>  ['index', 'doc1', 'doc2','doc1/index' , 'doc1/subdoc1', 'doc1/subdoc2', 'doc1/subdoc3', 'doc3/index'],
+                'tocFiles' => ['subdoc1', 'subdoc1'],
+                'expectedCount' => 1,
+            ],
             'testTocTreeAbsoluteGlobDoesNotAddIndex' => [
                 'current' => 'index',
                 'paths' =>  ['index', 'doc1', 'doc2', 'doc1/subdoc', 'doc3/doc1'],
@@ -103,6 +109,13 @@ public static function tocTreeEntryProvider(): array
                 'expectedCount' => 2,
                 'glob' => true,
             ],
+            'testRelativeGlobDoesNotAddDuplicates' => [
+                'current' => 'index',
+                'paths' =>  ['index', 'doc1', 'doc2', 'doc1/subdoc', 'doc3/doc1'],
+                'tocFiles' => ['doc1','*'],
+                'expectedCount' => 2,
+                'glob' => true,
+            ],
             'testRelativeGlobInSubdirDoesNotAddIndex' => [
                 'current' => 'doc1/index',
                 'paths' =>  ['index', 'doc1', 'doc2','doc1/index' , 'doc1/subdoc1', 'doc1/subdoc2', 'doc1/subdoc3', 'doc3/index'],
diff --git a/tests/Integration/tests/toctree-no-duplicates/expected/index.html b/tests/Integration/tests/toctree-no-duplicates/expected/index.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <title>index</title>
+
+</head>
+<body>
+    <div class="section" id="index">
+    <h1>index</h1>
+
+            <div class="toc">
+    <ul class="phpdocumentor-list">
+    <li class="toc-item"><a href="/overview.html#overview">Overview</a></li>
+
+    <li class="toc-item"><a href="/apple.html#apple">Apple</a></li>
+
+    <li class="toc-item"><a href="/banana.html#banana">Banana</a></li>
+
+    <li class="toc-item"><a href="/cherry.html#cherry">Cherry</a></li>
+
+    </ul>
+</div>
+
+    </div>
+
+</body>
+</html>
diff --git a/tests/Integration/tests/toctree-no-duplicates/expected/overview.html b/tests/Integration/tests/toctree-no-duplicates/expected/overview.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <title>Overview</title>
+
+</head>
+<body>
+    <div class="section" id="overview">
+    <h1>Overview</h1>
+
+            <p>Lorem Ipsum Dolor</p>
+            <div class="menu"><ul class="phpdocumentor-list"><li class="toc-item"><a href="/overview.html#overview">Overview</a></li><li class="toc-item"><a href="/apple.html#apple">Apple</a></li><li class="toc-item"><a href="/banana.html#banana">Banana</a></li><li class="toc-item"><a href="/cherry.html#cherry">Cherry</a></li></ul></div>
+    </div>
+
+</body>
+</html>
diff --git a/tests/Integration/tests/toctree-no-duplicates/input/apple.rst b/tests/Integration/tests/toctree-no-duplicates/input/apple.rst
@@ -0,0 +1,4 @@
+Apple
+=====
+
+Lorem Ipsum Dolor
diff --git a/tests/Integration/tests/toctree-no-duplicates/input/banana.rst b/tests/Integration/tests/toctree-no-duplicates/input/banana.rst
@@ -0,0 +1,4 @@
+Banana
+======
+
+Lorem Ipsum Dolor
diff --git a/tests/Integration/tests/toctree-no-duplicates/input/cherry.rst b/tests/Integration/tests/toctree-no-duplicates/input/cherry.rst
@@ -0,0 +1,4 @@
+Cherry
+======
+
+Lorem Ipsum Dolor
diff --git a/tests/Integration/tests/toctree-no-duplicates/input/index.rst b/tests/Integration/tests/toctree-no-duplicates/input/index.rst
@@ -0,0 +1,9 @@
+index
+=====
+
+..  toctree::
+    :titlesonly:
+    :glob:
+
+    overview
+    *
diff --git a/tests/Integration/tests/toctree-no-duplicates/input/overview.rst b/tests/Integration/tests/toctree-no-duplicates/input/overview.rst
@@ -0,0 +1,9 @@
+Overview
+========
+
+Lorem Ipsum Dolor
+
+..  menu::
+
+    overview
+    *

Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@`
`16`	`16`	`use Psr\Log\LoggerInterface;`
`17`	`17`
`18`	`18`	`use function array_pop;`
	`19`	`+use function array_unique;`
`19`	`20`	`use function assert;`
`20`	`21`	`use function explode;`
`21`	`22`	`use function implode;`
`@@ -85,6 +86,8 @@ public function leaveNode(Node $node, CompilerContext $compilerContext): Node\|nu`
`85`	`86`	`}`
`86`	`87`	`}`
`87`	`88`
	`89`	`+ $menuEntries = array_unique($menuEntries);`
	`90`	`+`
`88`	`91`	`$node = $node->withMenuEntries($menuEntries);`
`89`	`92`
`90`	`93`	`return $node;`
Original file line number	Diff line number	Diff line change
`@@ -72,4 +72,9 @@ public function addSection(MenuEntryNode $section): void`
`72`	`72`	`{`
`73`	`73`	`$this->sections[] = $section;`
`74`	`74`	`}`
	`75`	`+`
	`76`	`+ public function __toString(): string`
	`77`	`+ {`
	`78`	`+ return $this->url . '#' . $this->anchor;`
	`79`	`+ }`
`75`	`80`	`}`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +Apple
 +=====
++
 +Lorem Ipsum Dolor