Fix for table depth error when cell contains backticks

[phillip-haydon]

This addresses the issue in #504 using the example from #504 (comment)

https://gist.github.com/MihaZupan/98621af5788f570f0dd0b809f1f3489b

Not sure what the process is on contributing as there's no open PRs so I don't think this has been fixed before :)

[xoofx]

Thanks for the fix!

It could be an ok fix. I will try to have a look locally. My only concern is that it is coupling the code inline parser with a potential grid table | character, but yeah, not sure there are much cleaner way.

[xoofx]

cc: @MihaZupan if you have any comments on this

[phillip-haydon]

Thanks for accepting the PR ❤️ do you know when a new nuget would be available? I'm just running my app with a local nuget dir at the moment but would like ti get it from nuget.org when available.

[MihaZupan]

Table parsing is the one area of Markdig I'm not that familiar with unfortunately.

I fear this change is going to break otherwise valid inputs, e.g.

`
|| hidden text ||
`

is currently a perfectly valid CodeInline (and all parsers agree on that), but this change turns it into a literal instead.

Since this is clearly a targeted fix for table weirdness, I wonder if we should embrace that and add a && processor.Inline?.ContainsParentOfType<PipeTableDelimiterInline>() == true check before breaking the CodeInline?

[MihaZupan]

This is merely cleanup so that the syntax tree doesn't have random pipe delimiters even if we didn't have a table in the end, right?

None of the tests seem to care since the literal renders the same either way.

[phillip-haydon]

Yeah it's just cleanup, it swaps the left over | nodes back to plain text so the tree looks tidy. I did run the benchmark against it and there was no impact on perf so kept it in.

[phillip-haydon]

Table parsing is the one area of Markdig I'm not that familiar with unfortunately.

I fear this change is going to break otherwise valid inputs, e.g.

`
|| hidden text ||
`

is currently a perfectly valid CodeInline (and all parsers agree on that), but this change turns it into a literal instead.

Since this is clearly a targeted fix for table weirdness, I wonder if we should embrace that and add a && processor.Inline?.ContainsParentOfType<PipeTableDelimiterInline>() == true check before breaking the CodeInline?

I tested:

        const string markdown = "`|| hidden text ||`";

and

        const string markdown = """
                                `
                                || hidden text ||
                                `
                                """;

Both produced:

<p><code>|| hidden text ||</code></p>

[phillip-haydon]

Would you like me to add a follow up PR with unit tests for the code snippets?

[MihaZupan]

The second case (with new lines) isn't a code inline anymore after your change.

[Test]
public void Test()
{
    TestParser.TestSpec(
        """
        `
        || hidden text ||
        `
        """,
        "<p><code> || hidden text || </code></p>");
}

fails with

       ```````````````````Source
       `
       ||·hidden·text·||
       `
       ```````````````````Result
       <p>`
       ||·hidden·text·||
       `</p>
       ```````````````````Expected
       <p><code>·||·hidden·text·||·</code></p>
       ```````````````````

More unit tests could be useful (e.g. to also check cases with leading spaces), but I think we should tweak behavior here.

[phillip-haydon]

The second case (with new lines) isn't a code inline anymore after your change.

[Test]
public void Test()
{
    TestParser.TestSpec(
        """
        `
        || hidden text ||
        `
        """,
        "<p><code> || hidden text || </code></p>");
}

fails with

       ```````````````````Source
       `
       ||·hidden·text·||
       `
       ```````````````````Result
       <p>`
       ||·hidden·text·||
       `</p>
       ```````````````````Expected
       <p><code>·||·hidden·text·||·</code></p>
       ```````````````````

More unit tests could be useful (e.g. to also check cases with leading spaces), but I think we should tweak behavior here.

OH, ok leave that to me, ill get a unit test in and fix tonight.