Parse spoilers into nodes (#1150)

Implementation is inspired by already existing in Markwon image and link processing
but has to work around some limitations of writing an external plugin.

The first one is storing brackets ourselves. Stored brackets need to be cleared
when a new block starts. Markwon does it in MarkwonInlineProcessor but there is
no callback that we could use. Clearing storage from our own block parser is
unreliable as it is not guaranteed to be called. Instead, every time we need to
access the storage we compare current block with the last used block and clear
storage if necessary.

The second problem is actually a feature of Markwon that it applies spans in
reverse order of calls to MarkwonVisitor#setSpansForNode. This causes other spans
like links and code to be drawn over spoilers making them visible. Adding spans
with a different priority doesn't help as it would require negative priority.
Instead we just remove all the SpoilerSpans from the final string and add them
again, so they are applied last as we want.

8 files changed, 240 insertions, 188 deletions

diff --git a/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerClosingInlineProcessor.java b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerClosingInlineProcessor.java
new file mode 100644
index 00000000..e39dd6c9
--- /dev/null
+++ b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerClosingInlineProcessor.java
@@ -0,0 +1,61 @@
+package ml.docilealligator.infinityforreddit.markdown;
+
+import static io.noties.markwon.inlineparser.InlineParserUtils.mergeChildTextNodes;
+
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
+
+import org.commonmark.node.Node;
+
+import io.noties.markwon.inlineparser.InlineProcessor;
+
+/**
+ * Parses spoiler closing markdown ({@code !<}) and creates {@link SpoilerNode SpoilerNodes}.
+ * Relies on {@link SpoilerOpeningInlineProcessor} to handle opening.
+ *
+ * Implementation inspired by {@link io.noties.markwon.inlineparser.CloseBracketInlineProcessor}
+ */
+public class SpoilerClosingInlineProcessor extends InlineProcessor {
+    @NonNull
+    private final SpoilerOpeningBracketStorage spoilerOpeningBracketStorage;
+
+    public SpoilerClosingInlineProcessor(@NonNull SpoilerOpeningBracketStorage spoilerOpeningBracketStorage) {
+        this.spoilerOpeningBracketStorage = spoilerOpeningBracketStorage;
+    }
+
+    @Override
+    public char specialCharacter() {
+        return '!';
+    }
+
+    @Nullable
+    @Override
+    protected Node parse() {
+        index++;
+        if (peek() != '<') {
+            return null;
+        }
+        index++;
+
+        SpoilerOpeningBracket spoilerOpeningBracket = spoilerOpeningBracketStorage.pop(block);
+        if (spoilerOpeningBracket == null) {
+            return null;
+        }
+
+        SpoilerNode spoilerNode = new SpoilerNode();
+        Node node = spoilerOpeningBracket.node.getNext();
+        while (node != null) {
+            Node next = node.getNext();
+            spoilerNode.appendChild(node);
+            node = next;
+        }
+
+        // Process delimiters such as emphasis inside spoiler
+        processDelimiters(spoilerOpeningBracket.previousDelimiter);
+        mergeChildTextNodes(spoilerNode);
+        // We don't need the corresponding text node anymore, we turned it into a spoiler node
+        spoilerOpeningBracket.node.unlink();
+
+        return spoilerNode;
+    }
+}
diff --git a/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerNode.java b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerNode.java
new file mode 100644
index 00000000..2f1d9774
--- /dev/null
+++ b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerNode.java
@@ -0,0 +1,6 @@
+package ml.docilealligator.infinityforreddit.markdown;
+
+import org.commonmark.node.CustomNode;
+
+public class SpoilerNode extends CustomNode {
+}
diff --git a/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpening.java b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpening.java
deleted file mode 100644
index 279cba65..00000000
--- a/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpening.java
+++ /dev/null
@@ -1,21 +0,0 @@
-package ml.docilealligator.infinityforreddit.markdown;
-
-import org.commonmark.node.CustomNode;
-import org.commonmark.node.Visitor;
-
-class SpoilerOpening extends CustomNode {
-    private String literal;
-
-    @Override
-    public void accept(Visitor visitor) {
-        visitor.visit(this);
-    }
-
-    public String getLiteral() {
-        return literal;
-    }
-
-    public void setLiteral(String literal) {
-        this.literal = literal;
-    }
-}
diff --git a/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpeningBracket.java b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpeningBracket.java
new file mode 100644
index 00000000..64995255
--- /dev/null
+++ b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpeningBracket.java
@@ -0,0 +1,27 @@
+package ml.docilealligator.infinityforreddit.markdown;
+
+import org.commonmark.internal.Delimiter;
+import org.commonmark.node.Node;
+
+public class SpoilerOpeningBracket {
+    /**
+     * Node that contains spoiler opening markdown ({@code >!}).
+     */
+    public final Node node;
+
+    /**
+     * Previous bracket.
+     */
+    public final SpoilerOpeningBracket previous;
+
+    /**
+     * Previous delimiter (emphasis, etc) before this bracket.
+     */
+    public final Delimiter previousDelimiter;
+
+    public SpoilerOpeningBracket(Node node, SpoilerOpeningBracket previous, Delimiter previousDelimiter) {
+        this.node = node;
+        this.previous = previous;
+        this.previousDelimiter = previousDelimiter;
+    }
+}
diff --git a/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpeningBracketStorage.java b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpeningBracketStorage.java
new file mode 100644
index 00000000..444975b4
--- /dev/null
+++ b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpeningBracketStorage.java
@@ -0,0 +1,38 @@
+package ml.docilealligator.infinityforreddit.markdown;
+
+import androidx.annotation.Nullable;
+
+import org.commonmark.internal.Delimiter;
+import org.commonmark.node.Node;
+
+public class SpoilerOpeningBracketStorage {
+    @Nullable
+    private SpoilerOpeningBracket lastBracket;
+    private Node currentBlock;
+
+    public void clear() {
+        lastBracket = null;
+    }
+
+    public void add(Node block, Node node, Delimiter lastDelimiter) {
+        updateBlock(block);
+        lastBracket = new SpoilerOpeningBracket(node, lastBracket, lastDelimiter);
+    }
+
+    @Nullable
+    public SpoilerOpeningBracket pop(Node block) {
+        updateBlock(block);
+        SpoilerOpeningBracket bracket = lastBracket;
+        if (bracket != null) {
+            lastBracket = bracket.previous;
+        }
+        return bracket;
+    }
+
+    private void updateBlock(Node block) {
+        if (block != currentBlock) {
+            clear();
+        }
+        currentBlock = block;
+    }
+}
diff --git a/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpeningInlineProcessor.java b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpeningInlineProcessor.java
new file mode 100644
index 00000000..487325d7
--- /dev/null
+++ b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpeningInlineProcessor.java
@@ -0,0 +1,40 @@
+package ml.docilealligator.infinityforreddit.markdown;
+
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
+
+import org.commonmark.node.Node;
+import org.commonmark.node.Text;
+
+import io.noties.markwon.inlineparser.InlineProcessor;
+
+/**
+ * Parses spoiler opening markdown ({@code >!}). Relies on {@link SpoilerClosingInlineProcessor}
+ * to handle closing and create {@link SpoilerNode SpoilerNodes}.
+ */
+public class SpoilerOpeningInlineProcessor extends InlineProcessor {
+    @NonNull
+    private final SpoilerOpeningBracketStorage spoilerOpeningBracketStorage;
+
+    public SpoilerOpeningInlineProcessor(@NonNull SpoilerOpeningBracketStorage spoilerOpeningBracketStorage) {
+        this.spoilerOpeningBracketStorage = spoilerOpeningBracketStorage;
+    }
+
+    @Override
+    public char specialCharacter() {
+        return '>';
+    }
+
+    @Nullable
+    @Override
+    protected Node parse() {
+        index++;
+        if (peek() == '!') {
+            index++;
+            Text node = text(">!");
+            spoilerOpeningBracketStorage.add(block, node, lastDelimiter());
+            return node;
+        }
+        return null;
+    }
+}
diff --git a/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpeningParser.java b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpeningParser.java
deleted file mode 100644
index 391e4a88..00000000
--- a/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerOpeningParser.java
+++ /dev/null
@@ -1,27 +0,0 @@
-package ml.docilealligator.infinityforreddit.markdown;
-
-import androidx.annotation.Nullable;
-
-import org.commonmark.node.Node;
-
-import io.noties.markwon.inlineparser.InlineProcessor;
-
-public class SpoilerOpeningParser extends InlineProcessor {
-    @Override
-    public char specialCharacter() {
-        return '>';
-    }
-
-    @Nullable
-    @Override
-    protected Node parse() {
-        index++;
-        if (peek() == '!') {
-            index++;
-            SpoilerOpening node = new SpoilerOpening();
-            node.setLiteral(">!");
-            return node;
-        }
-        return null;
-    }
-}
diff --git a/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerParserPlugin.java b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerParserPlugin.java
index 0875fef7..623b3d98 100644
--- a/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerParserPlugin.java
+++ b/app/src/main/java/ml/docilealligator/infinityforreddit/markdown/SpoilerParserPlugin.java
@@ -1,6 +1,5 @@
 package ml.docilealligator.infinityforreddit.markdown;
 
-import android.graphics.Color;
 import android.text.SpannableStringBuilder;
 import android.text.Spanned;
 import android.widget.TextView;
@@ -10,25 +9,24 @@ import androidx.annotation.NonNull;
 import org.commonmark.node.Block;
 import org.commonmark.node.BlockQuote;
 import org.commonmark.node.HtmlBlock;
+import org.commonmark.node.Node;
 import org.commonmark.parser.Parser;
 
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.List;
 import java.util.Set;
-import java.util.Stack;
 
 import io.noties.markwon.AbstractMarkwonPlugin;
+import io.noties.markwon.MarkwonSpansFactory;
 import io.noties.markwon.MarkwonVisitor;
 import io.noties.markwon.core.CorePlugin;
-import io.noties.markwon.core.spans.CodeBlockSpan;
-import io.noties.markwon.core.spans.CodeSpan;
 import io.noties.markwon.inlineparser.MarkwonInlineParserPlugin;
 
 public class SpoilerParserPlugin extends AbstractMarkwonPlugin {
     private final int textColor;
     private final int backgroundColor;
-    private boolean textHasSpoiler = false;
-    private int firstSpoilerStart = -1;
+
+    private final SpoilerOpeningBracketStorage spoilerOpeningBracketStorage = new SpoilerOpeningBracketStorage();
 
     SpoilerParserPlugin(int textColor, int backgroundColor) {
         this.textColor = textColor;
@@ -40,20 +38,43 @@ public class SpoilerParserPlugin extends AbstractMarkwonPlugin {
     }
 
     @Override
+    public void configureSpansFactory(@NonNull MarkwonSpansFactory.Builder builder) {
+        builder.setFactory(SpoilerNode.class, (config, renderProps) ->
+                new SpoilerSpan(textColor, backgroundColor));
+    }
+
+    @Override
     public void configureVisitor(@NonNull MarkwonVisitor.Builder builder) {
-        builder.on(SpoilerOpening.class, (visitor, opening) -> {
-            textHasSpoiler = true;
-            if (firstSpoilerStart == -1) {
-                firstSpoilerStart = visitor.length();
+        builder.on(SpoilerNode.class, new MarkwonVisitor.NodeVisitor<>() {
+            int depth = 0;
+
+            @Override
+            public void visit(@NonNull MarkwonVisitor visitor, @NonNull SpoilerNode spoilerNode) {
+                int start = visitor.length();
+                depth++;
+                visitor.visitChildren(spoilerNode);
+                depth--;
+                if (depth == 0) {
+                    // don't add SpoilerSpans inside other SpoilerSpans
+                    visitor.setSpansForNode(spoilerNode, start);
+                }
             }
-            visitor.builder().append(opening.getLiteral());
         });
     }
 
     @Override
+    public void afterRender(@NonNull Node node, @NonNull MarkwonVisitor visitor) {
+        spoilerOpeningBracketStorage.clear();
+    }
+
+    @Override
     public void configure(@NonNull Registry registry) {
-        registry.require(MarkwonInlineParserPlugin.class, plugin ->
-                plugin.factoryBuilder().addInlineProcessor(new SpoilerOpeningParser())
+        registry.require(MarkwonInlineParserPlugin.class, plugin -> {
+                    plugin.factoryBuilder()
+                            .addInlineProcessor(new SpoilerOpeningInlineProcessor(spoilerOpeningBracketStorage));
+                    plugin.factoryBuilder()
+                            .addInlineProcessor(new SpoilerClosingInlineProcessor(spoilerOpeningBracketStorage));
+                }
         );
     }
 
@@ -70,142 +91,49 @@ public class SpoilerParserPlugin extends AbstractMarkwonPlugin {
 
     @Override
     public void afterSetText(@NonNull TextView textView) {
-        textView.setHighlightColor(Color.TRANSPARENT);
-
-        if (!textHasSpoiler || textView.getText().length() < 5) {
-            firstSpoilerStart = 0;
-            return;
-        }
-
-        SpannableStringBuilder markdownStringBuilder = new SpannableStringBuilder(textView.getText());
-
-        ArrayList<SpoilerRange> spoilers = parse(markdownStringBuilder, firstSpoilerStart);
-        firstSpoilerStart = 0;
-        textHasSpoiler = false; // Since PostDetail can contain multiple TextViews, we do this here
-        if (spoilers.size() == 0) {
-            return;
-        }
-
-        // Process all the found spoilers. We always want to delete the brackets
-        // because they are in matching pairs. But we want to apply SpoilerSpan
-        // only to the outermost spoilers because nested spans break revealing-hiding
-        int openingPosition = -1;
-        ArrayList<SpoilerBracket> brackets = new ArrayList<>();
-        for (SpoilerRange range : spoilers) {
-            brackets.add(new SpoilerBracket(range.start, true, range.nested));
-            brackets.add(new SpoilerBracket(range.end, false, range.nested));
-        }
-        //noinspection ComparatorCombinators as it requires api 24+
-        Collections.sort(brackets, (lhs, rhs) -> Integer.compare(lhs.position, rhs.position));
-
-        int offset = 0;
-        for (SpoilerBracket bracket: brackets) {
-            if (bracket.opening) {
-                int spoilerStart = bracket.position - offset;
-                if (!bracket.nested) {
-                    openingPosition = spoilerStart;
-                }
-                markdownStringBuilder.delete(spoilerStart, spoilerStart + 2);
-            } else {
-                int spoilerEnd = bracket.position - offset;
-                markdownStringBuilder.delete(spoilerEnd, spoilerEnd + 2);
-                if (!bracket.nested) {
-                    SpoilerSpan spoilerSpan = new SpoilerSpan(textColor, backgroundColor);
-                    markdownStringBuilder.setSpan(spoilerSpan, openingPosition, spoilerEnd,
-                            Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);
-                }
+        CharSequence text = textView.getText();
+        if (text instanceof Spanned) {
+            Spanned spannedText = (Spanned) text;
+            SpoilerSpan[] spans = spannedText.getSpans(0, text.length(), SpoilerSpan.class);
+            if (spans.length == 0) {
+                return;
             }
-            offset += 2;
-        }
-
-        if (offset > 0) {
-            textView.setText(markdownStringBuilder);
-        }
-    }
-
-    private boolean noCodeIntersection(SpannableStringBuilder markdown, int position) {
-        return markdown.getSpans(position, position + 2, CodeSpan.class).length == 0
-                && markdown.getSpans(position, position + 2, CodeBlockSpan.class).length == 0;
-    }
 
-    /** Parse spoilers in the string starting from {@code start}.
-     *
-     * Returns all spoilers, spoilers that are nested inside other spoilers will have
-     * {@code nested} set to {@code true}.
-     * Doesn't allow more than one new line after every non-blank line.
-     *
-     * NB: could be optimized to reduce the number of calls to {@link #noCodeIntersection(SpannableStringBuilder, int)}
-     */
-    private ArrayList<SpoilerRange> parse(SpannableStringBuilder markdown, int start) {
-        final int MAX_NEW_LINE = 1;
-        int length = markdown.length();
-        Stack<Integer> openSpoilerStack = new Stack<>();
-        Stack<SpoilerRange> spoilersStack = new Stack<>();
-        ArrayList<SpoilerRange> closedSpoilers = new ArrayList<>();
-        int new_lines = 0;
-        for (int i = start; i < length; i++) {
-            char currentChar = markdown.charAt(i);
-            if (currentChar == '\n') {
-                new_lines++;
-                if (new_lines > MAX_NEW_LINE) {
-                    openSpoilerStack.clear();
-                    new_lines = 0;
-                }
-            } else if ((currentChar != '>')
-                    && (currentChar != '<')
-                    && (currentChar != '!')) {
-                new_lines = 0;
-            } else if (currentChar == '>'
-                    && i + 1 < length
-                    && markdown.charAt(i + 1) == '!'
-                    && noCodeIntersection(markdown, i)) {
-                openSpoilerStack.push(i);
-                i++; // skip '!'
-            } else if (openSpoilerStack.size() > 0
-                    && currentChar == '!'
-                    && i + 1 < length
-                    && markdown.charAt(i + 1) == '<'
-                    && noCodeIntersection(markdown, i)) {
-                var pos = openSpoilerStack.pop();
-                while (!spoilersStack.isEmpty()
-                        && spoilersStack.peek().start > pos) {
-                    SpoilerRange nestedRange = spoilersStack.pop();
-                    nestedRange.nested = true;
-                    closedSpoilers.add(nestedRange);
-                }
-                SpoilerRange range = new SpoilerRange(pos, i);
-                spoilersStack.push(range);
-                i++; // skip '<'
-            } else {
-                new_lines = 0;
+            // This is a workaround for Markwon's behavior.
+            // Markwon adds spans in reversed order so SpoilerSpan is applied first
+            // and other things (i.e. links, code, etc.) get drawn over it.
+            // We fix it by removing all SpoilerSpans and adding them again
+            // so they are applied last.
+            List<SpanInfo> spanInfo = new ArrayList<>(spans.length);
+            for (SpoilerSpan span : spans) {
+                spanInfo.add(new SpanInfo(
+                        span,
+                        spannedText.getSpanStart(span),
+                        spannedText.getSpanEnd(span),
+                        spannedText.getSpanFlags(span)
+                ));
             }
-        }
-
-        closedSpoilers.addAll(spoilersStack);
-        return closedSpoilers;
-    }
-
-    private static class SpoilerBracket {
-        final int position;
-        final boolean opening;
-        final boolean nested;
 
-
-        private SpoilerBracket(int position, boolean opening, boolean nested) {
-            this.position = position;
-            this.opening = opening;
-            this.nested = nested;
+            SpannableStringBuilder spannableStringBuilder = new SpannableStringBuilder(text);
+            for (SpanInfo info : spanInfo) {
+                spannableStringBuilder.removeSpan(info.span);
+                spannableStringBuilder.setSpan(info.span, info.start, info.end, info.flags);
+            }
+            textView.setText(spannableStringBuilder);
         }
     }
 
-    private static class SpoilerRange {
-        final int start;
-        final int end;
-        boolean nested;
+    private static class SpanInfo {
+        public final SpoilerSpan span;
+        public final int start;
+        public final int end;
+        public final int flags;
 
-        SpoilerRange(int start, int end) {
+        private SpanInfo(SpoilerSpan span, int start, int end, int flags) {
+            this.span = span;
             this.start = start;
             this.end = end;
+            this.flags = flags;
         }
     }
 }