From 12ce6bc4244d9f0c06b2ff3bab753e6a42c0907e Mon Sep 17 00:00:00 2001 From: Robin Stocker <robin@nibor.org> Date: Wed, 17 Jan 2024 22:43:30 +1100 Subject: [PATCH] Fix tight list with nested loose list --- .../markdown/CoreMarkdownNodeRenderer.java | 90 +++++++++---------- .../renderer/markdown/MarkdownWriter.java | 70 +++++++++------ .../markdown/MarkdownRendererTest.java | 4 + .../markdown/SpecMarkdownRendererTest.java | 2 +- 4 files changed, 94 insertions(+), 72 deletions(-) diff --git a/commonmark/src/main/java/org/commonmark/renderer/markdown/CoreMarkdownNodeRenderer.java b/commonmark/src/main/java/org/commonmark/renderer/markdown/CoreMarkdownNodeRenderer.java index 28461c87..5e883333 100644 --- a/commonmark/src/main/java/org/commonmark/renderer/markdown/CoreMarkdownNodeRenderer.java +++ b/commonmark/src/main/java/org/commonmark/renderer/markdown/CoreMarkdownNodeRenderer.java @@ -92,6 +92,49 @@ public class CoreMarkdownNodeRenderer extends AbstractVisitor implements NodeRen visitChildren(bulletList); listHolder = listHolder.parent; writer.setTight(oldTight); + writer.block(); + } + + @Override + public void visit(OrderedList orderedList) { + boolean oldTight = writer.getTight(); + writer.setTight(orderedList.isTight()); + listHolder = new OrderedListHolder(listHolder, orderedList); + visitChildren(orderedList); + listHolder = listHolder.parent; + writer.setTight(oldTight); + writer.block(); + } + + @Override + public void visit(ListItem listItem) { + int contentIndent = listItem.getContentIndent(); + boolean pushedPrefix = false; + if (listHolder instanceof BulletListHolder) { + BulletListHolder bulletListHolder = (BulletListHolder) listHolder; + String marker = repeat(" ", listItem.getMarkerIndent()) + bulletListHolder.bulletMarker; + writer.write(marker); + writer.write(repeat(" ", contentIndent - marker.length())); + writer.pushPrefix(repeat(" ", contentIndent)); + pushedPrefix = true; + } else if (listHolder instanceof OrderedListHolder) { + OrderedListHolder orderedListHolder = (OrderedListHolder) listHolder; + String marker = repeat(" ", listItem.getMarkerIndent()) + orderedListHolder.number + orderedListHolder.delimiter; + orderedListHolder.number++; + writer.write(marker); + writer.write(repeat(" ", contentIndent - marker.length())); + writer.pushPrefix(repeat(" ", contentIndent)); + pushedPrefix = true; + } + if (listItem.getFirstChild() == null) { + // Empty list item + writer.block(); + } else { + visitChildren(listItem); + } + if (pushedPrefix) { + writer.popPrefix(); + } } @Override @@ -221,55 +264,10 @@ public class CoreMarkdownNodeRenderer extends AbstractVisitor implements NodeRen writeLinkLike(link.getTitle(), link.getDestination(), link, "["); } - @Override - public void visit(ListItem listItem) { - int contentIndent = listItem.getContentIndent(); - boolean pushedPrefix = false; - if (listHolder instanceof BulletListHolder) { - BulletListHolder bulletListHolder = (BulletListHolder) listHolder; - String marker = repeat(" ", listItem.getMarkerIndent()) + bulletListHolder.bulletMarker; - writer.write(marker); - writer.write(repeat(" ", contentIndent - marker.length())); - writer.pushPrefix(repeat(" ", contentIndent)); - pushedPrefix = true; - } else if (listHolder instanceof OrderedListHolder) { - OrderedListHolder orderedListHolder = (OrderedListHolder) listHolder; - String marker = repeat(" ", listItem.getMarkerIndent()) + orderedListHolder.number + orderedListHolder.delimiter; - orderedListHolder.number++; - writer.write(marker); - writer.write(repeat(" ", contentIndent - marker.length())); - writer.pushPrefix(repeat(" ", contentIndent)); - pushedPrefix = true; - } - if (listItem.getFirstChild() == null) { - // Empty list item - writer.block(); - } else { - visitChildren(listItem); - } - if (pushedPrefix) { - writer.popPrefix(); - } - } - - @Override - public void visit(OrderedList orderedList) { - boolean oldTight = writer.getTight(); - writer.setTight(orderedList.isTight()); - listHolder = new OrderedListHolder(listHolder, orderedList); - visitChildren(orderedList); - listHolder = listHolder.parent; - writer.setTight(oldTight); - } - @Override public void visit(Paragraph paragraph) { visitChildren(paragraph); - if (paragraph.getParent() instanceof ListItem) { - writer.block(); - } else { - writer.block(); - } + writer.block(); } @Override diff --git a/commonmark/src/main/java/org/commonmark/renderer/markdown/MarkdownWriter.java b/commonmark/src/main/java/org/commonmark/renderer/markdown/MarkdownWriter.java index c2c82619..ba682d46 100644 --- a/commonmark/src/main/java/org/commonmark/renderer/markdown/MarkdownWriter.java +++ b/commonmark/src/main/java/org/commonmark/renderer/markdown/MarkdownWriter.java @@ -9,7 +9,7 @@ public class MarkdownWriter { private final Appendable buffer; - private boolean finishBlock = false; + private int blockSeparator = 0; private boolean tight; private char lastChar; private final LinkedList<String> prefixes = new LinkedList<>(); @@ -22,22 +22,13 @@ public class MarkdownWriter { return lastChar; } - public void block() { - finishBlock = true; - } - - public void line() { - append('\n'); - writePrefixes(); - } - public void write(String s) { - finishBlockIfNeeded(); + flushBlockSeparator(); append(s); } public void write(char c) { - finishBlockIfNeeded(); + flushBlockSeparator(); append(c); } @@ -45,7 +36,7 @@ public class MarkdownWriter { if (s.isEmpty()) { return; } - finishBlockIfNeeded(); + flushBlockSeparator(); try { for (int i = 0; i < s.length(); i++) { char ch = s.charAt(i); @@ -61,6 +52,21 @@ public class MarkdownWriter { lastChar = s.charAt(s.length() - 1); } + public void line() { + append('\n'); + writePrefixes(); + } + + /** + * Enqueue a block separator to be written before the next text is written. Block separators are not written + * straight away because if there are no more blocks to write we don't want a separator (at the end of the document). + */ + public void block() { + // Remember whether this should be a tight or loose separator now because tight could get changed in between + // this and the next flush. + blockSeparator = tight ? 1 : 2; + } + public void pushPrefix(String prefix) { prefixes.addLast(prefix); } @@ -92,18 +98,6 @@ public class MarkdownWriter { lastChar = c; } - private void finishBlockIfNeeded() { - if (finishBlock) { - finishBlock = false; - append('\n'); - writePrefixes(); - if (!tight) { - append('\n'); - writePrefixes(); - } - } - } - private void writePrefixes() { if (!prefixes.isEmpty()) { for (String prefix : prefixes) { @@ -112,10 +106,36 @@ public class MarkdownWriter { } } + /** + * If a block separator has been enqueued with {@link #block()} but not yet written, write it now. + */ + private void flushBlockSeparator() { + if (blockSeparator != 0) { + append('\n'); + writePrefixes(); + if (blockSeparator > 1) { + append('\n'); + writePrefixes(); + } + blockSeparator = 0; + } + } + + /** + * @return whether blocks are currently set to tight or loose, see {@link #setTight(boolean)} + */ public boolean getTight() { return tight; } + /** + * Change whether blocks are tight or loose. Loose is the default where blocks are separated by a blank line. Tight + * is where blocks are not separated by a blank line. Tight blocks are used in lists, if there are no blank lines + * within the list. + * <p> + * Note that changing this does not affect block separators that have already been enqueued (with {@link #block()}, + * only future ones. + */ public void setTight(boolean tight) { this.tight = tight; } diff --git a/commonmark/src/test/java/org/commonmark/renderer/markdown/MarkdownRendererTest.java b/commonmark/src/test/java/org/commonmark/renderer/markdown/MarkdownRendererTest.java index 06ad79c2..f6ce1b4e 100644 --- a/commonmark/src/test/java/org/commonmark/renderer/markdown/MarkdownRendererTest.java +++ b/commonmark/src/test/java/org/commonmark/renderer/markdown/MarkdownRendererTest.java @@ -82,6 +82,8 @@ public class MarkdownRendererTest { // Tight list assertRoundTrip("* foo\n* bar\n"); + // Tight list where the second item contains a loose list + assertRoundTrip("- Foo\n - Bar\n \n - Baz\n"); // List item indent. This is a tricky one, but here the amount of space between the list marker and "one" // determines whether "two" is part of the list item or an indented code block. @@ -102,6 +104,8 @@ public class MarkdownRendererTest { // Tight list assertRoundTrip("1. foo\n2. bar\n"); + // Tight list where the second item contains a loose list + assertRoundTrip("1. Foo\n 1. Bar\n \n 2. Baz\n"); assertRoundTrip(" 1. one\n\n two\n"); } diff --git a/commonmark/src/test/java/org/commonmark/renderer/markdown/SpecMarkdownRendererTest.java b/commonmark/src/test/java/org/commonmark/renderer/markdown/SpecMarkdownRendererTest.java index 682fee9d..e21ec8cc 100644 --- a/commonmark/src/test/java/org/commonmark/renderer/markdown/SpecMarkdownRendererTest.java +++ b/commonmark/src/test/java/org/commonmark/renderer/markdown/SpecMarkdownRendererTest.java @@ -52,7 +52,7 @@ public class SpecMarkdownRendererTest { System.out.println("Failed examples by section (total " + fails.size() + "):"); printCountsBySection(fails); - int expectedPassed = 618; + int expectedPassed = 621; assertTrue("Expected at least " + expectedPassed + " examples to pass but was " + passes.size(), passes.size() >= expectedPassed); } -- GitLab