From 16c6ff26b1b62f40a394ec8578a2a54e7717aecb Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Thu, 14 Mar 2024 11:43:04 +1100 Subject: [PATCH] Fix markdown renderer for manually created list nodes --- .../commonmark/internal/ListBlockParser.java | 18 +++---- .../renderer/text/BulletListHolder.java | 6 +-- .../renderer/text/OrderedListHolder.java | 8 +-- .../java/org/commonmark/node/BulletList.java | 26 ++++++++-- .../java/org/commonmark/node/ListItem.java | 19 +++---- .../java/org/commonmark/node/OrderedList.java | 51 ++++++++++++++++--- .../renderer/html/CoreHtmlNodeRenderer.java | 2 +- .../markdown/CoreMarkdownNodeRenderer.java | 39 +++++++------- .../markdown/MarkdownRendererTest.java | 33 ++++++++++++ .../commonmark/test/ListBlockParserTest.java | 4 +- 10 files changed, 146 insertions(+), 60 deletions(-) diff --git a/commonmark/src/main/java/org/commonmark/internal/ListBlockParser.java b/commonmark/src/main/java/org/commonmark/internal/ListBlockParser.java index 0ff644a47..d77744da7 100644 --- a/commonmark/src/main/java/org/commonmark/internal/ListBlockParser.java +++ b/commonmark/src/main/java/org/commonmark/internal/ListBlockParser.java @@ -4,6 +4,8 @@ import org.commonmark.node.*; import org.commonmark.parser.block.*; +import java.util.Objects; + public class ListBlockParser extends AbstractBlockParser { private final ListBlock block; @@ -90,7 +92,7 @@ private static ListData parseList(CharSequence line, final int markerIndex, fina if (inParagraph) { // If the list item is ordered, the start number must be 1 to interrupt a paragraph. - if (listBlock instanceof OrderedList && ((OrderedList) listBlock).getStartNumber() != 1) { + if (listBlock instanceof OrderedList && ((OrderedList) listBlock).getMarkerStartNumber() != 1) { return null; } // Empty list item can not interrupt a paragraph. @@ -116,7 +118,7 @@ private static ListMarkerData parseListMarker(CharSequence line, int index) { case '*': if (isSpaceTabOrEnd(line, index + 1)) { BulletList bulletList = new BulletList(); - bulletList.setBulletMarker(c); + bulletList.setMarker(String.valueOf(c)); return new ListMarkerData(bulletList, index + 1); } else { return null; @@ -154,8 +156,8 @@ private static ListMarkerData parseOrderedList(CharSequence line, int index) { if (digits >= 1 && isSpaceTabOrEnd(line, i + 1)) { String number = line.subSequence(index, i).toString(); OrderedList orderedList = new OrderedList(); - orderedList.setStartNumber(Integer.parseInt(number)); - orderedList.setDelimiter(c); + orderedList.setMarkerStartNumber(Integer.parseInt(number)); + orderedList.setMarkerDelimiter(String.valueOf(c)); return new ListMarkerData(orderedList, i + 1); } else { return null; @@ -188,17 +190,13 @@ private static boolean isSpaceTabOrEnd(CharSequence line, int index) { */ private static boolean listsMatch(ListBlock a, ListBlock b) { if (a instanceof BulletList && b instanceof BulletList) { - return equals(((BulletList) a).getBulletMarker(), ((BulletList) b).getBulletMarker()); + return Objects.equals(((BulletList) a).getMarker(), ((BulletList) b).getMarker()); } else if (a instanceof OrderedList && b instanceof OrderedList) { - return equals(((OrderedList) a).getDelimiter(), ((OrderedList) b).getDelimiter()); + return Objects.equals(((OrderedList) a).getMarkerDelimiter(), ((OrderedList) b).getMarkerDelimiter()); } return false; } - private static boolean equals(Object a, Object b) { - return (a == null) ? (b == null) : a.equals(b); - } - public static class Factory extends AbstractBlockParserFactory { @Override diff --git a/commonmark/src/main/java/org/commonmark/internal/renderer/text/BulletListHolder.java b/commonmark/src/main/java/org/commonmark/internal/renderer/text/BulletListHolder.java index f08ccebd6..a9271dcdb 100644 --- a/commonmark/src/main/java/org/commonmark/internal/renderer/text/BulletListHolder.java +++ b/commonmark/src/main/java/org/commonmark/internal/renderer/text/BulletListHolder.java @@ -3,14 +3,14 @@ import org.commonmark.node.BulletList; public class BulletListHolder extends ListHolder { - private final char marker; + private final String marker; public BulletListHolder(ListHolder parent, BulletList list) { super(parent); - marker = list.getBulletMarker(); + marker = list.getMarker(); } - public char getMarker() { + public String getMarker() { return marker; } } diff --git a/commonmark/src/main/java/org/commonmark/internal/renderer/text/OrderedListHolder.java b/commonmark/src/main/java/org/commonmark/internal/renderer/text/OrderedListHolder.java index e02ecea7c..e5e470951 100644 --- a/commonmark/src/main/java/org/commonmark/internal/renderer/text/OrderedListHolder.java +++ b/commonmark/src/main/java/org/commonmark/internal/renderer/text/OrderedListHolder.java @@ -3,16 +3,16 @@ import org.commonmark.node.OrderedList; public class OrderedListHolder extends ListHolder { - private final char delimiter; + private final String delimiter; private int counter; public OrderedListHolder(ListHolder parent, OrderedList list) { super(parent); - delimiter = list.getDelimiter(); - counter = list.getStartNumber(); + delimiter = list.getMarkerDelimiter() != null ? list.getMarkerDelimiter() : "."; + counter = list.getMarkerStartNumber() != null ? list.getMarkerStartNumber() : 1; } - public char getDelimiter() { + public String getDelimiter() { return delimiter; } diff --git a/commonmark/src/main/java/org/commonmark/node/BulletList.java b/commonmark/src/main/java/org/commonmark/node/BulletList.java index 127862312..4d5c2a894 100644 --- a/commonmark/src/main/java/org/commonmark/node/BulletList.java +++ b/commonmark/src/main/java/org/commonmark/node/BulletList.java @@ -2,19 +2,37 @@ public class BulletList extends ListBlock { - private char bulletMarker; + private String marker; @Override public void accept(Visitor visitor) { visitor.visit(this); } + /** + * @return the bullet list marker that was used, e.g. {@code -}, {@code *} or {@code +}, if available, or null otherwise + */ + public String getMarker() { + return marker; + } + + public void setMarker(String marker) { + this.marker = marker; + } + + /** + * @deprecated use {@link #getMarker()} instead + */ + @Deprecated public char getBulletMarker() { - return bulletMarker; + return marker != null && !marker.isEmpty() ? marker.charAt(0) : '\0'; } + /** + * @deprecated use {@link #getMarker()} instead + */ + @Deprecated public void setBulletMarker(char bulletMarker) { - this.bulletMarker = bulletMarker; + this.marker = bulletMarker != '\0' ? String.valueOf(bulletMarker) : null; } - } diff --git a/commonmark/src/main/java/org/commonmark/node/ListItem.java b/commonmark/src/main/java/org/commonmark/node/ListItem.java index 21f4e2b82..4e63b6145 100644 --- a/commonmark/src/main/java/org/commonmark/node/ListItem.java +++ b/commonmark/src/main/java/org/commonmark/node/ListItem.java @@ -2,8 +2,8 @@ public class ListItem extends Block { - private int markerIndent; - private int contentIndent; + private Integer markerIndent; + private Integer contentIndent; @Override public void accept(Visitor visitor) { @@ -11,7 +11,8 @@ public void accept(Visitor visitor) { } /** - * Returns the indent of the marker such as "-" or "1." in columns (spaces or tab stop of 4). + * Returns the indent of the marker such as "-" or "1." in columns (spaces or tab stop of 4) if available, or null + * otherwise. *

* Some examples and their marker indent: * 

- Foo
@@ -21,17 +22,17 @@ public void accept(Visitor visitor) { * 
  1. Foo
* Marker indent: 2 */ - public int getMarkerIndent() { + public Integer getMarkerIndent() { return markerIndent; } - public void setMarkerIndent(int markerIndent) { + public void setMarkerIndent(Integer markerIndent) { this.markerIndent = markerIndent; } /** - * Returns the indent of the content in columns (spaces or tab stop of 4). The content indent is counted from the - * beginning of the line and includes the marker on the first line. + * Returns the indent of the content in columns (spaces or tab stop of 4) if available, or null otherwise. + * The content indent is counted from the beginning of the line and includes the marker on the first line. *

* Some examples and their content indent: * 

- Foo
@@ -44,11 +45,11 @@ public void setMarkerIndent(int markerIndent) { * Note that subsequent lines in the same list item need to be indented by at least the content indent to be counted * as part of the list item. */ - public int getContentIndent() { + public Integer getContentIndent() { return contentIndent; } - public void setContentIndent(int contentIndent) { + public void setContentIndent(Integer contentIndent) { this.contentIndent = contentIndent; } } diff --git a/commonmark/src/main/java/org/commonmark/node/OrderedList.java b/commonmark/src/main/java/org/commonmark/node/OrderedList.java index 1f988234c..0bbe09917 100644 --- a/commonmark/src/main/java/org/commonmark/node/OrderedList.java +++ b/commonmark/src/main/java/org/commonmark/node/OrderedList.java @@ -2,28 +2,65 @@ public class OrderedList extends ListBlock { - private int startNumber; - private char delimiter; + private String markerDelimiter; + private Integer markerStartNumber; @Override public void accept(Visitor visitor) { visitor.visit(this); } + /** + * @return the start number used in the marker, e.g. {@code 1}, if available, or null otherwise + */ + public Integer getMarkerStartNumber() { + return markerStartNumber; + } + + public void setMarkerStartNumber(Integer markerStartNumber) { + this.markerStartNumber = markerStartNumber; + } + + /** + * @return the delimiter used in the marker, e.g. {@code .} or {@code )}, if available, or null otherwise + */ + public String getMarkerDelimiter() { + return markerDelimiter; + } + + public void setMarkerDelimiter(String markerDelimiter) { + this.markerDelimiter = markerDelimiter; + } + + /** + * @deprecated use {@link #getMarkerStartNumber()} instead + */ + @Deprecated public int getStartNumber() { - return startNumber; + return markerStartNumber != null ? markerStartNumber : 0; } + /** + * @deprecated use {@link #setMarkerStartNumber} instead + */ + @Deprecated public void setStartNumber(int startNumber) { - this.startNumber = startNumber; + this.markerStartNumber = startNumber != 0 ? startNumber : null; } + /** + * @deprecated use {@link #getMarkerDelimiter()} instead + */ + @Deprecated public char getDelimiter() { - return delimiter; + return markerDelimiter != null && !markerDelimiter.isEmpty() ? markerDelimiter.charAt(0) : '\0'; } + /** + * @deprecated use {@link #setMarkerDelimiter} instead + */ + @Deprecated public void setDelimiter(char delimiter) { - this.delimiter = delimiter; + this.markerDelimiter = delimiter != '\0' ? String.valueOf(delimiter) : null; } - } diff --git a/commonmark/src/main/java/org/commonmark/renderer/html/CoreHtmlNodeRenderer.java b/commonmark/src/main/java/org/commonmark/renderer/html/CoreHtmlNodeRenderer.java index 7d3552668..47343b53c 100644 --- a/commonmark/src/main/java/org/commonmark/renderer/html/CoreHtmlNodeRenderer.java +++ b/commonmark/src/main/java/org/commonmark/renderer/html/CoreHtmlNodeRenderer.java @@ -168,7 +168,7 @@ public void visit(ListItem listItem) { @Override public void visit(OrderedList orderedList) { - int start = orderedList.getStartNumber(); + int start = orderedList.getMarkerStartNumber() != null ? orderedList.getMarkerStartNumber() : 1; Map attrs = new LinkedHashMap<>(); if (start != 1) { attrs.put("start", String.valueOf(start)); 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 e0cc4eb25..d5770155a 100644 --- a/commonmark/src/main/java/org/commonmark/renderer/markdown/CoreMarkdownNodeRenderer.java +++ b/commonmark/src/main/java/org/commonmark/renderer/markdown/CoreMarkdownNodeRenderer.java @@ -227,33 +227,32 @@ public void visit(OrderedList orderedList) { @Override public void visit(ListItem listItem) { - int contentIndent = listItem.getContentIndent(); - boolean pushedPrefix = false; + int markerIndent = listItem.getMarkerIndent() != null ? listItem.getMarkerIndent() : 0; + String marker; if (listHolder instanceof BulletListHolder) { BulletListHolder bulletListHolder = (BulletListHolder) listHolder; - String marker = repeat(" ", listItem.getMarkerIndent()) + bulletListHolder.bulletMarker; - writer.writePrefix(marker); - writer.writePrefix(repeat(" ", contentIndent - marker.length())); - writer.pushPrefix(repeat(" ", contentIndent)); - pushedPrefix = true; + marker = repeat(" ", markerIndent) + bulletListHolder.marker; } else if (listHolder instanceof OrderedListHolder) { OrderedListHolder orderedListHolder = (OrderedListHolder) listHolder; - String marker = repeat(" ", listItem.getMarkerIndent()) + orderedListHolder.number + orderedListHolder.delimiter; + marker = repeat(" ", markerIndent) + orderedListHolder.number + orderedListHolder.delimiter; orderedListHolder.number++; - writer.writePrefix(marker); - writer.writePrefix(repeat(" ", contentIndent - marker.length())); - writer.pushPrefix(repeat(" ", contentIndent)); - pushedPrefix = true; + } else { + throw new IllegalStateException("Unknown list holder type: " + listHolder); } + Integer contentIndent = listItem.getContentIndent(); + String spaces = contentIndent != null ? repeat(" ", contentIndent - marker.length()) : " "; + writer.writePrefix(marker); + writer.writePrefix(spaces); + writer.pushPrefix(repeat(" ", marker.length() + spaces.length())); + if (listItem.getFirstChild() == null) { // Empty list item writer.block(); } else { visitChildren(listItem); } - if (pushedPrefix) { - writer.popPrefix(); - } + + writer.popPrefix(); } @Override @@ -489,22 +488,22 @@ protected ListHolder(ListHolder parent) { } private static class BulletListHolder extends ListHolder { - final char bulletMarker; + final String marker; public BulletListHolder(ListHolder parent, BulletList bulletList) { super(parent); - this.bulletMarker = bulletList.getBulletMarker(); + this.marker = bulletList.getMarker() != null ? bulletList.getMarker() : "-"; } } private static class OrderedListHolder extends ListHolder { - final char delimiter; + final String delimiter; private int number; protected OrderedListHolder(ListHolder parent, OrderedList orderedList) { super(parent); - delimiter = orderedList.getDelimiter(); - number = orderedList.getStartNumber(); + delimiter = orderedList.getMarkerDelimiter() != null ? orderedList.getMarkerDelimiter() : "."; + number = orderedList.getMarkerStartNumber() != null ? orderedList.getMarkerStartNumber() : 1; } } 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 9bab92bcc..522b16cd4 100644 --- a/commonmark/src/test/java/org/commonmark/renderer/markdown/MarkdownRendererTest.java +++ b/commonmark/src/test/java/org/commonmark/renderer/markdown/MarkdownRendererTest.java @@ -2,8 +2,10 @@ import org.commonmark.node.*; import org.commonmark.parser.Parser; +import org.commonmark.testutil.Asserts; import org.junit.Test; +import static org.commonmark.testutil.Asserts.assertRendering; import static org.junit.Assert.assertEquals; public class MarkdownRendererTest { @@ -103,6 +105,21 @@ public void testBulletListItems() { assertRoundTrip("- \n\nFoo\n"); } + @Test + public void testBulletListItemsFromAst() { + var doc = new Document(); + var list = new BulletList(); + var item = new ListItem(); + item.appendChild(new Text("Test")); + list.appendChild(item); + doc.appendChild(list); + + assertRendering("", "- Test\n", render(doc)); + + list.setMarker("*"); + assertRendering("", "* Test\n", render(doc)); + } + @Test public void testOrderedListItems() { assertRoundTrip("1. foo\n"); @@ -116,6 +133,22 @@ public void testOrderedListItems() { assertRoundTrip(" 1. one\n\n two\n"); } + @Test + public void testOrderedListItemsFromAst() { + var doc = new Document(); + var list = new OrderedList(); + var item = new ListItem(); + item.appendChild(new Text("Test")); + list.appendChild(item); + doc.appendChild(list); + + assertRendering("", "1. Test\n", render(doc)); + + list.setMarkerStartNumber(2); + list.setMarkerDelimiter(")"); + assertRendering("", "2) Test\n", render(doc)); + } + // Inlines @Test diff --git a/commonmark/src/test/java/org/commonmark/test/ListBlockParserTest.java b/commonmark/src/test/java/org/commonmark/test/ListBlockParserTest.java index 2ba2116d4..667d60efe 100644 --- a/commonmark/src/test/java/org/commonmark/test/ListBlockParserTest.java +++ b/commonmark/src/test/java/org/commonmark/test/ListBlockParserTest.java @@ -60,7 +60,7 @@ public void testOrderedListIndents() { private void assertListItemIndents(String input, int expectedMarkerIndent, int expectedContentIndent) { Node doc = PARSER.parse(input); ListItem listItem = Nodes.find(doc, ListItem.class); - assertEquals(expectedMarkerIndent, listItem.getMarkerIndent()); - assertEquals(expectedContentIndent, listItem.getContentIndent()); + assertEquals(expectedMarkerIndent, (int) listItem.getMarkerIndent()); + assertEquals(expectedContentIndent, (int) listItem.getContentIndent()); } }