Merge remote-tracking branch 'origin/2.6'

Author: colinodell

.github/workflows/tests.yml

@@ -33,7 +33,7 @@ jobs:
         strategy:
             fail-fast: false
             matrix:
-                php: ['7.4', '8.0', '8.1', '8.2', '8.3']
+                php: ['7.4', '8.0', '8.1', '8.2', '8.3', '8.4']
                 stable: [true]
                 coverage: [true]
                 composer-flags: ['']
@@ -42,7 +42,7 @@ jobs:
                       stable: true
                       coverage: false
                       composer-flags: '--prefer-lowest'
-                    - php: '8.4'
+                    - php: '8.5'
                       stable: false
                       coverage: false
                       composer-flags: '--ignore-platform-req=php'
@@ -114,7 +114,7 @@ jobs:
         runs-on: ubuntu-latest
 
         steps:
-            - uses: actions/checkout@v3
+            - uses: actions/checkout@v4
 
             - uses: shivammathur/setup-php@v2
               with:

CHANGELOG.md

@@ -6,6 +6,12 @@ Updates should follow the [Keep a CHANGELOG](https://keepachangelog.com/) princi
 
 ## [Unreleased][unreleased]
 
+## [2.6.1] - 2024-12-29
+
+### Fixed
+
+- Rendered list items should only add newlines around block-level children (#1059, #1061)
+
 ## [2.6.0] - 2024-12-07
 
 This is a **security release** to address potential denial of service attacks when parsing specially crafted,
@@ -677,7 +683,8 @@ No changes were introduced since the previous release.
     - Alternative 1: Use `CommonMarkConverter` or `GithubFlavoredMarkdownConverter` if you don't need to customize the environment
     - Alternative 2: Instantiate a new `Environment` and add the necessary extensions yourself
 
-[unreleased]: https://github.com/thephpleague/commonmark/compare/2.6.0...main
+[unreleased]: https://github.com/thephpleague/commonmark/compare/2.6.1...main
+[2.6.1]: https://github.com/thephpleague/commonmark/compare/2.6.0...2.6.1
 [2.6.0]: https://github.com/thephpleague/commonmark/compare/2.5.3...2.6.0
 [2.5.3]: https://github.com/thephpleague/commonmark/compare/2.5.2...2.5.3
 [2.5.2]: https://github.com/thephpleague/commonmark/compare/2.5.1...2.5.2

src/Extension/CommonMark/Renderer/Block/ListItemRenderer.php

@@ -17,8 +17,9 @@
 namespace League\CommonMark\Extension\CommonMark\Renderer\Block;
 
 use League\CommonMark\Extension\CommonMark\Node\Block\ListItem;
-use League\CommonMark\Extension\TaskList\TaskListItemMarker;
+use League\CommonMark\Node\Block\AbstractBlock;
 use League\CommonMark\Node\Block\Paragraph;
+use League\CommonMark\Node\Block\TightBlockInterface;
 use League\CommonMark\Node\Node;
 use League\CommonMark\Renderer\ChildNodeRendererInterface;
 use League\CommonMark\Renderer\NodeRendererInterface;
@@ -39,11 +40,14 @@ public function render(Node $node, ChildNodeRendererInterface $childRenderer): \
         ListItem::assertInstanceOf($node);
 
         $contents = $childRenderer->renderNodes($node->children());
-        if (\substr($contents, 0, 1) === '<' && ! $this->startsTaskListItem($node)) {
+
+        $inTightList = ($parent = $node->parent()) && $parent instanceof TightBlockInterface && $parent->isTight();
+
+        if ($this->needsBlockSeparator($node->firstChild(), $inTightList)) {
             $contents = "\n" . $contents;
         }
 
-        if (\substr($contents, -1, 1) === '>') {
+        if ($this->needsBlockSeparator($node->lastChild(), $inTightList)) {
             $contents .= "\n";
         }
 
@@ -65,10 +69,12 @@ public function getXmlAttributes(Node $node): array
         return [];
     }
 
-    private function startsTaskListItem(ListItem $block): bool
+    private function needsBlockSeparator(?Node $child, bool $inTightList): bool
     {
-        $firstChild = $block->firstChild();
+        if ($child instanceof Paragraph && $inTightList) {
+            return false;
+        }
 
-        return $firstChild instanceof Paragraph && $firstChild->firstChild() instanceof TaskListItemMarker;
+        return $child instanceof AbstractBlock;
     }
 }

tests/functional/CMarkRegressionTest.php

@@ -27,12 +27,6 @@ public static function dataProvider(): \Generator
     {
         $tests = SpecReader::readFile(__DIR__ . '/../../vendor/commonmark/cmark/test/regression.txt');
         foreach ($tests as $example) {
-            // We can't currently render spec example 13 exactly how the upstream library does. We'll likely need to overhaul
-            // our rendering approach in order to fix that, so we'll use this temporary workaround for now.
-            if ($example['number'] === 13) {
-                $example['output'] = \str_replace('</script></li>', "</script>\n</li>", $example['output']);
-            }
-
             // The case-fold test from example 21 fails on PHP 8.0.* and below due to the behavior of mb_convert_case().
             // See https://3v4l.org/7TeXJ.
             if (\PHP_VERSION_ID < 81000 && $example['number'] === 21) {

tests/functional/CommonMarkJSRegressionTest.php

@@ -25,17 +25,6 @@ final class CommonMarkJSRegressionTest extends AbstractSpecTestCase
 {
     public static function dataProvider(): \Generator
     {
-        $tests = SpecReader::readFile(__DIR__ . '/../../vendor/commonmark/commonmark.js/test/regression.txt');
-        foreach ($tests as $example) {
-            // We can't currently render spec examples 18 or 24 exactly how the upstream library does. We'll likely need to overhaul
-            // our rendering approach in order to fix that, so we'll use this temporary workaround for now.
-            if ($example['number'] === 18) {
-                $example['output'] = \str_replace('</script></li>', "</script>\n</li>", $example['output']);
-            } elseif ($example['number'] === 24) {
-                $example['output'] = \str_replace("<pre>The following line is part of HTML block.\n\n</li>", "<pre>The following line is part of HTML block.\n</li>", $example['output']);
-            }
-
-            yield $example;
-        }
+        yield from SpecReader::readFile(__DIR__ . '/../../vendor/commonmark/commonmark.js/test/regression.txt');
     }
 }

tests/functional/Delimiter/DelimiterProcessingTest.php

@@ -49,7 +49,7 @@ public function testAsymmetricDelimiterProcessing(string $input, string $expecte
 
         $converter = new MarkdownConverter($e);
 
-        $this->assertEquals($expected, $converter->convert($input));
+        $this->assertEquals($expected, $converter->convert($input)->getContent());
     }
 
     /**

tests/functional/Extension/Autolink/UrlAutolinkParserTest.php

@@ -49,7 +49,7 @@ public static function dataProviderForAutolinkTests(): iterable
         yield ['www.google.com', '<p><a href="http://www.google.com">www.google.com</a></p>'];
         yield [' http://leadingwhitespace.example.com', '<p><a href="http://leadingwhitespace.example.com">http://leadingwhitespace.example.com</a></p>'];
         yield ['http://trailingwhitespace.example.com ', '<p><a href="http://trailingwhitespace.example.com">http://trailingwhitespace.example.com</a></p>'];
-        yield ['- https://example.com/list-item', "<ul>\n<li>\n<a href=\"https://example.com/list-item\">https://example.com/list-item</a>\n</li>\n</ul>"];
+        yield ['- https://example.com/list-item', "<ul>\n<li><a href=\"https://example.com/list-item\">https://example.com/list-item</a></li>\n</ul>"];
 
         // Tests of "incomplete" URLs
         yield ['google.com is missing www and/or a protocol', '<p>google.com is missing www and/or a protocol</p>'];
@@ -60,7 +60,7 @@ public static function dataProviderForAutolinkTests(): iterable
         yield ['Maybe you\'re interested in https://www.google.com/search?q=php+commonmark!', '<p>Maybe you\'re interested in <a href="https://www.google.com/search?q=php+commonmark">https://www.google.com/search?q=php+commonmark</a>!</p>'];
         yield ['Or perhaps you\'re looking for my personal website https://www.colinodell.com...?', '<p>Or perhaps you\'re looking for my personal website <a href="https://www.colinodell.com">https://www.colinodell.com</a>...?</p>'];
         yield ['Check https://www.stackoverflow.com: they have all the answers', '<p>Check <a href="https://www.stackoverflow.com">https://www.stackoverflow.com</a>: they have all the answers</p>'];
-        yield ['- https://example.com/list-item-with-trailing-colon:', "<ul>\n<li>\n<a href=\"https://example.com/list-item-with-trailing-colon\">https://example.com/list-item-with-trailing-colon</a>:</li>\n</ul>"];
+        yield ['- https://example.com/list-item-with-trailing-colon:', "<ul>\n<li><a href=\"https://example.com/list-item-with-trailing-colon\">https://example.com/list-item-with-trailing-colon</a>:</li>\n</ul>"];
         yield ['Visit www.commonmark.org.', '<p>Visit <a href="http://www.commonmark.org">www.commonmark.org</a>.</p>'];
         yield ['Visit www.commonmark.org/a.b.', '<p>Visit <a href="http://www.commonmark.org/a.b">www.commonmark.org/a.b</a>.</p>'];
 

tests/functional/Extension/Footnote/spec.txt

@@ -106,12 +106,9 @@ A [footnote identifier] can come immediately after a word, or have a space betwe
 [^5]: Not allowed
 .
 <ul>
-<li>No spaces<sup id="fnref:1"><a class="footnote-ref" href="#fn:1" role="doc-noteref">1</a></sup>
-</li>
-<li>One space <sup id="fnref:2"><a class="footnote-ref" href="#fn:2" role="doc-noteref">2</a></sup>
-</li>
-<li>Exclamation mark with space! <sup id="fnref:3"><a class="footnote-ref" href="#fn:3" role="doc-noteref">3</a></sup>
-</li>
+<li>No spaces<sup id="fnref:1"><a class="footnote-ref" href="#fn:1" role="doc-noteref">1</a></sup></li>
+<li>One space <sup id="fnref:2"><a class="footnote-ref" href="#fn:2" role="doc-noteref">2</a></sup></li>
+<li>Exclamation mark with space! <sup id="fnref:3"><a class="footnote-ref" href="#fn:3" role="doc-noteref">3</a></sup></li>
 <li>Exclamation mark with no space![^4]</li>
 <li>Another exclamation mark![^5](not allowed)</li>
 </ul>

tests/functional/Extension/Mention/MentionExtensionTest.php

@@ -44,7 +44,7 @@ public function testNoConfig(): void
 
         $converter = new MarkdownConverter($environment);
 
-        $this->assertEquals($expected, $converter->convert($input));
+        $this->assertEquals($expected, $converter->convert($input)->getContent());
     }
 
     public function testConfigStringGenerator(): void
@@ -72,7 +72,7 @@ public function testConfigStringGenerator(): void
 
         $converter = new MarkdownConverter($environment);
 
-        $this->assertEquals($expected, $converter->convert($input));
+        $this->assertEquals($expected, $converter->convert($input)->getContent());
     }
 
     public function testConfigCallableGenerator(): void
@@ -104,7 +104,7 @@ public function testConfigCallableGenerator(): void
 
         $converter = new MarkdownConverter($environment);
 
-        $this->assertEquals($expected, $converter->convert($input));
+        $this->assertEquals($expected, $converter->convert($input)->getContent());
     }
 
     public function testConfigObjectImplementingMentionGeneratorInterface(): void
@@ -139,7 +139,7 @@ public function generateMention(Mention $mention): ?AbstractInline
 
         $converter = new MarkdownConverter($environment);
 
-        $this->assertEquals($expected, $converter->convert($input));
+        $this->assertEquals($expected, $converter->convert($input)->getContent());
     }
 
     public function testConfigUnknownGenerator(): void

tests/functional/Extension/TableOfContents/md/custom-class.html

@@ -1,10 +1,7 @@
 <ul class="markdown-toc">
-<li>
-<a href="#content-hello">Hello</a>
+<li><a href="#content-hello">Hello</a>
 <ul>
-<li>
-<a href="#content-world">World</a>
-</li>
+<li><a href="#content-world">World</a></li>
 </ul>
 </li>
 </ul>

tests/functional/Extension/TableOfContents/md/headings-with-inlines.html

@@ -1,13 +1,7 @@
 <ul class="table-of-contents">
-<li>
-<a href="#content-this-heading-has-a-link">This heading has a link</a>
-</li>
-<li>
-<a href="#content-20-reasons-why-marquee-tags-should-make-a-comeback">20 Reasons Why &lt;marquee&gt; Tags Should Make A Comeback</a>
-</li>
-<li>
-<a href="#content-i-love-using--characters-in-markdown">I love using * characters in Markdown!</a>
-</li>
+<li><a href="#content-this-heading-has-a-link">This heading has a link</a></li>
+<li><a href="#content-20-reasons-why-marquee-tags-should-make-a-comeback">20 Reasons Why &lt;marquee&gt; Tags Should Make A Comeback</a></li>
+<li><a href="#content-i-love-using--characters-in-markdown">I love using * characters in Markdown!</a></li>
 </ul>
 <p>My Awesome Blog Post</p>
 <h2><a id="content-this-heading-has-a-link" href="#content-this-heading-has-a-link" class="heading-permalink" aria-hidden="true" title="Permalink">¶</a><a href="https://example.com">This heading</a> has a link</h2>

tests/functional/Extension/TableOfContents/md/html-in-headings.html

@@ -1,7 +1,5 @@
 <ul class="table-of-contents">
-<li>
-<a href="#content-hello-world">Hello World! </a>
-</li>
+<li><a href="#content-hello-world">Hello World! </a></li>
 </ul>
 <p>Here's a sample Markdown document</p>
 <h1><a id="content-hello-world" href="#content-hello-world" class="heading-permalink" aria-hidden="true" title="Permalink">¶</a>Hello World! <i class="icon icon-waving-hand"></i></h1>

tests/functional/Extension/TableOfContents/md/min-max.html

@@ -1,16 +1,11 @@
 <ul class="table-of-contents">
-<li>
-<a href="#content-level-2">Level 2</a>
+<li><a href="#content-level-2">Level 2</a>
 <ul>
-<li>
-<a href="#content-level-3">Level 3</a>
+<li><a href="#content-level-3">Level 3</a>
 <ul>
-<li>
-<a href="#content-level-4">Level 4</a>
+<li><a href="#content-level-4">Level 4</a>
 <ul>
-<li>
-<a href="#content-level-5">Level 5</a>
-</li>
+<li><a href="#content-level-5">Level 5</a></li>
 </ul>
 </li>
 </ul>

tests/functional/Extension/TableOfContents/md/position-before-headings.html

@@ -1,11 +1,8 @@
 <p>This is my document.</p>
 <ul class="table-of-contents">
-<li>
-<a href="#content-hello-world">Hello World!</a>
+<li><a href="#content-hello-world">Hello World!</a>
 <ul>
-<li>
-<a href="#content-isnt-markdown-great">Isn't Markdown Great?</a>
-</li>
+<li><a href="#content-isnt-markdown-great">Isn't Markdown Great?</a></li>
 </ul>
 </li>
 </ul>

tests/functional/Extension/TableOfContents/md/position-placeholder-multiple.html

@@ -1,28 +1,18 @@
 <p>This is my document.</p>
 <ul class="table-of-contents">
-<li>
-<a href="#content-another-copy-of-my-toc-is-here">Another copy of my TOC is here</a>
+<li><a href="#content-another-copy-of-my-toc-is-here">Another copy of my TOC is here</a>
 <ul>
-<li>
-<a href="#content-this-contains-something-that-looks-like-a-placeholder-but-actually-isnt">This contains something that looks like a placeholder but actually isn't</a>
-</li>
-<li>
-<a href="#content-just-a-link-reference-down-here">Just a link reference down here</a>
-</li>
+<li><a href="#content-this-contains-something-that-looks-like-a-placeholder-but-actually-isnt">This contains something that looks like a placeholder but actually isn't</a></li>
+<li><a href="#content-just-a-link-reference-down-here">Just a link reference down here</a></li>
 </ul>
 </li>
 </ul>
 <h1><a id="content-another-copy-of-my-toc-is-here" href="#content-another-copy-of-my-toc-is-here" class="heading-permalink" aria-hidden="true" title="Permalink">¶</a>Another copy of my TOC is here</h1>
 <ul class="table-of-contents">
-<li>
-<a href="#content-another-copy-of-my-toc-is-here">Another copy of my TOC is here</a>
+<li><a href="#content-another-copy-of-my-toc-is-here">Another copy of my TOC is here</a>
 <ul>
-<li>
-<a href="#content-this-contains-something-that-looks-like-a-placeholder-but-actually-isnt">This contains something that looks like a placeholder but actually isn't</a>
-</li>
-<li>
-<a href="#content-just-a-link-reference-down-here">Just a link reference down here</a>
-</li>
+<li><a href="#content-this-contains-something-that-looks-like-a-placeholder-but-actually-isnt">This contains something that looks like a placeholder but actually isn't</a></li>
+<li><a href="#content-just-a-link-reference-down-here">Just a link reference down here</a></li>
 </ul>
 </li>
 </ul>

tests/functional/Extension/TableOfContents/md/position-top.html

@@ -1,10 +1,7 @@
 <ul class="table-of-contents">
-<li>
-<a href="#content-hello-world">Hello World!</a>
+<li><a href="#content-hello-world">Hello World!</a>
 <ul>
-<li>
-<a href="#content-isnt-markdown-great">Isn't Markdown Great?</a>
-</li>
+<li><a href="#content-isnt-markdown-great">Isn't Markdown Great?</a></li>
 </ul>
 </li>
 </ul>