noties / Markwon

Android markdown library (no WebView)
https://noties.io/Markwon/
Apache License 2.0
2.76k stars 313 forks source link

Table spans are unstable if there's text around it #36

Closed Kaned1as closed 6 years ago

Kaned1as commented 6 years ago

Table spans are quite brittle, the don't survive additions of simple text content before them. This test-case outlines that (written in Kotlin, hope you don't mind).

    @Test
    fun testMarkwonTable() {
        val table = javaClass.getResourceAsStream("/sample-table.md").bufferedReader().readText()
        val spanned = Markwon.markdown(activity, table) as Spannable
        val spans = spanned.getSpans(0, spanned.length, TableRowSpan::class.java)
        Assert.assertTrue("Should have table spans!", spans.isNotEmpty())

        val tableWithHeader = """
            With header everything is ok
            =============================

                              """.trimIndent() + table
        val spannedHeader = Markwon.markdown(activity, tableWithHeader) as Spannable
        val spansHeader = spannedHeader.getSpans(0, spannedHeader.length, TableRowSpan::class.java)
        Assert.assertTrue("Should have table spans!", spansHeader.isNotEmpty())

        val tableAfterText = """
            But adding just text before the table causes it to collapse

                            """.trimIndent() + table

        val spannedAfterText = Markwon.markdown(activity, tableAfterText) as Spannable
        val spansAfterText = spannedAfterText.getSpans(0, spannedAfterText.length, TableRowSpan::class.java)
        Assert.assertTrue("Should have table spans!", spansAfterText.isNotEmpty())
    }

contents of sample table:

|          One ring           |         Patterns         |              Titanic              |   |   |   |
|-----------------------------|--------------------------|-----------------------------------|---|---|---|
|  One ring to rule them all  |There's one for the sorrow|      Roll on, Titanic, roll       |   |   |   |
|    One ring to find them    |   And two for the joy    |You're the pride of White Star Line|   |   |   |
| One ring to bring them all  | And three for the girls  |      Roll on, Titanic, roll       |   |   |   |
|And in the darkness bind them|  And four for the boys   |      Into the mists of time       |   |   |   |

This test fails on the last assert for me.

noties commented 6 years ago

Hello @Adonai (again) !

I cannot reproduce the issue. Can you please additionally log (for inspection purposes) raw markdown that is being processed? There might be some issue with trimIdent or file reading mechanism (some encoding issues, different new line characters, etc) or, hell, both 😃 .

Kaned1as commented 6 years ago

Sure, give me an hour, will post once I'm home

Kaned1as commented 6 years ago

This is the raw text:

But adding just text before or after the table causes it to collapse
|          One ring           |         Patterns         |              Titanic              |   |   |   |
|-----------------------------|--------------------------|-----------------------------------|---|---|---|
|  One ring to rule them all  |There's one for the sorrow|      Roll on, Titanic, roll       |   |   |   |
|    One ring to find them    |   And two for the joy    |You're the pride of White Star Line|   |   |   |
| One ring to bring them all  | And three for the girls  |      Roll on, Titanic, roll       |   |   |   |
|And in the darkness bind them|  And four for the boys   |      Into the mists of time       |   |   |   |

Possibly the problem is in commonmark parser

noties commented 6 years ago

Hey! The thing is it looks like a table needs a proper delimiter. For example, I'm using Github markdown here (using the text you have provided):


But adding just text before or after the table causes it to collapse One ring Patterns Titanic
One ring to rule them all There's one for the sorrow Roll on, Titanic, roll
One ring to find them And two for the joy You're the pride of White Star Line
One ring to bring them all And three for the girls Roll on, Titanic, roll
And in the darkness bind them And four for the boys Into the mists of time

and here with an additional new line after the header:


But adding just text before or after the table causes it to collapse

One ring Patterns Titanic
One ring to rule them all There's one for the sorrow Roll on, Titanic, roll
One ring to find them And two for the joy You're the pride of White Star Line
One ring to bring them all And three for the girls Roll on, Titanic, roll
And in the darkness bind them And four for the boys Into the mists of time

So, as in markdown one new line is not considered really a new line, parser thinks that you are just trying to ident your text. Adding another new line will fix your test case

Kaned1as commented 6 years ago

Ok, now I just should somehow explain it to my users...

Thanks though :)

noties commented 6 years ago

Tough one, good luck! 😃