Merge pull request #312 from commonmark/fix-markdown-renderer-fenced-code-blocks-from-ast

Fix markdown renderer for manually created fenced code blocks

Author: robinst

commonmark/src/main/java/org/commonmark/internal/FencedCodeBlockParser.java

@@ -12,13 +12,17 @@
 public class FencedCodeBlockParser extends AbstractBlockParser {
 
     private final FencedCodeBlock block = new FencedCodeBlock();
+    private final char fenceChar;
+    private final int openingFenceLength;
 
     private String firstLine;
     private StringBuilder otherLines = new StringBuilder();
 
     public FencedCodeBlockParser(char fenceChar, int fenceLength, int fenceIndent) {
-        block.setFenceChar(fenceChar);
-        block.setFenceLength(fenceLength);
+        this.fenceChar = fenceChar;
+        this.openingFenceLength = fenceLength;
+        block.setFenceCharacter(String.valueOf(fenceChar));
+        block.setOpeningFenceLength(fenceLength);
         block.setFenceIndent(fenceIndent);
     }
 
@@ -32,7 +36,7 @@ public BlockContinue tryContinue(ParserState state) {
         int nextNonSpace = state.getNextNonSpaceIndex();
         int newIndex = state.getIndex();
         CharSequence line = state.getLine().getContent();
-        if (state.getIndent() < Parsing.CODE_BLOCK_INDENT && nextNonSpace < line.length() && line.charAt(nextNonSpace) == block.getFenceChar() && isClosing(line, nextNonSpace)) {
+        if (state.getIndent() < Parsing.CODE_BLOCK_INDENT && nextNonSpace < line.length() && tryClosing(line, nextNonSpace)) {
             // closing fence - we're at end of line, so we can finalize now
             return BlockContinue.finished();
         } else {
@@ -76,7 +80,7 @@ public BlockStart tryStart(ParserState state, MatchedBlockParser matchedBlockPar
             int nextNonSpace = state.getNextNonSpaceIndex();
             FencedCodeBlockParser blockParser = checkOpener(state.getLine().getContent(), nextNonSpace, indent);
             if (blockParser != null) {
-                return BlockStart.of(blockParser).atIndex(nextNonSpace + blockParser.block.getFenceLength());
+                return BlockStart.of(blockParser).atIndex(nextNonSpace + blockParser.block.getOpeningFenceLength());
             } else {
                 return BlockStart.none();
             }
@@ -119,15 +123,17 @@ private static FencedCodeBlockParser checkOpener(CharSequence line, int index, i
     // spec: The content of the code block consists of all subsequent lines, until a closing code fence of the same type
     // as the code block began with (backticks or tildes), and with at least as many backticks or tildes as the opening
     // code fence.
-    private boolean isClosing(CharSequence line, int index) {
-        char fenceChar = block.getFenceChar();
-        int fenceLength = block.getFenceLength();
+    private boolean tryClosing(CharSequence line, int index) {
         int fences = Characters.skip(fenceChar, line, index, line.length()) - index;
-        if (fences < fenceLength) {
+        if (fences < openingFenceLength) {
             return false;
         }
         // spec: The closing code fence [...] may be followed only by spaces, which are ignored.
         int after = Characters.skipSpaceTab(line, index + fences, line.length());
-        return after == line.length();
+        if (after == line.length()) {
+            block.setClosingFenceLength(fences);
+            return true;
+        }
+        return false;
     }
 }

commonmark/src/main/java/org/commonmark/node/FencedCodeBlock.java

@@ -2,8 +2,9 @@
 
 public class FencedCodeBlock extends Block {
 
-    private char fenceChar;
-    private int fenceLength;
+    private String fenceCharacter;
+    private Integer openingFenceLength;
+    private Integer closingFenceLength;
     private int fenceIndent;
 
     private String info;
@@ -14,20 +15,47 @@ public void accept(Visitor visitor) {
         visitor.visit(this);
     }
 
-    public char getFenceChar() {
-        return fenceChar;
+    /**
+     * @return the fence character that was used, e.g. {@code `} or {@code ~}, if available, or null otherwise
+     */
+    public String getFenceCharacter() {
+        return fenceCharacter;
     }
 
-    public void setFenceChar(char fenceChar) {
-        this.fenceChar = fenceChar;
+    public void setFenceCharacter(String fenceCharacter) {
+        this.fenceCharacter = fenceCharacter;
     }
 
-    public int getFenceLength() {
-        return fenceLength;
+    /**
+     * @return the length of the opening fence (how many of {{@link #getFenceCharacter()}} were used to start the code
+     * block) if available, or null otherwise
+     */
+    public Integer getOpeningFenceLength() {
+        return openingFenceLength;
     }
 
-    public void setFenceLength(int fenceLength) {
-        this.fenceLength = fenceLength;
+    public void setOpeningFenceLength(Integer openingFenceLength) {
+        if (openingFenceLength != null && openingFenceLength < 3) {
+            throw new IllegalArgumentException("openingFenceLength needs to be >= 3");
+        }
+        checkFenceLengths(openingFenceLength, closingFenceLength);
+        this.openingFenceLength = openingFenceLength;
+    }
+
+    /**
+     * @return the length of the closing fence (how many of {@link #getFenceCharacter()} were used to end the code
+     * block) if available, or null otherwise
+     */
+    public Integer getClosingFenceLength() {
+        return closingFenceLength;
+    }
+
+    public void setClosingFenceLength(Integer closingFenceLength) {
+        if (closingFenceLength != null && closingFenceLength < 3) {
+            throw new IllegalArgumentException("closingFenceLength needs to be >= 3");
+        }
+        checkFenceLengths(openingFenceLength, closingFenceLength);
+        this.closingFenceLength = closingFenceLength;
     }
 
     public int getFenceIndent() {
@@ -56,4 +84,44 @@ public String getLiteral() {
     public void setLiteral(String literal) {
         this.literal = literal;
     }
+
+    /**
+     * @deprecated use {@link #getFenceCharacter()} instead
+     */
+    @Deprecated
+    public char getFenceChar() {
+        return fenceCharacter != null && !fenceCharacter.isEmpty() ? fenceCharacter.charAt(0) : '\0';
+    }
+
+    /**
+     * @deprecated use {@link #setFenceCharacter} instead
+     */
+    @Deprecated
+    public void setFenceChar(char fenceChar) {
+        this.fenceCharacter = fenceChar != '\0' ? String.valueOf(fenceChar) : null;
+    }
+
+    /**
+     * @deprecated use {@link #getOpeningFenceLength} instead
+     */
+    @Deprecated
+    public int getFenceLength() {
+        return openingFenceLength != null ? openingFenceLength : 0;
+    }
+
+    /**
+     * @deprecated use {@link #setOpeningFenceLength} instead
+     */
+    @Deprecated
+    public void setFenceLength(int fenceLength) {
+        this.openingFenceLength = fenceLength != 0 ? fenceLength : null;
+    }
+
+    private static void checkFenceLengths(Integer openingFenceLength, Integer closingFenceLength) {
+        if (openingFenceLength != null && closingFenceLength != null) {
+            if (closingFenceLength < openingFenceLength) {
+                throw new IllegalArgumentException("fence lengths required to be: closingFenceLength >= openingFenceLength");
+            }
+        }
+    }
 }

commonmark/src/main/java/org/commonmark/renderer/markdown/CoreMarkdownNodeRenderer.java

@@ -147,20 +147,35 @@ public void visit(IndentedCodeBlock indentedCodeBlock) {
     }
 
     @Override
-    public void visit(FencedCodeBlock fencedCodeBlock) {
-        String literal = fencedCodeBlock.getLiteral();
-        String fence = repeat(String.valueOf(fencedCodeBlock.getFenceChar()), fencedCodeBlock.getFenceLength());
-        int indent = fencedCodeBlock.getFenceIndent();
+    public void visit(FencedCodeBlock codeBlock) {
+        String literal = codeBlock.getLiteral();
+        String fenceChar = codeBlock.getFenceCharacter() != null ? codeBlock.getFenceCharacter() : "`";
+        int openingFenceLength;
+        if (codeBlock.getOpeningFenceLength() != null) {
+            // If we have a known fence length, use it
+            openingFenceLength = codeBlock.getOpeningFenceLength();
+        } else {
+            // Otherwise, calculate the closing fence length pessimistically, e.g. if the code block itself contains a
+            // line with ```, we need to use a fence of length 4. If ``` occurs with non-whitespace characters on a
+            // line, we technically don't need a longer fence, but it's not incorrect to do so.
+            int fenceCharsInLiteral = findMaxRunLength(fenceChar, literal);
+            openingFenceLength = Math.max(fenceCharsInLiteral + 1, 3);
+        }
+        int closingFenceLength = codeBlock.getClosingFenceLength() != null ? codeBlock.getClosingFenceLength() : openingFenceLength;
+
+        String openingFence = repeat(fenceChar, openingFenceLength);
+        String closingFence = repeat(fenceChar, closingFenceLength);
+        int indent = codeBlock.getFenceIndent();
 
         if (indent > 0) {
             String indentPrefix = repeat(" ", indent);
             writer.writePrefix(indentPrefix);
             writer.pushPrefix(indentPrefix);
         }
 
-        writer.raw(fence);
-        if (fencedCodeBlock.getInfo() != null) {
-            writer.raw(fencedCodeBlock.getInfo());
+        writer.raw(openingFence);
+        if (codeBlock.getInfo() != null) {
+            writer.raw(codeBlock.getInfo());
         }
         writer.line();
         if (!literal.isEmpty()) {
@@ -170,7 +185,7 @@ public void visit(FencedCodeBlock fencedCodeBlock) {
                 writer.line();
             }
         }
-        writer.raw(fence);
+        writer.raw(closingFence);
         if (indent > 0) {
             writer.popPrefix();
         }
@@ -259,7 +274,7 @@ public void visit(ListItem listItem) {
     public void visit(Code code) {
         String literal = code.getLiteral();
         // If the literal includes backticks, we can surround them by using one more backtick.
-        int backticks = findMaxRunLength('`', literal);
+        int backticks = findMaxRunLength("`", literal);
         for (int i = 0; i < backticks + 1; i++) {
             writer.raw('`');
         }
@@ -411,19 +426,22 @@ protected void visitChildren(Node parent) {
         }
     }
 
-    private static int findMaxRunLength(char c, CharSequence s) {
-        int backticks = 0;
-        int start = 0;
-        while (start < s.length()) {
-            int index = Characters.find(c, s, start);
-            if (index != -1) {
-                start = Characters.skip(c, s, index + 1, s.length());
-                backticks = Math.max(backticks, start - index);
-            } else {
-                break;
+    private static int findMaxRunLength(String needle, String s) {
+        int maxRunLength = 0;
+        int pos = 0;
+        while (pos < s.length()) {
+            pos = s.indexOf(needle, pos);
+            if (pos == -1) {
+                return maxRunLength;
             }
+            int runLength = 0;
+            do {
+                pos += needle.length();
+                runLength++;
+            } while (s.startsWith(needle, pos));
+            maxRunLength = Math.max(runLength, maxRunLength);
         }
-        return backticks;
+        return maxRunLength;
     }
 
     private static boolean contains(String s, CharMatcher charMatcher) {

commonmark/src/test/java/org/commonmark/renderer/markdown/MarkdownRendererTest.java

@@ -52,6 +52,23 @@ public void testFencedCodeBlocks() {
         assertRoundTrip("```info\ntest\n```\n");
         assertRoundTrip(" ```\n test\n ```\n");
         assertRoundTrip("```\n```\n");
+
+        // Preserve the length
+        assertRoundTrip("````\ntest\n````\n");
+        assertRoundTrip("~~~\ntest\n~~~~~~\n");
+    }
+
+    @Test
+    public void testFencedCodeBlocksFromAst() {
+        var doc = new Document();
+        var codeBlock = new FencedCodeBlock();
+        codeBlock.setLiteral("hi code");
+        doc.appendChild(codeBlock);
+
+        assertRendering("", "```\nhi code\n```\n", render(doc));
+
+        codeBlock.setLiteral("hi`\n```\n``test");
+        assertRendering("", "````\nhi`\n```\n``test\n````\n", render(doc));
     }
 
     @Test