Merge pull request #138 from wouterj/pull-86

Parse the contents of a definition list
doctrine · Mar 15, 2021 · 9f9887b · 9f9887b
2 parents 9034374 + 732ae67
commit 9f9887b
Show file tree

Hide file tree

Showing 10 changed files with 147 additions and 75 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -0,0 +1,19 @@
+# Upgrade to 0.3
+
+## `DefinitionListTerm::$definitions` is a list of `Node`'s instead of `SpanNode`'s
+
+If you define a custom `definition-list.html.twig`, no longer wrap the value in
+a `<p>` element (the `.first` and `.last` classes are automatically added):
+
+```diff
+  <dl>
+      {% for definitionListTerm in definitionList.terms %}
+      <dt>{{ definitionListTerm.term.render()|raw }}</dt>
+      <dd>
+          {% for definition in definitionListTerm.definitions %}
+-             <p>{{ definition.render()|raw }}</p>
++             {{ definition.render()|raw }}
+          {% endfor %}
+      </dd>
+  </dl>
+```
diff --git a/lib/Parser.php b/lib/Parser.php
@@ -152,13 +152,25 @@ public function createSpanNode($span): SpanNode
         return $this->getNodeFactory()->createSpanNode($this, $span);
     }
 
+    /**
+     * Parses the given contents as a new file.
+     */
     public function parse(string $contents): DocumentNode
     {
         $this->getEnvironment()->reset();
 
         return $this->parseLocal($contents);
     }
 
