From 8fed49c21ef8630cab03f556c73ba1835d8419dc Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Sat, 15 Aug 2020 16:52:08 +1000 Subject: [PATCH 1/8] Restart work for adding source positions The previous attempt required every block parser to implement source positions, which is not ideal. This one is for trying to see if we can get it done inside DocumentParser. Note that SourceSpan initially used 1-based indexes and beginIndex/endIndex, but it was changed to 0-based and a length because: 0-based fits better with the existing API and other Java APIs. And instead of having an endIndex, having a length would allow us to easily extend the SourceSpan to include a character offset in addition to line/column offset. --- .../commonmark/ext/gfm/tables/TablesTest.java | 46 +++++ commonmark-test-util/pom.xml | 4 + .../commonmark/internal/DocumentParser.java | 56 +++++- .../org/commonmark/internal/SourceSpans.java | 13 ++ .../main/java/org/commonmark/node/Node.java | 13 ++ .../java/org/commonmark/node/SourceSpan.java | 69 +++++++ .../commonmark/test/SourceSpanRenderer.java | 92 +++++++++ .../org/commonmark/test/SourceSpansTest.java | 178 ++++++++++++++++++ pom.xml | 5 + 9 files changed, 470 insertions(+), 6 deletions(-) create mode 100644 commonmark/src/main/java/org/commonmark/internal/SourceSpans.java create mode 100644 commonmark/src/main/java/org/commonmark/node/SourceSpan.java create mode 100644 commonmark/src/test/java/org/commonmark/test/SourceSpanRenderer.java create mode 100644 commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java diff --git a/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java b/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java index 563ae8c18..950926e1f 100644 --- a/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java +++ b/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java @@ -2,6 +2,7 @@ import org.commonmark.Extension; import org.commonmark.node.Node; +import org.commonmark.node.SourceSpan; import org.commonmark.parser.Parser; import org.commonmark.renderer.html.AttributeProvider; import org.commonmark.renderer.html.AttributeProviderContext; @@ -15,6 +16,8 @@ import java.util.Set; import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.emptyIterable; import static org.junit.Assert.assertThat; public class TablesTest extends RenderingTestCase { @@ -643,6 +646,49 @@ public void setAttributes(Node node, String tagName, Map attribu "\n")); } + @Test + public void sourceSpans() { + Node document = PARSER.parse("Abc|Def\n---|---\n|1|2\n 3|four|\n|||\n"); + + TableBlock block = (TableBlock) document.getFirstChild(); + assertThat(block.getSourceSpans(), contains(SourceSpan.of(0, 0, 7), SourceSpan.of(1, 0, 7), + SourceSpan.of(2, 0, 4), SourceSpan.of(3, 1, 7), SourceSpan.of(4, 0, 3))); + + TableHead head = (TableHead) block.getFirstChild(); + assertThat(head.getSourceSpans(), contains(SourceSpan.of(0, 0, 7))); + + TableRow headRow = (TableRow) head.getFirstChild(); + assertThat(headRow.getSourceSpans(), contains(SourceSpan.of(0, 0, 7))); + TableCell headRowCell1 = (TableCell) headRow.getFirstChild(); + TableCell headRowCell2 = (TableCell) headRow.getLastChild(); + assertThat(headRowCell1.getSourceSpans(), contains(SourceSpan.of(0, 0, 3))); + assertThat(headRowCell2.getSourceSpans(), contains(SourceSpan.of(0, 4, 3))); + + TableBody body = (TableBody) block.getLastChild(); + assertThat(body.getSourceSpans(), contains(SourceSpan.of(2, 0, 4), SourceSpan.of(3, 1, 7), SourceSpan.of(4, 0, 3))); + + TableRow bodyRow1 = (TableRow) body.getFirstChild(); + assertThat(bodyRow1.getSourceSpans(), contains(SourceSpan.of(2, 0, 4))); + TableCell bodyRow1Cell1 = (TableCell) bodyRow1.getFirstChild(); + TableCell bodyRow1Cell2 = (TableCell) bodyRow1.getLastChild(); + assertThat(bodyRow1Cell1.getSourceSpans(), contains(SourceSpan.of(2, 1, 1))); + assertThat(bodyRow1Cell2.getSourceSpans(), contains(SourceSpan.of(2, 3, 1))); + + TableRow bodyRow2 = (TableRow) body.getFirstChild().getNext(); + assertThat(bodyRow2.getSourceSpans(), contains(SourceSpan.of(3, 1, 7))); + TableCell bodyRow2Cell1 = (TableCell) bodyRow2.getFirstChild(); + TableCell bodyRow2Cell2 = (TableCell) bodyRow2.getLastChild(); + assertThat(bodyRow2Cell1.getSourceSpans(), contains(SourceSpan.of(3, 1, 1))); + assertThat(bodyRow2Cell2.getSourceSpans(), contains(SourceSpan.of(3, 3, 4))); + + TableRow bodyRow3 = (TableRow) body.getLastChild(); + assertThat(bodyRow3.getSourceSpans(), contains(SourceSpan.of(4, 0, 3))); + TableCell bodyRow3Cell1 = (TableCell) bodyRow3.getFirstChild(); + TableCell bodyRow3Cell2 = (TableCell) bodyRow3.getLastChild(); + assertThat(bodyRow3Cell1.getSourceSpans(), emptyIterable()); + assertThat(bodyRow3Cell2.getSourceSpans(), emptyIterable()); + } + @Override protected String render(String source) { return RENDERER.render(PARSER.parse(source)); diff --git a/commonmark-test-util/pom.xml b/commonmark-test-util/pom.xml index 527583c4c..92046f159 100644 --- a/commonmark-test-util/pom.xml +++ b/commonmark-test-util/pom.xml @@ -16,6 +16,10 @@ junit junit + + org.hamcrest + hamcrest-library + diff --git a/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java b/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java index 700519ac9..ce965f1e3 100644 --- a/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java +++ b/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java @@ -1,16 +1,40 @@ package org.commonmark.internal; import org.commonmark.internal.util.Parsing; -import org.commonmark.node.*; +import org.commonmark.node.Block; +import org.commonmark.node.BlockQuote; +import org.commonmark.node.Document; +import org.commonmark.node.FencedCodeBlock; +import org.commonmark.node.Heading; +import org.commonmark.node.HtmlBlock; +import org.commonmark.node.IndentedCodeBlock; +import org.commonmark.node.LinkReferenceDefinition; +import org.commonmark.node.ListBlock; +import org.commonmark.node.Paragraph; +import org.commonmark.node.SourceSpan; +import org.commonmark.node.ThematicBreak; import org.commonmark.parser.InlineParser; import org.commonmark.parser.InlineParserFactory; -import org.commonmark.parser.block.*; +import org.commonmark.parser.block.BlockContinue; +import org.commonmark.parser.block.BlockParser; +import org.commonmark.parser.block.BlockParserFactory; +import org.commonmark.parser.block.BlockStart; +import org.commonmark.parser.block.MatchedBlockParser; +import org.commonmark.parser.block.ParserState; import org.commonmark.parser.delimiter.DelimiterProcessor; import java.io.BufferedReader; import java.io.IOException; import java.io.Reader; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; public class DocumentParser implements ParserState { @@ -40,6 +64,11 @@ public class DocumentParser implements ParserState { private CharSequence line; + /** + * Line index (0-based) + */ + private int lineIndex = -1; + /** * current index (offset) in input line (0-based) */ @@ -137,6 +166,10 @@ public CharSequence getLine() { return line; } + public int getLineIndex() { + return lineIndex; + } + @Override public int getIndex() { return index; @@ -172,6 +205,7 @@ public BlockParser getActiveBlockParser() { * line of input, then finalizing the document. */ private void incorporateLine(CharSequence ln) { + lineIndex++; line = Parsing.prepareLine(ln); index = 0; column = 0; @@ -256,6 +290,10 @@ private void incorporateLine(CharSequence ln) { if (!startedNewBlock && !isBlank() && getActiveBlockParser().canHaveLazyContinuationLines()) { // lazy paragraph continuation + // TODO + for (BlockParser parser : activeBlockParsers.subList(1, activeBlockParsers.size())) { +// parser.onLazyContinuationLine(this); + } addLine(); } else { @@ -269,7 +307,11 @@ private void incorporateLine(CharSequence ln) { addLine(); } else if (!isBlank()) { // create paragraph container for line - addChild(new ParagraphParser()); + // TODO: +// SourceSpan sourceSpan = SourceSpans.fromState(this, this.getNextNonSpaceIndex()); +// ParagraphParser paragraphParser = new ParagraphParser(sourceSpan); + ParagraphParser paragraphParser = new ParagraphParser(); + addChild(paragraphParser); addLine(); } } @@ -392,6 +434,8 @@ private void finalize(BlockParser blockParser) { } blockParser.closeBlock(); + // TODO + //blockParser.getBlock().setSourceSpans(blockParser.getSourceSpans()); } private void addDefinitionsFrom(ParagraphParser paragraphParser) { @@ -420,8 +464,8 @@ private void processInlines() { } /** - * Add block of type tag as a child of the tip. If the tip can't accept children, close and finalize it and try - * its parent, and so on til we find a block that can accept children. + * Add block of type tag as a child of the tip. If the tip can't accept children, close and finalize it and try + * its parent, and so on until we find a block that can accept children. */ private T addChild(T blockParser) { while (!getActiveBlockParser().canContain(blockParser.getBlock())) { diff --git a/commonmark/src/main/java/org/commonmark/internal/SourceSpans.java b/commonmark/src/main/java/org/commonmark/internal/SourceSpans.java new file mode 100644 index 000000000..bf8715be8 --- /dev/null +++ b/commonmark/src/main/java/org/commonmark/internal/SourceSpans.java @@ -0,0 +1,13 @@ +package org.commonmark.internal; + +import org.commonmark.node.SourceSpan; +import org.commonmark.parser.block.ParserState; + +public class SourceSpans { + + public static SourceSpan fromState(ParserState parserState, int startIndex) { +// int length = parserState.getLine().length(); +// return SourceSpan.of(parserState.getLineIndex(), startIndex, length - startIndex); + return null; + } +} diff --git a/commonmark/src/main/java/org/commonmark/node/Node.java b/commonmark/src/main/java/org/commonmark/node/Node.java index e7b24c08c..7a742ea85 100644 --- a/commonmark/src/main/java/org/commonmark/node/Node.java +++ b/commonmark/src/main/java/org/commonmark/node/Node.java @@ -1,5 +1,9 @@ package org.commonmark.node; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + public abstract class Node { private Node parent = null; @@ -7,6 +11,7 @@ public abstract class Node { private Node lastChild = null; private Node prev = null; private Node next = null; + private List sourceSpans = new ArrayList<>(); public abstract void accept(Visitor visitor); @@ -30,6 +35,14 @@ public Node getParent() { return parent; } + public List getSourceSpans() { + return Collections.unmodifiableList(sourceSpans); + } + + public void setSourceSpans(List sourceSpans) { + this.sourceSpans = new ArrayList<>(sourceSpans); + } + protected void setParent(Node parent) { this.parent = parent; } diff --git a/commonmark/src/main/java/org/commonmark/node/SourceSpan.java b/commonmark/src/main/java/org/commonmark/node/SourceSpan.java new file mode 100644 index 000000000..c3c49531b --- /dev/null +++ b/commonmark/src/main/java/org/commonmark/node/SourceSpan.java @@ -0,0 +1,69 @@ +package org.commonmark.node; + +import java.util.Objects; + +public class SourceSpan { + + private final int lineIndex; + private final int columnIndex; + private final int length; + + public static SourceSpan of(int lineIndex, int columnIndex, int length) { + return new SourceSpan(lineIndex, columnIndex, length); + } + + private SourceSpan(int lineIndex, int columnIndex, int length) { + this.lineIndex = lineIndex; + this.columnIndex = columnIndex; + this.length = length; + } + + /** + * @return 0-based index of line in source + */ + public int getLineIndex() { + return lineIndex; + } + + /** + * @return 0-based index of column (character on line) in source + */ + public int getColumnIndex() { + return columnIndex; + } + + /** + * @return length of the span + */ + public int getLength() { + return length; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + SourceSpan that = (SourceSpan) o; + return lineIndex == that.lineIndex && + columnIndex == that.columnIndex && + length == that.length; + } + + @Override + public int hashCode() { + return Objects.hash(lineIndex, columnIndex, length); + } + + @Override + public String toString() { + return "SourceSpan{" + + "lineIndex=" + lineIndex + + ", columnIndex=" + columnIndex + + ", length=" + length + + "}"; + } +} diff --git a/commonmark/src/test/java/org/commonmark/test/SourceSpanRenderer.java b/commonmark/src/test/java/org/commonmark/test/SourceSpanRenderer.java new file mode 100644 index 000000000..4cb057b36 --- /dev/null +++ b/commonmark/src/test/java/org/commonmark/test/SourceSpanRenderer.java @@ -0,0 +1,92 @@ +package org.commonmark.test; + +import org.commonmark.node.AbstractVisitor; +import org.commonmark.node.Node; +import org.commonmark.node.SourceSpan; + +import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; + +public class SourceSpanRenderer { + + public static String render(Node document, String source) { + SourceSpanMarkersVisitor visitor = new SourceSpanMarkersVisitor(); + document.accept(visitor); + Map>> markers = visitor.getMarkers(); + + StringBuilder sb = new StringBuilder(); + + String[] lines = source.split("\n"); + + for (int lineIndex = 0; lineIndex < lines.length; lineIndex++) { + String line = lines[lineIndex]; + Map> lineMarkers = markers.get(lineIndex); + for (int i = 0; i < line.length(); i++) { + appendMarkers(lineMarkers, i, sb); + sb.append(line.charAt(i)); + } + appendMarkers(lineMarkers, line.length(), sb); + sb.append("\n"); + } + + return sb.toString(); + } + + private static void appendMarkers(Map> lineMarkers, int columnIndex, StringBuilder sb) { + if (lineMarkers != null) { + List columnMarkers = lineMarkers.get(columnIndex); + if (columnMarkers != null) { + for (String marker : columnMarkers) { + sb.append(marker); + } + } + } + } + + private static class SourceSpanMarkersVisitor extends AbstractVisitor { + + private final Map>> markers = new HashMap<>(); + private final String opening = "({[<⸢⸤"; + private final String closing = ")}]>⸣⸥"; + + private int markerIndex; + + public Map>> getMarkers() { + return markers; + } + + @Override + protected void visitChildren(Node parent) { + if (!parent.getSourceSpans().isEmpty()) { + for (SourceSpan sourceSpan : parent.getSourceSpans()) { + String opener = String.valueOf(opening.charAt(markerIndex % opening.length())); + String closer = String.valueOf(closing.charAt(markerIndex % closing.length())); + + int col = sourceSpan.getColumnIndex(); + getMarkers(sourceSpan.getLineIndex(), col).add(opener); + getMarkers(sourceSpan.getLineIndex(), col + sourceSpan.getLength()).add(0, closer); + } + markerIndex++; + } + super.visitChildren(parent); + } + + private List getMarkers(int lineIndex, int columnIndex) { + Map> columnMap = markers.get(lineIndex); + if (columnMap == null) { + columnMap = new HashMap<>(); + markers.put(lineIndex, columnMap); + } + + List markers = columnMap.get(columnIndex); + if (markers == null) { + markers = new LinkedList<>(); + columnMap.put(columnIndex, markers); + } + + return markers; + } + } +} diff --git a/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java b/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java new file mode 100644 index 000000000..2264ebfaf --- /dev/null +++ b/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java @@ -0,0 +1,178 @@ +package org.commonmark.test; + +import org.commonmark.node.BlockQuote; +import org.commonmark.node.FencedCodeBlock; +import org.commonmark.node.Heading; +import org.commonmark.node.HtmlBlock; +import org.commonmark.node.IndentedCodeBlock; +import org.commonmark.node.ListBlock; +import org.commonmark.node.ListItem; +import org.commonmark.node.Node; +import org.commonmark.node.Paragraph; +import org.commonmark.node.SourceSpan; +import org.commonmark.node.ThematicBreak; +import org.commonmark.parser.Parser; +import org.junit.Test; + +import static org.hamcrest.Matchers.contains; +import static org.junit.Assert.*; + +public class SourceSpansTest { + + private static final Parser PARSER = Parser.builder().build(); + + @Test + public void paragraph() { + assertSpans("foo\n", Paragraph.class, SourceSpan.of(0, 0, 3)); + assertSpans("foo\nbar\n", Paragraph.class, SourceSpan.of(0, 0, 3), SourceSpan.of(1, 0, 3)); + assertSpans(" foo\n bar\n", Paragraph.class, SourceSpan.of(0, 2, 3), SourceSpan.of(1, 2, 3)); + assertSpans("> foo\n> bar\n", Paragraph.class, SourceSpan.of(0, 2, 3), SourceSpan.of(1, 2, 3)); + assertSpans("* foo\n bar\n", Paragraph.class, SourceSpan.of(0, 2, 3), SourceSpan.of(1, 2, 3)); + assertSpans("* foo\nbar\n", Paragraph.class, SourceSpan.of(0, 2, 3), SourceSpan.of(1, 0, 3)); + } + + @Test + public void thematicBreak() { + assertSpans("---\n", ThematicBreak.class, SourceSpan.of(0, 0, 3)); + assertSpans(" ---\n", ThematicBreak.class, SourceSpan.of(0, 2, 3)); + assertSpans("> ---\n", ThematicBreak.class, SourceSpan.of(0, 2, 3)); + } + + @Test + public void atxHeading() { + assertSpans("# foo", Heading.class, SourceSpan.of(0, 0, 5)); + assertSpans(" # foo", Heading.class, SourceSpan.of(0, 1, 5)); + assertSpans("## foo ##", Heading.class, SourceSpan.of(0, 0, 9)); + assertSpans("> # foo", Heading.class, SourceSpan.of(0, 2, 5)); + } + + @Test + public void setextHeading() { + assertSpans("foo\n===\n", Heading.class, SourceSpan.of(0, 0, 3), SourceSpan.of(1, 0, 3)); + assertSpans("foo\nbar\n====\n", Heading.class, SourceSpan.of(0, 0, 3), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 4)); + assertSpans(" foo\n ===\n", Heading.class, SourceSpan.of(0, 2, 3), SourceSpan.of(1, 2, 3)); + assertSpans("> foo\n> ===\n", Heading.class, SourceSpan.of(0, 2, 3), SourceSpan.of(1, 2, 3)); + } + + @Test + public void indentedCodeBlock() { + assertSpans(" foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 3)); + assertSpans(" foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 4)); + assertSpans("\tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 1, 3)); + assertSpans(" \tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 2, 3)); + assertSpans(" \tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 3, 3)); + assertSpans(" \tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 3)); + assertSpans(" \tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 4)); + assertSpans(" \t foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 5)); + assertSpans("\t foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 1, 4)); + assertSpans("\t foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 1, 5)); + assertSpans(" foo\n bar\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 3), SourceSpan.of(1, 4, 4)); + assertSpans(" foo\n\tbar\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 3), SourceSpan.of(1, 1, 3)); + assertSpans(" foo\n \n \n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 3), SourceSpan.of(2, 4, 1)); + assertSpans("> foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 6, 3)); + } + + @Test + public void fencedCodeBlock() { + assertSpans("```\nfoo\n```\n", FencedCodeBlock.class, + SourceSpan.of(0, 0, 3), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 3)); + assertSpans("```\nfoo\nbar\n```\n", FencedCodeBlock.class, + SourceSpan.of(0, 0, 3), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 3), SourceSpan.of(3, 0, 3)); + assertSpans("```\nfoo\nbar\n```\n", FencedCodeBlock.class, + SourceSpan.of(0, 0, 3), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 3), SourceSpan.of(3, 0, 3)); + assertSpans(" ```\n foo\n ```\n", FencedCodeBlock.class, + SourceSpan.of(0, 3, 3), SourceSpan.of(1, 3, 3), SourceSpan.of(2, 3, 3)); + assertSpans(" ```\n foo\nfoo\n```\n", FencedCodeBlock.class, + SourceSpan.of(0, 1, 3), SourceSpan.of(1, 1, 3), SourceSpan.of(2, 0, 3), SourceSpan.of(3, 0, 3)); + assertSpans("```info\nfoo\n```\n", FencedCodeBlock.class, + SourceSpan.of(0, 0, 7), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 3)); + assertSpans("* ```\n foo\n ```\n", FencedCodeBlock.class, + SourceSpan.of(0, 2, 3), SourceSpan.of(1, 2, 3), SourceSpan.of(2, 2, 3)); + assertSpans("> ```\n> foo\n> ```\n", FencedCodeBlock.class, + SourceSpan.of(0, 2, 3), SourceSpan.of(1, 2, 3), SourceSpan.of(2, 2, 3)); + + Node document = PARSER.parse("```\nfoo\n```\nbar\n"); + Paragraph paragraph = (Paragraph) document.getLastChild(); + assertThat(paragraph.getSourceSpans(), contains(SourceSpan.of(3, 0, 3))); + } + + @Test + public void htmlBlock() { + assertSpans("
\n", HtmlBlock.class, SourceSpan.of(0, 0, 5)); + assertSpans("
\n foo\n
\n", HtmlBlock.class, SourceSpan.of(0, 0, 6), SourceSpan.of(1, 0, 4), SourceSpan.of(2, 0, 7)); + assertSpans("*
\n", HtmlBlock.class, SourceSpan.of(0, 2, 5)); + } + + @Test + public void blockQuote() { + assertSpans(">foo\n", BlockQuote.class, SourceSpan.of(0, 0, 4)); + assertSpans("> foo\n", BlockQuote.class, SourceSpan.of(0, 0, 5)); + assertSpans("> foo\n", BlockQuote.class, SourceSpan.of(0, 0, 6)); + assertSpans(" > foo\n", BlockQuote.class, SourceSpan.of(0, 1, 5)); + assertSpans(" > foo\n > bar\n", BlockQuote.class, SourceSpan.of(0, 3, 5), SourceSpan.of(1, 2, 5)); + // Lazy continuations + assertSpans("> foo\nbar\n", BlockQuote.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 3)); + assertSpans("> foo\nbar\n> baz\n", BlockQuote.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 5)); + assertSpans("> > foo\nbar\n", BlockQuote.class, SourceSpan.of(0, 0, 7), SourceSpan.of(1, 0, 3)); + } + + @Test + public void listBlock() { + assertSpans("* foo\n", ListBlock.class, SourceSpan.of(0, 0, 5)); + assertSpans("* foo\n bar\n", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 2, 3)); + assertSpans("* foo\n* bar\n", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 5)); + assertSpans("* foo\n # bar\n", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 2, 5)); + assertSpans("* foo\n * bar\n", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 2, 5)); + assertSpans("* foo\n> bar\n", ListBlock.class, SourceSpan.of(0, 0, 5)); + assertSpans("> * foo\n", ListBlock.class, SourceSpan.of(0, 2, 5)); + + // Lazy continuations + assertSpans("* foo\nbar\nbaz", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 3)); + assertSpans("* foo\nbar\n* baz", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 5)); + assertSpans("* foo\n * bar\nbaz", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 2, 5), SourceSpan.of(2, 0, 3)); + + Node document = PARSER.parse("* foo\n * bar\n"); + ListBlock listBlock = (ListBlock) document.getFirstChild().getFirstChild().getLastChild(); + assertThat(listBlock.getSourceSpans(), contains(SourceSpan.of(1, 2, 5))); + } + + @Test + public void listItem() { + assertSpans("* foo\n", ListItem.class, SourceSpan.of(0, 0, 5)); + assertSpans(" * foo\n", ListItem.class, SourceSpan.of(0, 1, 5)); + assertSpans(" * foo\n", ListItem.class, SourceSpan.of(0, 2, 5)); + assertSpans(" * foo\n", ListItem.class, SourceSpan.of(0, 3, 5)); + assertSpans("*\n foo\n", ListItem.class, SourceSpan.of(0, 0, 1), SourceSpan.of(1, 2, 3)); + assertSpans("*\n foo\n bar\n", ListItem.class, SourceSpan.of(0, 0, 1), SourceSpan.of(1, 2, 3), SourceSpan.of(2, 2, 3)); + assertSpans("> * foo\n", ListItem.class, SourceSpan.of(0, 2, 5)); + + // Lazy continuations + assertSpans("* foo\nbar\n", ListItem.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 3)); + assertSpans("* foo\nbar\nbaz\n", ListItem.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 3)); + } + + @Test + public void visualCheck() { + assertEquals("(> {[* ]})\n(> {[]})\n(> {⸢* ⸤baz⸥⸣})\n", + visualizeSourceSpans("> * foo\n> bar\n> * baz\n")); + assertEquals("(> {[* <```>]})\n(> {[]})\n(> {[<```>]})\n", + visualizeSourceSpans("> * ```\n> foo\n> ```")); + } + + private String visualizeSourceSpans(String source) { + Node document = PARSER.parse(source); + return SourceSpanRenderer.render(document, source); + } + + private static void assertSpans(String input, Class nodeClass, SourceSpan... expectedSourceSpans) { + Node node = PARSER.parse(input); + while (node != null && !nodeClass.isInstance(node)) { + node = node.getFirstChild(); + } + if (node == null) { + fail("Expected to find " + nodeClass + " node"); + } else { + assertThat(node.getSourceSpans(), contains(expectedSourceSpans)); + } + } +} diff --git a/pom.xml b/pom.xml index d81dc186a..2bc6e3e40 100644 --- a/pom.xml +++ b/pom.xml @@ -143,6 +143,11 @@ junit 4.12 + + org.hamcrest + hamcrest-library + 1.3 + org.openjdk.jmh jmh-core From e7d33ae7063bc8876a874edbe1959c750d514c4b Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Sat, 15 Aug 2020 21:05:08 +1000 Subject: [PATCH 2/8] Some tests passing The block parsers will still need to be extended so that they can communicate where the source started, in case they return positions after marker characters. I think we can put that into BlockStart and BlockContinue though. --- .../commonmark/internal/DocumentParser.java | 71 ++++++++++++------- .../main/java/org/commonmark/node/Node.java | 3 +- .../org/commonmark/test/SourceSpansTest.java | 8 ++- 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java b/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java index ce965f1e3..9691c94ff 100644 --- a/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java +++ b/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java @@ -95,7 +95,7 @@ public class DocumentParser implements ParserState { private final DocumentBlockParser documentBlockParser; private final Map definitions = new LinkedHashMap<>(); - private final List activeBlockParsers = new ArrayList<>(); + private final List openBlockParsers = new ArrayList<>(); private final List allBlockParsers = new ArrayList<>(); public DocumentParser(List blockParserFactories, InlineParserFactory inlineParserFactory, @@ -105,7 +105,7 @@ public DocumentParser(List blockParserFactories, InlineParse this.delimiterProcessors = delimiterProcessors; this.documentBlockParser = new DocumentBlockParser(); - activateBlockParser(this.documentBlockParser); + activateBlockParser(new OpenBlockParser(documentBlockParser, 0)); } public static Set> getDefaultBlockParserTypes() { @@ -197,7 +197,7 @@ public boolean isBlank() { @Override public BlockParser getActiveBlockParser() { - return activeBlockParsers.get(activeBlockParsers.size() - 1); + return openBlockParsers.get(openBlockParsers.size() - 1).blockParser; } /** @@ -214,15 +214,17 @@ private void incorporateLine(CharSequence ln) { // For each containing block, try to parse the associated line start. // The document will always match, so we can skip the first block parser and start at 1 matches int matches = 1; - for (int i = 1; i < activeBlockParsers.size(); i++) { - BlockParser blockParser = activeBlockParsers.get(i); + for (int i = 1; i < openBlockParsers.size(); i++) { + OpenBlockParser openBlockParser = openBlockParsers.get(i); + BlockParser blockParser = openBlockParser.blockParser; findNextNonSpace(); BlockContinue result = blockParser.tryContinue(this); if (result instanceof BlockContinueImpl) { BlockContinueImpl blockContinue = (BlockContinueImpl) result; if (blockContinue.isFinalize()) { - closeBlockParsers(activeBlockParsers.size() - i); + // TODO: Source pos here? + closeBlockParsers(openBlockParsers.size() - i); return; } else { if (blockContinue.getNewIndex() != -1) { @@ -230,6 +232,7 @@ private void incorporateLine(CharSequence ln) { } else if (blockContinue.getNewColumn() != -1) { setNewColumn(blockContinue.getNewColumn()); } + openBlockParser.index = index; matches++; } } else { @@ -237,8 +240,8 @@ private void incorporateLine(CharSequence ln) { } } - int unmatchedBlocks = activeBlockParsers.size() - matches; - BlockParser blockParser = activeBlockParsers.get(matches - 1); + int unmatchedBlocks = openBlockParsers.size() - matches; + BlockParser blockParser = openBlockParsers.get(matches - 1).blockParser; boolean startedNewBlock = false; // Unless last matched container is a code block, try new container starts, @@ -278,7 +281,8 @@ private void incorporateLine(CharSequence ln) { } for (BlockParser newBlockParser : blockStart.getBlockParsers()) { - blockParser = addChild(newBlockParser); + addChild(new OpenBlockParser(newBlockParser, index)); + blockParser = newBlockParser; tryBlockStarts = newBlockParser.isContainer(); } } @@ -289,11 +293,9 @@ private void incorporateLine(CharSequence ln) { // First check for a lazy paragraph continuation: if (!startedNewBlock && !isBlank() && getActiveBlockParser().canHaveLazyContinuationLines()) { + openBlockParsers.get(openBlockParsers.size() - 1).index = index; // lazy paragraph continuation // TODO - for (BlockParser parser : activeBlockParsers.subList(1, activeBlockParsers.size())) { -// parser.onLazyContinuationLine(this); - } addLine(); } else { @@ -311,7 +313,7 @@ private void incorporateLine(CharSequence ln) { // SourceSpan sourceSpan = SourceSpans.fromState(this, this.getNextNonSpaceIndex()); // ParagraphParser paragraphParser = new ParagraphParser(sourceSpan); ParagraphParser paragraphParser = new ParagraphParser(); - addChild(paragraphParser); + addChild(new OpenBlockParser(paragraphParser, index)); addLine(); } } @@ -410,6 +412,13 @@ private void addLine() { content = line.subSequence(index, line.length()); } getActiveBlockParser().addLine(content); + for (OpenBlockParser openBlockParser : openBlockParsers) { + int blockIndex = openBlockParser.index; + int length = line.length() - blockIndex; + if (length != 0) { + openBlockParser.blockParser.getBlock().getSourceSpans().add(SourceSpan.of(lineIndex, blockIndex, length)); + } + } } private BlockStartImpl findBlockStart(BlockParser blockParser) { @@ -467,28 +476,26 @@ private void processInlines() { * Add block of type tag as a child of the tip. If the tip can't accept children, close and finalize it and try * its parent, and so on until we find a block that can accept children. */ - private T addChild(T blockParser) { - while (!getActiveBlockParser().canContain(blockParser.getBlock())) { + private void addChild(OpenBlockParser openBlockParser) { + while (!getActiveBlockParser().canContain(openBlockParser.blockParser.getBlock())) { closeBlockParsers(1); } - getActiveBlockParser().getBlock().appendChild(blockParser.getBlock()); - activateBlockParser(blockParser); - - return blockParser; + getActiveBlockParser().getBlock().appendChild(openBlockParser.blockParser.getBlock()); + activateBlockParser(openBlockParser); } - private void activateBlockParser(BlockParser blockParser) { - activeBlockParsers.add(blockParser); + private void activateBlockParser(OpenBlockParser openBlockParser) { + openBlockParsers.add(openBlockParser); } - private BlockParser deactivateBlockParser() { - return activeBlockParsers.remove(activeBlockParsers.size() - 1); + private OpenBlockParser deactivateBlockParser() { + return openBlockParsers.remove(openBlockParsers.size() - 1); } private void prepareActiveBlockParserForReplacement() { // Note that we don't want to parse inlines or finalize this block, as it's getting replaced. - BlockParser old = deactivateBlockParser(); + BlockParser old = deactivateBlockParser().blockParser; if (old instanceof ParagraphParser) { ParagraphParser paragraphParser = (ParagraphParser) old; @@ -500,18 +507,20 @@ private void prepareActiveBlockParserForReplacement() { addDefinitionsFrom(paragraphParser); } + // TODO: Copy source positions of old block + old.getBlock().unlink(); } private Document finalizeAndProcess() { - closeBlockParsers(activeBlockParsers.size()); + closeBlockParsers(openBlockParsers.size()); processInlines(); return documentBlockParser.getBlock(); } private void closeBlockParsers(int count) { for (int i = 0; i < count; i++) { - BlockParser blockParser = deactivateBlockParser(); + BlockParser blockParser = deactivateBlockParser().blockParser; finalize(blockParser); // Remember for inline parsing. Note that a lot of blocks don't need inline parsing. We could have a // separate interface (e.g. BlockParserWithInlines) so that we only have to remember those that actually @@ -547,4 +556,14 @@ public CharSequence getParagraphContent() { return null; } } + + private static class OpenBlockParser { + private final BlockParser blockParser; + private int index; + + OpenBlockParser(BlockParser blockParser, int index) { + this.blockParser = blockParser; + this.index = index; + } + } } diff --git a/commonmark/src/main/java/org/commonmark/node/Node.java b/commonmark/src/main/java/org/commonmark/node/Node.java index 7a742ea85..462acc545 100644 --- a/commonmark/src/main/java/org/commonmark/node/Node.java +++ b/commonmark/src/main/java/org/commonmark/node/Node.java @@ -1,7 +1,6 @@ package org.commonmark.node; import java.util.ArrayList; -import java.util.Collections; import java.util.List; public abstract class Node { @@ -36,7 +35,7 @@ public Node getParent() { } public List getSourceSpans() { - return Collections.unmodifiableList(sourceSpans); + return sourceSpans; } public void setSourceSpans(List sourceSpans) { diff --git a/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java b/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java index 2264ebfaf..9e03c94da 100644 --- a/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java +++ b/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java @@ -14,6 +14,8 @@ import org.commonmark.parser.Parser; import org.junit.Test; +import java.util.Arrays; + import static org.hamcrest.Matchers.contains; import static org.junit.Assert.*; @@ -21,6 +23,10 @@ public class SourceSpansTest { private static final Parser PARSER = Parser.builder().build(); + // TODO: Tests for when column is within tab + + // TODO: Tests for where paragraph starts with link reference definitions + @Test public void paragraph() { assertSpans("foo\n", Paragraph.class, SourceSpan.of(0, 0, 3)); @@ -172,7 +178,7 @@ private static void assertSpans(String input, Class nodeClass, S if (node == null) { fail("Expected to find " + nodeClass + " node"); } else { - assertThat(node.getSourceSpans(), contains(expectedSourceSpans)); + assertEquals(Arrays.asList(expectedSourceSpans), node.getSourceSpans()); } } } From 149ed277b4fb8b9165cec7cca3ec0732a71b9a78 Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Wed, 2 Sep 2020 22:00:42 +1000 Subject: [PATCH 3/8] Set block source spans based on the index This means that e.g. this: ``` paragraph starting with a space ``` Will include the starting space in the source positions. I was initially thinking it should not be included, but that makes a lot of things harder, for not much gain IMO. E.g. for a code block with 5 spaces: ``` code ``` Where should the source position start, at index 0, 4, or 5? This implements 0. 4 is kind of hard to implement due to tabs being treated as 4-space tabstops. --- .../commonmark/ext/gfm/tables/TablesTest.java | 37 ++++--- .../commonmark/internal/DocumentParser.java | 63 +++++++---- .../LinkReferenceDefinitionParser.java | 15 ++- .../commonmark/internal/ParagraphParser.java | 12 +- .../java/org/commonmark/node/SourceSpan.java | 4 +- .../parser/block/AbstractBlockParser.java | 6 + .../commonmark/parser/block/BlockParser.java | 8 ++ .../commonmark/test/SourceSpanRenderer.java | 9 +- .../org/commonmark/test/SourceSpansTest.java | 104 +++++++++++------- 9 files changed, 172 insertions(+), 86 deletions(-) diff --git a/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java b/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java index 950926e1f..b38358d8b 100644 --- a/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java +++ b/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java @@ -11,13 +11,13 @@ import org.commonmark.testutil.RenderingTestCase; import org.junit.Test; +import java.util.Arrays; import java.util.Collections; import java.util.Map; import java.util.Set; import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.emptyIterable; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; public class TablesTest extends RenderingTestCase { @@ -651,42 +651,43 @@ public void sourceSpans() { Node document = PARSER.parse("Abc|Def\n---|---\n|1|2\n 3|four|\n|||\n"); TableBlock block = (TableBlock) document.getFirstChild(); - assertThat(block.getSourceSpans(), contains(SourceSpan.of(0, 0, 7), SourceSpan.of(1, 0, 7), - SourceSpan.of(2, 0, 4), SourceSpan.of(3, 1, 7), SourceSpan.of(4, 0, 3))); + assertEquals(Arrays.asList(SourceSpan.of(0, 0, 7), SourceSpan.of(1, 0, 7), + SourceSpan.of(2, 0, 4), SourceSpan.of(3, 0, 8), SourceSpan.of(4, 0, 3)), + block.getSourceSpans()); TableHead head = (TableHead) block.getFirstChild(); - assertThat(head.getSourceSpans(), contains(SourceSpan.of(0, 0, 7))); + assertEquals(Arrays.asList(SourceSpan.of(0, 0, 7)), head.getSourceSpans()); TableRow headRow = (TableRow) head.getFirstChild(); - assertThat(headRow.getSourceSpans(), contains(SourceSpan.of(0, 0, 7))); + assertEquals(Arrays.asList(SourceSpan.of(0, 0, 7)), headRow.getSourceSpans()); TableCell headRowCell1 = (TableCell) headRow.getFirstChild(); TableCell headRowCell2 = (TableCell) headRow.getLastChild(); - assertThat(headRowCell1.getSourceSpans(), contains(SourceSpan.of(0, 0, 3))); - assertThat(headRowCell2.getSourceSpans(), contains(SourceSpan.of(0, 4, 3))); + assertEquals(Arrays.asList(SourceSpan.of(0, 0, 3)), headRowCell1.getSourceSpans()); + assertEquals(Arrays.asList(SourceSpan.of(0, 4, 3)), headRowCell2.getSourceSpans()); TableBody body = (TableBody) block.getLastChild(); - assertThat(body.getSourceSpans(), contains(SourceSpan.of(2, 0, 4), SourceSpan.of(3, 1, 7), SourceSpan.of(4, 0, 3))); + assertEquals(Arrays.asList(SourceSpan.of(2, 0, 4), SourceSpan.of(3, 1, 7), SourceSpan.of(4, 0, 3)), body.getSourceSpans()); TableRow bodyRow1 = (TableRow) body.getFirstChild(); - assertThat(bodyRow1.getSourceSpans(), contains(SourceSpan.of(2, 0, 4))); + assertEquals(Arrays.asList(SourceSpan.of(2, 0, 4)), bodyRow1.getSourceSpans()); TableCell bodyRow1Cell1 = (TableCell) bodyRow1.getFirstChild(); TableCell bodyRow1Cell2 = (TableCell) bodyRow1.getLastChild(); - assertThat(bodyRow1Cell1.getSourceSpans(), contains(SourceSpan.of(2, 1, 1))); - assertThat(bodyRow1Cell2.getSourceSpans(), contains(SourceSpan.of(2, 3, 1))); + assertEquals(Arrays.asList(SourceSpan.of(2, 1, 1)), bodyRow1Cell1.getSourceSpans()); + assertEquals(Arrays.asList(SourceSpan.of(2, 3, 1)), bodyRow1Cell2.getSourceSpans()); TableRow bodyRow2 = (TableRow) body.getFirstChild().getNext(); - assertThat(bodyRow2.getSourceSpans(), contains(SourceSpan.of(3, 1, 7))); + assertEquals(Arrays.asList(SourceSpan.of(3, 1, 7)), bodyRow2.getSourceSpans()); TableCell bodyRow2Cell1 = (TableCell) bodyRow2.getFirstChild(); TableCell bodyRow2Cell2 = (TableCell) bodyRow2.getLastChild(); - assertThat(bodyRow2Cell1.getSourceSpans(), contains(SourceSpan.of(3, 1, 1))); - assertThat(bodyRow2Cell2.getSourceSpans(), contains(SourceSpan.of(3, 3, 4))); + assertEquals(Arrays.asList(SourceSpan.of(3, 1, 1)), bodyRow2Cell1.getSourceSpans()); + assertEquals(Arrays.asList(SourceSpan.of(3, 3, 4)), bodyRow2Cell2.getSourceSpans()); TableRow bodyRow3 = (TableRow) body.getLastChild(); - assertThat(bodyRow3.getSourceSpans(), contains(SourceSpan.of(4, 0, 3))); + assertEquals(Arrays.asList(SourceSpan.of(4, 0, 3)), bodyRow3.getSourceSpans()); TableCell bodyRow3Cell1 = (TableCell) bodyRow3.getFirstChild(); TableCell bodyRow3Cell2 = (TableCell) bodyRow3.getLastChild(); - assertThat(bodyRow3Cell1.getSourceSpans(), emptyIterable()); - assertThat(bodyRow3Cell2.getSourceSpans(), emptyIterable()); + assertEquals(Collections.emptyList(), bodyRow3Cell1.getSourceSpans()); + assertEquals(Collections.emptyList(), bodyRow3Cell2.getSourceSpans()); } @Override diff --git a/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java b/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java index 9691c94ff..6287d42d2 100644 --- a/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java +++ b/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java @@ -222,8 +222,9 @@ private void incorporateLine(CharSequence ln) { BlockContinue result = blockParser.tryContinue(this); if (result instanceof BlockContinueImpl) { BlockContinueImpl blockContinue = (BlockContinueImpl) result; + openBlockParser.sourceIndex = getIndex(); if (blockContinue.isFinalize()) { - // TODO: Source pos here? + addSourceSpans(); closeBlockParsers(openBlockParsers.size() - i); return; } else { @@ -232,7 +233,6 @@ private void incorporateLine(CharSequence ln) { } else if (blockContinue.getNewColumn() != -1) { setNewColumn(blockContinue.getNewColumn()); } - openBlockParser.index = index; matches++; } } else { @@ -244,10 +244,13 @@ private void incorporateLine(CharSequence ln) { BlockParser blockParser = openBlockParsers.get(matches - 1).blockParser; boolean startedNewBlock = false; + int lastIndex = index; + // Unless last matched container is a code block, try new container starts, // adding children to the last matched container: boolean tryBlockStarts = blockParser.getBlock() instanceof Paragraph || blockParser.isContainer(); while (tryBlockStarts) { + lastIndex = index; findNextNonSpace(); // this is a little performance optimization: @@ -263,6 +266,7 @@ private void incorporateLine(CharSequence ln) { } startedNewBlock = true; + int sourceIndex = getIndex(); // We're starting a new block. If we have any previous blocks that need to be closed, we need to do it now. if (unmatchedBlocks > 0) { @@ -276,12 +280,17 @@ private void incorporateLine(CharSequence ln) { setNewColumn(blockStart.getNewColumn()); } + List replacedSourceSpans = null; if (blockStart.isReplaceActiveBlockParser()) { - prepareActiveBlockParserForReplacement(); + Block replacedBlock = prepareActiveBlockParserForReplacement(); + replacedSourceSpans = replacedBlock.getSourceSpans(); } for (BlockParser newBlockParser : blockStart.getBlockParsers()) { - addChild(new OpenBlockParser(newBlockParser, index)); + addChild(new OpenBlockParser(newBlockParser, sourceIndex)); + if (replacedSourceSpans != null) { + newBlockParser.getBlock().setSourceSpans(replacedSourceSpans); + } blockParser = newBlockParser; tryBlockStarts = newBlockParser.isContainer(); } @@ -293,9 +302,8 @@ private void incorporateLine(CharSequence ln) { // First check for a lazy paragraph continuation: if (!startedNewBlock && !isBlank() && getActiveBlockParser().canHaveLazyContinuationLines()) { - openBlockParsers.get(openBlockParsers.size() - 1).index = index; + openBlockParsers.get(openBlockParsers.size() - 1).sourceIndex = lastIndex; // lazy paragraph continuation - // TODO addLine(); } else { @@ -309,12 +317,18 @@ private void incorporateLine(CharSequence ln) { addLine(); } else if (!isBlank()) { // create paragraph container for line - // TODO: -// SourceSpan sourceSpan = SourceSpans.fromState(this, this.getNextNonSpaceIndex()); -// ParagraphParser paragraphParser = new ParagraphParser(sourceSpan); ParagraphParser paragraphParser = new ParagraphParser(); - addChild(new OpenBlockParser(paragraphParser, index)); + addChild(new OpenBlockParser(paragraphParser, lastIndex)); addLine(); + } else { + // This can happen for a list item like this: + // ``` + // * + // list item + // ``` + // + // The first line does not start a paragraph yet, but we still want to record source positions. + addSourceSpans(); } } } @@ -412,11 +426,17 @@ private void addLine() { content = line.subSequence(index, line.length()); } getActiveBlockParser().addLine(content); - for (OpenBlockParser openBlockParser : openBlockParsers) { - int blockIndex = openBlockParser.index; + addSourceSpans(); + } + + private void addSourceSpans() { + // Don't add source spans for Document itself (it would get the whole source text) + for (int i = 1; i < openBlockParsers.size(); i++) { + OpenBlockParser openBlockParser = openBlockParsers.get(i); + int blockIndex = openBlockParser.sourceIndex; int length = line.length() - blockIndex; if (length != 0) { - openBlockParser.blockParser.getBlock().getSourceSpans().add(SourceSpan.of(lineIndex, blockIndex, length)); + openBlockParser.blockParser.addSourceSpan(SourceSpan.of(lineIndex, blockIndex, length)); } } } @@ -443,8 +463,6 @@ private void finalize(BlockParser blockParser) { } blockParser.closeBlock(); - // TODO - //blockParser.getBlock().setSourceSpans(blockParser.getSourceSpans()); } private void addDefinitionsFrom(ParagraphParser paragraphParser) { @@ -493,8 +511,8 @@ private OpenBlockParser deactivateBlockParser() { return openBlockParsers.remove(openBlockParsers.size() - 1); } - private void prepareActiveBlockParserForReplacement() { - // Note that we don't want to parse inlines or finalize this block, as it's getting replaced. + private Block prepareActiveBlockParserForReplacement() { + // Note that we don't want to parse inlines, as it's getting replaced. BlockParser old = deactivateBlockParser().blockParser; if (old instanceof ParagraphParser) { @@ -507,9 +525,10 @@ private void prepareActiveBlockParserForReplacement() { addDefinitionsFrom(paragraphParser); } - // TODO: Copy source positions of old block - + // Do this so that source positions are calculated, which we will carry over to the replacing block. + old.closeBlock(); old.getBlock().unlink(); + return old.getBlock(); } private Document finalizeAndProcess() { @@ -559,11 +578,11 @@ public CharSequence getParagraphContent() { private static class OpenBlockParser { private final BlockParser blockParser; - private int index; + private int sourceIndex; - OpenBlockParser(BlockParser blockParser, int index) { + OpenBlockParser(BlockParser blockParser, int sourceIndex) { this.blockParser = blockParser; - this.index = index; + this.sourceIndex = sourceIndex; } } } diff --git a/commonmark/src/main/java/org/commonmark/internal/LinkReferenceDefinitionParser.java b/commonmark/src/main/java/org/commonmark/internal/LinkReferenceDefinitionParser.java index 1fe2cbea7..e0c23160e 100644 --- a/commonmark/src/main/java/org/commonmark/internal/LinkReferenceDefinitionParser.java +++ b/commonmark/src/main/java/org/commonmark/internal/LinkReferenceDefinitionParser.java @@ -4,6 +4,7 @@ import org.commonmark.internal.util.LinkScanner; import org.commonmark.internal.util.Parsing; import org.commonmark.node.LinkReferenceDefinition; +import org.commonmark.node.SourceSpan; import java.util.ArrayList; import java.util.List; @@ -19,6 +20,7 @@ public class LinkReferenceDefinitionParser { private final StringBuilder paragraph = new StringBuilder(); private final List definitions = new ArrayList<>(); + private final List sourceSpans = new ArrayList<>(); private StringBuilder label; private String normalizedLabel; @@ -70,10 +72,18 @@ public void parse(CharSequence line) { } } + public void addSourceSpan(SourceSpan sourceSpan) { + sourceSpans.add(sourceSpan); + } + CharSequence getParagraphContent() { return paragraph; } + List getParagraphSourceSpans() { + return sourceSpans; + } + List getDefinitions() { finishReference(); return definitions; @@ -235,7 +245,10 @@ private void finishReference() { String d = Escaping.unescapeString(destination); String t = title != null ? Escaping.unescapeString(title.toString()) : null; - definitions.add(new LinkReferenceDefinition(normalizedLabel, d, t)); + LinkReferenceDefinition definition = new LinkReferenceDefinition(normalizedLabel, d, t); + definition.setSourceSpans(sourceSpans); + sourceSpans.clear(); + definitions.add(definition); label = null; referenceValid = false; diff --git a/commonmark/src/main/java/org/commonmark/internal/ParagraphParser.java b/commonmark/src/main/java/org/commonmark/internal/ParagraphParser.java index ee2899a93..09cf9fdec 100644 --- a/commonmark/src/main/java/org/commonmark/internal/ParagraphParser.java +++ b/commonmark/src/main/java/org/commonmark/internal/ParagraphParser.java @@ -3,6 +3,7 @@ import org.commonmark.node.Block; import org.commonmark.node.LinkReferenceDefinition; import org.commonmark.node.Paragraph; +import org.commonmark.node.SourceSpan; import org.commonmark.parser.InlineParser; import org.commonmark.parser.block.AbstractBlockParser; import org.commonmark.parser.block.BlockContinue; @@ -13,7 +14,7 @@ public class ParagraphParser extends AbstractBlockParser { private final Paragraph block = new Paragraph(); - private LinkReferenceDefinitionParser linkReferenceDefinitionParser = new LinkReferenceDefinitionParser(); + private final LinkReferenceDefinitionParser linkReferenceDefinitionParser = new LinkReferenceDefinitionParser(); @Override public boolean canHaveLazyContinuationLines() { @@ -39,10 +40,19 @@ public void addLine(CharSequence line) { linkReferenceDefinitionParser.parse(line); } + @Override + public void addSourceSpan(SourceSpan sourceSpan) { + // Some source spans might belong to link reference definitions, others to the paragraph. + // The parser will handle that. + linkReferenceDefinitionParser.addSourceSpan(sourceSpan); + } + @Override public void closeBlock() { if (linkReferenceDefinitionParser.getParagraphContent().length() == 0) { block.unlink(); + } else { + block.getSourceSpans().addAll(linkReferenceDefinitionParser.getParagraphSourceSpans()); } } diff --git a/commonmark/src/main/java/org/commonmark/node/SourceSpan.java b/commonmark/src/main/java/org/commonmark/node/SourceSpan.java index c3c49531b..3788f86f0 100644 --- a/commonmark/src/main/java/org/commonmark/node/SourceSpan.java +++ b/commonmark/src/main/java/org/commonmark/node/SourceSpan.java @@ -61,8 +61,8 @@ public int hashCode() { @Override public String toString() { return "SourceSpan{" + - "lineIndex=" + lineIndex + - ", columnIndex=" + columnIndex + + "line=" + lineIndex + + ", column=" + columnIndex + ", length=" + length + "}"; } diff --git a/commonmark/src/main/java/org/commonmark/parser/block/AbstractBlockParser.java b/commonmark/src/main/java/org/commonmark/parser/block/AbstractBlockParser.java index f806d105c..c66873dd1 100644 --- a/commonmark/src/main/java/org/commonmark/parser/block/AbstractBlockParser.java +++ b/commonmark/src/main/java/org/commonmark/parser/block/AbstractBlockParser.java @@ -1,6 +1,7 @@ package org.commonmark.parser.block; import org.commonmark.node.Block; +import org.commonmark.node.SourceSpan; import org.commonmark.parser.InlineParser; public abstract class AbstractBlockParser implements BlockParser { @@ -24,6 +25,11 @@ public boolean canContain(Block childBlock) { public void addLine(CharSequence line) { } + @Override + public void addSourceSpan(SourceSpan sourceSpan) { + getBlock().getSourceSpans().add(sourceSpan); + } + @Override public void closeBlock() { } diff --git a/commonmark/src/main/java/org/commonmark/parser/block/BlockParser.java b/commonmark/src/main/java/org/commonmark/parser/block/BlockParser.java index 0c903198c..ca603ac8f 100644 --- a/commonmark/src/main/java/org/commonmark/parser/block/BlockParser.java +++ b/commonmark/src/main/java/org/commonmark/parser/block/BlockParser.java @@ -1,6 +1,7 @@ package org.commonmark.parser.block; import org.commonmark.node.Block; +import org.commonmark.node.SourceSpan; import org.commonmark.parser.InlineParser; /** @@ -34,6 +35,13 @@ public interface BlockParser { void addLine(CharSequence line); + /** + * Add a source span of the currently parsed block. The default implementation in {@link AbstractBlockParser} adds + * it to the block. Unless you have some complicated parsing where you need to check source positions, you don't + * need to override this. + */ + void addSourceSpan(SourceSpan sourceSpan); + void closeBlock(); void parseInlines(InlineParser inlineParser); diff --git a/commonmark/src/test/java/org/commonmark/test/SourceSpanRenderer.java b/commonmark/src/test/java/org/commonmark/test/SourceSpanRenderer.java index 4cb057b36..1b76ed5bd 100644 --- a/commonmark/src/test/java/org/commonmark/test/SourceSpanRenderer.java +++ b/commonmark/src/test/java/org/commonmark/test/SourceSpanRenderer.java @@ -47,9 +47,10 @@ private static void appendMarkers(Map> lineMarkers, int co private static class SourceSpanMarkersVisitor extends AbstractVisitor { + private static final String OPENING = "({[<⸢⸤"; + private static final String CLOSING = ")}]>⸣⸥"; + private final Map>> markers = new HashMap<>(); - private final String opening = "({[<⸢⸤"; - private final String closing = ")}]>⸣⸥"; private int markerIndex; @@ -61,8 +62,8 @@ public Map>> getMarkers() { protected void visitChildren(Node parent) { if (!parent.getSourceSpans().isEmpty()) { for (SourceSpan sourceSpan : parent.getSourceSpans()) { - String opener = String.valueOf(opening.charAt(markerIndex % opening.length())); - String closer = String.valueOf(closing.charAt(markerIndex % closing.length())); + String opener = String.valueOf(OPENING.charAt(markerIndex % OPENING.length())); + String closer = String.valueOf(CLOSING.charAt(markerIndex % CLOSING.length())); int col = sourceSpan.getColumnIndex(); getMarkers(sourceSpan.getLineIndex(), col).add(opener); diff --git a/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java b/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java index 9e03c94da..3e2de2be2 100644 --- a/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java +++ b/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java @@ -5,6 +5,7 @@ import org.commonmark.node.Heading; import org.commonmark.node.HtmlBlock; import org.commonmark.node.IndentedCodeBlock; +import org.commonmark.node.LinkReferenceDefinition; import org.commonmark.node.ListBlock; import org.commonmark.node.ListItem; import org.commonmark.node.Node; @@ -23,15 +24,11 @@ public class SourceSpansTest { private static final Parser PARSER = Parser.builder().build(); - // TODO: Tests for when column is within tab - - // TODO: Tests for where paragraph starts with link reference definitions - @Test public void paragraph() { assertSpans("foo\n", Paragraph.class, SourceSpan.of(0, 0, 3)); assertSpans("foo\nbar\n", Paragraph.class, SourceSpan.of(0, 0, 3), SourceSpan.of(1, 0, 3)); - assertSpans(" foo\n bar\n", Paragraph.class, SourceSpan.of(0, 2, 3), SourceSpan.of(1, 2, 3)); + assertSpans(" foo\n bar\n", Paragraph.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 5)); assertSpans("> foo\n> bar\n", Paragraph.class, SourceSpan.of(0, 2, 3), SourceSpan.of(1, 2, 3)); assertSpans("* foo\n bar\n", Paragraph.class, SourceSpan.of(0, 2, 3), SourceSpan.of(1, 2, 3)); assertSpans("* foo\nbar\n", Paragraph.class, SourceSpan.of(0, 2, 3), SourceSpan.of(1, 0, 3)); @@ -40,14 +37,14 @@ public void paragraph() { @Test public void thematicBreak() { assertSpans("---\n", ThematicBreak.class, SourceSpan.of(0, 0, 3)); - assertSpans(" ---\n", ThematicBreak.class, SourceSpan.of(0, 2, 3)); + assertSpans(" ---\n", ThematicBreak.class, SourceSpan.of(0, 0, 5)); assertSpans("> ---\n", ThematicBreak.class, SourceSpan.of(0, 2, 3)); } @Test public void atxHeading() { assertSpans("# foo", Heading.class, SourceSpan.of(0, 0, 5)); - assertSpans(" # foo", Heading.class, SourceSpan.of(0, 1, 5)); + assertSpans(" # foo", Heading.class, SourceSpan.of(0, 0, 6)); assertSpans("## foo ##", Heading.class, SourceSpan.of(0, 0, 9)); assertSpans("> # foo", Heading.class, SourceSpan.of(0, 2, 5)); } @@ -56,40 +53,42 @@ public void atxHeading() { public void setextHeading() { assertSpans("foo\n===\n", Heading.class, SourceSpan.of(0, 0, 3), SourceSpan.of(1, 0, 3)); assertSpans("foo\nbar\n====\n", Heading.class, SourceSpan.of(0, 0, 3), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 4)); - assertSpans(" foo\n ===\n", Heading.class, SourceSpan.of(0, 2, 3), SourceSpan.of(1, 2, 3)); + assertSpans(" foo\n ===\n", Heading.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 5)); assertSpans("> foo\n> ===\n", Heading.class, SourceSpan.of(0, 2, 3), SourceSpan.of(1, 2, 3)); } @Test public void indentedCodeBlock() { - assertSpans(" foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 3)); - assertSpans(" foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 4)); - assertSpans("\tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 1, 3)); - assertSpans(" \tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 2, 3)); - assertSpans(" \tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 3, 3)); - assertSpans(" \tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 3)); - assertSpans(" \tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 4)); - assertSpans(" \t foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 5)); - assertSpans("\t foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 1, 4)); - assertSpans("\t foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 1, 5)); - assertSpans(" foo\n bar\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 3), SourceSpan.of(1, 4, 4)); - assertSpans(" foo\n\tbar\n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 3), SourceSpan.of(1, 1, 3)); - assertSpans(" foo\n \n \n", IndentedCodeBlock.class, SourceSpan.of(0, 4, 3), SourceSpan.of(2, 4, 1)); - assertSpans("> foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 6, 3)); + assertSpans(" foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 0, 7)); + assertSpans(" foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 0, 8)); + assertSpans("\tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 0, 4)); + assertSpans(" \tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 0, 5)); + assertSpans(" \tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 0, 6)); + assertSpans(" \tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 0, 7)); + assertSpans(" \tfoo\n", IndentedCodeBlock.class, SourceSpan.of(0, 0, 8)); + assertSpans(" \t foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 0, 9)); + assertSpans("\t foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 0, 5)); + assertSpans("\t foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 0, 6)); + assertSpans(" foo\n bar\n", IndentedCodeBlock.class, SourceSpan.of(0, 0, 7), SourceSpan.of(1, 0, 8)); + assertSpans(" foo\n\tbar\n", IndentedCodeBlock.class, SourceSpan.of(0, 0, 7), SourceSpan.of(1, 0, 4)); + assertSpans(" foo\n \n \n", IndentedCodeBlock.class, SourceSpan.of(0, 0, 7), SourceSpan.of(1, 0, 4), SourceSpan.of(2, 0, 5)); + assertSpans("> foo\n", IndentedCodeBlock.class, SourceSpan.of(0, 2, 7)); } @Test public void fencedCodeBlock() { assertSpans("```\nfoo\n```\n", FencedCodeBlock.class, SourceSpan.of(0, 0, 3), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 3)); + assertSpans("```\n foo\n```\n", FencedCodeBlock.class, + SourceSpan.of(0, 0, 3), SourceSpan.of(1, 0, 4), SourceSpan.of(2, 0, 3)); assertSpans("```\nfoo\nbar\n```\n", FencedCodeBlock.class, SourceSpan.of(0, 0, 3), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 3), SourceSpan.of(3, 0, 3)); assertSpans("```\nfoo\nbar\n```\n", FencedCodeBlock.class, SourceSpan.of(0, 0, 3), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 3), SourceSpan.of(3, 0, 3)); assertSpans(" ```\n foo\n ```\n", FencedCodeBlock.class, - SourceSpan.of(0, 3, 3), SourceSpan.of(1, 3, 3), SourceSpan.of(2, 3, 3)); + SourceSpan.of(0, 0, 6), SourceSpan.of(1, 0, 6), SourceSpan.of(2, 0, 6)); assertSpans(" ```\n foo\nfoo\n```\n", FencedCodeBlock.class, - SourceSpan.of(0, 1, 3), SourceSpan.of(1, 1, 3), SourceSpan.of(2, 0, 3), SourceSpan.of(3, 0, 3)); + SourceSpan.of(0, 0, 4), SourceSpan.of(1, 0, 4), SourceSpan.of(2, 0, 3), SourceSpan.of(3, 0, 3)); assertSpans("```info\nfoo\n```\n", FencedCodeBlock.class, SourceSpan.of(0, 0, 7), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 3)); assertSpans("* ```\n foo\n ```\n", FencedCodeBlock.class, @@ -105,7 +104,10 @@ public void fencedCodeBlock() { @Test public void htmlBlock() { assertSpans("
\n", HtmlBlock.class, SourceSpan.of(0, 0, 5)); - assertSpans("
\n foo\n
\n", HtmlBlock.class, SourceSpan.of(0, 0, 6), SourceSpan.of(1, 0, 4), SourceSpan.of(2, 0, 7)); + assertSpans("
\n foo\n
\n", HtmlBlock.class, + SourceSpan.of(0, 0, 6), + SourceSpan.of(1, 0, 4), + SourceSpan.of(2, 0, 7)); assertSpans("*
\n", HtmlBlock.class, SourceSpan.of(0, 2, 5)); } @@ -114,8 +116,8 @@ public void blockQuote() { assertSpans(">foo\n", BlockQuote.class, SourceSpan.of(0, 0, 4)); assertSpans("> foo\n", BlockQuote.class, SourceSpan.of(0, 0, 5)); assertSpans("> foo\n", BlockQuote.class, SourceSpan.of(0, 0, 6)); - assertSpans(" > foo\n", BlockQuote.class, SourceSpan.of(0, 1, 5)); - assertSpans(" > foo\n > bar\n", BlockQuote.class, SourceSpan.of(0, 3, 5), SourceSpan.of(1, 2, 5)); + assertSpans(" > foo\n", BlockQuote.class, SourceSpan.of(0, 0, 6)); + assertSpans(" > foo\n > bar\n", BlockQuote.class, SourceSpan.of(0, 0, 8), SourceSpan.of(1, 0, 7)); // Lazy continuations assertSpans("> foo\nbar\n", BlockQuote.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 3)); assertSpans("> foo\nbar\n> baz\n", BlockQuote.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 5)); @@ -125,17 +127,17 @@ public void blockQuote() { @Test public void listBlock() { assertSpans("* foo\n", ListBlock.class, SourceSpan.of(0, 0, 5)); - assertSpans("* foo\n bar\n", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 2, 3)); + assertSpans("* foo\n bar\n", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 5)); assertSpans("* foo\n* bar\n", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 5)); - assertSpans("* foo\n # bar\n", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 2, 5)); - assertSpans("* foo\n * bar\n", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 2, 5)); + assertSpans("* foo\n # bar\n", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 7)); + assertSpans("* foo\n * bar\n", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 7)); assertSpans("* foo\n> bar\n", ListBlock.class, SourceSpan.of(0, 0, 5)); assertSpans("> * foo\n", ListBlock.class, SourceSpan.of(0, 2, 5)); // Lazy continuations assertSpans("* foo\nbar\nbaz", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 3)); assertSpans("* foo\nbar\n* baz", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 5)); - assertSpans("* foo\n * bar\nbaz", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 2, 5), SourceSpan.of(2, 0, 3)); + assertSpans("* foo\n * bar\nbaz", ListBlock.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 7), SourceSpan.of(2, 0, 3)); Node document = PARSER.parse("* foo\n * bar\n"); ListBlock listBlock = (ListBlock) document.getFirstChild().getFirstChild().getLastChild(); @@ -145,11 +147,11 @@ public void listBlock() { @Test public void listItem() { assertSpans("* foo\n", ListItem.class, SourceSpan.of(0, 0, 5)); - assertSpans(" * foo\n", ListItem.class, SourceSpan.of(0, 1, 5)); - assertSpans(" * foo\n", ListItem.class, SourceSpan.of(0, 2, 5)); - assertSpans(" * foo\n", ListItem.class, SourceSpan.of(0, 3, 5)); - assertSpans("*\n foo\n", ListItem.class, SourceSpan.of(0, 0, 1), SourceSpan.of(1, 2, 3)); - assertSpans("*\n foo\n bar\n", ListItem.class, SourceSpan.of(0, 0, 1), SourceSpan.of(1, 2, 3), SourceSpan.of(2, 2, 3)); + assertSpans(" * foo\n", ListItem.class, SourceSpan.of(0, 0, 6)); + assertSpans(" * foo\n", ListItem.class, SourceSpan.of(0, 0, 7)); + assertSpans(" * foo\n", ListItem.class, SourceSpan.of(0, 0, 8)); + assertSpans("*\n foo\n", ListItem.class, SourceSpan.of(0, 0, 1), SourceSpan.of(1, 0, 5)); + assertSpans("*\n foo\n bar\n", ListItem.class, SourceSpan.of(0, 0, 1), SourceSpan.of(1, 0, 5), SourceSpan.of(2, 0, 5)); assertSpans("> * foo\n", ListItem.class, SourceSpan.of(0, 2, 5)); // Lazy continuations @@ -157,11 +159,37 @@ public void listItem() { assertSpans("* foo\nbar\nbaz\n", ListItem.class, SourceSpan.of(0, 0, 5), SourceSpan.of(1, 0, 3), SourceSpan.of(2, 0, 3)); } + @Test + public void linkReferenceDefinition() { + // This is tricky due to how link reference definition parsing works. It is stripped from the paragraph if it's + // successfully parsed, otherwise it stays part of the paragraph. + Node document = PARSER.parse("[foo]: /url\ntext\n"); + + LinkReferenceDefinition linkReferenceDefinition = (LinkReferenceDefinition) document.getFirstChild(); + assertEquals(Arrays.asList(SourceSpan.of(0, 0, 11)), linkReferenceDefinition.getSourceSpans()); + + Paragraph paragraph = (Paragraph) document.getLastChild(); + assertEquals(Arrays.asList(SourceSpan.of(1, 0, 4)), paragraph.getSourceSpans()); + } + + @Test + public void linkReferenceDefinitionHeading() { + // This is probably the trickiest because we have a link reference definition at the start of a paragraph + // that gets replaced because of a heading. Phew. + Node document = PARSER.parse("[foo]: /url\nHeading\n===\n"); + + LinkReferenceDefinition linkReferenceDefinition = (LinkReferenceDefinition) document.getFirstChild(); + assertEquals(Arrays.asList(SourceSpan.of(0, 0, 11)), linkReferenceDefinition.getSourceSpans()); + + Heading heading = (Heading) document.getLastChild(); + assertEquals(Arrays.asList(SourceSpan.of(1, 0, 7), SourceSpan.of(2, 0, 3)), heading.getSourceSpans()); + } + @Test public void visualCheck() { - assertEquals("(> {[* ]})\n(> {[]})\n(> {⸢* ⸤baz⸥⸣})\n", + assertEquals("(> {[* ]})\n(> {[ ]})\n(> {⸢* ⸤baz⸥⸣})\n", visualizeSourceSpans("> * foo\n> bar\n> * baz\n")); - assertEquals("(> {[* <```>]})\n(> {[]})\n(> {[<```>]})\n", + assertEquals("(> {[* <```>]})\n(> {[ ]})\n(> {[ <```>]})\n", visualizeSourceSpans("> * ```\n> foo\n> ```")); } From e86d5ab8ce966bbfc274305017dfbe1215ddce79 Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Mon, 7 Sep 2020 16:20:34 +1000 Subject: [PATCH 4/8] Implement source spans for table nodes --- .../gfm/tables/internal/TableBlockParser.java | 112 ++++++++++++------ .../commonmark/ext/gfm/tables/TablesTest.java | 6 +- 2 files changed, 81 insertions(+), 37 deletions(-) diff --git a/commonmark-ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java b/commonmark-ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java index 112764ba0..99214fcfd 100644 --- a/commonmark-ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java +++ b/commonmark-ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java @@ -1,26 +1,34 @@ package org.commonmark.ext.gfm.tables.internal; -import org.commonmark.ext.gfm.tables.*; +import org.commonmark.ext.gfm.tables.TableBlock; +import org.commonmark.ext.gfm.tables.TableBody; +import org.commonmark.ext.gfm.tables.TableCell; +import org.commonmark.ext.gfm.tables.TableHead; +import org.commonmark.ext.gfm.tables.TableRow; import org.commonmark.node.Block; import org.commonmark.node.Node; +import org.commonmark.node.SourceSpan; import org.commonmark.parser.InlineParser; -import org.commonmark.parser.block.*; +import org.commonmark.parser.block.AbstractBlockParser; +import org.commonmark.parser.block.AbstractBlockParserFactory; +import org.commonmark.parser.block.BlockContinue; +import org.commonmark.parser.block.BlockStart; +import org.commonmark.parser.block.MatchedBlockParser; +import org.commonmark.parser.block.ParserState; import java.util.ArrayList; +import java.util.Collections; import java.util.List; public class TableBlockParser extends AbstractBlockParser { private final TableBlock block = new TableBlock(); - private final List bodyLines = new ArrayList<>(); + private final List rowLines = new ArrayList<>(); private final List columns; - private final List headerCells; - private boolean nextIsSeparatorLine = true; - - private TableBlockParser(List columns, List headerCells) { + private TableBlockParser(List columns, CharSequence headerLine) { this.columns = columns; - this.headerCells = headerCells; + this.rowLines.add(headerLine); } @Override @@ -44,37 +52,45 @@ public BlockContinue tryContinue(ParserState state) { @Override public void addLine(CharSequence line) { - if (nextIsSeparatorLine) { - nextIsSeparatorLine = false; - } else { - bodyLines.add(line); - } + rowLines.add(line); } @Override public void parseInlines(InlineParser inlineParser) { - int headerColumns = headerCells.size(); + List sourceSpans = block.getSourceSpans(); + SourceSpan headerSourceSpan = !sourceSpans.isEmpty() ? sourceSpans.get(0) : null; Node head = new TableHead(); + if (headerSourceSpan != null) { + head.getSourceSpans().add(headerSourceSpan); + } block.appendChild(head); TableRow headerRow = new TableRow(); + headerRow.setSourceSpans(head.getSourceSpans()); head.appendChild(headerRow); + + List headerCells = split(rowLines.get(0), headerSourceSpan); + int headerColumns = headerCells.size(); for (int i = 0; i < headerColumns; i++) { - String cell = headerCells.get(i); + CellSource cell = headerCells.get(i); TableCell tableCell = parseCell(cell, i, inlineParser); tableCell.setHeader(true); headerRow.appendChild(tableCell); } - Node body = null; - for (CharSequence rowLine : bodyLines) { - List cells = split(rowLine); + TableBody body = null; + // Body starts at index 2. 0 is header, 1 is separator. + for (int rowIndex = 2; rowIndex < rowLines.size(); rowIndex++) { + CharSequence rowLine = rowLines.get(rowIndex); + SourceSpan sourceSpan = sourceSpans.get(rowIndex); + List cells = split(rowLine, sourceSpan); TableRow row = new TableRow(); + row.getSourceSpans().add(sourceSpan); // Body can not have more columns than head for (int i = 0; i < headerColumns; i++) { - String cell = i < cells.size() ? cells.get(i) : ""; + CellSource cell = i < cells.size() ? cells.get(i) : new CellSource("", null); TableCell tableCell = parseCell(cell, i, inlineParser); row.appendChild(tableCell); } @@ -85,33 +101,35 @@ public void parseInlines(InlineParser inlineParser) { block.appendChild(body); } body.appendChild(row); + body.getSourceSpans().add(sourceSpan); } } - private TableCell parseCell(String cell, int column, InlineParser inlineParser) { + private TableCell parseCell(CellSource cell, int column, InlineParser inlineParser) { TableCell tableCell = new TableCell(); if (column < columns.size()) { tableCell.setAlignment(columns.get(column)); } - inlineParser.parse(cell.trim(), tableCell); + if (cell.sourceSpan != null) { + tableCell.setSourceSpans(Collections.singletonList(cell.sourceSpan)); + } + + inlineParser.parse(cell.content, tableCell); return tableCell; } - private static List split(CharSequence input) { - String line = input.toString().trim(); - if (line.startsWith("|")) { - line = line.substring(1); - } - List cells = new ArrayList<>(); + private static List split(CharSequence row, SourceSpan rowSourceSpan) { + int cellStart = row.charAt(0) == '|' ? 1 : 0; + List cells = new ArrayList<>(); StringBuilder sb = new StringBuilder(); - for (int i = 0; i < line.length(); i++) { - char c = line.charAt(i); + for (int i = cellStart; i < row.length(); i++) { + char c = row.charAt(i); switch (c) { case '\\': - if (i + 1 < line.length() && line.charAt(i + 1) == '|') { + if (i + 1 < row.length() && row.charAt(i + 1) == '|') { // Pipe is special for table parsing. An escaped pipe doesn't result in a new cell, but is // passed down to inline parsing as an unescaped pipe. Note that that applies even for the `\|` // in an input like `\\|` - in other words, table parsing doesn't support escaping backslashes. @@ -123,15 +141,19 @@ private static List split(CharSequence input) { } break; case '|': - cells.add(sb.toString()); + String content = sb.toString(); + cells.add(CellSource.of(content, rowSourceSpan, cellStart)); sb.setLength(0); + // + 1 to skip the pipe itself for the next cell's span + cellStart = i + 1; break; default: sb.append(c); } } if (sb.length() > 0) { - cells.add(sb.toString()); + String content = sb.toString(); + cells.add(CellSource.of(content, rowSourceSpan, cellStart)); } return cells; } @@ -229,9 +251,9 @@ public BlockStart tryStart(ParserState state, MatchedBlockParser matchedBlockPar CharSequence separatorLine = line.subSequence(state.getIndex(), line.length()); List columns = parseSeparator(separatorLine); if (columns != null && !columns.isEmpty()) { - List headerCells = split(paragraph); + List headerCells = split(paragraph, null); if (columns.size() >= headerCells.size()) { - return BlockStart.of(new TableBlockParser(columns, headerCells)) + return BlockStart.of(new TableBlockParser(columns, paragraph)) .atIndex(state.getIndex()) .replaceActiveBlockParser(); } @@ -241,4 +263,26 @@ public BlockStart tryStart(ParserState state, MatchedBlockParser matchedBlockPar } } + private static class CellSource { + + private final String content; + private final SourceSpan sourceSpan; + + public static CellSource of(String content, SourceSpan rowSourceSpan, int offset) { + if (!content.isEmpty()) { + SourceSpan sourceSpan = null; + if (rowSourceSpan != null) { + sourceSpan = SourceSpan.of(rowSourceSpan.getLineIndex(), rowSourceSpan.getColumnIndex() + offset, content.length()); + } + return new CellSource(content, sourceSpan); + } else { + return new CellSource(content, null); + } + } + + private CellSource(String content, SourceSpan sourceSpan) { + this.content = content; + this.sourceSpan = sourceSpan; + } + } } diff --git a/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java b/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java index b38358d8b..4aca197b2 100644 --- a/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java +++ b/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java @@ -666,7 +666,7 @@ public void sourceSpans() { assertEquals(Arrays.asList(SourceSpan.of(0, 4, 3)), headRowCell2.getSourceSpans()); TableBody body = (TableBody) block.getLastChild(); - assertEquals(Arrays.asList(SourceSpan.of(2, 0, 4), SourceSpan.of(3, 1, 7), SourceSpan.of(4, 0, 3)), body.getSourceSpans()); + assertEquals(Arrays.asList(SourceSpan.of(2, 0, 4), SourceSpan.of(3, 0, 8), SourceSpan.of(4, 0, 3)), body.getSourceSpans()); TableRow bodyRow1 = (TableRow) body.getFirstChild(); assertEquals(Arrays.asList(SourceSpan.of(2, 0, 4)), bodyRow1.getSourceSpans()); @@ -676,10 +676,10 @@ public void sourceSpans() { assertEquals(Arrays.asList(SourceSpan.of(2, 3, 1)), bodyRow1Cell2.getSourceSpans()); TableRow bodyRow2 = (TableRow) body.getFirstChild().getNext(); - assertEquals(Arrays.asList(SourceSpan.of(3, 1, 7)), bodyRow2.getSourceSpans()); + assertEquals(Arrays.asList(SourceSpan.of(3, 0, 8)), bodyRow2.getSourceSpans()); TableCell bodyRow2Cell1 = (TableCell) bodyRow2.getFirstChild(); TableCell bodyRow2Cell2 = (TableCell) bodyRow2.getLastChild(); - assertEquals(Arrays.asList(SourceSpan.of(3, 1, 1)), bodyRow2Cell1.getSourceSpans()); + assertEquals(Arrays.asList(SourceSpan.of(3, 0, 2)), bodyRow2Cell1.getSourceSpans()); assertEquals(Arrays.asList(SourceSpan.of(3, 3, 4)), bodyRow2Cell2.getSourceSpans()); TableRow bodyRow3 = (TableRow) body.getLastChild(); From d766a568894682bc0774df6feeb745a549dd275a Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Mon, 7 Sep 2020 16:24:54 +1000 Subject: [PATCH 5/8] Return immutable collection from getSourceSpans Just better to not accidentally modify it. --- .../ext/gfm/tables/internal/TableBlockParser.java | 6 +++--- .../main/java/org/commonmark/internal/ParagraphParser.java | 2 +- commonmark/src/main/java/org/commonmark/node/Node.java | 7 ++++++- .../org/commonmark/parser/block/AbstractBlockParser.java | 2 +- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/commonmark-ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java b/commonmark-ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java index 99214fcfd..5434dc18c 100644 --- a/commonmark-ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java +++ b/commonmark-ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java @@ -62,7 +62,7 @@ public void parseInlines(InlineParser inlineParser) { SourceSpan headerSourceSpan = !sourceSpans.isEmpty() ? sourceSpans.get(0) : null; Node head = new TableHead(); if (headerSourceSpan != null) { - head.getSourceSpans().add(headerSourceSpan); + head.addSourceSpan(headerSourceSpan); } block.appendChild(head); @@ -86,7 +86,7 @@ public void parseInlines(InlineParser inlineParser) { SourceSpan sourceSpan = sourceSpans.get(rowIndex); List cells = split(rowLine, sourceSpan); TableRow row = new TableRow(); - row.getSourceSpans().add(sourceSpan); + row.addSourceSpan(sourceSpan); // Body can not have more columns than head for (int i = 0; i < headerColumns; i++) { @@ -101,7 +101,7 @@ public void parseInlines(InlineParser inlineParser) { block.appendChild(body); } body.appendChild(row); - body.getSourceSpans().add(sourceSpan); + body.addSourceSpan(sourceSpan); } } diff --git a/commonmark/src/main/java/org/commonmark/internal/ParagraphParser.java b/commonmark/src/main/java/org/commonmark/internal/ParagraphParser.java index 09cf9fdec..7e28cf0fb 100644 --- a/commonmark/src/main/java/org/commonmark/internal/ParagraphParser.java +++ b/commonmark/src/main/java/org/commonmark/internal/ParagraphParser.java @@ -52,7 +52,7 @@ public void closeBlock() { if (linkReferenceDefinitionParser.getParagraphContent().length() == 0) { block.unlink(); } else { - block.getSourceSpans().addAll(linkReferenceDefinitionParser.getParagraphSourceSpans()); + block.setSourceSpans(linkReferenceDefinitionParser.getParagraphSourceSpans()); } } diff --git a/commonmark/src/main/java/org/commonmark/node/Node.java b/commonmark/src/main/java/org/commonmark/node/Node.java index 462acc545..6235f84ee 100644 --- a/commonmark/src/main/java/org/commonmark/node/Node.java +++ b/commonmark/src/main/java/org/commonmark/node/Node.java @@ -1,6 +1,7 @@ package org.commonmark.node; import java.util.ArrayList; +import java.util.Collections; import java.util.List; public abstract class Node { @@ -35,13 +36,17 @@ public Node getParent() { } public List getSourceSpans() { - return sourceSpans; + return Collections.unmodifiableList(sourceSpans); } public void setSourceSpans(List sourceSpans) { this.sourceSpans = new ArrayList<>(sourceSpans); } + public void addSourceSpan(SourceSpan sourceSpan) { + this.sourceSpans.add(sourceSpan); + } + protected void setParent(Node parent) { this.parent = parent; } diff --git a/commonmark/src/main/java/org/commonmark/parser/block/AbstractBlockParser.java b/commonmark/src/main/java/org/commonmark/parser/block/AbstractBlockParser.java index c66873dd1..98fddd458 100644 --- a/commonmark/src/main/java/org/commonmark/parser/block/AbstractBlockParser.java +++ b/commonmark/src/main/java/org/commonmark/parser/block/AbstractBlockParser.java @@ -27,7 +27,7 @@ public void addLine(CharSequence line) { @Override public void addSourceSpan(SourceSpan sourceSpan) { - getBlock().getSourceSpans().add(sourceSpan); + getBlock().addSourceSpan(sourceSpan); } @Override From 8101aa1afd42525361629692c838c13db054d8bc Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Mon, 7 Sep 2020 20:14:36 +1000 Subject: [PATCH 6/8] Add more documentation, CHANGELOG --- CHANGELOG.md | 8 ++++ .../gfm/tables/internal/TableBlockParser.java | 2 +- .../commonmark/ext/gfm/tables/TablesTest.java | 7 ++- .../SourceSpanIntegrationTest.java | 25 ++++++++++ .../integration/SpecIntegrationTest.java | 8 ++-- .../commonmark/internal/DocumentParser.java | 21 ++++---- .../main/java/org/commonmark/node/Block.java | 3 ++ .../main/java/org/commonmark/node/Node.java | 48 ++++++++++++++----- .../java/org/commonmark/node/Paragraph.java | 3 ++ .../java/org/commonmark/node/SourceSpan.java | 21 +++++++- .../commonmark/parser/IncludeSourceSpans.java | 16 +++++++ .../java/org/commonmark/parser/Parser.java | 31 ++++++++++-- .../org/commonmark/test/SourceSpansTest.java | 3 +- 13 files changed, 164 insertions(+), 32 deletions(-) create mode 100644 commonmark-integration-test/src/test/java/org/commonmark/integration/SourceSpanIntegrationTest.java create mode 100644 commonmark/src/main/java/org/commonmark/parser/IncludeSourceSpans.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 64e160aac..d035037dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html), with the exception that 0.x versions can break between minor versions. +## Unreleased +### Added +- Support for including source spans on block nodes: + - Answer for "Where in the source input (line/column position and length) does this block come from?" + - Useful for things like editors that want to keep the input and rendered output scrolled to the same lines, + or start editing on the block that was selected. + - Use `includeSourceSpans` on `Parser.Builder` to enable, read data with `Node.getSourceSpans` + ## [0.15.2] - 2020-07-20 ### Fixed - image-attributes extension: Fix unexpected altering of text in case diff --git a/commonmark-ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java b/commonmark-ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java index 5434dc18c..2952a8785 100644 --- a/commonmark-ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java +++ b/commonmark-ext-gfm-tables/src/main/java/org/commonmark/ext/gfm/tables/internal/TableBlockParser.java @@ -83,7 +83,7 @@ public void parseInlines(InlineParser inlineParser) { // Body starts at index 2. 0 is header, 1 is separator. for (int rowIndex = 2; rowIndex < rowLines.size(); rowIndex++) { CharSequence rowLine = rowLines.get(rowIndex); - SourceSpan sourceSpan = sourceSpans.get(rowIndex); + SourceSpan sourceSpan = rowIndex < sourceSpans.size() ? sourceSpans.get(rowIndex) : null; List cells = split(rowLine, sourceSpan); TableRow row = new TableRow(); row.addSourceSpan(sourceSpan); diff --git a/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java b/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java index 4aca197b2..8af4a0ec7 100644 --- a/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java +++ b/commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesTest.java @@ -4,6 +4,7 @@ import org.commonmark.node.Node; import org.commonmark.node.SourceSpan; import org.commonmark.parser.Parser; +import org.commonmark.parser.IncludeSourceSpans; import org.commonmark.renderer.html.AttributeProvider; import org.commonmark.renderer.html.AttributeProviderContext; import org.commonmark.renderer.html.AttributeProviderFactory; @@ -648,7 +649,11 @@ public void setAttributes(Node node, String tagName, Map attribu @Test public void sourceSpans() { - Node document = PARSER.parse("Abc|Def\n---|---\n|1|2\n 3|four|\n|||\n"); + Parser parser = Parser.builder() + .extensions(EXTENSIONS) + .includeSourceSpans(IncludeSourceSpans.BLOCKS) + .build(); + Node document = parser.parse("Abc|Def\n---|---\n|1|2\n 3|four|\n|||\n"); TableBlock block = (TableBlock) document.getFirstChild(); assertEquals(Arrays.asList(SourceSpan.of(0, 0, 7), SourceSpan.of(1, 0, 7), diff --git a/commonmark-integration-test/src/test/java/org/commonmark/integration/SourceSpanIntegrationTest.java b/commonmark-integration-test/src/test/java/org/commonmark/integration/SourceSpanIntegrationTest.java new file mode 100644 index 000000000..6e51b0af5 --- /dev/null +++ b/commonmark-integration-test/src/test/java/org/commonmark/integration/SourceSpanIntegrationTest.java @@ -0,0 +1,25 @@ +package org.commonmark.integration; + +import org.commonmark.parser.IncludeSourceSpans; +import org.commonmark.parser.Parser; +import org.commonmark.testutil.example.Example; + +/** + * Spec and all extensions, with source spans enabed. + */ +public class SourceSpanIntegrationTest extends SpecIntegrationTest { + + protected static final Parser PARSER = Parser.builder() + .extensions(EXTENSIONS) + .includeSourceSpans(IncludeSourceSpans.BLOCKS) + .build(); + + public SourceSpanIntegrationTest(Example example) { + super(example); + } + + @Override + protected String render(String source) { + return RENDERER.render(PARSER.parse(source)); + } +} diff --git a/commonmark-integration-test/src/test/java/org/commonmark/integration/SpecIntegrationTest.java b/commonmark-integration-test/src/test/java/org/commonmark/integration/SpecIntegrationTest.java index ed2d22cde..13d5918b8 100644 --- a/commonmark-integration-test/src/test/java/org/commonmark/integration/SpecIntegrationTest.java +++ b/commonmark-integration-test/src/test/java/org/commonmark/integration/SpecIntegrationTest.java @@ -21,7 +21,7 @@ */ public class SpecIntegrationTest extends SpecTestCase { - private static final List EXTENSIONS = Arrays.asList( + protected static final List EXTENSIONS = Arrays.asList( AutolinkExtension.create(), ImageAttributesExtension.create(), InsExtension.create(), @@ -29,10 +29,10 @@ public class SpecIntegrationTest extends SpecTestCase { TablesExtension.create(), TaskListItemsExtension.create(), YamlFrontMatterExtension.create()); - private static final Parser PARSER = Parser.builder().extensions(EXTENSIONS).build(); + protected static final Parser PARSER = Parser.builder().extensions(EXTENSIONS).build(); // The spec says URL-escaping is optional, but the examples assume that it's enabled. - private static final HtmlRenderer RENDERER = HtmlRenderer.builder().extensions(EXTENSIONS).percentEncodeUrls(true).build(); - private static final Map OVERRIDDEN_EXAMPLES = getOverriddenExamples(); + protected static final HtmlRenderer RENDERER = HtmlRenderer.builder().extensions(EXTENSIONS).percentEncodeUrls(true).build(); + protected static final Map OVERRIDDEN_EXAMPLES = getOverriddenExamples(); public SpecIntegrationTest(Example example) { super(example); diff --git a/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java b/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java index 6287d42d2..51791d90b 100644 --- a/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java +++ b/commonmark/src/main/java/org/commonmark/internal/DocumentParser.java @@ -15,6 +15,7 @@ import org.commonmark.node.ThematicBreak; import org.commonmark.parser.InlineParser; import org.commonmark.parser.InlineParserFactory; +import org.commonmark.parser.IncludeSourceSpans; import org.commonmark.parser.block.BlockContinue; import org.commonmark.parser.block.BlockParser; import org.commonmark.parser.block.BlockParserFactory; @@ -92,6 +93,7 @@ public class DocumentParser implements ParserState { private final List blockParserFactories; private final InlineParserFactory inlineParserFactory; private final List delimiterProcessors; + private final IncludeSourceSpans includeSourceSpans; private final DocumentBlockParser documentBlockParser; private final Map definitions = new LinkedHashMap<>(); @@ -99,10 +101,11 @@ public class DocumentParser implements ParserState { private final List allBlockParsers = new ArrayList<>(); public DocumentParser(List blockParserFactories, InlineParserFactory inlineParserFactory, - List delimiterProcessors) { + List delimiterProcessors, IncludeSourceSpans includeSourceSpans) { this.blockParserFactories = blockParserFactories; this.inlineParserFactory = inlineParserFactory; this.delimiterProcessors = delimiterProcessors; + this.includeSourceSpans = includeSourceSpans; this.documentBlockParser = new DocumentBlockParser(); activateBlockParser(new OpenBlockParser(documentBlockParser, 0)); @@ -430,13 +433,15 @@ private void addLine() { } private void addSourceSpans() { - // Don't add source spans for Document itself (it would get the whole source text) - for (int i = 1; i < openBlockParsers.size(); i++) { - OpenBlockParser openBlockParser = openBlockParsers.get(i); - int blockIndex = openBlockParser.sourceIndex; - int length = line.length() - blockIndex; - if (length != 0) { - openBlockParser.blockParser.addSourceSpan(SourceSpan.of(lineIndex, blockIndex, length)); + if (includeSourceSpans != IncludeSourceSpans.NONE) { + // Don't add source spans for Document itself (it would get the whole source text) + for (int i = 1; i < openBlockParsers.size(); i++) { + OpenBlockParser openBlockParser = openBlockParsers.get(i); + int blockIndex = openBlockParser.sourceIndex; + int length = line.length() - blockIndex; + if (length != 0) { + openBlockParser.blockParser.addSourceSpan(SourceSpan.of(lineIndex, blockIndex, length)); + } } } } diff --git a/commonmark/src/main/java/org/commonmark/node/Block.java b/commonmark/src/main/java/org/commonmark/node/Block.java index e6a317d7c..753447c5c 100644 --- a/commonmark/src/main/java/org/commonmark/node/Block.java +++ b/commonmark/src/main/java/org/commonmark/node/Block.java @@ -1,5 +1,8 @@ package org.commonmark.node; +/** + * Block nodes such as paragraphs, list blocks, code blocks etc. + */ public abstract class Block extends Node { public Block getParent() { diff --git a/commonmark/src/main/java/org/commonmark/node/Node.java b/commonmark/src/main/java/org/commonmark/node/Node.java index 6235f84ee..f8a8cace7 100644 --- a/commonmark/src/main/java/org/commonmark/node/Node.java +++ b/commonmark/src/main/java/org/commonmark/node/Node.java @@ -4,6 +4,11 @@ import java.util.Collections; import java.util.List; +/** + * The base class of all CommonMark AST nodes ({@link Block} and inlines). + *

+ * A node can have multiple children, and a parent (except for the root node). + */ public abstract class Node { private Node parent = null; @@ -11,7 +16,7 @@ public abstract class Node { private Node lastChild = null; private Node prev = null; private Node next = null; - private List sourceSpans = new ArrayList<>(); + private List sourceSpans = null; public abstract void accept(Visitor visitor); @@ -35,18 +40,6 @@ public Node getParent() { return parent; } - public List getSourceSpans() { - return Collections.unmodifiableList(sourceSpans); - } - - public void setSourceSpans(List sourceSpans) { - this.sourceSpans = new ArrayList<>(sourceSpans); - } - - public void addSourceSpan(SourceSpan sourceSpan) { - this.sourceSpans.add(sourceSpan); - } - protected void setParent(Node parent) { this.parent = parent; } @@ -121,6 +114,35 @@ public void insertBefore(Node sibling) { } } + + /** + * @return the source spans of this node if included by the parser, an empty list otherwise + */ + public List getSourceSpans() { + return sourceSpans != null ? Collections.unmodifiableList(sourceSpans) : Collections.emptyList(); + } + + /** + * Replace the current source spans with the provided list. + * + * @param sourceSpans the new source spans to set + */ + public void setSourceSpans(List sourceSpans) { + this.sourceSpans = new ArrayList<>(sourceSpans); + } + + /** + * Add a source span to the end of the list. + * + * @param sourceSpan the source span to add + */ + public void addSourceSpan(SourceSpan sourceSpan) { + if (sourceSpans == null) { + this.sourceSpans = new ArrayList<>(); + } + this.sourceSpans.add(sourceSpan); + } + @Override public String toString() { return getClass().getSimpleName() + "{" + toStringAttributes() + "}"; diff --git a/commonmark/src/main/java/org/commonmark/node/Paragraph.java b/commonmark/src/main/java/org/commonmark/node/Paragraph.java index 0c3f88f39..176eaaa76 100644 --- a/commonmark/src/main/java/org/commonmark/node/Paragraph.java +++ b/commonmark/src/main/java/org/commonmark/node/Paragraph.java @@ -1,5 +1,8 @@ package org.commonmark.node; +/** + * A paragraph block, contains inline nodes such as {@link Text} + */ public class Paragraph extends Block { @Override diff --git a/commonmark/src/main/java/org/commonmark/node/SourceSpan.java b/commonmark/src/main/java/org/commonmark/node/SourceSpan.java index 3788f86f0..a643bd4dc 100644 --- a/commonmark/src/main/java/org/commonmark/node/SourceSpan.java +++ b/commonmark/src/main/java/org/commonmark/node/SourceSpan.java @@ -2,6 +2,25 @@ import java.util.Objects; +/** + * A source span references a snippet of text from the source input. + *

+ * It has a starting position (line and column index) and a length of how many characters it spans. + *

+ * For example, this CommonMark source text: + * 


+ * > foo
+ *
+ * The {@link BlockQuote} node would have this source span: line 0, column 0, length 5. + *

+ * The {@link Paragraph} node inside it would have: line 0, column 2, length 3. + *

+ * If a block has multiple lines, it will have a source span for each line. + *

+ * Note that the column index and length are measured in Java characters (UTF-16 code units). If you're outputting them + * to be consumed by another programming language, e.g. one that uses UTF-8 strings, you will need to translate them, + * otherwise characters such as emojis will result in incorrect positions. + */ public class SourceSpan { private final int lineIndex; @@ -33,7 +52,7 @@ public int getColumnIndex() { } /** - * @return length of the span + * @return length of the span in characters */ public int getLength() { return length; diff --git a/commonmark/src/main/java/org/commonmark/parser/IncludeSourceSpans.java b/commonmark/src/main/java/org/commonmark/parser/IncludeSourceSpans.java new file mode 100644 index 000000000..d6fc459eb --- /dev/null +++ b/commonmark/src/main/java/org/commonmark/parser/IncludeSourceSpans.java @@ -0,0 +1,16 @@ +package org.commonmark.parser; + +/** + * Whether to include {@link org.commonmark.node.SourceSpan} or not while parsing, + * see {@link Parser.Builder#includeSourceSpans(IncludeSourceSpans)}. + */ +public enum IncludeSourceSpans { + /** + * Do not include source spans. + */ + NONE, + /** + * Include source spans on {@link org.commonmark.node.Block} nodes. + */ + BLOCKS, +} diff --git a/commonmark/src/main/java/org/commonmark/parser/Parser.java b/commonmark/src/main/java/org/commonmark/parser/Parser.java index 5e15158ad..a4d5c8531 100644 --- a/commonmark/src/main/java/org/commonmark/parser/Parser.java +++ b/commonmark/src/main/java/org/commonmark/parser/Parser.java @@ -4,7 +4,16 @@ import org.commonmark.internal.DocumentParser; import org.commonmark.internal.InlineParserContextImpl; import org.commonmark.internal.InlineParserImpl; -import org.commonmark.node.*; +import org.commonmark.node.Block; +import org.commonmark.node.BlockQuote; +import org.commonmark.node.FencedCodeBlock; +import org.commonmark.node.Heading; +import org.commonmark.node.HtmlBlock; +import org.commonmark.node.IndentedCodeBlock; +import org.commonmark.node.LinkReferenceDefinition; +import org.commonmark.node.ListBlock; +import org.commonmark.node.Node; +import org.commonmark.node.ThematicBreak; import org.commonmark.parser.block.BlockParserFactory; import org.commonmark.parser.delimiter.DelimiterProcessor; @@ -31,12 +40,14 @@ public class Parser { private final List delimiterProcessors; private final InlineParserFactory inlineParserFactory; private final List postProcessors; + private final IncludeSourceSpans includeSourceSpans; private Parser(Builder builder) { this.blockParserFactories = DocumentParser.calculateBlockParserFactories(builder.blockParserFactories, builder.enabledBlockTypes); this.inlineParserFactory = builder.getInlineParserFactory(); this.postProcessors = builder.postProcessors; this.delimiterProcessors = builder.delimiterProcessors; + this.includeSourceSpans = builder.includeSourceSpans; // Try to construct an inline parser. Invalid configuration might result in an exception, which we want to // detect as soon as possible. @@ -99,7 +110,7 @@ public Node parseReader(Reader input) throws IOException { } private DocumentParser createDocumentParser() { - return new DocumentParser(blockParserFactories, inlineParserFactory, delimiterProcessors); + return new DocumentParser(blockParserFactories, inlineParserFactory, delimiterProcessors, includeSourceSpans); } private Node postProcess(Node document) { @@ -118,6 +129,7 @@ public static class Builder { private final List postProcessors = new ArrayList<>(); private Set> enabledBlockTypes = DocumentParser.getDefaultBlockParserTypes(); private InlineParserFactory inlineParserFactory; + private IncludeSourceSpans includeSourceSpans = IncludeSourceSpans.NONE; /** * @return the configured {@link Parser} @@ -167,7 +179,7 @@ public Builder extensions(Iterable extensions) { * * * @param enabledBlockTypes A list of block nodes the parser will parse. - * If this list is empty, the parser will not recognize any CommonMark core features. + * If this list is empty, the parser will not recognize any CommonMark core features. * @return {@code this} */ public Builder enabledBlockTypes(Set> enabledBlockTypes) { @@ -178,6 +190,19 @@ public Builder enabledBlockTypes(Set> enabledBlockTypes) return this; } + /** + * Whether to calculate {@link org.commonmark.node.SourceSpan} for {@link Node}. + *

+ * By default, source spans are disabled. + * + * @param includeSourceSpans which kind of source spans should be included + * @return {@code this} + */ + public Builder includeSourceSpans(IncludeSourceSpans includeSourceSpans) { + this.includeSourceSpans = includeSourceSpans; + return this; + } + /** * Adds a custom block parser factory. *

diff --git a/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java b/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java index 3e2de2be2..f6796c925 100644 --- a/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java +++ b/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java @@ -13,6 +13,7 @@ import org.commonmark.node.SourceSpan; import org.commonmark.node.ThematicBreak; import org.commonmark.parser.Parser; +import org.commonmark.parser.IncludeSourceSpans; import org.junit.Test; import java.util.Arrays; @@ -22,7 +23,7 @@ public class SourceSpansTest { - private static final Parser PARSER = Parser.builder().build(); + private static final Parser PARSER = Parser.builder().includeSourceSpans(IncludeSourceSpans.BLOCKS).build(); @Test public void paragraph() { From db6aa0b802fb2101105a0565dcd0af0b7f29c286 Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Mon, 7 Sep 2020 21:22:52 +1000 Subject: [PATCH 7/8] Remove unnecessary dependency --- commonmark-test-util/pom.xml | 7 ++----- .../test/java/org/commonmark/test/SourceSpansTest.java | 10 +++++----- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/commonmark-test-util/pom.xml b/commonmark-test-util/pom.xml index 92046f159..cad82f51d 100644 --- a/commonmark-test-util/pom.xml +++ b/commonmark-test-util/pom.xml @@ -1,5 +1,6 @@ - + 4.0.0 com.atlassian.commonmark @@ -16,10 +17,6 @@ junit junit - - org.hamcrest - hamcrest-library - diff --git a/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java b/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java index f6796c925..9fe1fac03 100644 --- a/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java +++ b/commonmark/src/test/java/org/commonmark/test/SourceSpansTest.java @@ -12,14 +12,14 @@ import org.commonmark.node.Paragraph; import org.commonmark.node.SourceSpan; import org.commonmark.node.ThematicBreak; -import org.commonmark.parser.Parser; import org.commonmark.parser.IncludeSourceSpans; +import org.commonmark.parser.Parser; import org.junit.Test; import java.util.Arrays; -import static org.hamcrest.Matchers.contains; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; public class SourceSpansTest { @@ -99,7 +99,7 @@ public void fencedCodeBlock() { Node document = PARSER.parse("```\nfoo\n```\nbar\n"); Paragraph paragraph = (Paragraph) document.getLastChild(); - assertThat(paragraph.getSourceSpans(), contains(SourceSpan.of(3, 0, 3))); + assertEquals(Arrays.asList(SourceSpan.of(3, 0, 3)), paragraph.getSourceSpans()); } @Test @@ -142,7 +142,7 @@ public void listBlock() { Node document = PARSER.parse("* foo\n * bar\n"); ListBlock listBlock = (ListBlock) document.getFirstChild().getFirstChild().getLastChild(); - assertThat(listBlock.getSourceSpans(), contains(SourceSpan.of(1, 2, 5))); + assertEquals(Arrays.asList(SourceSpan.of(1, 2, 5)), listBlock.getSourceSpans()); } @Test From ccafb953f8fe8c12aa023de70178ab6a8c8e3572 Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Tue, 8 Sep 2020 10:49:17 +1000 Subject: [PATCH 8/8] Remove unused dependencyManagement version --- pom.xml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pom.xml b/pom.xml index 2bc6e3e40..d81dc186a 100644 --- a/pom.xml +++ b/pom.xml @@ -143,11 +143,6 @@ junit 4.12 - - org.hamcrest - hamcrest-library - 1.3 - org.openjdk.jmh jmh-core