ext-tasklist, changed task list parser implementation

CHANGELOG.md

@@ -6,12 +6,16 @@
 * `ext-tables` - `TableAwareMovementMethod` a special movement method to handle clicks inside tables ([#289])
 
 #### Changed
+* `ext-tasklist` - changed implementation to be in line with GFM (Github flavored markdown), 
+  task list item is a regular list item (BulletList and OrderedList can contain it). 
+  Internal implementation changed from block parsing to node post processing ([#291])
 * `image-glide` - update to `4.11.0` version
 * `inline-parser` - revert parsing index when `InlineProcessor` returns `null` as result
 * `image-coil` - update `Coil` to `0.12.0` ([Coil changelog](https://coil-kt.github.io/coil/changelog/)) ([#284])<br>Thanks [@magnusvs] 
 
 [#284]: https://github.com/noties/Markwon/pull/284
 [#289]: https://github.com/noties/Markwon/issues/289
+[#291]: https://github.com/noties/Markwon/issues/291
 
 [@magnusvs]: https://github.com/magnusvs
 

app-sample/samples.json

@@ -1,4 +1,16 @@
 [
+  {
+    "javaClassName": "io.noties.markwon.app.samples.tasklist.ListTaskListSample",
+    "id": "20200902174132",
+    "title": "Task list items with other lists",
+    "description": "Mix of task list items with other lists (bullet and ordered)",
+    "artifacts": [
+      "EXT_TASKLIST"
+    ],
+    "tags": [
+      "lists"
+    ]
+  },
   {
     "javaClassName": "io.noties.markwon.app.samples.DeeplinksSample",
     "id": "20200826122247",

app-sample/src/main/java/io/noties/markwon/app/samples/tasklist/ListTaskListSample.java

@@ -0,0 +1,38 @@
+package io.noties.markwon.app.samples.tasklist;
+
+import io.noties.markwon.Markwon;
+import io.noties.markwon.app.sample.Tags;
+import io.noties.markwon.app.sample.ui.MarkwonTextViewSample;
+import io.noties.markwon.ext.tasklist.TaskListPlugin;
+import io.noties.markwon.sample.annotations.MarkwonArtifact;
+import io.noties.markwon.sample.annotations.MarkwonSampleInfo;
+
+@MarkwonSampleInfo(
+  id = "20200902174132",
+  title = "Task list items with other lists",
+  description = "Mix of task list items with other lists (bullet and ordered)",
+  artifacts = MarkwonArtifact.EXT_TASKLIST,
+  tags = Tags.lists
+)
+public class ListTaskListSample extends MarkwonTextViewSample {
+  @Override
+  public void render() {
+    final String md = "" +
+      "- [ ] Task **1**\n" +
+      "- [ ] _Task_ 2\n" +
+      "- [ ] Task 3\n" +
+      "  - Sub Task 3.1\n" +
+      "  - Sub Task 3.2\n" +
+      "    * [X] Sub Task 4.1\n" +
+      "    * [X] Sub Task 4.2\n" +
+      "- [ ] Task 4\n" +
+      "  - [ ] Sub Task 3.1\n" +
+      "  - [ ] Sub Task 3.2";
+
+    final Markwon markwon = Markwon.builder(context)
+      .usePlugin(TaskListPlugin.create(context))
+      .build();
+
+    markwon.setMarkdown(textView, md);
+  }
+}

markwon-core/src/main/java/io/noties/markwon/utils/DumpNodes.java

@@ -1,9 +1,9 @@
 package io.noties.markwon.utils;
 
+import androidx.annotation.CheckResult;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
-import org.commonmark.node.Block;
 import org.commonmark.node.Node;
 import org.commonmark.node.Visitor;
 
@@ -15,17 +15,22 @@ import java.lang.reflect.Proxy;
 @SuppressWarnings({"unused", "WeakerAccess"})
 public abstract class DumpNodes {
 
+    /**
+     * Creates String representation of a node which will be used in output
+     */
     public interface NodeProcessor {
         @NonNull
         String process(@NonNull Node node);
     }
 
     @NonNull
+    @CheckResult
     public static String dump(@NonNull Node node) {
         return dump(node, null);
     }
 
     @NonNull
+    @CheckResult
     public static String dump(@NonNull Node node, @Nullable NodeProcessor nodeProcessor) {
 
         final NodeProcessor processor = nodeProcessor != null
@@ -49,7 +54,9 @@ public abstract class DumpNodes {
                         // node info
                         builder.append(processor.process(argument));
 
-                        if (argument instanceof Block) {
+                        // @since $SNAPSHOT; check for first child instead of casting to Block
+                        //  (regular nodes can contain other nodes, for example Text)
+                        if (argument.getFirstChild() != null) {
                             builder.append(" [\n");
                             indent.increment();
                             visitChildren((Visitor) proxy, argument);
@@ -57,8 +64,9 @@ public abstract class DumpNodes {
                             indent.appendTo(builder);
                             builder.append("]\n");
                         } else {
-                            builder.append('\n');
+                            builder.append("\n");
                         }
+
                         return null;
                     }
                 });

markwon-core/src/main/java/io/noties/markwon/utils/ParserUtils.java

@@ -0,0 +1,25 @@
+package io.noties.markwon.utils;
+
+import androidx.annotation.NonNull;
+
+import org.commonmark.node.Node;
+
+/**
+ * @since $SNAPSHOT;
+ */
+public abstract class ParserUtils {
+
+    public static void moveChildren(@NonNull Node to, @NonNull Node from) {
+        Node next = from.getNext();
+        Node temp;
+        while (next != null) {
+            // appendChild would unlink passed node (thus making next info un-available)
+            temp = next.getNext();
+            to.appendChild(next);
+            next = temp;
+        }
+    }
+
+    private ParserUtils() {
+    }
+}

markwon-ext-tasklist/src/main/java/io/noties/markwon/ext/tasklist/TaskListBlock.java

@@ -1,9 +0,0 @@
-package io.noties.markwon.ext.tasklist;
-
-import org.commonmark.node.CustomBlock;
-
-/**
- * @since 1.0.1
- */
-public class TaskListBlock extends CustomBlock {
-}

markwon-ext-tasklist/src/main/java/io/noties/markwon/ext/tasklist/TaskListBlockParser.java

@@ -1,152 +0,0 @@
-package io.noties.markwon.ext.tasklist;
-
-import androidx.annotation.NonNull;
-import androidx.annotation.Nullable;
-
-import org.commonmark.node.Block;
-import org.commonmark.parser.InlineParser;
-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.List;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
-/**
- * @since 1.0.1
- */
-@SuppressWarnings("WeakerAccess")
-class TaskListBlockParser extends AbstractBlockParser {
-
-    private static final Pattern PATTERN = Pattern.compile("\\s*([-*+]|\\d{1,9}[.)])\\s+\\[(x|X|\\s)]\\s+(.*)");
-
-    private final TaskListBlock block = new TaskListBlock();
-
-    private final List<Item> items = new ArrayList<>(3);
-
-    private int indent = 0;
-
-    TaskListBlockParser(@NonNull String startLine, int startIndent) {
-        items.add(new Item(startLine, startIndent));
-        indent = startIndent;
-    }
-
-    @Override
-    public Block getBlock() {
-        return block;
-    }
-
-    @Override
-    public BlockContinue tryContinue(ParserState parserState) {
-
-        final BlockContinue blockContinue;
-
-        final String line = line(parserState);
-
-        final int currentIndent = parserState.getIndent();
-        if (currentIndent > indent) {
-            indent += 2;
-        } else if (currentIndent < indent && indent > 1) {
-            indent -= 2;
-        }
-
-        if (line != null
-                && line.length() > 0
-                && PATTERN.matcher(line).matches()) {
-            blockContinue = BlockContinue.atIndex(parserState.getIndex());
-        } else {
-            // @since 2.0.0, previously called `BlockContinue.finished()`
-            //  that was swallowing non-matching lines
-            blockContinue = BlockContinue.none();
-        }
-
-        return blockContinue;
-    }
-
-    @Override
-    public void addLine(CharSequence line) {
-        if (length(line) > 0) {
-            items.add(new Item(line.toString(), indent));
-        }
-    }
-
-    @Override
-    public void parseInlines(InlineParser inlineParser) {
-
-        Matcher matcher;
-
-        TaskListItem listItem;
-
-        for (Item item : items) {
-            matcher = PATTERN.matcher(item.line);
-            if (!matcher.matches()) {
-                continue;
-            }
-            listItem = new TaskListItem()
-                    .done(isDone(matcher.group(2)))
-                    .indent(item.indent / 2);
-            inlineParser.parse(matcher.group(3), listItem);
-            block.appendChild(listItem);
-        }
-    }
-
-    static class Factory extends AbstractBlockParserFactory {
-
-        @Override
-        public BlockStart tryStart(ParserState state, MatchedBlockParser matchedBlockParser) {
-
-            final String line = line(state);
-
-            if (line != null
-                    && line.length() > 0
-                    && PATTERN.matcher(line).matches()) {
-
-                final int length = line.length();
-                final int index = state.getIndex();
-                final int atIndex = index != 0
-                        ? index + (length - index)
-                        : length;
-
-                return BlockStart.of(new TaskListBlockParser(line, state.getIndent()))
-                        .atIndex(atIndex);
-            }
-
-            return BlockStart.none();
-        }
-    }
-
-    @Nullable
-    private static String line(@NonNull ParserState state) {
-        final CharSequence lineRaw = state.getLine();
-        return lineRaw != null
-                ? lineRaw.toString()
-                : null;
-    }
-
-    private static int length(@Nullable CharSequence text) {
-        return text != null
-                ? text.length()
-                : 0;
-    }
-
-    private static boolean isDone(@NonNull String value) {
-        return "X".equals(value)
-                || "x".equals(value);
-    }
-
-    private static class Item {
-
-        final String line;
-        final int indent;
-
-        Item(@NonNull String line, int indent) {
-            this.line = line;
-            this.indent = indent;
-        }
-    }
-}

markwon-ext-tasklist/src/main/java/io/noties/markwon/ext/tasklist/TaskListItem.java

@@ -1,31 +1,30 @@
 package io.noties.markwon.ext.tasklist;
 
-import org.commonmark.node.CustomNode;
+import androidx.annotation.NonNull;
+
+import org.commonmark.node.CustomBlock;
 
 /**
  * @since 1.0.1
  */
 @SuppressWarnings("WeakerAccess")
-public class TaskListItem extends CustomNode {
+public class TaskListItem extends CustomBlock {
 
-    private boolean done;
+    private final boolean isDone;
-    private int indent;
 
-    public boolean done() {
+    public TaskListItem(boolean isDone) {
-        return done;
+        this.isDone = isDone;
     }
 
-    public TaskListItem done(boolean done) {
+    public boolean isDone() {
-        this.done = done;
+        return isDone;
-        return this;
     }
 
-    public int indent() {
+    @Override
-        return indent;
+    @NonNull
-    }
+    public String toString() {
-
+        return "TaskListItem{" +
-    public TaskListItem indent(int indent) {
+                "isDone=" + isDone +
-        this.indent = indent;
+                '}';
-        return this;
     }
 }

markwon-ext-tasklist/src/main/java/io/noties/markwon/ext/tasklist/TaskListPlugin.java

@@ -9,13 +9,11 @@ import androidx.annotation.AttrRes;
 import androidx.annotation.ColorInt;
 import androidx.annotation.NonNull;
 
-import org.commonmark.node.Node;
 import org.commonmark.parser.Parser;
 
 import io.noties.markwon.AbstractMarkwonPlugin;
 import io.noties.markwon.MarkwonSpansFactory;
 import io.noties.markwon.MarkwonVisitor;
-import io.noties.markwon.RenderProps;
 import io.noties.markwon.core.SimpleBlockNodeVisitor;
 
 /**
@@ -66,7 +64,7 @@ public class TaskListPlugin extends AbstractMarkwonPlugin {
 
     @Override
     public void configureParser(@NonNull Parser.Builder builder) {
-        builder.customBlockParserFactory(new TaskListBlockParser.Factory());
+        builder.postProcessor(new TaskListPostProcessor());
     }
 
     @Override
@@ -77,7 +75,6 @@ public class TaskListPlugin extends AbstractMarkwonPlugin {
     @Override
     public void configureVisitor(@NonNull MarkwonVisitor.Builder builder) {
         builder
-                .on(TaskListBlock.class, new SimpleBlockNodeVisitor())
                 .on(TaskListItem.class, new MarkwonVisitor.NodeVisitor<TaskListItem>() {
                     @Override
                     public void visit(@NonNull MarkwonVisitor visitor, @NonNull TaskListItem taskListItem) {
@@ -86,10 +83,7 @@ public class TaskListPlugin extends AbstractMarkwonPlugin {
 
                         visitor.visitChildren(taskListItem);
 
-                        final RenderProps context = visitor.renderProps();
+                        TaskListProps.DONE.set(visitor.renderProps(), taskListItem.isDone());
-
-                        TaskListProps.BLOCK_INDENT.set(context, indent(taskListItem) + taskListItem.indent());
-                        TaskListProps.DONE.set(context, taskListItem.done());
 
                         visitor.setSpansForNode(taskListItem, length);
 
@@ -110,17 +104,4 @@ public class TaskListPlugin extends AbstractMarkwonPlugin {
             typedArray.recycle();
         }
     }
-
-    private static int indent(@NonNull Node node) {
-        int indent = 0;
-        Node parent = node.getParent();
-        if (parent != null) {
-            parent = parent.getParent();
-            while (parent != null) {
-                indent += 1;
-                parent = parent.getParent();
-            }
-        }
-        return indent;
-    }
 }

markwon-ext-tasklist/src/main/java/io/noties/markwon/ext/tasklist/TaskListPostProcessor.java

@@ -0,0 +1,82 @@
+package io.noties.markwon.ext.tasklist;
+
+import android.text.TextUtils;
+
+import org.commonmark.node.AbstractVisitor;
+import org.commonmark.node.ListItem;
+import org.commonmark.node.Node;
+import org.commonmark.node.Paragraph;
+import org.commonmark.node.Text;
+import org.commonmark.parser.PostProcessor;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import io.noties.markwon.utils.ParserUtils;
+
+// @since $SNAPSHOT;
+// Hint taken from commonmark-ext-task-list-items artifact
+class TaskListPostProcessor implements PostProcessor {
+
+    @Override
+    public Node process(Node node) {
+        final TaskListVisitor visitor = new TaskListVisitor();
+        node.accept(visitor);
+        return node;
+    }
+
+    private static class TaskListVisitor extends AbstractVisitor {
+
+        private static final Pattern REGEX_TASK_LIST_ITEM = Pattern.compile("^\\[([xX\\s])]\\s+(.*)");
+
+        @Override
+        public void visit(ListItem listItem) {
+            // Takes first child and checks if it is Text (we are looking for exact `[xX\s]` without any formatting)
+            final Node child = listItem.getFirstChild();
+            // check if it is paragraph (can contain text)
+            if (child instanceof Paragraph) {
+                final Node node = child.getFirstChild();
+                if (node instanceof Text) {
+
+                    final Text textNode = (Text) node;
+                    final Matcher matcher = REGEX_TASK_LIST_ITEM.matcher(textNode.getLiteral());
+
+                    if (matcher.matches()) {
+                        final String checked = matcher.group(1);
+                        final boolean isChecked = "x".equals(checked) || "X".equals(checked);
+
+                        final TaskListItem taskListItem = new TaskListItem(isChecked);
+
+                        final Paragraph paragraph = new Paragraph();
+
+                        // insert before list item (directly before inside parent)
+                        listItem.insertBefore(taskListItem);
+
+                        // append the rest of matched text (can be empty)
+                        final String restMatchedText = matcher.group(2);
+                        if (!TextUtils.isEmpty(restMatchedText)) {
+                            paragraph.appendChild(new Text(restMatchedText));
+                        }
+
+                        // move all the rest children (from the first paragraph)
+                        ParserUtils.moveChildren(paragraph, node);
+
+                        // append our created paragraph
+                        taskListItem.appendChild(paragraph);
+
+                        // move all the rest children from the listItem (further nested lists, etc)
+                        ParserUtils.moveChildren(taskListItem, child);
+
+                        // remove list item from node
+                        listItem.unlink();
+
+                        // visit taskListItem children
+                        visitChildren(taskListItem);
+                        return;
+                    }
+                }
+            }
+            visitChildren(listItem);
+        }
+    }
+}

markwon-ext-tasklist/src/main/java/io/noties/markwon/ext/tasklist/TaskListProps.java

@@ -7,8 +7,6 @@ import io.noties.markwon.Prop;
  */
 public abstract class TaskListProps {
 
-    public static final Prop<Integer> BLOCK_INDENT = Prop.of("task-list-block-indent");
-
     public static final Prop<Boolean> DONE = Prop.of("task-list-done");
 
     private TaskListProps() {

markwon-ext-tasklist/src/main/java/io/noties/markwon/ext/tasklist/TaskListSpan.java

@@ -22,16 +22,13 @@ public class TaskListSpan implements LeadingMarginSpan {
 
     private final MarkwonTheme theme;
     private final Drawable drawable;
-    private final int blockIndent;
 
     // @since 2.0.1 field is NOT final (to allow mutation)
     private boolean isDone;
 
-
+    public TaskListSpan(@NonNull MarkwonTheme theme, @NonNull Drawable drawable, boolean isDone) {
-    public TaskListSpan(@NonNull MarkwonTheme theme, @NonNull Drawable drawable, int blockIndent, boolean isDone) {
         this.theme = theme;
         this.drawable = drawable;
-        this.blockIndent = blockIndent;
         this.isDone = isDone;
     }
 
@@ -54,7 +51,7 @@ public class TaskListSpan implements LeadingMarginSpan {
 
     @Override
     public int getLeadingMargin(boolean first) {
-        return theme.getBlockMargin() * blockIndent;
+        return theme.getBlockMargin();
     }
 
     @Override
@@ -65,11 +62,14 @@ public class TaskListSpan implements LeadingMarginSpan {
             return;
         }
 
+        final float descent = p.descent();
+        final float ascent = p.ascent();
+
         final int save = c.save();
         try {
 
             final int width = theme.getBlockMargin();
-            final int height = bottom - top;
+            final int height = (int) (descent - ascent + 0.5F);
 
             final int w = (int) (width * .75F + .5F);
             final int h = (int) (height * .75F + .5F);
@@ -88,12 +88,12 @@ public class TaskListSpan implements LeadingMarginSpan {
 
             final int l;
             if (dir > 0) {
-                l = x + (width * (blockIndent - 1)) + ((width - w) / 2);
+                l = x + ((width - w) / 2);
             } else {
-                l = x - (width * blockIndent) + ((width - w) / 2);
+                l = x - ((width - w) / 2) - w;
             }
 
-            final int t = top + ((height - h) / 2);
+            final int t = (int) (baseline + ascent + 0.5F) + ((height - h) / 2);
 
             c.translate(l, t);
             drawable.draw(c);

markwon-ext-tasklist/src/main/java/io/noties/markwon/ext/tasklist/TaskListSpanFactory.java

@@ -23,7 +23,6 @@ public class TaskListSpanFactory implements SpanFactory {
         return new TaskListSpan(
                 configuration.theme(),
                 drawable,
-                TaskListProps.BLOCK_INDENT.get(props, 0),
                 TaskListProps.DONE.get(props, false)
         );
     }

markwon-ext-tasklist/src/test/java/io/noties/markwon/ext/tasklist/TaskListTest.java

@@ -21,8 +21,6 @@ import io.noties.markwon.SpanFactory;
 import io.noties.markwon.test.TestSpan;
 import io.noties.markwon.test.TestSpanMatcher;
 
-import static io.noties.markwon.test.TestSpan.span;
-
 @RunWith(RobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
 public class TaskListTest {
@@ -33,6 +31,9 @@ public class TaskListTest {
     @Test
     public void test() {
 
+        // NB! different markers lead to different types of lists,
+        //  that's why there are 2 new lines after each type
+
         final TestSpan.Document document = TestSpan.document(
                 TestSpan.span(SPAN, TestSpan.args(IS_DONE, false), TestSpan.text("First")),
                 newLine(),
@@ -40,20 +41,24 @@ public class TaskListTest {
                 newLine(),
                 TestSpan.span(SPAN, TestSpan.args(IS_DONE, true), TestSpan.text("Third")),
                 newLine(),
+                newLine(),
                 TestSpan.span(SPAN, TestSpan.args(IS_DONE, false), TestSpan.text("First star")),
                 newLine(),
                 TestSpan.span(SPAN, TestSpan.args(IS_DONE, true), TestSpan.text("Second star")),
                 newLine(),
                 TestSpan.span(SPAN, TestSpan.args(IS_DONE, true), TestSpan.text("Third star")),
                 newLine(),
+                newLine(),
                 TestSpan.span(SPAN, TestSpan.args(IS_DONE, false), TestSpan.text("First plus")),
                 newLine(),
                 TestSpan.span(SPAN, TestSpan.args(IS_DONE, true), TestSpan.text("Second plus")),
                 newLine(),
                 TestSpan.span(SPAN, TestSpan.args(IS_DONE, true), TestSpan.text("Third plus")),
                 newLine(),
+                newLine(),
                 TestSpan.span(SPAN, TestSpan.args(IS_DONE, true), TestSpan.text("Number with dot")),
                 newLine(),
+                newLine(),
                 TestSpan.span(SPAN, TestSpan.args(IS_DONE, false), TestSpan.text("Number"))
         );