From 20a814862e091e6ff03191bc50af4858cabea981 Mon Sep 17 00:00:00 2001 From: Dimitry Ivanov Date: Tue, 4 Sep 2018 13:15:27 +0300 Subject: [PATCH] Deal with white spaces by checking for next node --- .../ru/noties/markwon/MarkdownRenderer.java | 1 - .../ru/noties/markwon/SpannableBuilder.java | 53 -------- .../markwon/SpannableConfiguration.java | 29 +---- .../renderer/SpannableMarkdownVisitor.java | 115 +++++++++++++----- .../visitor/SpannableMarkdownVisitorTest.java | 2 +- .../renderer/visitor/TestValidator.java | 20 ++- .../src/test/resources/tests/code-blocks.yaml | 17 +++ markwon/src/test/resources/tests/second.yaml | 4 +- .../src/test/resources/tests/ul-levels.yaml | 2 +- 9 files changed, 120 insertions(+), 123 deletions(-) create mode 100644 markwon/src/test/resources/tests/code-blocks.yaml diff --git a/app/src/main/java/ru/noties/markwon/MarkdownRenderer.java b/app/src/main/java/ru/noties/markwon/MarkdownRenderer.java index a868bbcd..23ff268b 100644 --- a/app/src/main/java/ru/noties/markwon/MarkdownRenderer.java +++ b/app/src/main/java/ru/noties/markwon/MarkdownRenderer.java @@ -96,7 +96,6 @@ public class MarkdownRenderer { .codeTextColor(prism4jTheme.textColor()) .build()) .factory(new GifAwareSpannableFactory(gifPlaceholder)) - .trimWhiteSpaceEnd(false) .build(); final long start = SystemClock.uptimeMillis(); diff --git a/markwon/src/main/java/ru/noties/markwon/SpannableBuilder.java b/markwon/src/main/java/ru/noties/markwon/SpannableBuilder.java index a02b53de..cba31c27 100644 --- a/markwon/src/main/java/ru/noties/markwon/SpannableBuilder.java +++ b/markwon/src/main/java/ru/noties/markwon/SpannableBuilder.java @@ -61,7 +61,6 @@ public class SpannableBuilder implements Appendable, CharSequence { } public SpannableBuilder(@NonNull CharSequence cs) { -// this.builder = new SpannableStringBuilderImpl(cs.toString()); this.builder = new StringBuilder(cs); copySpans(0, cs); } @@ -198,58 +197,6 @@ public class SpannableBuilder implements Appendable, CharSequence { return builder.toString(); } - /** - * Moved from {@link #text()} method - * - * @since 2.0.0 - */ - public void trimWhiteSpaceEnd() { - - // now, let's remove trailing & leading newLines (so small amounts of text are displayed correctly) - // @since 1.0.2 - - int length = builder.length(); - - if (length > 0) { - - length = builder.length(); - int amount = 0; - for (int i = length - 1; i >= 0; i--) { - if (Character.isWhitespace(builder.charAt(i))) { - amount += 1; - } else { - break; - } - } - - if (amount > 0) { - - final int newLength = length - amount; - builder.replace(newLength, length, ""); - - // additionally we should apply new length to the spans (otherwise - // sometimes system cannot handle spans with length greater than total length - // which causes some internal failure (no exceptions, no logs) in Layout class - // and no markdown is displayed at all - if (spans.size() > 0) { - Span span; - final Iterator iterator = spans.iterator(); - while (iterator.hasNext()) { - span = iterator.next(); - // if span start is greater than newLength, then remove it... one should - // not use white space for spanning resulting text - if (span.start > newLength) { - iterator.remove(); - } else if (span.end > newLength) { - span.end = newLength; - } - } - } - } - - } - } - @NonNull public CharSequence text() { // @since 2.0.0 redirects this call to `#spannableStringBuilder()` diff --git a/markwon/src/main/java/ru/noties/markwon/SpannableConfiguration.java b/markwon/src/main/java/ru/noties/markwon/SpannableConfiguration.java index 319698ea..cb2a34bc 100644 --- a/markwon/src/main/java/ru/noties/markwon/SpannableConfiguration.java +++ b/markwon/src/main/java/ru/noties/markwon/SpannableConfiguration.java @@ -3,7 +3,6 @@ package ru.noties.markwon; import android.content.Context; import android.support.annotation.NonNull; -import ru.noties.markwon.html.api.HtmlTag; import ru.noties.markwon.html.api.MarkwonHtmlParser; import ru.noties.markwon.renderer.ImageSizeResolver; import ru.noties.markwon.renderer.ImageSizeResolverDef; @@ -34,7 +33,6 @@ public class SpannableConfiguration { private final ImageSizeResolver imageSizeResolver; private final SpannableFactory factory; // @since 1.1.0 private final boolean softBreakAddsNewLine; // @since 1.1.1 - private final boolean trimWhiteSpaceEnd; // @since 2.0.0 private final MarkwonHtmlParser htmlParser; // @since 2.0.0 private final MarkwonHtmlRenderer htmlRenderer; // @since 2.0.0 private final boolean htmlAllowNonClosedTags; // @since 2.0.0 @@ -48,7 +46,6 @@ public class SpannableConfiguration { this.imageSizeResolver = builder.imageSizeResolver; this.factory = builder.factory; this.softBreakAddsNewLine = builder.softBreakAddsNewLine; - this.trimWhiteSpaceEnd = builder.trimWhiteSpaceEnd; this.htmlParser = builder.htmlParser; this.htmlRenderer = builder.htmlRenderer; this.htmlAllowNonClosedTags = builder.htmlAllowNonClosedTags; @@ -98,13 +95,6 @@ public class SpannableConfiguration { return softBreakAddsNewLine; } - /** - * @since 2.0.0 - */ - public boolean trimWhiteSpaceEnd() { - return trimWhiteSpaceEnd; - } - /** * @since 2.0.0 */ @@ -140,7 +130,6 @@ public class SpannableConfiguration { private ImageSizeResolver imageSizeResolver; private SpannableFactory factory; // @since 1.1.0 private boolean softBreakAddsNewLine; // @since 1.1.1 - private boolean trimWhiteSpaceEnd = true; // @since 2.0.0 private MarkwonHtmlParser htmlParser; // @since 2.0.0 private MarkwonHtmlRenderer htmlRenderer; // @since 2.0.0 private boolean htmlAllowNonClosedTags; // @since 2.0.0 @@ -210,18 +199,6 @@ public class SpannableConfiguration { return this; } - /** - * Will trim white space(s) from the end from resulting text. - * By default `true` - * - * @since 2.0.0 - */ - @NonNull - public Builder trimWhiteSpaceEnd(boolean trimWhiteSpaceEnd) { - this.trimWhiteSpaceEnd = trimWhiteSpaceEnd; - return this; - } - /** * @since 2.0.0 */ @@ -242,9 +219,9 @@ public class SpannableConfiguration { /** * @param htmlAllowNonClosedTags that indicates if non-closed html tags should be rendered. - * If this argument is true then all non-closed HTML tags - * will be closed at the end of a document. Otherwise they will - * be delivered non-closed {@code HtmlTag#isClosed()} + * If this argument is true then all non-closed HTML tags + * will be closed at the end of a document. Otherwise they will + * be delivered non-closed {@code HtmlTag#isClosed()} * @since 2.0.0 */ @NonNull diff --git a/markwon/src/main/java/ru/noties/markwon/renderer/SpannableMarkdownVisitor.java b/markwon/src/main/java/ru/noties/markwon/renderer/SpannableMarkdownVisitor.java index 3ce3db87..f3342203 100644 --- a/markwon/src/main/java/ru/noties/markwon/renderer/SpannableMarkdownVisitor.java +++ b/markwon/src/main/java/ru/noties/markwon/renderer/SpannableMarkdownVisitor.java @@ -6,6 +6,7 @@ import android.support.annotation.Nullable; import org.commonmark.ext.gfm.strikethrough.Strikethrough; 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.AbstractVisitor; import org.commonmark.node.BlockQuote; @@ -79,10 +80,6 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { super.visit(document); configuration.htmlRenderer().render(configuration, builder, htmlParser); - - if (configuration.trimWhiteSpaceEnd()) { - builder.trimWhiteSpaceEnd(); - } } @Override @@ -122,9 +119,11 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { blockQuoteIndent -= 1; - newLine(); - if (blockQuoteIndent == 0) { - builder.append('\n'); + if (hasNext(blockQuote)) { + newLine(); + if (blockQuoteIndent == 0) { + builder.append('\n'); + } } } @@ -145,7 +144,7 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { @Override public void visit(FencedCodeBlock fencedCodeBlock) { // @since 1.0.4 - visitCodeBlock(fencedCodeBlock.getInfo(), fencedCodeBlock.getLiteral()); + visitCodeBlock(fencedCodeBlock.getInfo(), fencedCodeBlock.getLiteral(), fencedCodeBlock); } /** @@ -153,7 +152,7 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { */ @Override public void visit(IndentedCodeBlock indentedCodeBlock) { - visitCodeBlock(null, indentedCodeBlock.getLiteral()); + visitCodeBlock(null, indentedCodeBlock.getLiteral(), indentedCodeBlock); } /** @@ -161,7 +160,8 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { * @param code content of a code block * @since 1.0.4 */ - private void visitCodeBlock(@Nullable String info, @NonNull String code) { + private void visitCodeBlock(@Nullable String info, @NonNull String code, @NonNull Node node) { + newLine(); final int length = builder.length(); @@ -172,12 +172,16 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { configuration.syntaxHighlight() .highlight(info, code) ); - builder.append('\u00a0').append('\n'); + + newLine(); + builder.append('\u00a0'); setSpan(length, factory.code(theme, true)); - newLine(); - builder.append('\n'); + if (hasNext(node)) { + newLine(); + builder.append('\n'); + } } @Override @@ -191,11 +195,16 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { } private void visitList(Node node) { + newLine(); + visitChildren(node); - newLine(); - if (listLevel == 0 && blockQuoteIndent == 0) { - builder.append('\n'); + + if (hasNext(node)) { + newLine(); + if (listLevel == 0 && blockQuoteIndent == 0) { + builder.append('\n'); + } } } @@ -230,7 +239,9 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { blockQuoteIndent -= 1; listLevel -= 1; - newLine(); + if (hasNext(listItem)) { + newLine(); + } } @Override @@ -243,8 +254,10 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { setSpan(length, factory.thematicBreak(theme)); - newLine(); - builder.append('\n'); + if (hasNext(thematicBreak)) { + newLine(); + builder.append('\n'); + } } @Override @@ -256,10 +269,11 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { visitChildren(heading); setSpan(length, factory.heading(theme, heading.getLevel())); - newLine(); - - // after heading we add another line anyway (no additional checks) - builder.append('\n'); + if (hasNext(heading)) { + newLine(); + // after heading we add another line anyway (no additional checks) + builder.append('\n'); + } } @Override @@ -282,12 +296,17 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { */ @Override public void visit(CustomBlock customBlock) { + if (customBlock instanceof TaskListBlock) { blockQuoteIndent += 1; visitChildren(customBlock); blockQuoteIndent -= 1; - newLine(); - builder.append('\n'); + + if (hasNext(customBlock)) { + newLine(); + builder.append('\n'); + } + } else { super.visit(customBlock); } @@ -316,7 +335,9 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { setSpan(length, factory.taskListItem(theme, blockQuoteIndent, listItem.done())); - newLine(); + if (hasNext(customNode)) { + newLine(); + } blockQuoteIndent -= listItem.indent(); @@ -330,18 +351,37 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { final boolean handled; if (node instanceof TableBody) { + visitChildren(node); tableRows = 0; handled = true; - newLine(); - builder.append('\n'); - } else if (node instanceof TableRow) { + + if (hasNext(node)) { + newLine(); + builder.append('\n'); + } + + } else if (node instanceof TableRow || node instanceof TableHead) { final int length = builder.length(); + visitChildren(node); if (pendingTableRow != null) { + // @since 2.0.0 + // we cannot rely on hasNext(TableHead) as it's not reliable + // we must apply new line manually and then exclude it from tableRow span + final boolean addNewLine; + { + final int builderLength = builder.length(); + addNewLine = builderLength > 0 + && '\n' != builder.charAt(builderLength - 1); + } + if (addNewLine) { + builder.append('\n'); + } + // @since 1.0.4 Replace table char with non-breakable space // we need this because if table is at the end of the text, then it will be // trimmed from the final result @@ -357,12 +397,13 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { ? 0 : tableRows + 1; - setSpan(length, span); - newLine(); + setSpan(addNewLine ? length + 1 : length, span); + pendingTableRow = null; } handled = true; + } else if (node instanceof TableCell) { final TableCell cell = (TableCell) node; @@ -383,11 +424,13 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { } else { handled = false; } + return handled; } @Override public void visit(Paragraph paragraph) { + final boolean inTightList = isInTightList(paragraph); if (!inTightList) { @@ -400,9 +443,8 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { // @since 1.1.1 apply paragraph span setSpan(length, factory.paragraph(inTightList)); - if (!inTightList) { + if (hasNext(paragraph) && !inTightList) { newLine(); - if (blockQuoteIndent == 0) { builder.append('\n'); } @@ -518,4 +560,11 @@ public class SpannableMarkdownVisitor extends AbstractVisitor { } return out; } + + /** + * @since 2.0.0 + */ + protected static boolean hasNext(@NonNull Node node) { + return node.getNext() != null; + } } diff --git a/markwon/src/test/java/ru/noties/markwon/renderer/visitor/SpannableMarkdownVisitorTest.java b/markwon/src/test/java/ru/noties/markwon/renderer/visitor/SpannableMarkdownVisitorTest.java index 9f123307..047a0584 100644 --- a/markwon/src/test/java/ru/noties/markwon/renderer/visitor/SpannableMarkdownVisitorTest.java +++ b/markwon/src/test/java/ru/noties/markwon/renderer/visitor/SpannableMarkdownVisitorTest.java @@ -61,7 +61,7 @@ public class SpannableMarkdownVisitorTest { } // assert that the whole thing is processed - assertEquals(stringBuilder.length(), index); + assertEquals("`" + stringBuilder + "`", stringBuilder.length(), index); final Object[] spans = stringBuilder.getSpans(0, stringBuilder.length(), Object.class); final int length = spans != null diff --git a/markwon/src/test/java/ru/noties/markwon/renderer/visitor/TestValidator.java b/markwon/src/test/java/ru/noties/markwon/renderer/visitor/TestValidator.java index f6ad246e..59c000fe 100644 --- a/markwon/src/test/java/ru/noties/markwon/renderer/visitor/TestValidator.java +++ b/markwon/src/test/java/ru/noties/markwon/renderer/visitor/TestValidator.java @@ -5,7 +5,6 @@ import android.support.annotation.Nullable; import android.text.SpannableStringBuilder; import android.text.Spanned; -import java.util.Arrays; import java.util.Map; import ix.Ix; @@ -68,7 +67,10 @@ abstract class TestValidator { } } - assertEquals(text, builder.subSequence(index, index + text.length()).toString()); + assertEquals( + String.format("text: %s, position: {%d-%d}", text, index, index + text.length()), + text, + builder.subSequence(index, index + text.length()).toString()); return index + text.length(); } @@ -106,6 +108,7 @@ abstract class TestValidator { .filter(new IxPredicate() { @Override public boolean test(TestSpan testSpan) { + // in case of nested spans with the same name (lists) // we also must validate attributes // and thus we are moving most of assertions to this filter method @@ -170,16 +173,21 @@ abstract class TestValidator { spansText = "[]"; } else { final StringBuilder builder = new StringBuilder(); - for (Object o: spans) { + for (Object o : spans) { final TestSpan testSpan = (TestSpan) o; if (builder.length() > 0) { builder.append(", "); } + builder .append("{name: '").append(testSpan.name()).append('\'') - .append(", position{").append(start).append(", ").append(end).append('}') - .append(", attributes: ").append(testSpan.attributes()) - .append('}'); + .append(", position{").append(start).append(", ").append(end).append('}'); + + if (testSpan.attributes().size() > 0) { + builder.append(", attributes: ").append(testSpan.attributes()); + } + + builder.append('}'); } spansText = builder.toString(); } diff --git a/markwon/src/test/resources/tests/code-blocks.yaml b/markwon/src/test/resources/tests/code-blocks.yaml new file mode 100644 index 00000000..53b9bd50 --- /dev/null +++ b/markwon/src/test/resources/tests/code-blocks.yaml @@ -0,0 +1,17 @@ +input: |- + ```java + final String s = null; + ``` + ```html + + ``` + ``` + nothing here + ``` + +output: + - code-block: "final String s = null;" + - "\n\n" + - code-block: "" + - "\n\n" + - code-block: "nothing here" \ No newline at end of file diff --git a/markwon/src/test/resources/tests/second.yaml b/markwon/src/test/resources/tests/second.yaml index 747a645e..bd088dc2 100644 --- a/markwon/src/test/resources/tests/second.yaml +++ b/markwon/src/test/resources/tests/second.yaml @@ -21,8 +21,8 @@ output: - text: " " - s: "strike" - text: " down\n\n" - - blockquote: "Some quote here!\n" - - text: "\n" + - blockquote: "Some quote here!" + - text: "\n\n" - h1: "Header 1" - text: "\n\n" - h2: "Header 2" diff --git a/markwon/src/test/resources/tests/ul-levels.yaml b/markwon/src/test/resources/tests/ul-levels.yaml index 00708e17..a7a06c96 100644 --- a/markwon/src/test/resources/tests/ul-levels.yaml +++ b/markwon/src/test/resources/tests/ul-levels.yaml @@ -10,8 +10,8 @@ output: - ul: - ul: "Second" level: 1 - - text: "\n" level: 0 + - text: "\n" - ul: - ul: - ul: "Third"