Defensive copy in SpannableBuilder

app/src/main/java/ru/noties/markwon/MainActivity.java

@@ -2,12 +2,8 @@ package ru.noties.markwon;
 
 import android.app.Activity;
 import android.content.Intent;
-import android.graphics.Canvas;
-import android.graphics.Paint;
 import android.net.Uri;
 import android.os.Bundle;
-import android.text.Layout;
-import android.text.style.LeadingMarginSpan;
 import android.view.View;
 import android.widget.TextView;
 
@@ -62,56 +58,6 @@ public class MainActivity extends Activity {
 
         appBarRenderer.render(appBarState());
 
-        if (false) {
-
-            final class Whatever {
-                CharSequence text() {
-
-                    final SpannableBuilder builder = new SpannableBuilder();
-
-                    builder.append("First line\n\n");
-                    builder.append(Markwon.markdown(MainActivity.this, "* first\n* second\n* * third\n* * * forth\n\n"));
-                    builder.setSpan(new LeadingMarginSpan() {
-                        @Override
-                        public int getLeadingMargin(boolean first) {
-                            return 100;
-                        }
-
-                        @Override
-                        public void drawLeadingMargin(Canvas c, Paint p, int x, int dir, int top, int baseline, int bottom, CharSequence text, int start, int end, boolean first, Layout layout) {
-
-                        }
-                    }, 0);
-
-                    builder.append("Last line\n\n");
-                    return builder.text();
-                }
-            }
-            final Whatever whatever = new Whatever();
-
-            final SpannableBuilder builder = new SpannableBuilder();
-            builder.append(whatever.text());
-            builder.append(whatever.text());
-
-            builder.setSpan(new LeadingMarginSpan() {
-                @Override
-                public int getLeadingMargin(boolean first) {
-                    return 50;
-                }
-
-                @Override
-                public void drawLeadingMargin(Canvas c, Paint p, int x, int dir, int top, int baseline, int bottom, CharSequence text, int start, int end, boolean first, Layout layout) {
-
-                }
-            }, 0);
-
-            builder.append(whatever.text());
-
-            textView.setText(builder.text());
-
-            return;
-        }
-
         markdownLoader.load(uri(), new MarkdownLoader.OnMarkdownTextLoaded() {
             @Override
             public void apply(final String text) {

library/src/main/java/ru/noties/markwon/SpannableBuilder.java

@@ -14,6 +14,8 @@ import java.util.Iterator;
  * is using an array to store all the information about spans. So, a span that is added first
  * will be drawn first, which leads to subtle bugs (spans receive wrong `x` values when
  * requested to draw itself)
+ *
+ * @since 1.0.1
  */
 @SuppressWarnings({"WeakerAccess", "unused"})
 public class SpannableBuilder {
@@ -23,6 +25,8 @@ public class SpannableBuilder {
     // we will be using SpannableStringBuilder anyway as a backing store
     // as it has tight connection with system (implements some hidden methods, etc)
     private final SpannableStringBuilder builder;
+
+    // actually we might be just using ArrayList
     private final Deque<Span> spans = new ArrayDeque<>(8);
 
     public SpannableBuilder() {
@@ -31,7 +35,7 @@ public class SpannableBuilder {
 
     public SpannableBuilder(@NonNull CharSequence cs) {
         this.builder = new SpannableStringBuilderImpl(cs.toString());
-        copySpans(cs);
+        copySpans(0, cs);
     }
 
     /**
@@ -55,7 +59,7 @@ public class SpannableBuilder {
     @NonNull
     public SpannableBuilder append(@NonNull CharSequence cs) {
 
-        copySpans(cs);
+        copySpans(length(), cs);
 
         builder.append(cs.toString());
 
@@ -109,7 +113,8 @@ public class SpannableBuilder {
     @NonNull
     public CharSequence remove(int start, int end) {
 
-        // okay: here is what we will try to do:
+        // this method is intended to be used only by markdown visitor
+        // it's a workaround to allow tables
 
         final SpannableStringBuilderImpl impl = new SpannableStringBuilderImpl(builder.subSequence(start, end));
 
@@ -165,24 +170,26 @@ public class SpannableBuilder {
         return builder.toString();
     }
 
-    // Unfortunately I cannot see any way to NOT expose this internal value, which opens a gate
-    // to external modification (first of all InputFilters, that potentially break span indexes
-    // as we keep track of them independently). Must warn user to NOT apply inputFilters
     @NonNull
     public CharSequence text() {
 
-        // if called once, it will apply spans, which will modify our state
+        // okay, in order to not allow external modification and keep our spans order
+        // we should not return our builder
+        //
+        // plus, if this method was called -> all spans would be applied, which potentially
+        // breaks the order that we intend to use
+        // so, we will defensively copy builder
 
-        applySpans();
+        final SpannableStringBuilderImpl impl = new SpannableStringBuilderImpl(builder.toString());
 
-        // we could return here for example new SpannableStringBuilder(builder)
+        for (Span span : spans) {
-        // but, if returned value will be used in other SpannableBuilder,
+            impl.setSpan(span.what, span.start, span.end, span.flags);
-        // we won't be able to detect in what order to store the spans
+        }
 
-        return builder;
+        return impl;
     }
 
-    private void copySpans(@Nullable CharSequence cs) {
+    private void copySpans(final int index, @Nullable CharSequence cs) {
 
         // we must identify already reversed Spanned...
         // and (!) iterate backwards when adding (to preserve order)
@@ -191,7 +198,6 @@ public class SpannableBuilder {
 
             final Spanned spanned = (Spanned) cs;
             final boolean reverse = spanned instanceof SpannedReversed;
-            final int index = length();
 
             final Object[] spans = spanned.getSpans(0, spanned.length(), Object.class);
 
@@ -209,17 +215,6 @@ public class SpannableBuilder {
         }
     }
 
-    private void applySpans() {
-
-        // will apply appended spans in reverse order
-        // clear the stack (that keeps track of them)
-
-        Span span;
-        while ((span = spans.poll()) != null) {
-            builder.setSpan(span.what, span.start, span.end, span.flags);
-        }
-    }
-
     private static class Span {
 
         final Object what;

library/src/main/java/ru/noties/markwon/SpannableStringBuilderImpl.java

@@ -2,12 +2,11 @@ package ru.noties.markwon;
 
 import android.text.SpannableStringBuilder;
 
+/**
+ * @since 1.0.1
+ */
 class SpannableStringBuilderImpl extends SpannableStringBuilder implements SpannedReversed {
 
-    SpannableStringBuilderImpl() {
-        super();
-    }
-
     SpannableStringBuilderImpl(CharSequence text) {
         super(text);
     }