Merge pull request #269 from commonmark/issue-267-strikethrough

Adjust GFM strikethrough parsing (fixes #267)

Author: robinst

CHANGELOG.md

@@ -8,8 +8,12 @@ with the exception that 0.x versions can break between minor versions.
 
 ## [Unreleased]
 ### Fixed
-- A single pipe (optional whitespace) now ends a table instead of crashing or
-  being treated as an empty row, for consistency with GitHub (#255).
+- GitHub tables: A single pipe (optional whitespace) now ends a table
+  instead of crashing or being treated as an empty row, for consistency
+  with GitHub (#255).
+- GitHub strikethrough: A single tilde now also works, and more than two
+  tildes are not accepted anymore. This brings us in line with what
+  GitHub actually does, which is a bit underspecified (#267)
 
 ## [0.19.0] - 2022-06-02
 ### Added

commonmark-ext-gfm-strikethrough/src/main/java/org/commonmark/ext/gfm/strikethrough/internal/StrikethroughDelimiterProcessor.java

@@ -22,33 +22,33 @@ public char getClosingCharacter() {
 
     @Override
     public int getMinLength() {
-        return 2;
+        return 1;
     }
 
     @Override
     public int process(DelimiterRun openingRun, DelimiterRun closingRun) {
-        if (openingRun.length() >= 2 && closingRun.length() >= 2) {
-            // Use exactly two delimiters even if we have more, and don't care about internal openers/closers.
+        if (openingRun.length() == closingRun.length() && openingRun.length() <= 2) {
+            // GitHub only accepts either one or two delimiters, but not a mix or more than that.
 
             Text opener = openingRun.getOpener();
 
             // Wrap nodes between delimiters in strikethrough.
             Node strikethrough = new Strikethrough();
 
             SourceSpans sourceSpans = new SourceSpans();
-            sourceSpans.addAllFrom(openingRun.getOpeners(2));
+            sourceSpans.addAllFrom(openingRun.getOpeners(openingRun.length()));
 
             for (Node node : Nodes.between(opener, closingRun.getCloser())) {
                 strikethrough.appendChild(node);
                 sourceSpans.addAll(node.getSourceSpans());
             }
 
-            sourceSpans.addAllFrom(closingRun.getClosers(2));
+            sourceSpans.addAllFrom(closingRun.getClosers(closingRun.length()));
             strikethrough.setSourceSpans(sourceSpans.getSourceSpans());
 
             opener.insertAfter(strikethrough);
 
-            return 2;
+            return openingRun.length();
         } else {
             return 0;
         }

commonmark-ext-gfm-strikethrough/src/test/java/org/commonmark/ext/gfm/strikethrough/StrikethroughSpecTest.java

@@ -0,0 +1,46 @@
+package org.commonmark.ext.gfm.strikethrough;
+
+import org.commonmark.Extension;
+import org.commonmark.parser.Parser;
+import org.commonmark.renderer.html.HtmlRenderer;
+import org.commonmark.testutil.RenderingTestCase;
+import org.commonmark.testutil.TestResources;
+import org.commonmark.testutil.example.Example;
+import org.commonmark.testutil.example.ExampleReader;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+
+@RunWith(Parameterized.class)
+public class StrikethroughSpecTest extends RenderingTestCase {
+
+    private static final Set<Extension> EXTENSIONS = Collections.singleton(StrikethroughExtension.create());
+    private static final Parser PARSER = Parser.builder().extensions(EXTENSIONS).build();
+    private static final HtmlRenderer RENDERER = HtmlRenderer.builder().extensions(EXTENSIONS).build();
+
+    private final Example example;
+
+    public StrikethroughSpecTest(Example example) {
+        this.example = example;
+    }
+
+    @Parameters(name = "{0}")
+    public static List<Object[]> data() {
+        return ExampleReader.readExampleObjects(TestResources.getGfmSpec(), "strikethrough");
+    }
+
+    @Test
+    public void testHtmlRendering() {
+        assertRendering(example.getSource(), example.getHtml());
+    }
+
+    @Override
+    protected String render(String source) {
+        return RENDERER.render(PARSER.parse(source));
+    }
+}

commonmark-ext-gfm-strikethrough/src/test/java/org/commonmark/ext/gfm/strikethrough/StrikethroughTest.java

@@ -26,12 +26,12 @@ public class StrikethroughTest extends RenderingTestCase {
             .extensions(EXTENSIONS).build();
 
     @Test
-    public void oneTildeIsNotEnough() {
-        assertRendering("~foo~", "<p>~foo~</p>\n");
+    public void oneTildeIsEnough() {
+        assertRendering("~foo~", "<p><del>foo</del></p>\n");
     }
 
     @Test
-    public void twoTildesYay() {
+    public void twoTildesWorksToo() {
         assertRendering("~~foo~~", "<p><del>foo</del></p>\n");
     }
 
@@ -48,23 +48,22 @@ public void unmatched() {
 
     @Test
     public void threeInnerThree() {
-        assertRendering("a ~~~foo~~~", "<p>a ~<del>foo</del>~</p>\n");
+        assertRendering("a ~~~foo~~~", "<p>a ~~~foo~~~</p>\n");
     }
 
     @Test
     public void twoInnerThree() {
-        assertRendering("~~foo~~~", "<p><del>foo</del>~</p>\n");
+        assertRendering("~~foo~~~", "<p>~~foo~~~</p>\n");
     }
 
     @Test
     public void tildesInside() {
         assertRendering("~~foo~bar~~", "<p><del>foo~bar</del></p>\n");
         assertRendering("~~foo~~bar~~", "<p><del>foo</del>bar~~</p>\n");
-        assertRendering("~~foo~~~bar~~", "<p><del>foo</del>~bar~~</p>\n");
-        assertRendering("~~foo~~~~bar~~", "<p><del>foo</del><del>bar</del></p>\n");
-        assertRendering("~~foo~~~~~bar~~", "<p><del>foo</del>~<del>bar</del></p>\n");
-        assertRendering("~~foo~~~~~~bar~~", "<p><del>foo</del>~~<del>bar</del></p>\n");
-        assertRendering("~~foo~~~~~~~bar~~", "<p><del>foo</del>~~~<del>bar</del></p>\n");
+        assertRendering("~~foo~~~bar~~", "<p><del>foo~~~bar</del></p>\n");
+        assertRendering("~~foo~~~~bar~~", "<p><del>foo~~~~bar</del></p>\n");
+        assertRendering("~~foo~~~~~bar~~", "<p><del>foo~~~~~bar</del></p>\n");
+        assertRendering("~~foo~~~~~~bar~~", "<p><del>foo~~~~~~bar</del></p>\n");
     }
 
     @Test

commonmark-ext-gfm-tables/src/test/java/org/commonmark/ext/gfm/tables/TablesSpecTest.java

@@ -32,14 +32,7 @@ public TablesSpecTest(Example example) {
 
     @Parameters(name = "{0}")
     public static List<Object[]> data() {
-        List<Example> examples = ExampleReader.readExamples(TestResources.class.getResource("/gfm-spec.txt"));
-        List<Object[]> data = new ArrayList<>();
-        for (Example example : examples) {
-            if (example.getInfo().contains("table")) {
-                data.add(new Object[]{example});
-            }
-        }
-        return data;
+        return ExampleReader.readExampleObjects(TestResources.getGfmSpec(), "table");
     }
 
     @Test

commonmark-test-util/src/main/java/org/commonmark/testutil/TestResources.java

@@ -14,6 +14,10 @@ public static URL getSpec() {
         return TestResources.class.getResource("/spec.txt");
     }
 
+    public static URL getGfmSpec() {
+        return TestResources.class.getResource("/gfm-spec.txt");
+    }
+
     public static List<URL> getRegressions() {
         return Arrays.asList(
                 TestResources.class.getResource("/cmark-regression.txt"),

commonmark-test-util/src/main/java/org/commonmark/testutil/example/ExampleReader.java

@@ -42,6 +42,17 @@ public static List<Example> readExamples(URL url) {
         }
     }
 
+    public static List<Object[]> readExampleObjects(URL url, String info) {
+        List<Example> examples = readExamples(url);
+        List<Object[]> data = new ArrayList<>();
+        for (Example example : examples) {
+            if (example.getInfo().contains(info)) {
+                data.add(new Object[]{example});
+            }
+        }
+        return data;
+    }
+
     public static List<String> readExampleSources(URL url) {
         List<Example> examples = ExampleReader.readExamples(url);
         List<String> result = new ArrayList<>();

…m-tables/src/test/resources/gfm-spec.txt …est-util/src/main/resources/gfm-spec.txtcommonmark-ext-gfm-tables/src/test/resources/gfm-spec.txt renamed to commonmark-test-util/src/main/resources/gfm-spec.txt commonmark-ext-gfm-tables/src/test/resources/gfm-spec.txt renamed to commonmark-test-util/src/main/resources/gfm-spec.txt

@@ -2077,7 +2077,7 @@ followed by one of the strings (case-insensitive) `address`,
 `h1`, `h2`, `h3`, `h4`, `h5`, `h6`, `head`, `header`, `hr`,
 `html`, `iframe`, `legend`, `li`, `link`, `main`, `menu`, `menuitem`,
 `nav`, `noframes`, `ol`, `optgroup`, `option`, `p`, `param`,
-`section`, `source`, `summary`, `table`, `tbody`, `td`,
+`section`, `summary`, `table`, `tbody`, `td`,
 `tfoot`, `th`, `thead`, `title`, `tr`, `track`, `ul`, followed
 by [whitespace], the end of the line, the string `>`, or
 the string `/>`.\
@@ -10224,4 +10224,3 @@ closers:
 
 After we're done, we remove all delimiters above `stack_bottom` from the
 delimiter stack.
-

etc/update-spec.sh

@@ -7,7 +7,7 @@ fi
 
 version=$1
 curl -L "https://raw.githubusercontent.com/commonmark/commonmark-spec/$version/spec.txt" -o commonmark-test-util/src/main/resources/spec.txt
-curl -L "https://raw.githubusercontent.com/github/cmark-gfm/master/test/spec.txt" -o commonmark-ext-gfm-tables/src/test/resources/gfm-spec.txt
+curl -L "https://raw.githubusercontent.com/github/cmark-gfm/master/test/spec.txt" -o commonmark-test-util/src/main/resources/gfm-spec.txt
 
 echo "Check cmark and commonmark.js regression.txt:"
 echo "https://github.com/commonmark/cmark/blob/master/test/regression.txt"