+    /**
+     * Parses the given contents in a new document node.
+     *
+     * CAUTION: This modifies the state of the Parser, do not use this method
+     * on the main parser (use `Parser::getSubParser()->parseLocal(...)` instead).
+     *
+     * Use this method to parse contents of an other node. Nodes created by
+     * this new parser are not added to the main DocumentNode.
+     */
     public function parseLocal(string $contents): DocumentNode
     {
         $this->documentParser = $this->createDocumentParser();

diff --git a/lib/Parser/DefinitionListTerm.php b/lib/Parser/DefinitionListTerm.php
@@ -4,6 +4,7 @@
 
 namespace Doctrine\RST\Parser;
 
+use Doctrine\RST\Nodes\Node;
 use Doctrine\RST\Nodes\SpanNode;
 use RuntimeException;
 
@@ -15,12 +16,12 @@ class DefinitionListTerm
     /** @var SpanNode[] */
     private $classifiers = [];
 
-    /** @var SpanNode[] */
+    /** @var Node[] */
     private $definitions = [];
 
     /**
      * @param SpanNode[] $classifiers
-     * @param SpanNode[] $definitions
+     * @param Node[]     $definitions
      */
     public function __construct(SpanNode $term, array $classifiers, array $definitions)
     {
@@ -43,14 +44,14 @@ public function getClassifiers(): array
     }
 
     /**
-     * @return SpanNode[]
+     * @return Node[]
      */
     public function getDefinitions(): array
     {
         return $this->definitions;
     }
 
-    public function getFirstDefinition(): SpanNode
+    public function getFirstDefinition(): Node
     {
         if (! isset($this->definitions[0])) {
             throw new RuntimeException('No definitions found.');

diff --git a/lib/Parser/LineDataParser.php b/lib/Parser/LineDataParser.php
@@ -6,6 +6,7 @@
 
 use Doctrine\Common\EventManager;
 use Doctrine\RST\Event\OnLinkParsedEvent;
+use Doctrine\RST\Nodes\ParagraphNode;
 use Doctrine\RST\Nodes\SpanNode;
 use Doctrine\RST\Parser;
 
@@ -147,27 +148,42 @@ public function parseListLine(string $line): ?ListLine
      */
     public function parseDefinitionList(array $lines): DefinitionList
     {
-        $definitionList     = [];
+        /** @var array{term: SpanNode, classifiers: list<SpanNode>, definition: string}|null $definitionListTerm */
         $definitionListTerm = null;
-        $currentDefinition  = null;
+        $definitionList     = [];
+
+        $createDefinitionTerm = function (array $definitionListTerm): DefinitionListTerm {
+            // parse any markup in the definition (e.g. lists, directives)
+            $definitionNodes = $this->parser->getSubParser()->parseLocal($definitionListTerm['definition'])->getNodes();
+            if (count($definitionNodes) === 1 && $definitionNodes[0] instanceof ParagraphNode) {
+                // if there is only one paragraph node, the value is put directly in the <dd> element
+                $definitionNodes = [$definitionNodes[0]->getValue()];
+            } else {
+                // otherwise, .first and .last are added to the first and last nodes of the definition
+                $definitionNodes[0]->setClasses($definitionNodes[0]->getClasses() + ['first']);
+                $definitionNodes[count($definitionNodes) - 1]->setClasses($definitionNodes[count($definitionNodes) - 1]->getClasses() + ['last']);
+            }
+
+            return new DefinitionListTerm(
+                $definitionListTerm['term'],
+                $definitionListTerm['classifiers'],
+                $definitionNodes
+            );
+        };
 
         foreach ($lines as $key => $line) {
-            // term definition line
-            if ($definitionListTerm !== null && substr($line, 0, 4) === '    ') {
-                $definition = trim($line);
+            // indent or empty line = term definition line
+            if ($definitionListTerm !== null && (substr($line, 0, 4) === '    ' || trim($line) === '')) {
+                $definition = substr($line, 4);
 
-                $currentDefinition .= $definition . ' ';
+                $definitionListTerm['definition'] .= $definition . "\n";
 
-            // non empty string
+            // non empty string at the start of the line = definition term
             } elseif (trim($line) !== '') {
                 // we are starting a new term so if we have an existing
                 // term with definitions, add it to the definition list
                 if ($definitionListTerm !== null) {
-                    $definitionList[] = new DefinitionListTerm(
-                        $definitionListTerm['term'],
-                        $definitionListTerm['classifiers'],
-                        $definitionListTerm['definitions']
-                    );
+                    $definitionList[] = $createDefinitionTerm($definitionListTerm);
                 }
 
                 $parts = explode(':', trim($line));
@@ -182,31 +198,16 @@ public function parseDefinitionList(array $lines): DefinitionList
                 $definitionListTerm = [
                     'term' => $this->parser->createSpanNode($term),
                     'classifiers' => $classifiers,
-                    'definitions' => [],
+                    'definition' => '',
                 ];
-
-            // last line
-            } elseif ($definitionListTerm !== null && count($lines) - 1 === $key) {
-                if ($currentDefinition !== null) {
-                    $definitionListTerm['definitions'][] = $this->parser->createSpanNode($currentDefinition);
-
-                    $currentDefinition = null;
-                }
-
-                $definitionList[] = new DefinitionListTerm(
-                    $definitionListTerm['term'],
-                    $definitionListTerm['classifiers'],
-                    $definitionListTerm['definitions']
-                );
-
-            // empty line, start of a new definition for the current term
-            } elseif ($currentDefinition !== null && $definitionListTerm !== null) {
-                $definitionListTerm['definitions'][] = $this->parser->createSpanNode($currentDefinition);
-
-                $currentDefinition = null;
             }
         }
 
+        // append the last definition of the list
+        if ($definitionListTerm !== null) {
+            $definitionList[] = $createDefinitionTerm($definitionListTerm);
+        }
+
         return new DefinitionList($definitionList);
     }
 }
diff --git a/lib/Templates/default/html/definition-list.html.twig b/lib/Templates/default/html/definition-list.html.twig
@@ -14,21 +14,11 @@
             </dt>
         {% endif %}
 
-        {% if definitionListTerm.definitions|length > 1 %}
-            <dd>
-                {% for definition in definitionListTerm.definitions %}
-                    {% if loop.first %}
-                        <p class="first">{{ definition.render()|raw }}</p>
-                    {% elseif loop.last %}
-                        <p class="last">{{ definition.render()|raw }}</p>
-                    {% else %}
-                        <p>{{ definition.render()|raw }}</p>
-                    {% endif %}
-                {% endfor %}
-            </dd>
-        {% elseif definitionListTerm.definitions|length == 1 %}
-            <dd>{{ definitionListTerm.firstDefinition.render()|raw }}</dd>
-        {% endif %}
+        <dd>
+            {% for definition in definitionListTerm.definitions %}
+                {{ definition.render()|raw }}
+            {% endfor %}
+        </dd>
     {% endfor %}
 </dl>
 {% endapply %}
diff --git a/tests/Functional/tests/class-directive/class-directive.html b/tests/Functional/tests/class-directive/class-directive.html
@@ -17,7 +17,7 @@
 </blockquote>
 <dl class="special-definition-list">
     <dt>term 1</dt>
-    <dd>Definition 1 </dd>
+    <dd> Definition 1 </dd>
 </dl>
 <table class="special-table">
     <thead>

diff --git a/tests/Functional/tests/definition-list/definition-list.html b/tests/Functional/tests/definition-list/definition-list.html
@@ -8,33 +8,35 @@ <h1>
     <p>Text around the definition list.</p>
     <dl>
         <dt>term 1</dt>
-        <dd>Definition 1 </dd>
+        <dd> Definition 1 </dd>
         <dt>term 2</dt>
         <dd>
-            <p class="first">Definition 1 </p>
-            <p>Definition 2 </p>
-            <p class="last">Definition 3 </p>
+            <p class="first">Definition 1</p>
+            <p>Definition 2</p>
+            <p class="last">Definition 3</p>
         </dd>
         <dt> term 3 <span class="classifier-delimiter">:</span><span class="classifier">classifier</span></dt>
-        <dd>Definition 1 </dd>
+        <dd> Definition 1 </dd>
         <dt> term 4 <span class="classifier-delimiter">:</span><span class="classifier">classifier one</span><span class="classifier-delimiter">:</span><span class="classifier">classifier two</span></dt>
-        <dd>Definition 1 </dd>
+        <dd> Definition 1 </dd>
         <dt> term with &amp; <span class="classifier-delimiter">:</span><span class="classifier">classifier with &amp;</span></dt>
-        <dd>Definition 1 with &amp; </dd>
+        <dd> Definition 1 with &amp; </dd>
         <dt> term with &amp; <span class="classifier-delimiter">:</span><span class="classifier">classifier with &amp;</span><span class="classifier-delimiter">:</span><span class="classifier">classifier with &amp;</span></dt>
         <dd>
-            <p class="first">Definition 1 with &amp; </p>
-            <p class="last">Definition 2 with &amp; </p>
+            <p class="first">Definition 1 with &amp;</p>
+            <p class="last">Definition 2 with &amp;</p>
         </dd>
         <dt>
             <code>term 5</code><span class="classifier-delimiter">:</span>
             <span class="classifier"><code>classifier</code></span>
         </dt>
-        <dd>Definition 1 </dd>
+        <dd> Definition 1 </dd>
         <dt>multi-line definition term</dt>
         <dd>
-            <p class="first">Definition 1 line 1 Definition 1 line 2 </p>
-            <p class="last">Definition 2 line 1 Definition 2 line 2 </p>
+            <p class="first">Definition 1 line 1
+Definition 1 line 2</p>
+            <p class="last">Definition 2 line 1
+Definition 2 line 2</p>
         </dd>
     </dl>
 </div>
@@ -47,13 +49,15 @@ <h1>
         <dl>
             <dt>term 1</dt>
             <dd>
-                <p class="first">Definition 1 line 1 Definition 1 line 2 </p>
-                <p class="last">Definition 2 </p>
+                <p class="first">Definition 1 line 1
+Definition 1 line 2</p>
+                <p class="last">Definition 2</p>
             </dd>
             <dt>term 2</dt>
             <dd>
-                <p class="first">Definition 1 line 1 Definition 1 line 2 </p>
-                <p class="last">Definition 2 </p>
+                <p class="first">Definition 1 line 1
+Definition 1 line 2</p>
+                <p class="last">Definition 2</p>
             </dd>
         </dl>
     </div>
@@ -65,11 +69,26 @@ <h1>
     <p>Paragraph 1</p>
     <dl>
         <dt>term 1</dt>
-        <dd>definition 1 definition 2 </dd>
+        <dd> definition 1
+definition 2 </dd>
     </dl>
     <p>Paragraph 2</p>
     <dl>
         <dt>term 2</dt>
-        <dd>definition 1 definition 2 </dd>
+        <dd> definition 1
+definition 2 </dd>
+    </dl>
+</div>
+<div class="section" id="directive-in-definition-list">
+    <h1>
+        Directive in definition list
+    </h1>
+    <dl>
+        <dt>term 1</dt>
+        <dd>
+            <div class="note">
+                <p>directive in definition list</p>
+            </div>
+        </dd>
     </dl>
 </div>
diff --git a/tests/Functional/tests/definition-list/definition-list.rst b/tests/Functional/tests/definition-list/definition-list.rst
@@ -73,3 +73,11 @@ Paragraph 2
 term 2
     definition 1
     definition 2
+
+Directive in definition list
+============================
+
+term 1
+    .. note::
+
+        directive in definition list
diff --git a/tests/Functional/tests/standalone-hyperlinks/standalone-hyperlinks.html b/tests/Functional/tests/standalone-hyperlinks/standalone-hyperlinks.html
@@ -20,10 +20,31 @@
 <a href="mailto:name@example.com">mailto:name@example.com</a><a href="http://www.asianewsphoto.com/(S(neugxif4twuizg551ywh3f55))/Web_ENG/View_DetailPhoto.aspx?PicId=752">http://www.asianewsphoto.com/(S(neugxif4twuizg551ywh3f55))/Web_ENG/View_DetailPhoto.aspx?PicId=752</a><a href="http://www.asianewsphoto.com/(S(neugxif4twuizg551ywh3f55))">http://www.asianewsphoto.com/(S(neugxif4twuizg551ywh3f55))</a><a href="http://lcweb2.loc.gov/cgi-bin/query/h?pp/horyd:@field(NUMBER+@band(thc+5a46634))">http://lcweb2.loc.gov/cgi-bin/query/h?pp/horyd:@field(NUMBER+@band(thc+5a46634))</a><a href="http://www.example.com/wpstyle/?p=364">http://www.example.com/wpstyle/?p=364</a><a href="https://www.example.com/foo/?bar=baz&inga=42&quux">https://www.example.com/foo/?bar=baz&amp;inga=42&amp;quux</a><a href="http://userid:password@example.com:8080">http://userid:password@example.com:8080</a><a href="http://userid:password@example.com:8080/">http://userid:password@example.com:8080/</a><a href="http://userid@example.com">http://userid@example.com</a><a href="http://userid@example.com/">http://userid@example.com/</a><a href="http://userid@example.com:8080">http://userid@example.com:8080</a><a href="http://userid@example.com:8080/">http://userid@example.com:8080/</a><a href="http://userid:password@example.com">http://userid:password@example.com</a><a href="http://userid:password@example.com/">http://userid:password@example.com/</a><a href="http://142.42.1.1/">http://142.42.1.1/</a><a href="http://142.42.1.1:8080/">http://142.42.1.1:8080/</a><a href="http://⌘.ws">http://⌘.ws</a><a href="http://⌘.ws/">http://⌘.ws/</a><a href="http://☺.damowmow.com/">http://☺.damowmow.com/</a><a href="http://code.google.com/events/#&product=browser">http://code.google.com/events/#&amp;product=browser</a><a href="http://j.mp">http://j.mp</a><a href="ftp://foo.bar/baz">ftp://foo.bar/baz</a><a href="http://foo.bar/?q=Test%20URL-encoded%20stuff">http://foo.bar/?q=Test%20URL-encoded%20stuff</a><a href="http://例子.测试">http://例子.测试</a><a href="http://उदाहरण.परीक्षा">http://उदाहरण.परीक्षा</a><a href="http://-._!$&'()*+,;=:%40:80%2f::::::@example.com">http://-._!$&amp;'()*+,;=:%40:80%2f::::::@example.com</a><a href="http://1337.net">http://1337.net</a><a href="http://a.b-c.de">http://a.b-c.de</a><a href="http://223.255.255.254">http://223.255.255.254</a><a href="mailto:jane.doe@example.com">mailto:jane.doe@example.com</a><a href="chrome://chrome">chrome://chrome</a><a href="irc://irc.freenode.net:6667/freenode">irc://irc.freenode.net:6667/freenode</a><a href="microsoft.windows.camera:foobar">microsoft.windows.camera:foobar</a><a href="coaps+ws://foobar">coaps+ws://foobar</a>
 How about multiple <a href="http://example.com/uris">http://example.com/uris</a> on the <a href="mailto:same-line@example.com">mailto:same-line@example.com</a></p>
 </blockquote>
-<dl>
-    <dt> Should fail against <span class="classifier-delimiter">:</span><span class="classifier"></span></dt>
-    <dd>6:00p filename.txt www.c.ws/䨹 Just a www.example.com link. bit.ly/foo “is.gd/foo/” WWW.EXAMPLE.COM http:// http://. http://.. http://? http://?? // //a /// foo.com h://test http:// shouldfail.com :// should fail ✪df.ws/1234 example.com example.com/ </dd>
-</dl>
+<p>Should fail against:</p>
+<blockquote>
+    <p>6:00p
+filename.txt
+www.c.ws/䨹
+Just a www.example.com link.
+bit.ly/foo
+“is.gd/foo/”
+WWW.EXAMPLE.COM
+http://
+http://.
+http://..
+http://?
+http://??
+//
+//a
+///
+foo.com
+h://test
+http:// shouldfail.com
+:// should fail
+✪df.ws/1234
+example.com
+example.com/</p>
+</blockquote>
 <p>These are currently problematic and fail, since the parser appears to see named
 links appearing within these URLs (i.e. &quot;blah_&quot;) and attempts to replace these
 with tokens. These should match the standalone hyperlink pattern, but they do

diff --git a/tests/Functional/tests/standalone-hyperlinks/standalone-hyperlinks.rst b/tests/Functional/tests/standalone-hyperlinks/standalone-hyperlinks.rst
@@ -64,6 +64,7 @@ Matches the right thing in the following lines:
     How about multiple http://example.com/uris on the mailto:same-line@example.com
 
 Should fail against:
+
     6:00p
     filename.txt
     www.c.ws/䨹