From e0a7933a38162368b2a2cc8939674866354a021e Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Sun, 19 Nov 2023 19:19:52 +1300 Subject: [PATCH] Fix list renderer to respect original content (#14) * Fix list renderer to respect original content * Fix ListItemRendererTest * Update ListBlockRendererTest * Fix Test * Add AST Test with multiple List Items --------- Co-authored-by: Stefan Zweifel --- src/Renderer/Block/ListBlockRenderer.php | 30 ------ src/Renderer/Block/ListItemRenderer.php | 58 +++++++++-- .../Renderer/Block/ListBlockRendererTest.php | 96 ++++++++++++++++--- tests/Renderer/Block/ListItemRendererTest.php | 8 +- tests/stubs/kitchen-sink-expected.md | 21 +++- tests/stubs/kitchen-sink.md | 11 +++ 6 files changed, 166 insertions(+), 58 deletions(-) diff --git a/src/Renderer/Block/ListBlockRenderer.php b/src/Renderer/Block/ListBlockRenderer.php index 1ede272..59076ff 100644 --- a/src/Renderer/Block/ListBlockRenderer.php +++ b/src/Renderer/Block/ListBlockRenderer.php @@ -21,39 +21,9 @@ public function render(Node $node, ChildNodeRendererInterface $childRenderer): s { ListBlock::assertInstanceOf($node); - $listData = $node->getListData(); - $content = $childRenderer->renderNodes($node->children()); $content = explode("\n", $content); - $content = array_map(fn ($item) => $this->replaceInternalLineBreakCharacter($item), $content); - - if ($listData->type === ListBlock::TYPE_BULLET) { - $content = array_map(fn ($item) => "- {$item}", $content); - } - - if ($listData->type === ListBlock::TYPE_ORDERED) { - $returnArray = []; - foreach ($content as $key => $value) { - $key++; - $returnArray[] = "{$key}. $value"; - } - - $content = $returnArray; - } - return implode("\n", $content) . "\n"; } - - /** - * Replace custom line break character with _native_ line breaks. - * Whitespace is added so that other Markdown clients correctly - * render the list and its line breaks. - * @param string $content - * @return string - */ - private function replaceInternalLineBreakCharacter(string $content): string - { - return str_replace(ListItemRenderer::INLINE_LINE_BREAK, " \n ", $content); - } } diff --git a/src/Renderer/Block/ListItemRenderer.php b/src/Renderer/Block/ListItemRenderer.php index 2bcadd1..010f38b 100644 --- a/src/Renderer/Block/ListItemRenderer.php +++ b/src/Renderer/Block/ListItemRenderer.php @@ -4,16 +4,17 @@ namespace Wnx\CommonmarkMarkdownRenderer\Renderer\Block; +use League\CommonMark\Extension\CommonMark\Node\Block\ListBlock; +use League\CommonMark\Extension\CommonMark\Node\Block\ListData; use League\CommonMark\Extension\CommonMark\Node\Block\ListItem; use League\CommonMark\Extension\TaskList\TaskListItemMarker; use League\CommonMark\Node\Block\Paragraph; use League\CommonMark\Node\Node; use League\CommonMark\Renderer\ChildNodeRendererInterface; +use LogicException; final class ListItemRenderer implements \League\CommonMark\Renderer\NodeRendererInterface { - public const INLINE_LINE_BREAK = '_COMMONMARK_MARKDOWN_RENDERER_LINE_BREAK_'; - /** * @param ListItem $node * @@ -25,15 +26,11 @@ public function render(Node $node, ChildNodeRendererInterface $childRenderer): s { ListItem::assertInstanceOf($node); + $listData = $node->getListData(); + $contents = $childRenderer->renderNodes($node->children()); - // If the ListItem contains a line break, replace the line break with a custom string. - // The custom line break string is being replaced with a _native_ line break again, when - // being rendered in a ListBlock. - // This workaround is required to support multi-line list items. - if (str_contains($contents, "\n")) { - $contents = str_replace("\n", self::INLINE_LINE_BREAK, $contents); - } + $contents = $this->addPadding($listData->padding, $contents); if (str_starts_with($contents, '<') && ! $this->startsTaskListItem($node)) { $contents = "\n" . $contents; @@ -43,7 +40,26 @@ public function render(Node $node, ChildNodeRendererInterface $childRenderer): s $contents .= "\n"; } - return "{$contents}"; + $contents = $this->addBulletChar($listData, $contents); + + return $contents; + } + + private function addPadding(int $paddingLevel, string $content): string + { + $padding = str_repeat(' ', $paddingLevel); + $lines = []; + $isFirstLine = true; + foreach (explode("\n", $content) as $line) { + // We don't need to indent the first line. + if ($isFirstLine) { + $isFirstLine = false; + } else { + $line = "{$padding}{$line}"; + } + $lines[] = $line; + } + return implode("\n", $lines); } private function startsTaskListItem(ListItem $block): bool @@ -52,4 +68,26 @@ private function startsTaskListItem(ListItem $block): bool return $firstChild instanceof Paragraph && $firstChild->firstChild() instanceof TaskListItemMarker; } + + private function addBulletChar(ListData $listData, string $content): string + { + switch ($listData->type) { + case ListBlock::TYPE_BULLET: + return "{$listData->bulletChar} {$content}"; + case ListBlock::TYPE_ORDERED: + switch ($listData->delimiter) { + case ListBlock::DELIM_PAREN: + $delimiter = ')'; + break; + case ListBlock::DELIM_PERIOD: + $delimiter = '.'; + break; + default: + throw new LogicException('Unexpected list delimiter: ' . $listData->delimiter); + } + return "{$listData->start}{$delimiter} $content"; + default: + throw new LogicException('Unexpected list type: ' . $listData->type); + } + } } diff --git a/tests/Renderer/Block/ListBlockRendererTest.php b/tests/Renderer/Block/ListBlockRendererTest.php index 558dd36..805e2e8 100644 --- a/tests/Renderer/Block/ListBlockRendererTest.php +++ b/tests/Renderer/Block/ListBlockRendererTest.php @@ -4,12 +4,17 @@ namespace Wnx\CommonmarkMarkdownRenderer\Tests\Renderer\Block; +use League\CommonMark\Environment\Environment; use League\CommonMark\Extension\CommonMark\Node\Block\ListBlock; use League\CommonMark\Extension\CommonMark\Node\Block\ListData; +use League\CommonMark\Extension\CommonMark\Node\Block\ListItem; +use League\CommonMark\Node\Block\Paragraph; +use League\CommonMark\Node\Inline\Text; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; +use Wnx\CommonmarkMarkdownRenderer\MarkdownRendererExtension; use Wnx\CommonmarkMarkdownRenderer\Renderer\Block\ListBlockRenderer; -use Wnx\CommonmarkMarkdownRenderer\Tests\Support\FakeChildNodeRenderer; +use Wnx\CommonmarkMarkdownRenderer\Renderer\MarkdownRenderer; final class ListBlockRendererTest extends TestCase { @@ -23,34 +28,103 @@ protected function setUp(): void #[Test] public function it_renders_ordered_list_block(): void { + // Build up Children $data = new ListData(); $data->type = ListBlock::TYPE_ORDERED; - $data->start = 0; + $data->start = 1; + $data->padding = 3; + $data->delimiter = 'period'; + $data->bulletChar = '-'; + + $listItem = new ListItem($data); + + $paragraph = new Paragraph(); + $paragraph->appendChild(new Text('List Item Value')); + $listItem->appendChild($paragraph); $block = new ListBlock($data); - $fakeRenderer = new FakeChildNodeRenderer(); - $fakeRenderer->pretendChildrenExist(); + $block->appendChild($listItem); + + // Build up Child Renderer + $environment = new Environment(); + $environment->addExtension(new MarkdownRendererExtension()); + $childRenderer = new MarkdownRenderer($environment); - $result = $this->renderer->render($block, $fakeRenderer); + // Render AST + $result = $this->renderer->render($block, $childRenderer); + // Assert $this->assertIsString($result); - $this->assertEquals("1. ::children::\n", $result); + $this->assertEquals("1. List Item Value\n \n", $result); } #[Test] public function it_renders_unordered_list_block(): void { + // Build up Children $data = new ListData(); $data->type = ListBlock::TYPE_BULLET; - $data->start = 0; + $data->padding = 2; + $data->bulletChar = '-'; + + $listItem = new ListItem($data); + + $paragraph = new Paragraph(); + $paragraph->appendChild(new Text('List Item Value')); + $listItem->appendChild($paragraph); $block = new ListBlock($data); - $fakeRenderer = new FakeChildNodeRenderer(); - $fakeRenderer->pretendChildrenExist(); + $block->appendChild($listItem); + + // Build up Child Renderer + $environment = new Environment(); + $environment->addExtension(new MarkdownRendererExtension()); + $childRenderer = new MarkdownRenderer($environment); - $result = $this->renderer->render($block, $fakeRenderer); + // Render AST + $result = $this->renderer->render($block, $childRenderer); $this->assertIsString($result); - $this->assertEquals("- ::children::\n", $result); + $this->assertEquals("- List Item Value\n \n", $result); + } + + #[Test] + public function it_renders_unordered_list_with_multiple_list_item_values_correctly(): void + { + // Build up Children + $data = new ListData(); + $data->type = ListBlock::TYPE_BULLET; + $data->padding = 2; + $data->bulletChar = '-'; + + $listItem = new ListItem($data); + + $paragraph = new Paragraph(); + $paragraph->appendChild(new Text('List Item Value')); + $listItem->appendChild($paragraph); + + $block = new ListBlock($data); + $block->appendChild($listItem); + $block->appendChild(clone $listItem); + $block->appendChild(clone $listItem); + + // Build up Child Renderer + $environment = new Environment(); + $environment->addExtension(new MarkdownRendererExtension()); + $childRenderer = new MarkdownRenderer($environment); + + // Render AST + $result = $this->renderer->render($block, $childRenderer); + + $this->assertIsString($result); + $this->assertEquals(<<<'TXT' + - List Item Value + + - List Item Value + + - List Item Value + + + TXT, $result); } } diff --git a/tests/Renderer/Block/ListItemRendererTest.php b/tests/Renderer/Block/ListItemRendererTest.php index c243cc3..bc2fa4c 100644 --- a/tests/Renderer/Block/ListItemRendererTest.php +++ b/tests/Renderer/Block/ListItemRendererTest.php @@ -4,6 +4,7 @@ namespace Wnx\CommonmarkMarkdownRenderer\Tests\Renderer\Block; +use League\CommonMark\Extension\CommonMark\Node\Block\ListBlock; use League\CommonMark\Extension\CommonMark\Node\Block\ListData; use League\CommonMark\Extension\CommonMark\Node\Block\ListItem; use PHPUnit\Framework\Attributes\Test; @@ -23,13 +24,16 @@ protected function setUp(): void #[Test] public function it_renders_unordered_list(): void { - $block = new ListItem(new ListData()); + $data = new ListData(); + $data->type = ListBlock::TYPE_BULLET; + + $block = new ListItem($data); $block->data->set('attributes/id', 'foo'); $fakeRenderer = new FakeChildNodeRenderer(); $fakeRenderer->pretendChildrenExist(); $result = $this->renderer->render($block, $fakeRenderer); - $this->assertEquals('::children::', $result); + $this->assertEquals(' ::children::', $result); } } diff --git a/tests/stubs/kitchen-sink-expected.md b/tests/stubs/kitchen-sink-expected.md index 8b52dd6..ce6e76e 100644 --- a/tests/stubs/kitchen-sink-expected.md +++ b/tests/stubs/kitchen-sink-expected.md @@ -5,15 +5,26 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Curiosities -- This is a list item which is followed by an indented content on the next line +- This is a list item which is followed by an indented content on the next line which is part of the same list item -- This is a list item which contains multiple paragraphs - This is the second paragraph. +- This is a list item which contains multiple paragraphs + This is the second paragraph. This is the third paragraph. -- This is a list item with multiple paragraphs, but without trailing whitespace in the original document at the end - This is the second paragraph. +- This is a list item with multiple paragraphs, but without trailing whitespace in the original document at the end + This is the second paragraph. This is the third paragraph. +* This list is indented a little further + It's using 4 characters to indent items. For some reason league/commonmark reports 2 though... +* And it uses asterisks to denote list items + +0. this ordered list starts with 0. +0. And it uses 0 for the second item. +3. And then it uses 3 here! But that's valid too. + +1) And this list uses parenthesis for its delimiter +2) and that's fine too. + ## Task List - [ ] Task 1 diff --git a/tests/stubs/kitchen-sink.md b/tests/stubs/kitchen-sink.md index 33affb9..0497ecf 100644 --- a/tests/stubs/kitchen-sink.md +++ b/tests/stubs/kitchen-sink.md @@ -14,6 +14,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). This is the second paragraph. This is the third paragraph. +* This list is indented a little further + It's using 4 characters to indent items. For some reason league/commonmark reports 2 though... +* And it uses asterisks to denote list items + +0. this ordered list starts with 0. +0. And it uses 0 for the second item. +3. And then it uses 3 here! But that's valid too. + +1) And this list uses parenthesis for its delimiter +2) and that's fine too. + ## Task List - [ ] Task 1