Fix tight list with nested loose list

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();
-        }
     }
 
     @Override

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;
     }

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");
     }

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);
     }